[1/8] extcon: gpio: Add DT bindings

Message ID 20170924145622.4031-2-linus.walleij@linaro.org
State New
Headers show
Series
  • [1/8] extcon: gpio: Add DT bindings
Related show

Commit Message

Linus Walleij Sept. 24, 2017, 2:56 p.m.
Add some reasonable device tree bindings and also add the cable defines
to the extcon include in <dt-bindings/extcon/connectors.h> since
the GPIO extcon definately need to specify which cable/connector it is
detecting.

Adding the include file makes the (as it happens) Linux numbers into an
ABI, but I do not see any better method. It is possible to define strings
for all cable types but it seems like overkill, just reusing Linux'
enumerators seems like a good idea.

The binding supports any number of GPIOs and connectors, but the driver
currently only supports one connector on one GPIO line.

Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
 .../devicetree/bindings/extcon/extcon-gpio.txt     | 24 ++++++++++++++
 include/dt-bindings/extcon/connectors.h            | 38 ++++++++++++++++++++++
 2 files changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/extcon/extcon-gpio.txt
 create mode 100644 include/dt-bindings/extcon/connectors.h

-- 
2.13.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Chanwoo Choi Oct. 19, 2017, 10:47 a.m. | #1
Hi Rob,

[snip]

>>

>>>> +External Connector Using GPIO

>>>

>>> What kind of connector? All connectors external to something... And

>>> GPIO is not a kind of connector on its own, but an implementation.

>>

>> Yeah that is a problem with the whole subsystem I guess. Should

>> I add a paragraph describing the usecases?

>>

>> The whole thing is a bit

>> of Androidism and is named like this because Android named it like

>> this in their kernel tree.

>>

>>>> +Required properties:

>>>> +- compatible: should be "extcon-gpio"

>>>> +- extcon-gpios: the GPIO lines used for the external connectors

>>>

>>> This doesn't tell me what the GPIOs functions are and should. For

>>> example we have hpd-gpios for HDMI hotplug detect in HDMI connector

>>> binding.

>>

>> The idea is that this array corresponds to the extcon-connector-types

>> right below, I'll clarify that if you think the overall idea is OK.

>>

>>>> +  See gpio/gpio.txt

>>>> +- extcon-connector-types: set to an unsigned integer value arrat representing the types

>>>> +  of this connector, matched to the corresponding GPIO lines in the previous array.

>>>

>>> This should be determined by the compatible string.

>>

>> So a GPIO connector is very versatile. It is "general purpose" by definition,

>> so it needs to be subclassed.

>>

>> One possibility is to allow just one GPIO and have one comptible-string per

>> use case.

>> compatible = "gpio-usb-connector";

>> compatible = "gpio-charger-connector";

>> compatible = "gpio-headphone-connector";

>> etc etc

>>

>> These bindings on the other hand, assume that the driver will be able

>> to handle an array of gpios with an associated array of connector types,

>> which reflects how many of the existing extcon-type driver hardware works:

>> a single PMIC providing some 5 misc extcons for example.

>>

>> For this reason there is a generic compatible string and then the device

>> is subclassed per-gpio with the per-gpio connector type.


The "drivers/input/keyboard/gpio_keys.c" driver used the 'linux,code' property
to get the key type as following in the device-tree file:

gpio-keys {                                       
	compatible = "gpio-keys";

	key-1 {
		gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
		linux,code = <KEY_1>;
		label = "SW2-1";
		wakeup-source;
	};

	key-2 {
		gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
		linux,code = <KEY_2>;
		label = "SW2-2";
		wakeup-source;
	};
};


IMO, extcon-gpio.c uses the 'gpio-connectors' compatible
instead of 'extcon-gpio' and then define the connector type
in the include/dt-bindings/extcon/connectors.h. How about this?

For example,
In the include/dt-bindings/extcon/connectors.h,

#define CONNECTOR_USB		1 /* EXTCON_USB */
#define CONNECTOR_USB_HOST	2 /* EXTCON_USB_HOST */
#define CONNECTOR_CHG_USB_SDP	5 /* EXTCON_CHG_USB_SDP */
#define CONNECTOR_CHG_USB_DCP	6 /* EXTCON_CHG_USB_DCP */
#define CONNECTOR_CHG_USB_CDP	7 /* EXTCON_CHG_USB_CDP */
#define CONNECTOR_CHG_USB_ACA	8 /* EXTCON_CHG_USB_ACA */
...
#define CONNECTOR_HDMI		40 /* EXTCON_DISP_HDMI */
...

In the devicetree example for 'gpio-connectors', 

	usb-connector {
		compatible = "gpio-connectors";
		gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
		linux,connector = <CONNECTOR_USB>;
		wakeup-source;
	};

	hdmi-connector {
		gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
		linux,connector = <CONNECTOR_HDMI>;
		wakeup-source;	
	};


>>

>>>> +/* USB external connector */

>>>> +#define EXTCON_USB             1

>>>> +#define EXTCON_USB_HOST                2

>>>> +#define EXTCON_CHG_USB_SDP     5       /* Standard Downstream Port */

>>>> +#define EXTCON_CHG_USB_DCP     6       /* Dedicated Charging Port */

>>>> +#define EXTCON_CHG_USB_CDP     7       /* Charging Downstream Port */

>>>> +#define EXTCON_CHG_USB_ACA     8       /* Accessory Charger Adapter */

>>>> +#define EXTCON_CHG_USB_FAST    9

