Re: [PATCH 1/4] input: Add new sun4i-lradc-keys drivers

On Thu, Jan 02, 2014 at 12:38:31PM -0800, Dmitry Torokhov wrote:

On Thu, Jan 02, 2014 at 09:20:22PM +0100, Maxime Ripard wrote:

quoted

On Thu, Jan 02, 2014 at 02:45:29PM +0100, Hans de Goede wrote:

quoted

quoted

Also, instead of inventing yet another vendor-specific property, why not re-use
a button binding similar to gpio-keys like:

       lradc: lradc@01c22800 {
               compatible = "allwinner,sun4i-lradc-keys";
               reg = <0x01c22800 0x100>;
               interrupts = <31>;
               allwinner,chan0-step = <200>;

	#address-cells = <1>;
	#size-cells = <0>;

	button@0 {
		reg = <0>; /* your channel index from above */
		linux,code = <115>; /* already used as dt-property */
	};

	button@1 {
		reg = <1>;
		linux,code = <114>;
	};

Ugh no. Having a vendor specific property which is KISS certainly
beats this, both wrt ease of writing dts files as well as wrt the
dts parsing code in the driver.

I'd agree with Heiko here. This is pretty much the same construct
that's already in use in other input drivers, like gpio-keys.

This is also something that can really easily be made generic, since
this is something that is rather common.

Except that button definition from gpio-keys does not use 'reg' property
but rather gpio. I'd rather we did not cram non-applicable attributes
into that definition just to make it "reusable" like that.

I'd be OK with having similar (but not claiming to be the same) mappings
though.

Yes, this is what I was meaning. Sorry if it was not clear enough.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com