public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: heiko@sntech.de (Heiko Stübner)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/4] input: Add new sun4i-lradc-keys drivers
Date: Mon, 06 Jan 2014 11:04:34 +0100	[thread overview]
Message-ID: <7852278.jiSUdg0kdd@phil> (raw)
In-Reply-To: <20140103182349.GA13489@core.coreip.homeip.net>

Am Freitag, 3. Januar 2014, 10:23:50 schrieb Dmitry Torokhov:
> On Fri, Jan 03, 2014 at 06:36:05PM +0100, Maxime Ripard wrote:
> > On Thu, Jan 02, 2014 at 11:36:33PM +0100, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 01/02/2014 09:20 PM, Maxime Ripard wrote:
> > > >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 at 01c22800 {
> > > >>>        
> > > >>>                compatible = "allwinner,sun4i-lradc-keys";
> > > >>>                reg = <0x01c22800 0x100>;
> > > >>>                interrupts = <31>;
> > > >>>                allwinner,chan0-step = <200>;
> > > >>>		
> > > >>>		#address-cells = <1>;
> > > >>>		#size-cells = <0>;
> > > >>>		
> > > >>>		button at 0 {
> > > >>>		
> > > >>>			reg = <0>; /* your channel index from above */
> > > >>>			linux,code = <115>; /* already used as dt-property */
> > > >>>		
> > > >>>		};
> > > >>>		
> > > >>>		button at 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.
> > > 
> > > In the gpio case there is a 1 on 1 relation between a single hw
> > > entity (the gpio-pin) and a single keycode. Here there is 1 hw entity
> > > which maps to an array of key-codes, certainly using an array rather
> > > then a much more complicated construct is the correct data-structure
> > > to represent this.
> > 
> > You can build an array in your driver out of this very easily, it's 10
> > lines in your probe. And you gain from this something that is more
> > generic, can be shared by other similar drivers and is consistent with
> > what is already in use.
> 
> How will it be shared? Surely not code-wise, but basically in spirit
> only. It seems to me that the originally proposed binding is simple and
> concise and works well for the driver.

I don't think "binding [...] works well for the driver" is the correct 
direction. From my understanding the binding should describe the hardware in 
an os-agnostic way (so "linux,foo" properties should stay the exception) and 
not the data structures used in the driver. The driver itself then implements 
the binding to convert the binding-data into a structure it wants to use.

The sharing would, as you suggested, be in spirit and in the use of already 
established dt-properties without introducing more non-standard ones.


Heiko

  reply	other threads:[~2014-01-06 10:04 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-01 19:30 [PATCH 0/4] input: Add new sun4i-lradc-keys driver Hans de Goede
2014-01-01 19:30 ` [PATCH 1/4] input: Add new sun4i-lradc-keys drivers Hans de Goede
2014-01-01 20:56   ` Dmitry Torokhov
2014-01-02  9:37     ` Hans de Goede
2014-01-02 11:59       ` Heiko Stübner
2014-01-02 13:45         ` Hans de Goede
2014-01-02 20:20           ` Maxime Ripard
2014-01-02 20:38             ` Dmitry Torokhov
2014-01-03 17:15               ` Maxime Ripard
2014-01-02 22:36             ` Hans de Goede
2014-01-03 17:36               ` Maxime Ripard
2014-01-03 18:23                 ` Dmitry Torokhov
2014-01-06 10:04                   ` Heiko Stübner [this message]
2014-01-06 10:13                   ` Maxime Ripard
2014-01-01 19:30 ` [PATCH 2/4] ARM: dts: sun4i: Add lradc node Hans de Goede
2014-01-01 19:30 ` [PATCH 3/4] ARM: dts: sun5i: " Hans de Goede
2014-01-01 19:45   ` Andrew Lunn
2014-01-02  9:37     ` [linux-sunxi] " Hans de Goede
2014-01-01 19:30 ` [PATCH 4/4] ARM: dts: sun7i: " Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7852278.jiSUdg0kdd@phil \
    --to=heiko@sntech.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox