Hi Neil, While I'm not fundamentally opposed to this binding, I have some issues with its current form and would not want to see this version hit mainline. On Fri, Nov 01, 2013 at 09:50:05AM +0000, NeilBrown wrote: > > As this device is not vendor specific, I haven't included any "vendor," > prefixes. For my model I used "regulator-gpio" which takes a similar > approach. > > Signed-off-by: NeilBrown <neilb@xxxxxxx> > > diff --git a/Documentation/devicetree/bindings/extcon/extcon-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt > new file mode 100644 > index 000000000000..2346b61cc620 > --- /dev/null > +++ b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt > @@ -0,0 +1,26 @@ > +* EXTCON detector using GPIO EXTCON is _extremely_ Linux-specific. The binding document needs a description of the class of device it's inteded to describe that does not just refer to Linux internals. I would prefer if we could have a better name for this that was not tied to the Linux driver name. Perhaps "gpio-presence-detector"? > + > +Required Properties: > + - compatible: "extcon-gpio" > + - gpios: gpio line that detects connector > + - interrupts: interrupt generated by that gpio We don't need this. If we need the interrupt a gpio generates, we should ask the gpio controller driver to map the gpio to an interrupt. We have gpiod_to_irq for this in Linux. > + - debounce-delay-ms: debouncing delay > + > +Optional Properties: > + - label: name for connector. If not given, device name is used. > + - state-on: string to report when GPIO is high (else '0') > + - state-off: string to report when GPIO is low (else '1') I do not like these properties, they are very much a Linux implementation detail. Are extcon devices ever used standalone? If so, why? If not I see _no_ reason at all for the label property. If a userspace application needs to detect the presence of a particular external connector, it will need to know this in relation to the device the external connectors are attached to. In that case the application should find that device and traverse its set of extcon devices. The names for the external connections will be a property of the device, not the extcon devices themselves (along hte same lines as clocks), and need not be a property of the extcon device. As for state-on and state-off, we are exposing a binary property to userspace, the meaning of which is already dependent on the particular device the extcon devices are connected to. Given userspace already has to understand that, I see no value in embedding arbitrary strings in the device tree which attempt to replicate this knowledge. They provide no additional value and in my mind make the interface to userspace harder to use because you have ot handle an arbitrary set of values depending on how the dt author felt one morning rather than just the binary value you actually care about. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
