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

Hi,

On 01/02/2014 12:59 PM, Heiko Stübner wrote:

Hi Hans, Dmitry,

Am Donnerstag, 2. Januar 2014, 10:37:47 schrieb Hans de Goede:

quoted

Hi,

On 01/01/2014 09:56 PM, Dmitry Torokhov wrote:

quoted

Hi Hans,

On Wed, Jan 01, 2014 at 08:30:07PM +0100, Hans de Goede wrote:

quoted

+Required properties:
+ - compatible: "allwinner,sun4i-lradc-keys"
+ - reg: mmio address range of the chip
+ - interrupts: interrupt to which the chip is connected
+ - allwinner,chan0-step: step in mV between keys must be 150 or 200
+ - allwinner,chan0-keycodes: array of include/uapi/linux/input.h KEY_
codes>

I think this should be "linux,chan0-keycodes".

Right, because the codes are Linux specific, will fix in v2.

but the property with its "chan0-" thingy would be allwinner-specific if I'm
not mistaken.

Correct, but denoting that this is linux only is more important, so as to
avoid namespace collisions.

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.

Regards,

Hans

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/groups/opt_out.