On Thu, Jan 02, 2014 at 02:45:29PM +0100, Hans de Goede wrote: > >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. Speaking of which. I believe this should actually come in two different drivers: - The ADC driver itself, using IIO - A generic button handler driver on top of IIO. The fact that on most board this adc is used for buttons doesn't make any difference, it's actually a hardware designer choice, we should support that choice, but we should also be able to use it just as an ADC. Carlo Caione already started to work on an IIO driver for the LRADC: https://github.com/carlocaione/linux/tree/sunxi-lradc maybe you can take over his work. I also wonder wether it would be possible in that case to use reg as the "button" voltage, to get rid of both the chan0-step property, and those big fat arrays in the driver. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
Attachment:
signature.asc
Description: Digital signature