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

[Rakesh Iyer]

Hello All.

I have combined Pavel's and Dmitry's comments so I can address them as one issue.

Dmitry's comment

> On Mon, Jan 10, 2011 at 02:49:25PM -0800, Rakesh Iyer wrote:
> > > 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.
> >
> > We had the same thought regarding Function as a modifier akin to Shift, Ctrl an Alt.
> > But this does not appear to be the case.
> >
> > This keyboard is used primarily for Laptop-like form factors where function keys are used
> to overload the existing keys.
> 
> So it is same SHIFT.
> 
> > In addition the Fn key mappings aren't identical in different keyboard layouts.
> 
> 
> Same as SHIFT - depending on layout used shifted KEY_1 produces either '!' or '1'.

The FN key modifiers are not identical to Shift Modifier. They map to totally different keys for this keyboard.
If you are suggesting that you weren't implying they map to the same thing, then I agree it can be treated as a modifier.
But the problem is the higher layers don't necessarily do so. 

The documentation I have read all suggest the Fn key isn't a valid modifier for the operating system, 
and that most keyboard hardware abstracts the Fn keypress and generates the modified key.
Maybe my understanding is incorrect regarding this so please feel free to give me the information.

Since our keyboard controller hardware (made for a SOC) is very simplistic the driver has to do this.


Pavel's comment

> > > +	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.
> 
> We had the same thought regarding Function as a modifier akin to Shift, Ctrl an Alt.
> But this does not appear to be the case. 
> 
> This keyboard is used primarily for Laptop-like form factors where function keys are used to overload the existing keys. 
> In addition the Fn key mappings aren't identical in different keyboard layouts.
> 
> So at this point in order to function with the target hardware and
> the target operating system which is Chrome, this modifier
> keymap is necessary.

> Please find a way to do this cleanly. Other machines (zaurus) have
> similar keyboards...

With regards to Pavel's comment of handling this within the driver without another keymap, we have explored using a smaller 
key translation map, but we cannot use this as we support a varied number of keyboards depending on the OEM. 
Thus our keymaps and the resultant function modified keys are dependent on the target keyboard and hence the need for the 
Override keymaps.


Please let me know if you think the current approach is okay given our targeted configurations, or if there are alternative mechanisms.

Thanks and Regards
Rakesh
--
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/