Re: [PATCH] input: Add keypad support for w90p910 evb

[Wan ZongShun <mcuos.com@gmail.com>]

Dear Hartley,

Thanks for your good ideas, before resubmitting the updated patch,I have to need your help.

H Hartley Sweeten:
> On Thursday, July 09, 2009 2:31 AM, Wan ZongShun wrote:
>> Dear sirs,
>>
>> According to Trilok's suggestion, I fixed up my driver and
>> resubmitted it.
>>
>> patch text:
>>
>> Add w90p910 keypad driver for w90p910 evalution board
>> based on w90p910,there is a 4X4 keypad on my board.
>>
>> Signed-off-by: Wan ZongShun <mcuos.com@gmail.com>
>>
>> ---
> +
> +#define keypad_readl(off)	__raw_readl(keypad->mmio_base + (off))
> +#define keypad_writel(off, v)	__raw_writel((v), keypad->mmio_base + (off))
> +

Do you mean that using the following function instead of that above macro?


static inline unsigned long keypad_readl(unsigned int off, struct w90p910_keypad *keypad)
{
	return __raw_readl(keypad->mmio_base + (off));
}

> You might want to make these two macro's inline functions instead.
> 
> Depending on having the local variable 'keypad' is prone to breakage.
> See Documentation/CodingStyle, Chapter 12.
> 
> +	input_dev->name = pdev->name;
> +	input_dev->id.bustype = BUS_HOST;
> +	input_dev->open = w90p910_keypad_open;
> +	input_dev->close = w90p910_keypad_close;
> +	input_dev->dev.parent = &pdev->dev;
> 
> If you add the following the keymap can be changed from userspace:
> 
> 	input_dev->keycode = keypad->matrix_keycodes;
> 	input_dev->keycodesize = sizeof(keypad->matrix_keycodes[0]);
> 	input_dev->keycodemax = ARRAY_SIZE(keypad->matrix_keycodes);

It is a good idea.

> Regards,
> Hartley
>