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? Thanks. -- Dmitry -- 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