>>>> +#define EXTCON_CHG_USB_SLOW    10

>>>> +#define EXTCON_CHG_WPT         11      /* Wireless Power Transfer */

>>>> +#define EXTCON_CHG_USB_PD      12      /* USB Power Delivery */

>>>

>>> These don't all look to be mutually exclusive.

>>>

>>> For USB PD, isn't that discoverable?

> 

> Currently, EXTCON_CHG_USB_PD is not used on any extcon provider drivers.

> I think that EXTCON_CHG_USB_PD is discoverable according to the H/W design

> such as using ADC.

> 

> Also, The charger connectors of extcon are related to power_supply subsystem

> because power_supply is in charge of behavior when the connector is attached/detached.

> So, the extcon defines the EXTCON_CHG_USB_PD in order to detect this type.

> 

>>

>> Someone else would have to answer, this is based on the current

>> connector types supported by the Linux kernel.

>>

>>>> +/* Display external connector */

>>>> +#define EXTCON_DISP_HDMI       40      /* High-Definition Multimedia Interface */

>>>> +#define EXTCON_DISP_MHL                41      /* Mobile High-Definition Link */

>>>> +#define EXTCON_DISP_DVI                42      /* Digital Visual Interface */

>>>> +#define EXTCON_DISP_VGA                43      /* Video Graphics Array */

>>>> +#define EXTCON_DISP_DP         44      /* Display Port */

>>>> +#define EXTCON_DISP_HMD                45      /* Head-Mounted Display */

>>>

>>> We already have connector bindings for most of these. Use those as a

>>> model for whatever you want to do.

>>

>> I guess they are not in Documentation/devicetree/bindings/extcon/*

>>

>> Please point me in the right direction and I'll check it out.

>>

>> Yours,

>> Linus Walleij

>>

>>

>>

> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/Documentation/devicetree/bindings/extcon/extcon-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt
new file mode 100644
index 000000000000..2f5e21b94a64
--- /dev/null
+++ b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt
@@ -0,0 +1,24 @@ 
+External Connector Using GPIO
+
+Required properties:
+- compatible: should be "extcon-gpio"
+- extcon-gpios: the GPIO lines used for the external connectors
+  See gpio/gpio.txt
+- extcon-connector-types: set to an unsigned integer value arrat representing the types
+  of this connector, matched to the corresponding GPIO lines in the previous array.
+  Those are defined with unique IDs in <dt-bindings/extcon/connectors.h>
+- input-debounce: The number of microseconds to wait for the
+  connector state to stabilize. This property is reused from pin control
+  See pinctrl/pinctrl-bindings.txt
+
+Example:
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/extcon/connectors.h>
+
+extcon {
+	compatible = "extcon-gpio";
+	extcon-gpios = <&gpio0 42 GPIO_ACTIVE_LOW>;
+	extcon-connector-types = <EXTCON_USB>;
+	input-debounce = <20000>; /* 20 ms */
+};
diff --git a/include/dt-bindings/extcon/connectors.h b/include/dt-bindings/extcon/connectors.h
new file mode 100644
index 000000000000..61bed24eaadc
--- /dev/null
+++ b/include/dt-bindings/extcon/connectors.h
@@ -0,0 +1,38 @@ 
+#ifndef _DT_BINDINGS_EXTCON_CONNECTORS_H
+#define _DT_BINDINGS_EXTCON_CONNECTORS_H
+
+/* USB external connector */
+#define EXTCON_USB		1
+#define EXTCON_USB_HOST		2
+#define EXTCON_CHG_USB_SDP	5	/* Standard Downstream Port */
+#define EXTCON_CHG_USB_DCP	6	/* Dedicated Charging Port */
+#define EXTCON_CHG_USB_CDP	7	/* Charging Downstream Port */
+#define EXTCON_CHG_USB_ACA	8	/* Accessory Charger Adapter */
+#define EXTCON_CHG_USB_FAST	9
+#define EXTCON_CHG_USB_SLOW	10
+#define EXTCON_CHG_WPT		11	/* Wireless Power Transfer */
+#define EXTCON_CHG_USB_PD	12	/* USB Power Delivery */
+/* Jack external connector */
+#define EXTCON_JACK_MICROPHONE	20
+#define EXTCON_JACK_HEADPHONE	21
+#define EXTCON_JACK_LINE_IN	22
+#define EXTCON_JACK_LINE_OUT	23
+#define EXTCON_JACK_VIDEO_IN	24
+#define EXTCON_JACK_VIDEO_OUT	25
+#define EXTCON_JACK_SPDIF_IN	26	/* Sony Philips Digital InterFace */
+#define EXTCON_JACK_SPDIF_OUT	27
+/* Display external connector */
+#define EXTCON_DISP_HDMI	40	/* High-Definition Multimedia Interface */
+#define EXTCON_DISP_MHL		41	/* Mobile High-Definition Link */
+#define EXTCON_DISP_DVI		42	/* Digital Visual Interface */
+#define EXTCON_DISP_VGA		43	/* Video Graphics Array */
+#define EXTCON_DISP_DP		44	/* Display Port */
+#define EXTCON_DISP_HMD		45	/* Head-Mounted Display */
+/* Miscellaneous external connector */
+#define EXTCON_DOCK		60
+#define EXTCON_JIG		61
+#define EXTCON_MECHANICAL	62
+
+#define EXTCON_NUM		63
+
+#endif /* _DT_BINDINGS_EXTCON_CONNECTORS_H */