| Message ID | 20170924145622.4031-2-linus.walleij@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/8] extcon: gpio: Add DT bindings | expand |
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
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 */
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