On Mon, Sep 24, 2007 at 01:42:53 +0900, KIYOHARA Takashi wrote:

> I will commit the psh3pwr.  Moreover, it supports sysmon_envsys(9).

Thanks.  Some comments below.


> #include <machine/config_hook.h>

I don't see any config_hook calls.


> 	/* regsiter sleep function to APM */
> 	__sleep_func = psh3pwr_sleep;
> 	__sleep_ctx = self;

Also arrange necessary config_hooks to get hpcapm connected to this
driver?


> 		printf("%s: plug status: out\n",
> 		    sc->sc_dev.dv_xname);

device_xname(&sc->sc_dev) is preferred way to do it (I know, j6x0pwr.c
hasn't been converted yet :)


> 	sc->sc_data.sensor = 0;
> 	sc->sc_data.units = ENVSYS_INDICATOR;
> 	sc->sc_data.state = ENVSYS_SVALID;
> 	sc->sc_data.value_cur = sc->sc_plug;
> 	snprintf(sc->sc_data.desc, sizeof(sc->sc_data.desc),
> 	    "%s %s", sc->sc_dev.dv_xname, "plug");

I wonder if it makes sense to convert hpcapm to use sysmon instead of
doing that in each driver (just thinking out loud here).


> 	irr0 = _reg_read_1(SH7709_IRR0);
> 	if (irr0 & IRR0_IRQ0)
> 		_reg_write_1(SH7709_IRR0, irr0 & ~IRR0_IRQ0);
> 

If the bit is not set, you probably want to return 0 (interrupt not
handled) and bail.  Ditto in _plug_in().



> 	/* splhigh on entry */
> 	extern void pfckbd_poll_hitachi_power(void);

I assume this will be in pfckbd.c.  Isn't there any interrupt to wake
us up?


Just commit it, and the details can be hashed out in the tree.

Thanks!

SY, Uwe
-- 
uwe@stderr.spb.ru                       |       Zu Grunde kommen
http://snark.ptc.spbu.ru/~uwe/          |       Ist zu Grunde gehen