Re: [PATCH] extcon-gpio: add devicetree support.

From: Guenter Roeck
Date: Sun Nov 03 2013 - 01:39:18 EST

On 11/01/2013 04:33 PM, NeilBrown wrote:
On Fri, 1 Nov 2013 10:16:44 -0700 Mark Rutland <mark.rutland@xxxxxxx> wrote:

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.

Thanks for the review.

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

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"?

Maybe "cable-presence-detector" as in this case the GPIO is just an
implementation detail. Which isn't much different from "external-connector"
which is where "extcon" comes from...

I propose "external-connector" if you don't like "extcon".

+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.

The reason I did this was that the pre-existing platform_data wants
'irq_flags'. I could have an 'irq-flags' property, but it seems to make more
sense to use "interrupts" as that already provides a way to pass irq-flags to
a device.

On reflection though, I cannot imagine why any extcon-gpio would use anything
other than IRQ_TYPE_EDGE_BOTH. Maybe MyungJoo Ham can explain that???

If there is no need for specifying irq-flags per-platform, the "interrupts"
property can definitely go.

When I tried to add DT support to extcon-pio, I used IRQ_TYPE_EDGE_BOTH.
See [1]. A useful fallback for 'label' - if not specified - may be
np->name, to match LED functionality.

A patch to add 'active low' to the platform data for extcon-gpio is pending
in linux-next. It might make sense to extract this flag from the gpio flags
and add it as well. Again see [1] for an example how this could be implemented.



To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at