On Thu, 23 Jun 2011 01:24:10 -0700 Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote: > On Fri, Jun 17, 2011 at 06:54:17AM -0600, Grant Likely wrote: > > On Fri, Jun 17, 2011 at 2:58 AM, David Jander <david.jander@xxxxxxxxxxx> > > wrote: > > > > > > Hi Grant, > > > > > > On Thu, 16 Jun 2011 13:25:59 -0600 > > > Grant Likely <grant.likely@xxxxxxxxxxxx> wrote: > > > > > >> On Tue, Jun 14, 2011 at 11:08:10AM +0200, David Jander wrote: > > >> > This patch enables fetching configuration data which is normally > > >> > provided via platform_data from the device-tree instead. > > >> > If the device is configured from device-tree data, the platform_data > > >> > struct is not used, and button data needs to be allocated dynamically. > > >> > Big part of this patch deals with confining pdata usage to the probe > > >> > function, to make this possible. > > >> > > > >> > Signed-off-by: David Jander <david@xxxxxxxxxxx> > > >> > --- > > >> > .../devicetree/bindings/gpio/gpio_keys.txt | 49 +++++++ > > >> > drivers/input/keyboard/gpio_keys.c | 147 > > >> > ++++++++++++++++++-- 2 files changed, 186 insertions(+), 10 > > >> > deletions(-) create mode 100644 > > >> > Documentation/devicetree/bindings/gpio/gpio_keys.txt > > >> > > > >> > diff --git a/Documentation/devicetree/bindings/gpio/gpio_keys.txt > > >> > b/Documentation/devicetree/bindings/gpio/gpio_keys.txt new file mode > > >> > 100644 index 0000000..60a4d8e > > >> > --- /dev/null > > >> > +++ b/Documentation/devicetree/bindings/gpio/gpio_keys.txt > > >> > @@ -0,0 +1,49 @@ > > >> > +Device-Tree bindings for input/gpio_keys.c keyboard driver > > >> > + > > >> > +Required properties: > > >> > + - compatible = "gpio-keys"; > > >> > + > > >> > +Optional properties: > > >> > + - autorepeat: Boolean, Enable auto repeat feature of Linux input > > >> > + subsystem. > > >> > + > > >> > +Each button (key) is represented as a sub-node of "gpio-keys": > > >> > +Subnode properties: > > >> > + > > >> > + - reg: GPIO number the key is bound to if linux GPIO numbering is > > >> > used. > > >> > > >> Wait. That won't work. There is no concept of "linux gpio numbering" > > >> in the device tree data. When using device tree, the gpio numbers > > >> usually get dynamically assigned. > > > > > > Yes I know, but suppose you want to use this driver with a GPIO-driver > > > that does not have devaice-tree support yet? I need a way of binding the > > > button to a GPIO pin that does not have a device-tree definition. How > > > should I do this otherwise? > > > I am using this driver with a pca963x, as you might have suspected > > > already, and I just added OF device-tree support to it, so that will > > > work, but others...? > > > > The solution is to add OF support to the GPIO driver being used. > > > > > Before "fixing" pca963x, I used this property and it worked perfectly > > > well, so please explain what is not supposed to work. Please keep in > > > mind that this is meant as more of a backwards-compatibility feature. If > > > you think that being backwards compatible with non-OF GPIO drivers is a > > > bad idea, I'll remove this. > > > > It is. Something that we've been very careful about is to avoid > > encoding Linux-specific implementation details into the device tree > > bindings. The implementation details, such as how gpio controllers > > are enumerated, are subject to change just in the normal progress of > > development. By focusing the DT bindings on HW description, the DT > > data is insulated to a degree from kernel churn. > > > > >> > + - wakeup: Boolean, button can wake-up the system. > > >> > > >> "wakeup" is potentially too generic a property name (potential to > > >> conflict with a generic wakeup binding if one ever exists). Just to > > >> be defencive, I'd suggest prefixing these custom gpio keys properties > > >> with something like "gpio-key,". > > > > > > Ok, "gpio-key,wakeup" it will be then? But isn't this function something > > > potentially other IO-pins/keys/buttons/interrupts/etc... could have to > > > be able to wake up the system? > > > > Can you foresee how all bindings would potentially use a 'wakeup' > > property? It's really hard to define a common binding without first > > having several use cases ready to use it. It's better to start being > > cautious, and then create a common binding at some point in the > > future. > > > > > > >> > + reg = of_get_property(pp, "linux,input-type", &len); > > >> > + if (reg) > > >> > + buttons[i].type = be32_to_cpup(reg); > > >> > + else > > >> > + buttons[i].type = EV_KEY; > > >> how about: > > >> buttons[i].type = reg ? be32_to_cpup(reg) : EV_KEY; > > > > > > Ok, if you prefer this notation.... just an "if...else" in another > > > dress ;-) > > > > Yup, but it's shorter, and I like painting bike sheds. > > > > So is there an updated version of this patch coming? Yes, I'm preparing v5 right now. Best regards, -- David Jander Protonic Holland. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html