Re: [PATCH v3] input: tegra-kbc - Add tegra keyboard driver

[Dmitry Torokhov]

Hi Rakesh,

On Fri, Jan 07, 2011 at 09:45:07AM -0800, riyer@xxxxxxxxxx wrote:
> +
> +struct tegra_kbc {
> +	void __iomem *mmio;
> +	struct input_dev *idev;
> +	int irq;
> +	unsigned int wake_enable_rows;
> +	unsigned int wake_enable_cols;
> +	spinlock_t lock;
> +	unsigned int repoll_time;
> +	unsigned long cp_dly_jiffies;
> +	int fifo[KBC_MAX_KPENT];
> +	const struct tegra_kbc_platform_data *pdata;
> +	int *plain_keycode;
> +	int *fn_keycode;

There should not be separate keycodes for FN and normal kys - FN is just
a modifier, like SHIFT or CTRL or ALT are and shoudl be handled in upper
layers.

Also you should wire up keycode/keycodemax/keycodesize in input_dev
structuire so that keymap can be retrieved via EVIOCGKEYCODE and
modified via EVIOGSKEYCODE. Also, because keymap is modifiable, the
original keymap should be copied in per-device structure, leaving
original intact. It (the original) also should be marked as const.

Since this papears to be a matrix keypad consider using definitions from
linux/input/matrix_keypad.h

Thank you.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/