Subject: Re: once more psh3pwr(4)
To: KIYOHARA Takashi <kiyohara@kk.iij4u.or.jp>
From: Valeriy E. Ushakov <uwe@stderr.spb.ru>
List: port-hpcsh
Date: 09/23/2007 22:08:56
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