Re: once more psh3pwr(4)

["Valeriy E. Ushakov" <uwe%stderr.spb.ru@localhost>]

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@localhost                       |       Zu Grunde kommen
http://snark.ptc.spbu.ru/~uwe/          |       Ist zu Grunde gehen