Subject: Re: RFC: cleaning up j720ssp.c
To: None <port-hpcarm@NetBSD.org>
From: Peter Postma <peter@pointless.nl>
List: port-hpcarm
Date: 02/21/2006 12:28:26
On Tue, Jan 17, 2006 at 03:30:21PM +0100, Peter Postma wrote:
> j720ssp.c contains drivers for the touch-panel, keyboard, lcd control
> and battery status. I'd like to split this into multiple files, because
> it's a bit messy now and having each driver in a separate file is much
> cleaner. It will then also be easier then to improve the drivers (e.g.
> changing keyboard driver to use hpckbd(4)). This is what I propose:
>
Changes to hpcarm files are available here:
ftp://ftp.netbsd.org/pub/NetBSD/misc/peter/j720-cleanup.diff
New files are available here:
ftp://ftp.netbsd.org/pub/NetBSD/misc/peter/j720-cleanup.new
The following files have been removed: apm.c, j720kbdmap.c.
Note that there are also changes to the platforms and hpckbd keymap,
this is not included but of course that needs some testing and I'll write
a seperate mail to call for testers.
There are also a few things unclear to me, I'll try to explain this below.
Hopefully there's someone on the list willing to review these changes
and/or comment to my observations.
> j720kbd.c - keyboard driver (using hpckbd(4))
Three issues remain here:
- When hpckbd goes into polling mode, I need to disable the keyboard
interrupt, but this does not seem to be possible with hpckbd (maybe we
should add a callback in hpckbd_cnpollc?). This is not a big problem
because polling works, but it can generate some debug messages (which is
actually not a big problem either).
- Button event is #if 0'ed and in sed_saip.c (framebuffer driver) for
now. Eventually, sed_saip.c should be modified to support lcd light
toggling, but then it should keep a global instance of sed1356_softc.
An alternative might be to call the lcd light power hook in
j720kbd_button_event, but this is not optimal because the onoff value
passed by hpckbd can be wrong (and actually is wrong at the first keypress,
it should at least be initialized to 1 -- assuming lcd light is on at
boottime).
- The keymaps should be tested. I've no idea if they all work (will write
a separate mail to call for testers).
> j720lcd.c - lcd contrast/brightness/power control
I'm not sure if j720lcd_power is at the right place, because it seems more
to be specific to epson 1356 <-> sa1110 (sed_saip.c). However, the XXX in
sed_saip.c makes me think it's not...?
> j720pwr.c - power management (using hpcapm(4))
Not using hpcapm(4) yet, because I can't attach devices to mainbus (and
I don't think putting #ifdef hpcarm in arm/mainbus/mainbus.c is right, so
suggestions are welcome).
I've added my own fixes for displaying the battery status.
> j720tp.c - touch-panel driver
Seems to work fine.
--
Peter Postma