Re: RFC: cleaning up j720ssp.c

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