Hi Rob,

[snip]

quoted

quoted

quoted

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

quoted

quoted

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

quoted

quoted

+  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;	
	};

quoted

quoted

quoted

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

quoted

Someone else would have to answer, this is based on the current
connector types supported by the Linux kernel.

quoted

quoted

+/* 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