Re: [Qemu-devel] [Patch 1/2][PXA27x] initial keypad support

[andrzej zaborowski]

Hi,

On 12/12/2007, Armin <address@hidden> wrote:
> Here is an attempt to add PXA27x keypad support. It currently only
> supports the matrix type interface. It still needs direct and
> mulitswitch support added.
>
> Just wanted to get something out there for folks to pound on.
>
> Comment and feedback welcome.

Here are some comments:

* Several bits in KPC and KPREC should be reset on read. The IRQ
should be reset when it becomes masked in KPC, and potentially
asserted when it becomes unmasked.

* It's wasteful to go through all keys in the 8x8 matrix on every
event. It can be avoided easily by mapping qemu keycode to matrix
position, rather than position to keycode, for example the PalmT|E
matrix keypad does that.

* Please convert the indentation in the file to use spaces, before
considering for inclusion.

* The bit definitions and pxa2xx_keypad_s struct are local to the
keypad module so they shouldn't be in the header file exposed to the
whole world.

* Please set a copyright owner.

Regarding mainstone keypad patch:

* Probably not worth to stick the keymap in a file of its own since it
is not shared between different machines - it's an integral part of
mainstone.

Thanks,
Andrew