[PATCH v5 0/6] dt-bindings: add bindings for USB physical connector

[PATCH v5 0/6] dt-bindings: add bindings for USB physical connector

[Andrzej Hajda]

Hi,

Thanks for reviews of previous iterations.

This patchset introduces USB physical connector bindings, together with
working example.
I have removed RFC prefix - the patchset seems to be heading
to a happy end :)

v5: fixed extra parenthesis in dts, renamed extcon function.
v4: improved binding descriptions, added missing reg in dts.
v3: Separate binding for Samsung 11-pin connector, added full-blown USB-C
    example.
v2: I have addressed comments by Rob and Laurent, thanks 

Changes in datail are described in the patches.

Regards
Andrzej


Andrzej Hajda (5):
  dt-bindings: add bindings for USB physical connector
  dt-bindings: add bindings for Samsung micro-USB 11-pin connector
  arm64: dts: exynos: add micro-USB connector node to TM2 platforms
  arm64: dts: exynos: add OF graph between MHL and USB connector
  extcon: add possibility to get extcon device by OF node

Maciej Purski (1):
  drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL

 .../connector/samsung,usb-connector-11pin.txt      | 49 +++++++++++
 .../bindings/connector/usb-connector.txt           | 75 +++++++++++++++++
 .../boot/dts/exynos/exynos5433-tm2-common.dtsi     | 38 ++++++++-
 drivers/extcon/extcon.c                            | 44 +++++++---
 drivers/gpu/drm/bridge/sil-sii8620.c               | 97 +++++++++++++++++++++-
 include/linux/extcon.h                             |  6 ++
 6 files changed, 293 insertions(+), 16 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/connector/samsung,usb-connector-11pin.txt
 create mode 100644 Documentation/devicetree/bindings/connector/usb-connector.txt

-- 
2.16.2

[PATCH v5 1/6] dt-bindings: add bindings for USB physical connector

[Andrzej Hajda]

These bindings allow to describe most known standard USB connectors
and it should be possible to extend it if necessary.
USB connectors, beside USB can be used to route other protocols,
for example UART, Audio, MHL. In such case every device passing data
through the connector should have appropriate graph bindings.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
v4:
- improved 'type' description (Rob),
- improved description of 2nd example (Rob).
v3:
- removed MHL port (samsung connector will have separate bindings),
- added 2nd example for USB-C,
- improved formatting.
v2:
- moved connector type(A,B,C) to compatible string (Rob),
- renamed size property to type (Rob),
- changed type description to be less confusing (Laurent),
- removed vendor specific compatibles (implied by graph port number),
- added requirement of connector being a child of IC (Rob),
- removed max-mode (subtly suggested by Rob, it should be detected anyway
  by USB Controller in runtime, downside is that device is not able to
  report its real capabilities, maybe better would be to make it optional(?)),
- assigned port numbers to data buses (Rob).

Regards
Andrzej
---
 .../bindings/connector/usb-connector.txt           | 75 ++++++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/connector/usb-connector.txt

diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt b/Documentation/devicetree/bindings/connector/usb-connector.txt
new file mode 100644
index 000000000000..e1463f14af38
--- /dev/null
+++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
@@ -0,0 +1,75 @@
+USB Connector
+=============
+
+USB connector node represents physical USB connector. It should be
+a child of USB interface controller.
+
+Required properties:
+- compatible: describes type of the connector, must be one of:
+    "usb-a-connector",
+    "usb-b-connector",
+    "usb-c-connector".
+
+Optional properties:
+- label: symbolic name for the connector,
+- type: size of the connector, should be specified in case of USB-A, USB-B
+  non-fullsize connectors: "mini", "micro".
+
+Required nodes:
+- any data bus to the connector should be modeled using the OF graph bindings
+  specified in bindings/graph.txt, unless the bus is between parent node and
+  the connector. Since single connector can have multpile data buses every bus
+  has assigned OF graph port number as follows:
+    0: High Speed (HS), present in all connectors,
+    1: Super Speed (SS), present in SS capable connectors,
+    2: Sideband use (SBU), present in USB-C.
+
+Examples
+--------
+
+1. Micro-USB connector with HS lines routed via controller (MUIC):
+
+muic-max77843 at 66 {
+	...
+	usb_con: connector {
+		compatible = "usb-b-connector";
+		label = "micro-USB";
+		type = "micro";
+	};
+};
+
+2. USB-C connector attached to CC controller (s2mm005), HS lines routed
+to companion PMIC (max77865), SS lines to USB3 PHY and SBU to DisplayPort.
+DisplayPort video lines are routed to the connector via SS mux in USB3 PHY.
+
+ccic: s2mm005 at 33 {
+	...
+	usb_con: connector {
+		compatible = "usb-c-connector";
+		label = "USB-C";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port at 0 {
+				reg = <0>;
+				usb_con_hs: endpoint {
+					remote-endpoint = <&max77865_usbc_hs>;
+				};
+			};
+			port at 1 {
+				reg = <1>;
+				usb_con_ss: endpoint {
+					remote-endpoint = <&usbdrd_phy_ss>;
+				};
+			};
+			port at 2 {
+				reg = <2>;
+				usb_con_sbu: endpoint {
+					remote-endpoint = <&dp_aux>;
+				};
+			};
+		};
+	};
+};
-- 
2.16.2

[PATCH v5 1/6] dt-bindings: add bindings for USB physical connector

[Heikki Krogerus]

Hi,

On Tue, Feb 27, 2018 at 08:11:29AM +0100, Andrzej Hajda wrote:
> +2. USB-C connector attached to CC controller (s2mm005), HS lines routed
> +to companion PMIC (max77865), SS lines to USB3 PHY and SBU to DisplayPort.
> +DisplayPort video lines are routed to the connector via SS mux in USB3 PHY.
> +
> +ccic: s2mm005 at 33 {
> +	...
> +	usb_con: connector {
> +		compatible = "usb-c-connector";
> +		label = "USB-C";

Is this child node really necessary? There will never be more then
one connector per CC line.

We should prefer device_graph* functions over of_graph* and
acpi_graph* functions in the drivers so we don't have to handle the
same thing multiple times with separate APIs. Is it still possible if
there is that connector child node?

> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port at 0 {
> +				reg = <0>;
> +				usb_con_hs: endpoint {
> +					remote-endpoint = <&max77865_usbc_hs>;
> +				};
> +			};
> +			port at 1 {
> +				reg = <1>;
> +				usb_con_ss: endpoint {
> +					remote-endpoint = <&usbdrd_phy_ss>;
> +				};
> +			};
> +			port at 2 {
> +				reg = <2>;
> +				usb_con_sbu: endpoint {
> +					remote-endpoint = <&dp_aux>;
> +				};
> +			};
> +		};
> +	};
> +};


Thanks,

-- 
heikki

[PATCH v5 1/6] dt-bindings: add bindings for USB physical connector

[Andrzej Hajda]

On 02.03.2018 14:13, Heikki Krogerus wrote:
> Hi,
>
> On Tue, Feb 27, 2018 at 08:11:29AM +0100, Andrzej Hajda wrote:
>> +2. USB-C connector attached to CC controller (s2mm005), HS lines routed
>> +to companion PMIC (max77865), SS lines to USB3 PHY and SBU to DisplayPort.
>> +DisplayPort video lines are routed to the connector via SS mux in USB3 PHY.
>> +
>> +ccic: s2mm005 at 33 {
>> +	...
>> +	usb_con: connector {
>> +		compatible = "usb-c-connector";
>> +		label = "USB-C";
> Is this child node really necessary? There will never be more then
> one connector per CC line.

But there can be more connectors/cc-lines per IC, for example EZ-PD CCG5[1].

[1]:
http://www.cypress.com/products/ez-pd-ccg5-two-port-usb-type-c-and-power-delivery

>
> We should prefer device_graph* functions over of_graph* and
I guess you mean fwnode_graph* functions.
> acpi_graph* functions in the drivers so we don't have to handle the
> same thing multiple times with separate APIs. Is it still possible if
> there is that connector child node?

Bindings proposed here are OF bindings, I suppose the most important is
to follow OF specification and guidelines and these bindings tries to
follow it.
It looks like it should not be a problem for fwnode framework to handle
such bindings, but it is just my guess. I have not seen any fwnode*
specification I am not sure what is the real purpose of this framework,
but it seems to be just in-kernel abstraction for different firmware
standards (OF, ACPI), so even if it lacks at the moment some
functionality it should not be a barrier for OF bindings.

Regards
Andrzej

>
>> +		ports {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			port at 0 {
>> +				reg = <0>;
>> +				usb_con_hs: endpoint {
>> +					remote-endpoint = <&max77865_usbc_hs>;
>> +				};
>> +			};
>> +			port at 1 {
>> +				reg = <1>;
>> +				usb_con_ss: endpoint {
>> +					remote-endpoint = <&usbdrd_phy_ss>;
>> +				};
>> +			};
>> +			port at 2 {
>> +				reg = <2>;
>> +				usb_con_sbu: endpoint {
>> +					remote-endpoint = <&dp_aux>;
>> +				};
>> +			};
>> +		};
>> +	};
>> +};
>
> Thanks,
>

[PATCH v5 1/6] dt-bindings: add bindings for USB physical connector

[Heikki Krogerus]

On Mon, Mar 05, 2018 at 09:18:10AM +0100, Andrzej Hajda wrote:
> On 02.03.2018 14:13, Heikki Krogerus wrote:
> > Hi,
> >
> > On Tue, Feb 27, 2018 at 08:11:29AM +0100, Andrzej Hajda wrote:
> >> +2. USB-C connector attached to CC controller (s2mm005), HS lines routed
> >> +to companion PMIC (max77865), SS lines to USB3 PHY and SBU to DisplayPort.
> >> +DisplayPort video lines are routed to the connector via SS mux in USB3 PHY.
> >> +
> >> +ccic: s2mm005 at 33 {
> >> +	...
> >> +	usb_con: connector {
> >> +		compatible = "usb-c-connector";
> >> +		label = "USB-C";
> > Is this child node really necessary? There will never be more then
> > one connector per CC line.
> 
> But there can be more connectors/cc-lines per IC, for example EZ-PD CCG5[1].

OK, in that case the child node is of course needed.

> [1]:
> http://www.cypress.com/products/ez-pd-ccg5-two-port-usb-type-c-and-power-delivery
> 
> >
> > We should prefer device_graph* functions over of_graph* and
> I guess you mean fwnode_graph* functions.

Yes.

> > acpi_graph* functions in the drivers so we don't have to handle the
> > same thing multiple times with separate APIs. Is it still possible if
> > there is that connector child node?
> 
> Bindings proposed here are OF bindings, I suppose the most important is
> to follow OF specification and guidelines and these bindings tries to
> follow it.
> It looks like it should not be a problem for fwnode framework to handle
> such bindings, but it is just my guess. I have not seen any fwnode*
> specification I am not sure what is the real purpose of this framework,
> but it seems to be just in-kernel abstraction for different firmware
> standards (OF, ACPI), so even if it lacks at the moment some
> functionality it should not be a barrier for OF bindings.

Sure thing.


Thanks,

-- 
heikki

[PATCH v5 1/6] dt-bindings: add bindings for USB physical connector

[Rob Herring]

On Tue, Feb 27, 2018 at 08:11:29AM +0100, Andrzej Hajda wrote:
> These bindings allow to describe most known standard USB connectors
> and it should be possible to extend it if necessary.
> USB connectors, beside USB can be used to route other protocols,
> for example UART, Audio, MHL. In such case every device passing data
> through the connector should have appropriate graph bindings.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> v4:
> - improved 'type' description (Rob),
> - improved description of 2nd example (Rob).
> v3:
> - removed MHL port (samsung connector will have separate bindings),
> - added 2nd example for USB-C,
> - improved formatting.
> v2:
> - moved connector type(A,B,C) to compatible string (Rob),
> - renamed size property to type (Rob),
> - changed type description to be less confusing (Laurent),
> - removed vendor specific compatibles (implied by graph port number),
> - added requirement of connector being a child of IC (Rob),
> - removed max-mode (subtly suggested by Rob, it should be detected anyway
>   by USB Controller in runtime, downside is that device is not able to
>   report its real capabilities, maybe better would be to make it optional(?)),
> - assigned port numbers to data buses (Rob).
> 
> Regards
> Andrzej
> ---
>  .../bindings/connector/usb-connector.txt           | 75 ++++++++++++++++++++++
>  1 file changed, 75 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/connector/usb-connector.txt

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH v5 1/6] dt-bindings: add bindings for USB physical connector

[Roger Quadros]

Hi,

On 27/02/18 09:11, Andrzej Hajda wrote:
> These bindings allow to describe most known standard USB connectors
> and it should be possible to extend it if necessary.
> USB connectors, beside USB can be used to route other protocols,
> for example UART, Audio, MHL. In such case every device passing data
> through the connector should have appropriate graph bindings.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> v4:
> - improved 'type' description (Rob),
> - improved description of 2nd example (Rob).
> v3:
> - removed MHL port (samsung connector will have separate bindings),
> - added 2nd example for USB-C,
> - improved formatting.
> v2:
> - moved connector type(A,B,C) to compatible string (Rob),
> - renamed size property to type (Rob),
> - changed type description to be less confusing (Laurent),
> - removed vendor specific compatibles (implied by graph port number),
> - added requirement of connector being a child of IC (Rob),
> - removed max-mode (subtly suggested by Rob, it should be detected anyway
>   by USB Controller in runtime, downside is that device is not able to
>   report its real capabilities, maybe better would be to make it optional(?)),
> - assigned port numbers to data buses (Rob).
> 
> Regards
> Andrzej
> ---
>  .../bindings/connector/usb-connector.txt           | 75 ++++++++++++++++++++++
>  1 file changed, 75 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/connector/usb-connector.txt
> 
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt b/Documentation/devicetree/bindings/connector/usb-connector.txt
> new file mode 100644
> index 000000000000..e1463f14af38
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> @@ -0,0 +1,75 @@
> +USB Connector
> +=============
> +
> +USB connector node represents physical USB connector. It should be
> +a child of USB interface controller.
> +
> +Required properties:
> +- compatible: describes type of the connector, must be one of:
> +    "usb-a-connector",
> +    "usb-b-connector",
> +    "usb-c-connector".

compatible should be just "usb-connector"

Type should be a property

type: type of usb connector "A", "B", "AB", "C"
AB is for dual-role connectors.

micro super-speed and high-speed connectors are different. How do you differentiate that?

> +
> +Optional properties:
> +- label: symbolic name for the connector,

Why do you need label? We can't maintain consistency as people will put creative names there.
Device/bus driver could generate a valid label.

> +- type: size of the connector, should be specified in case of USB-A, USB-B
> +  non-fullsize connectors: "mini", "micro".

type is misleading. Type is usually A/B/C. It should be size here instead.

size: size of the connector if not standard size. "mini", "micro"

If not specified it is treated as standard sized connector.
e.g. for Type-C there is no mini/micro. so size doesn't have to be specificed

What about Type-C connector?
> +
> +Required nodes:
> +- any data bus to the connector should be modeled using the OF graph bindings

s/modeled/modelled

> +  specified in bindings/graph.txt, unless the bus is between parent node and
> +  the connector. Since single connector can have multpile data buses every bus

s/multpile/multiple

> +  has assigned OF graph port number as follows:
> +    0: High Speed (HS), present in all connectors,
> +    1: Super Speed (SS), present in SS capable connectors,
> +    2: Sideband use (SBU), present in USB-C.
> +
> +Examples
> +--------
> +
> +1. Micro-USB connector with HS lines routed via controller (MUIC):
> +
> +muic-max77843 at 66 {
> +	...
> +	usb_con: connector {
> +		compatible = "usb-b-connector";
> +		label = "micro-USB";
> +		type = "micro";
> +	};
> +};
> +
> +2. USB-C connector attached to CC controller (s2mm005), HS lines routed
> +to companion PMIC (max77865), SS lines to USB3 PHY and SBU to DisplayPort.
> +DisplayPort video lines are routed to the connector via SS mux in USB3 PHY.
> +
> +ccic: s2mm005 at 33 {
> +	...
> +	usb_con: connector {
> +		compatible = "usb-c-connector";
> +		label = "USB-C";

The label is not consistent with the earlier example.

> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port at 0 {
> +				reg = <0>;
> +				usb_con_hs: endpoint {
> +					remote-endpoint = <&max77865_usbc_hs>;
> +				};
> +			};
> +			port at 1 {
> +				reg = <1>;
> +				usb_con_ss: endpoint {
> +					remote-endpoint = <&usbdrd_phy_ss>;
> +				};
> +			};
> +			port at 2 {
> +				reg = <2>;
> +				usb_con_sbu: endpoint {
> +					remote-endpoint = <&dp_aux>;
> +				};
> +			};
> +		};
> +	};
> +};
> 

-- 
cheers,
-roger

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[PATCH v5 1/6] dt-bindings: add bindings for USB physical connector

[Andrzej Hajda]

On 09.03.2018 11:24, Roger Quadros wrote:
> Hi,
>
> On 27/02/18 09:11, Andrzej Hajda wrote:
>> These bindings allow to describe most known standard USB connectors
>> and it should be possible to extend it if necessary.
>> USB connectors, beside USB can be used to route other protocols,
>> for example UART, Audio, MHL. In such case every device passing data
>> through the connector should have appropriate graph bindings.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>> v4:
>> - improved 'type' description (Rob),
>> - improved description of 2nd example (Rob).
>> v3:
>> - removed MHL port (samsung connector will have separate bindings),
>> - added 2nd example for USB-C,
>> - improved formatting.
>> v2:
>> - moved connector type(A,B,C) to compatible string (Rob),
>> - renamed size property to type (Rob),
>> - changed type description to be less confusing (Laurent),
>> - removed vendor specific compatibles (implied by graph port number),
>> - added requirement of connector being a child of IC (Rob),
>> - removed max-mode (subtly suggested by Rob, it should be detected anyway
>>   by USB Controller in runtime, downside is that device is not able to
>>   report its real capabilities, maybe better would be to make it optional(?)),
>> - assigned port numbers to data buses (Rob).
>>
>> Regards
>> Andrzej
>> ---
>>  .../bindings/connector/usb-connector.txt           | 75 ++++++++++++++++++++++
>>  1 file changed, 75 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/connector/usb-connector.txt
>>
>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt b/Documentation/devicetree/bindings/connector/usb-connector.txt
>> new file mode 100644
>> index 000000000000..e1463f14af38
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
>> @@ -0,0 +1,75 @@
>> +USB Connector
>> +=============
>> +
>> +USB connector node represents physical USB connector. It should be
>> +a child of USB interface controller.
>> +
>> +Required properties:
>> +- compatible: describes type of the connector, must be one of:
>> +    "usb-a-connector",
>> +    "usb-b-connector",
>> +    "usb-c-connector".
> compatible should be just "usb-connector"
>
> Type should be a property
>
> type: type of usb connector "A", "B", "AB", "C"
> AB is for dual-role connectors.

I have proposed such property (and size also) in my first RFC [1]. Rod
did not like it :)

[1]: https://marc.info/?l=devicetree&m=150660411515233&w=2


>
> micro super-speed and high-speed connectors are different. How do you differentiate that?

I am aware of it, and property differentiating it was also in my earlier
iterations.
If there will be need to differentiate such connectors, adding property
max-speed or max-mode would do the thing.

>
>> +
>> +Optional properties:
>> +- label: symbolic name for the connector,
> Why do you need label? We can't maintain consistency as people will put creative names there.
> Device/bus driver could generate a valid label.

It follows convention for other connectors: HDMI, DVI, ....

>
>> +- type: size of the connector, should be specified in case of USB-A, USB-B
>> +  non-fullsize connectors: "mini", "micro".
> type is misleading. Type is usually A/B/C. It should be size here instead.

As you can see in discussion for previous iterations it follows
convention already established for other connectors.

>
> size: size of the connector if not standard size. "mini", "micro"
>
> If not specified it is treated as standard sized connector.
> e.g. for Type-C there is no mini/micro. so size doesn't have to be specificed
Few lines above it is described: should be specified in case of USB-A,
USB-B non-fullsize connectors.

>
> What about Type-C connector?
>> +
>> +Required nodes:
>> +- any data bus to the connector should be modeled using the OF graph bindings
> s/modeled/modelled

>> +  specified in bindings/graph.txt, unless the bus is between parent node and
>> +  the connector. Since single connector can have multpile data buses every bus
> s/multpile/multiple

OK, I will fix both typos.

>
>> +  has assigned OF graph port number as follows:
>> +    0: High Speed (HS), present in all connectors,
>> +    1: Super Speed (SS), present in SS capable connectors,
>> +    2: Sideband use (SBU), present in USB-C.
>> +
>> +Examples
>> +--------
>> +
>> +1. Micro-USB connector with HS lines routed via controller (MUIC):
>> +
>> +muic-max77843 at 66 {
>> +	...
>> +	usb_con: connector {
>> +		compatible = "usb-b-connector";
>> +		label = "micro-USB";
>> +		type = "micro";
>> +	};
>> +};
>> +
>> +2. USB-C connector attached to CC controller (s2mm005), HS lines routed
>> +to companion PMIC (max77865), SS lines to USB3 PHY and SBU to DisplayPort.
>> +DisplayPort video lines are routed to the connector via SS mux in USB3 PHY.
>> +
>> +ccic: s2mm005 at 33 {
>> +	...
>> +	usb_con: connector {
>> +		compatible = "usb-c-connector";
>> +		label = "USB-C";
> The label is not consistent with the earlier example.

Why?

Regards
Andrzej

>
>> +
>> +		ports {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			port at 0 {
>> +				reg = <0>;
>> +				usb_con_hs: endpoint {
>> +					remote-endpoint = <&max77865_usbc_hs>;
>> +				};
>> +			};
>> +			port at 1 {
>> +				reg = <1>;
>> +				usb_con_ss: endpoint {
>> +					remote-endpoint = <&usbdrd_phy_ss>;
>> +				};
>> +			};
>> +			port at 2 {
>> +				reg = <2>;
>> +				usb_con_sbu: endpoint {
>> +					remote-endpoint = <&dp_aux>;
>> +				};
>> +			};
>> +		};
>> +	};
>> +};
>>

[PATCH v5 1/6] dt-bindings: add bindings for USB physical connector

[Andrzej Hajda]

On 12.03.2018 08:02, Andrzej Hajda wrote:
> On 09.03.2018 11:24, Roger Quadros wrote:
>> Hi,
>>
>> On 27/02/18 09:11, Andrzej Hajda wrote:
>>> These bindings allow to describe most known standard USB connectors
>>> and it should be possible to extend it if necessary.
>>> USB connectors, beside USB can be used to route other protocols,
>>> for example UART, Audio, MHL. In such case every device passing data
>>> through the connector should have appropriate graph bindings.
>>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>> v4:
>>> - improved 'type' description (Rob),
>>> - improved description of 2nd example (Rob).
>>> v3:
>>> - removed MHL port (samsung connector will have separate bindings),
>>> - added 2nd example for USB-C,
>>> - improved formatting.
>>> v2:
>>> - moved connector type(A,B,C) to compatible string (Rob),
>>> - renamed size property to type (Rob),
>>> - changed type description to be less confusing (Laurent),
>>> - removed vendor specific compatibles (implied by graph port number),
>>> - added requirement of connector being a child of IC (Rob),
>>> - removed max-mode (subtly suggested by Rob, it should be detected anyway
>>>   by USB Controller in runtime, downside is that device is not able to
>>>   report its real capabilities, maybe better would be to make it optional(?)),
>>> - assigned port numbers to data buses (Rob).
>>>
>>> Regards
>>> Andrzej
>>> ---
>>>  .../bindings/connector/usb-connector.txt           | 75 ++++++++++++++++++++++
>>>  1 file changed, 75 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/connector/usb-connector.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt b/Documentation/devicetree/bindings/connector/usb-connector.txt
>>> new file mode 100644
>>> index 000000000000..e1463f14af38
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
>>> @@ -0,0 +1,75 @@
>>> +USB Connector
>>> +=============
>>> +
>>> +USB connector node represents physical USB connector. It should be
>>> +a child of USB interface controller.
>>> +
>>> +Required properties:
>>> +- compatible: describes type of the connector, must be one of:
>>> +    "usb-a-connector",
>>> +    "usb-b-connector",
>>> +    "usb-c-connector".
>> compatible should be just "usb-connector"
>>
>> Type should be a property
>>
>> type: type of usb connector "A", "B", "AB", "C"
>> AB is for dual-role connectors.
> I have proposed such property (and size also) in my first RFC [1]. Rod
> did not like it :)

Ups, ugly typo, I meant Rob, of course.

Regards
Andrzej

>
> [1]: https://marc.info/?l=devicetree&m=150660411515233&w=2
>
>
>> micro super-speed and high-speed connectors are different. How do you differentiate that?
> I am aware of it, and property differentiating it was also in my earlier
> iterations.
> If there will be need to differentiate such connectors, adding property
> max-speed or max-mode would do the thing.
>
>>> +
>>> +Optional properties:
>>> +- label: symbolic name for the connector,
>> Why do you need label? We can't maintain consistency as people will put creative names there.
>> Device/bus driver could generate a valid label.
> It follows convention for other connectors: HDMI, DVI, ....
>
>>> +- type: size of the connector, should be specified in case of USB-A, USB-B
>>> +  non-fullsize connectors: "mini", "micro".
>> type is misleading. Type is usually A/B/C. It should be size here instead.
> As you can see in discussion for previous iterations it follows
> convention already established for other connectors.
>
>> size: size of the connector if not standard size. "mini", "micro"
>>
>> If not specified it is treated as standard sized connector.
>> e.g. for Type-C there is no mini/micro. so size doesn't have to be specificed
> Few lines above it is described: should be specified in case of USB-A,
> USB-B non-fullsize connectors.
>
>> What about Type-C connector?
>>> +
>>> +Required nodes:
>>> +- any data bus to the connector should be modeled using the OF graph bindings
>> s/modeled/modelled
>>> +  specified in bindings/graph.txt, unless the bus is between parent node and
>>> +  the connector. Since single connector can have multpile data buses every bus
>> s/multpile/multiple
> OK, I will fix both typos.
>
>>> +  has assigned OF graph port number as follows:
>>> +    0: High Speed (HS), present in all connectors,
>>> +    1: Super Speed (SS), present in SS capable connectors,
>>> +    2: Sideband use (SBU), present in USB-C.
>>> +
>>> +Examples
>>> +--------
>>> +
>>> +1. Micro-USB connector with HS lines routed via controller (MUIC):
>>> +
>>> +muic-max77843 at 66 {
>>> +	...
>>> +	usb_con: connector {
>>> +		compatible = "usb-b-connector";
>>> +		label = "micro-USB";
>>> +		type = "micro";
>>> +	};
>>> +};
>>> +
>>> +2. USB-C connector attached to CC controller (s2mm005), HS lines routed
>>> +to companion PMIC (max77865), SS lines to USB3 PHY and SBU to DisplayPort.
>>> +DisplayPort video lines are routed to the connector via SS mux in USB3 PHY.
>>> +
>>> +ccic: s2mm005 at 33 {
>>> +	...
>>> +	usb_con: connector {
>>> +		compatible = "usb-c-connector";
>>> +		label = "USB-C";
>> The label is not consistent with the earlier example.
> Why?
>
> Regards
> Andrzej
>
>>> +
>>> +		ports {
>>> +			#address-cells = <1>;
>>> +			#size-cells = <0>;
>>> +
>>> +			port at 0 {
>>> +				reg = <0>;
>>> +				usb_con_hs: endpoint {
>>> +					remote-endpoint = <&max77865_usbc_hs>;
>>> +				};
>>> +			};
>>> +			port at 1 {
>>> +				reg = <1>;
>>> +				usb_con_ss: endpoint {
>>> +					remote-endpoint = <&usbdrd_phy_ss>;
>>> +				};
>>> +			};
>>> +			port at 2 {
>>> +				reg = <2>;
>>> +				usb_con_sbu: endpoint {
>>> +					remote-endpoint = <&dp_aux>;
>>> +				};
>>> +			};
>>> +		};
>>> +	};
>>> +};
>>>

[PATCH v5 1/6] dt-bindings: add bindings for USB physical connector

[Roger Quadros]

Andrezej,

Why don't you have any of the USB maintainers in to/cc?

Greg Kroah-Hartman <gregkh@linuxfoundation.org> (supporter:USB SUBSYSTEM)
Felipe Balbi <balbi@kernel.org> (maintainer:USB GADGET/PERIPHERAL SUBSYSTEM)

On 12/03/18 09:02, Andrzej Hajda wrote:
> On 09.03.2018 11:24, Roger Quadros wrote:
>> Hi,
>>
>> On 27/02/18 09:11, Andrzej Hajda wrote:
>>> These bindings allow to describe most known standard USB connectors
>>> and it should be possible to extend it if necessary.
>>> USB connectors, beside USB can be used to route other protocols,
>>> for example UART, Audio, MHL. In such case every device passing data
>>> through the connector should have appropriate graph bindings.
>>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>> v4:
>>> - improved 'type' description (Rob),
>>> - improved description of 2nd example (Rob).
>>> v3:
>>> - removed MHL port (samsung connector will have separate bindings),
>>> - added 2nd example for USB-C,
>>> - improved formatting.
>>> v2:
>>> - moved connector type(A,B,C) to compatible string (Rob),
>>> - renamed size property to type (Rob),
>>> - changed type description to be less confusing (Laurent),
>>> - removed vendor specific compatibles (implied by graph port number),
>>> - added requirement of connector being a child of IC (Rob),
>>> - removed max-mode (subtly suggested by Rob, it should be detected anyway
>>>   by USB Controller in runtime, downside is that device is not able to
>>>   report its real capabilities, maybe better would be to make it optional(?)),
>>> - assigned port numbers to data buses (Rob).
>>>
>>> Regards
>>> Andrzej
>>> ---
>>>  .../bindings/connector/usb-connector.txt           | 75 ++++++++++++++++++++++
>>>  1 file changed, 75 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/connector/usb-connector.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt b/Documentation/devicetree/bindings/connector/usb-connector.txt
>>> new file mode 100644
>>> index 000000000000..e1463f14af38
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt

Should this lie in  bindings/usb/connector?

>>> @@ -0,0 +1,75 @@
>>> +USB Connector
>>> +=============
>>> +
>>> +USB connector node represents physical USB connector. It should be
>>> +a child of USB interface controller.
>>> +
>>> +Required properties:
>>> +- compatible: describes type of the connector, must be one of:
>>> +    "usb-a-connector",
>>> +    "usb-b-connector",
>>> +    "usb-c-connector".
>> compatible should be just "usb-connector"
>>
>> Type should be a property
>>
>> type: type of usb connector "A", "B", "AB", "C"
>> AB is for dual-role connectors.
> 
> I have proposed such property (and size also) in my first RFC [1]. Rod
> did not like it :)
> 
> [1]: https://marc.info/?l=devicetree&m=150660411515233&w=2
> 

This is what Rob says here https://patchwork.kernel.org/patch/9976043/
"We did "type" for hdmi-connector, but I think I'd really prefer 
compatible be used to distinguish as least where it may matter to s/w. 
In the HDMI case, they all are pretty much the same, just different 
physical size."

So the question is. Does it matter to this particular software implementation
if it is type A,B,C connector?
If yes, how?

Type A will never have any alternate function. It is always dedicated to USB.

Also does the size "full", "micro", "mini" matter to software?

> 
>>
>> micro super-speed and high-speed connectors are different. How do you differentiate that?
> 
> I am aware of it, and property differentiating it was also in my earlier
> iterations.
> If there will be need to differentiate such connectors, adding property
> max-speed or max-mode would do the thing.

USB controller binding (bindings/usb/generic.txt) has the speed.
I don't think there is any value for replicating it for the connector. Maybe it could
be added later if needed.

> 
>>
>>> +
>>> +Optional properties:
>>> +- label: symbolic name for the connector,
>> Why do you need label? We can't maintain consistency as people will put creative names there.
>> Device/bus driver could generate a valid label.
> 
> It follows convention for other connectors: HDMI, DVI, ....
> 
>>
>>> +- type: size of the connector, should be specified in case of USB-A, USB-B
>>> +  non-fullsize connectors: "mini", "micro".
>> type is misleading. Type is usually A/B/C. It should be size here instead.
> 
> As you can see in discussion for previous iterations it follows
> convention already established for other connectors.
> 
>>
>> size: size of the connector if not standard size. "mini", "micro"
>>
>> If not specified it is treated as standard sized connector.
>> e.g. for Type-C there is no mini/micro. so size doesn't have to be specificed
> Few lines above it is described: should be specified in case of USB-A,
> USB-B non-fullsize connectors.
> 
>>
>> What about Type-C connector?
>>> +
>>> +Required nodes:
>>> +- any data bus to the connector should be modeled using the OF graph bindings
>> s/modeled/modelled
> 
>>> +  specified in bindings/graph.txt, unless the bus is between parent node and
>>> +  the connector. Since single connector can have multpile data buses every bus
>> s/multpile/multiple
> 
> OK, I will fix both typos.
> 
>>
>>> +  has assigned OF graph port number as follows:
>>> +    0: High Speed (HS), present in all connectors,
>>> +    1: Super Speed (SS), present in SS capable connectors,
>>> +    2: Sideband use (SBU), present in USB-C.
>>> +
>>> +Examples
>>> +--------
>>> +
>>> +1. Micro-USB connector with HS lines routed via controller (MUIC):
>>> +
>>> +muic-max77843 at 66 {
>>> +	...
>>> +	usb_con: connector {
>>> +		compatible = "usb-b-connector";
>>> +		label = "micro-USB";
>>> +		type = "micro";
>>> +	};
>>> +};
>>> +
>>> +2. USB-C connector attached to CC controller (s2mm005), HS lines routed
>>> +to companion PMIC (max77865), SS lines to USB3 PHY and SBU to DisplayPort.
>>> +DisplayPort video lines are routed to the connector via SS mux in USB3 PHY.
>>> +
>>> +ccic: s2mm005 at 33 {
>>> +	...
>>> +	usb_con: connector {
>>> +		compatible = "usb-c-connector";
>>> +		label = "USB-C";
>> The label is not consistent with the earlier example.
> 
> Why?

Because 1st example is "<size>-USB" and second one is "USB-<type>".

How is label going to be used? Is it being presented to end user?
If yes it should indicate what's important to the user. i.e. its function. e.g. "USB-MHL"
or "USB-DisplayPort"

> 
> Regards
> Andrzej
> 
>>
>>> +
>>> +		ports {
>>> +			#address-cells = <1>;
>>> +			#size-cells = <0>;
>>> +
>>> +			port at 0 {
>>> +				reg = <0>;
>>> +				usb_con_hs: endpoint {
>>> +					remote-endpoint = <&max77865_usbc_hs>;
>>> +				};
>>> +			};
>>> +			port at 1 {
>>> +				reg = <1>;
>>> +				usb_con_ss: endpoint {
>>> +					remote-endpoint = <&usbdrd_phy_ss>;
>>> +				};
>>> +			};
>>> +			port at 2 {
>>> +				reg = <2>;
>>> +				usb_con_sbu: endpoint {
>>> +					remote-endpoint = <&dp_aux>;
>>> +				};
>>> +			};
>>> +		};
>>> +	};
>>> +};
>>>
> 

-- 
cheers,
-roger

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[PATCH v5 1/6] dt-bindings: add bindings for USB physical connector

[Andrzej Hajda]

On 12.03.2018 11:41, Roger Quadros wrote:
> Andrezej,
>
> Why don't you have any of the USB maintainers in to/cc?
>
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> (supporter:USB SUBSYSTEM)
> Felipe Balbi <balbi@kernel.org> (maintainer:USB GADGET/PERIPHERAL SUBSYSTEM)

Serious omission, sorry for that.

>
> On 12/03/18 09:02, Andrzej Hajda wrote:
>> On 09.03.2018 11:24, Roger Quadros wrote:
>>> Hi,
>>>
>>> On 27/02/18 09:11, Andrzej Hajda wrote:
>>>> These bindings allow to describe most known standard USB connectors
>>>> and it should be possible to extend it if necessary.
>>>> USB connectors, beside USB can be used to route other protocols,
>>>> for example UART, Audio, MHL. In such case every device passing data
>>>> through the connector should have appropriate graph bindings.
>>>>
>>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>>> ---
>>>> v4:
>>>> - improved 'type' description (Rob),
>>>> - improved description of 2nd example (Rob).
>>>> v3:
>>>> - removed MHL port (samsung connector will have separate bindings),
>>>> - added 2nd example for USB-C,
>>>> - improved formatting.
>>>> v2:
>>>> - moved connector type(A,B,C) to compatible string (Rob),
>>>> - renamed size property to type (Rob),
>>>> - changed type description to be less confusing (Laurent),
>>>> - removed vendor specific compatibles (implied by graph port number),
>>>> - added requirement of connector being a child of IC (Rob),
>>>> - removed max-mode (subtly suggested by Rob, it should be detected anyway
>>>>   by USB Controller in runtime, downside is that device is not able to
>>>>   report its real capabilities, maybe better would be to make it optional(?)),
>>>> - assigned port numbers to data buses (Rob).
>>>>
>>>> Regards
>>>> Andrzej
>>>> ---
>>>>  .../bindings/connector/usb-connector.txt           | 75 ++++++++++++++++++++++
>>>>  1 file changed, 75 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/connector/usb-connector.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt b/Documentation/devicetree/bindings/connector/usb-connector.txt
>>>> new file mode 100644
>>>> index 000000000000..e1463f14af38
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> Should this lie in  bindings/usb/connector?


I guess this is question for DT people. For me both locations are OK.

>
>>>> @@ -0,0 +1,75 @@
>>>> +USB Connector
>>>> +=============
>>>> +
>>>> +USB connector node represents physical USB connector. It should be
>>>> +a child of USB interface controller.
>>>> +
>>>> +Required properties:
>>>> +- compatible: describes type of the connector, must be one of:
>>>> +    "usb-a-connector",
>>>> +    "usb-b-connector",
>>>> +    "usb-c-connector".
>>> compatible should be just "usb-connector"
>>>
>>> Type should be a property
>>>
>>> type: type of usb connector "A", "B", "AB", "C"
>>> AB is for dual-role connectors.
>> I have proposed such property (and size also) in my first RFC [1]. Rod
>> did not like it :)
>>
>> [1]: https://marc.info/?l=devicetree&m=150660411515233&w=2
>>
> This is what Rob says here https://patchwork.kernel.org/patch/9976043/
> "We did "type" for hdmi-connector, but I think I'd really prefer 
> compatible be used to distinguish as least where it may matter to s/w. 
> In the HDMI case, they all are pretty much the same, just different 
> physical size."
>
> So the question is. Does it matter to this particular software implementation
> if it is type A,B,C connector?
> If yes, how?

IMHO both approaches can work from S/W POV. As I understood it moving
usb type to compatible was rather matter of devicetree guidelines I am
not aware of. Again question to Rob.

>
> Type A will never have any alternate function. It is always dedicated to USB.
>
> Also does the size "full", "micro", "mini" matter to software?
>
>>> micro super-speed and high-speed connectors are different. How do you differentiate that?
>> I am aware of it, and property differentiating it was also in my earlier
>> iterations.
>> If there will be need to differentiate such connectors, adding property
>> max-speed or max-mode would do the thing.
> USB controller binding (bindings/usb/generic.txt) has the speed.
> I don't think there is any value for replicating it for the connector. Maybe it could
> be added later if needed.
>
>>>> +
>>>> +Optional properties:
>>>> +- label: symbolic name for the connector,
>>> Why do you need label? We can't maintain consistency as people will put creative names there.
>>> Device/bus driver could generate a valid label.
>> It follows convention for other connectors: HDMI, DVI, ....
>>
>>>> +- type: size of the connector, should be specified in case of USB-A, USB-B
>>>> +  non-fullsize connectors: "mini", "micro".
>>> type is misleading. Type is usually A/B/C. It should be size here instead.
>> As you can see in discussion for previous iterations it follows
>> convention already established for other connectors.
>>
>>> size: size of the connector if not standard size. "mini", "micro"
>>>
>>> If not specified it is treated as standard sized connector.
>>> e.g. for Type-C there is no mini/micro. so size doesn't have to be specificed
>> Few lines above it is described: should be specified in case of USB-A,
>> USB-B non-fullsize connectors.
>>
>>> What about Type-C connector?
>>>> +
>>>> +Required nodes:
>>>> +- any data bus to the connector should be modeled using the OF graph bindings
>>> s/modeled/modelled
>>>> +  specified in bindings/graph.txt, unless the bus is between parent node and
>>>> +  the connector. Since single connector can have multpile data buses every bus
>>> s/multpile/multiple
>> OK, I will fix both typos.
>>
>>>> +  has assigned OF graph port number as follows:
>>>> +    0: High Speed (HS), present in all connectors,
>>>> +    1: Super Speed (SS), present in SS capable connectors,
>>>> +    2: Sideband use (SBU), present in USB-C.
>>>> +
>>>> +Examples
>>>> +--------
>>>> +
>>>> +1. Micro-USB connector with HS lines routed via controller (MUIC):
>>>> +
>>>> +muic-max77843 at 66 {
>>>> +	...
>>>> +	usb_con: connector {
>>>> +		compatible = "usb-b-connector";
>>>> +		label = "micro-USB";
>>>> +		type = "micro";
>>>> +	};
>>>> +};
>>>> +
>>>> +2. USB-C connector attached to CC controller (s2mm005), HS lines routed
>>>> +to companion PMIC (max77865), SS lines to USB3 PHY and SBU to DisplayPort.
>>>> +DisplayPort video lines are routed to the connector via SS mux in USB3 PHY.
>>>> +
>>>> +ccic: s2mm005 at 33 {
>>>> +	...
>>>> +	usb_con: connector {
>>>> +		compatible = "usb-c-connector";
>>>> +		label = "USB-C";
>>> The label is not consistent with the earlier example.
>> Why?
> Because 1st example is "<size>-USB" and second one is "USB-<type>".
>
> How is label going to be used? Is it being presented to end user?

I suppose it could be presented to user, and not-interpreted by S/W.

> If yes it should indicate what's important to the user. i.e. its function. e.g. "USB-MHL"
> or "USB-DisplayPort"

Good idea, to emphasize ports capabilities. I guess it could be most
useful in case of multiple USB ports, they could be then named according
to their visible properties/labels, for example: Front-USB, Green-USB,
USB-1.

Regards
Andrzej

>
>> Regards
>> Andrzej
>>
>>>> +
>>>> +		ports {
>>>> +			#address-cells = <1>;
>>>> +			#size-cells = <0>;
>>>> +
>>>> +			port at 0 {
>>>> +				reg = <0>;
>>>> +				usb_con_hs: endpoint {
>>>> +					remote-endpoint = <&max77865_usbc_hs>;
>>>> +				};
>>>> +			};
>>>> +			port at 1 {
>>>> +				reg = <1>;
>>>> +				usb_con_ss: endpoint {
>>>> +					remote-endpoint = <&usbdrd_phy_ss>;
>>>> +				};
>>>> +			};
>>>> +			port at 2 {
>>>> +				reg = <2>;
>>>> +				usb_con_sbu: endpoint {
>>>> +					remote-endpoint = <&dp_aux>;
>>>> +				};
>>>> +			};
>>>> +		};
>>>> +	};
>>>> +};
>>>>

[PATCH v5 1/6] dt-bindings: add bindings for USB physical connector

[Robin Murphy]

On 12/03/18 10:41, Roger Quadros wrote:
[...]
>>>> @@ -0,0 +1,75 @@
>>>> +USB Connector
>>>> +=============
>>>> +
>>>> +USB connector node represents physical USB connector. It should be
>>>> +a child of USB interface controller.
>>>> +
>>>> +Required properties:
>>>> +- compatible: describes type of the connector, must be one of:
>>>> +    "usb-a-connector",
>>>> +    "usb-b-connector",
>>>> +    "usb-c-connector".
>>> compatible should be just "usb-connector"
>>>
>>> Type should be a property
>>>
>>> type: type of usb connector "A", "B", "AB", "C"
>>> AB is for dual-role connectors.
>>
>> I have proposed such property (and size also) in my first RFC [1]. Rod
>> did not like it :)
>>
>> [1]: https://marc.info/?l=devicetree&m=150660411515233&w=2
>>
> 
> This is what Rob says here https://patchwork.kernel.org/patch/9976043/
> "We did "type" for hdmi-connector, but I think I'd really prefer
> compatible be used to distinguish as least where it may matter to s/w.
> In the HDMI case, they all are pretty much the same, just different
> physical size."
> 
> So the question is. Does it matter to this particular software implementation
> if it is type A,B,C connector?
> If yes, how?
> 
> Type A will never have any alternate function. It is always dedicated to USB.

In USB spec terms, at least. In reality there are things like the cool 
trick Rockchip SoCs do whereby they can expose the debug UART Rx/Tx 
through the OTG port's D+/D- pins, and that is on a type A connector in 
many products. I'm guessing that's probably beyond the scope of this 
binding, though.

> Also does the size "full", "micro", "mini" matter to software?

If it means the user can look in sysfs to easily correlate logical ports 
with physical connectors that's certainly handy (e.g. on something like 
Odroid-XU where the two USB3 ports are brought out to an A and a 
micro-AB connector respectively).

Robin.

[PATCH v5 1/6] dt-bindings: add bindings for USB physical connector

[Roger Quadros]

On 15/03/18 13:46, Robin Murphy wrote:
> On 12/03/18 10:41, Roger Quadros wrote:
> [...]
>>>>> @@ -0,0 +1,75 @@
>>>>> +USB Connector
>>>>> +=============
>>>>> +
>>>>> +USB connector node represents physical USB connector. It should be
>>>>> +a child of USB interface controller.
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible: describes type of the connector, must be one of:
>>>>> +??? "usb-a-connector",
>>>>> +??? "usb-b-connector",
>>>>> +??? "usb-c-connector".
>>>> compatible should be just "usb-connector"
>>>>
>>>> Type should be a property
>>>>
>>>> type: type of usb connector "A", "B", "AB", "C"
>>>> AB is for dual-role connectors.
>>>
>>> I have proposed such property (and size also) in my first RFC [1]. Rod
>>> did not like it :)
>>>
>>> [1]: https://marc.info/?l=devicetree&m=150660411515233&w=2
>>>
>>
>> This is what Rob says here https://patchwork.kernel.org/patch/9976043/
>> "We did "type" for hdmi-connector, but I think I'd really prefer
>> compatible be used to distinguish as least where it may matter to s/w.
>> In the HDMI case, they all are pretty much the same, just different
>> physical size."
>>
>> So the question is. Does it matter to this particular software implementation
>> if it is type A,B,C connector?
>> If yes, how?
>>
>> Type A will never have any alternate function. It is always dedicated to USB.
> 
> In USB spec terms, at least. In reality there are things like the cool trick Rockchip SoCs do whereby they can expose the debug UART Rx/Tx through the OTG port's D+/D- pins, and that is on a type A connector in many products. I'm guessing that's probably beyond the scope of this binding, though.
> 
>> Also does the size "full", "micro", "mini" matter to software?
> 
> If it means the user can look in sysfs to easily correlate logical ports with physical connectors that's certainly handy (e.g. on something like Odroid-XU where the two USB3 ports are brought out to an A and a micro-AB connector respectively).

But this logic fails if both connectors are the same type/size.
This is where the label comes in handy. The labels can be unique and end user can identify the port using that.

> 
> Robin.

-- 
cheers,
-roger

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[PATCH v5 2/6] dt-bindings: add bindings for Samsung micro-USB 11-pin connector

[Andrzej Hajda]

Samsung micro-USB 11-pin connector beside standard micro-USB pins,
has pins dedicated to route MHL traffic.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
v4:
- removed description of property inherited from usb-connector (Rob),
- fixed example description.
---
 .../connector/samsung,usb-connector-11pin.txt      | 49 ++++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/connector/samsung,usb-connector-11pin.txt

diff --git a/Documentation/devicetree/bindings/connector/samsung,usb-connector-11pin.txt b/Documentation/devicetree/bindings/connector/samsung,usb-connector-11pin.txt
new file mode 100644
index 000000000000..22256e295a7a
--- /dev/null
+++ b/Documentation/devicetree/bindings/connector/samsung,usb-connector-11pin.txt
@@ -0,0 +1,49 @@
+Samsung micro-USB 11-pin connector
+==================================
+
+Samsung micro-USB 11-pin connector is an extension of micro-USB connector.
+It is present in multiple Samsung mobile devices.
+It has additional pins to route MHL traffic simultanously with USB.
+
+The bindings are superset of usb-connector bindings for micro-USB connector[1].
+
+Required properties:
+- compatible: must be: "samsung,usb-connector-11pin", "usb-b-connector",
+- type: must be "micro".
+
+Required nodes:
+- any data bus to the connector should be modeled using the OF graph bindings
+  specified in bindings/graph.txt, unless the bus is between parent node and
+  the connector. Since single connector can have multpile data buses every bus
+  has assigned OF graph port number as follows:
+    0: High Speed (HS),
+    3: Mobile High-Definition Link (MHL), specific to 11-pin Samsung micro-USB.
+
+[1]: bindings/connector/usb-connector.txt
+
+Example
+-------
+
+Micro-USB connector with HS lines routed via controller (MUIC) and MHL lines
+connected to HDMI-MHL bridge (sii8620):
+
+muic-max77843 at 66 {
+	...
+	usb_con: connector {
+		compatible = "samsung,usb-connector-11pin", "usb-b-connector";
+		label = "micro-USB";
+		type = "micro";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port at 3 {
+				reg = <3>;
+				usb_con_mhl: endpoint {
+					remote-endpoint = <&sii8620_mhl>;
+				};
+			};
+		};
+	};
+};
-- 
2.16.2

[PATCH v5 3/6] arm64: dts: exynos: add micro-USB connector node to TM2 platforms

[Andrzej Hajda]

Since USB connector bindings are available we can describe it on TM2(e).

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
index 0b61dda99569..f604f6b1a9c2 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
@@ -836,6 +836,13 @@
 
 		muic: max77843-muic {
 			compatible = "maxim,max77843-muic";
+
+			musb_con: musb_connector {
+				compatible = "samsung,usb-connector-11pin",
+					     "usb-b-connector";
+				label = "micro-USB";
+				type = "micro";
+			};
 		};
 
 		regulators {
-- 
2.16.2

[PATCH v5 3/6] arm64: dts: exynos: add micro-USB connector node to TM2 platforms

[Krzysztof Kozlowski]

On Tue, Feb 27, 2018 at 08:11:31AM +0100, Andrzej Hajda wrote:
> Since USB connector bindings are available we can describe it on TM2(e).
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 7 +++++++
>  1 file changed, 7 insertions(+)
> 

Thanks, applied.

Best regards,
Krzysztof

[PATCH v5 4/6] arm64: dts: exynos: add OF graph between MHL and USB connector

[Andrzej Hajda]

OF graph describes MHL data lanes between MHL and respective USB
connector.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
v5: removed extra parenthesis (kbuild test robot)
v4: added missing reg property in connector's port node (Krzysztof)
---
 .../boot/dts/exynos/exynos5433-tm2-common.dtsi     | 31 +++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
index f604f6b1a9c2..03453b822093 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
@@ -817,9 +817,22 @@
 		clocks = <&pmu_system_controller 0>;
 		clock-names = "xtal";
 
-		port {
-			mhl_to_hdmi: endpoint {
-				remote-endpoint = <&hdmi_to_mhl>;
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port at 0 {
+				reg = <0>;
+				mhl_to_hdmi: endpoint {
+					remote-endpoint = <&hdmi_to_mhl>;
+				};
+			};
+
+			port at 1 {
+				reg = <1>;
+				mhl_to_musb_con: endpoint {
+					remote-endpoint = <&musb_con_to_mhl>;
+				};
 			};
 		};
 	};
@@ -842,6 +855,18 @@
 					     "usb-b-connector";
 				label = "micro-USB";
 				type = "micro";
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port at 3 {
+						reg = <3>;
+						musb_con_to_mhl: endpoint {
+							remote-endpoint = <&mhl_to_musb_con>;
+						};
+					};
+				};
 			};
 		};
 
-- 
2.16.2

[PATCH v5 4/6] arm64: dts: exynos: add OF graph between MHL and USB connector

[Krzysztof Kozlowski]

On Tue, Feb 27, 2018 at 08:11:32AM +0100, Andrzej Hajda wrote:
> OF graph describes MHL data lanes between MHL and respective USB
> connector.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> v5: removed extra parenthesis (kbuild test robot)
> v4: added missing reg property in connector's port node (Krzysztof)
> ---
>  .../boot/dts/exynos/exynos5433-tm2-common.dtsi     | 31 +++++++++++++++++++---
>  1 file changed, 28 insertions(+), 3 deletions(-)
> 

Thanks, applied.

Best regards,
Krzysztof

[PATCH v5 5/6] extcon: add possibility to get extcon device by OF node

[Andrzej Hajda]

Since extcon property is not allowed in DT, extcon subsystem requires
another way to get extcon device. Lets try the simplest approach - get
edev by of_node.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
---
v5: function renamed to extcon_find_edev_by_node (Andy)
v2: changed label to follow local convention (Chanwoo)
---
 drivers/extcon/extcon.c | 44 ++++++++++++++++++++++++++++++++++----------
 include/linux/extcon.h  |  6 ++++++
 2 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index cb38c2747684..47a5ca9eb86d 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -1336,6 +1336,28 @@ void extcon_dev_unregister(struct extcon_dev *edev)
 EXPORT_SYMBOL_GPL(extcon_dev_unregister);
 
 #ifdef CONFIG_OF
+
+/*
+ * extcon_find_edev_by_node - Find the extcon device from devicetree.
+ * @node	: OF node identyfying edev
+ *
+ * Return the pointer of extcon device if success or ERR_PTR(err) if fail.
+ */
+struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)
+{
+	struct extcon_dev *edev;
+
+	mutex_lock(&extcon_dev_list_lock);
+	list_for_each_entry(edev, &extcon_dev_list, entry)
+		if (edev->dev.parent && edev->dev.parent->of_node == node)
+			goto out;
+	edev = ERR_PTR(-EPROBE_DEFER);
+out:
+	mutex_unlock(&extcon_dev_list_lock);
+
+	return edev;
+}
+
 /*
  * extcon_get_edev_by_phandle - Get the extcon device from devicetree.
  * @dev		: the instance to the given device
@@ -1363,25 +1385,27 @@ struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev, int index)
 		return ERR_PTR(-ENODEV);
 	}
 
-	mutex_lock(&extcon_dev_list_lock);
-	list_for_each_entry(edev, &extcon_dev_list, entry) {
-		if (edev->dev.parent && edev->dev.parent->of_node == node) {
-			mutex_unlock(&extcon_dev_list_lock);
-			of_node_put(node);
-			return edev;
-		}
-	}
-	mutex_unlock(&extcon_dev_list_lock);
+	edev = extcon_find_edev_by_node(node);
 	of_node_put(node);
 
-	return ERR_PTR(-EPROBE_DEFER);
+	return edev;
 }
+
 #else
+
+struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
 struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev, int index)
 {
 	return ERR_PTR(-ENOSYS);
 }
+
 #endif /* CONFIG_OF */
+
+EXPORT_SYMBOL_GPL(extcon_find_edev_by_node);
 EXPORT_SYMBOL_GPL(extcon_get_edev_by_phandle);
 
 /**
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index 6d94e82c8ad9..7f033b1ea568 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -230,6 +230,7 @@ extern void devm_extcon_unregister_notifier_all(struct device *dev,
  * Following APIs get the extcon_dev from devicetree or by through extcon name.
  */
 extern struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name);
+extern struct extcon_dev *extcon_find_edev_by_node(struct device_node *node);
 extern struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev,
 						     int index);
 
@@ -283,6 +284,11 @@ static inline struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
 	return ERR_PTR(-ENODEV);
 }
 
+static inline struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)
+{
+	return ERR_PTR(-ENODEV);
+}
+
 static inline struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev,
 				int index)
 {
-- 
2.16.2

[PATCH v5 5/6] extcon: add possibility to get extcon device by OF node

[Chanwoo Choi]

Hi,

On 2018? 02? 27? 16:11, Andrzej Hajda wrote:
> Since extcon property is not allowed in DT, extcon subsystem requires
> another way to get extcon device. Lets try the simplest approach - get
> edev by of_node.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
> v5: function renamed to extcon_find_edev_by_node (Andy)
> v2: changed label to follow local convention (Chanwoo)
> ---
>  drivers/extcon/extcon.c | 44 ++++++++++++++++++++++++++++++++++----------
>  include/linux/extcon.h  |  6 ++++++
>  2 files changed, 40 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index cb38c2747684..47a5ca9eb86d 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -1336,6 +1336,28 @@ void extcon_dev_unregister(struct extcon_dev *edev)
>  EXPORT_SYMBOL_GPL(extcon_dev_unregister);
>  
>  #ifdef CONFIG_OF
> +
> +/*
> + * extcon_find_edev_by_node - Find the extcon device from devicetree.
> + * @node	: OF node identyfying edev

s/identyfying/identifying

> + *
> + * Return the pointer of extcon device if success or ERR_PTR(err) if fail.
> + */
> +struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)
> +{
> +	struct extcon_dev *edev;
> +
> +	mutex_lock(&extcon_dev_list_lock);
> +	list_for_each_entry(edev, &extcon_dev_list, entry)
> +		if (edev->dev.parent && edev->dev.parent->of_node == node)
> +			goto out;
> +	edev = ERR_PTR(-EPROBE_DEFER);
> +out:
> +	mutex_unlock(&extcon_dev_list_lock);
> +
> +	return edev;
> +}
> +
>  /*
>   * extcon_get_edev_by_phandle - Get the extcon device from devicetree.
>   * @dev		: the instance to the given device
> @@ -1363,25 +1385,27 @@ struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev, int index)
>  		return ERR_PTR(-ENODEV);
>  	}
>  
> -	mutex_lock(&extcon_dev_list_lock);
> -	list_for_each_entry(edev, &extcon_dev_list, entry) {
> -		if (edev->dev.parent && edev->dev.parent->of_node == node) {
> -			mutex_unlock(&extcon_dev_list_lock);
> -			of_node_put(node);
> -			return edev;
> -		}
> -	}
> -	mutex_unlock(&extcon_dev_list_lock);
> +	edev = extcon_find_edev_by_node(node);
>  	of_node_put(node);
>  
> -	return ERR_PTR(-EPROBE_DEFER);
> +	return edev;
>  }
> +
>  #else
> +
> +struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)
> +{
> +	return ERR_PTR(-ENOSYS);
> +}
> +
>  struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev, int index)
>  {
>  	return ERR_PTR(-ENOSYS);
>  }
> +
>  #endif /* CONFIG_OF */
> +
> +EXPORT_SYMBOL_GPL(extcon_find_edev_by_node);
>  EXPORT_SYMBOL_GPL(extcon_get_edev_by_phandle);
>  
>  /**
> diff --git a/include/linux/extcon.h b/include/linux/extcon.h
> index 6d94e82c8ad9..7f033b1ea568 100644
> --- a/include/linux/extcon.h
> +++ b/include/linux/extcon.h
> @@ -230,6 +230,7 @@ extern void devm_extcon_unregister_notifier_all(struct device *dev,
>   * Following APIs get the extcon_dev from devicetree or by through extcon name.
>   */
>  extern struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name);
> +extern struct extcon_dev *extcon_find_edev_by_node(struct device_node *node);
>  extern struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev,
>  						     int index);
>  
> @@ -283,6 +284,11 @@ static inline struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
>  	return ERR_PTR(-ENODEV);
>  }
>  
> +static inline struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)
> +{
> +	return ERR_PTR(-ENODEV);
> +}
> +
>  static inline struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev,
>  				int index)
>  {
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

[PATCH v6 5/6] extcon: add possibility to get extcon device by OF node

[Andrzej Hajda]

Since extcon property is not allowed in DT, extcon subsystem requires
another way to get extcon device. Lets try the simplest approach - get
edev by of_node.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
---
v5: function renamed to extcon_find_edev_by_node (Andy)
v2: changed label to follow local convention (Chanwoo)
---
 drivers/extcon/extcon.c | 44 ++++++++++++++++++++++++++++++++++----------
 include/linux/extcon.h  |  6 ++++++
 2 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index cb38c2747684..8bff5fd18185 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -1336,6 +1336,28 @@ void extcon_dev_unregister(struct extcon_dev *edev)
 EXPORT_SYMBOL_GPL(extcon_dev_unregister);
 
 #ifdef CONFIG_OF
+
+/*
+ * extcon_find_edev_by_node - Find the extcon device from devicetree.
+ * @node	: OF node identifying edev
+ *
+ * Return the pointer of extcon device if success or ERR_PTR(err) if fail.
+ */
+struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)
+{
+	struct extcon_dev *edev;
+
+	mutex_lock(&extcon_dev_list_lock);
+	list_for_each_entry(edev, &extcon_dev_list, entry)
+		if (edev->dev.parent && edev->dev.parent->of_node == node)
+			goto out;
+	edev = ERR_PTR(-EPROBE_DEFER);
+out:
+	mutex_unlock(&extcon_dev_list_lock);
+
+	return edev;
+}
+
 /*
  * extcon_get_edev_by_phandle - Get the extcon device from devicetree.
  * @dev		: the instance to the given device
@@ -1363,25 +1385,27 @@ struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev, int index)
 		return ERR_PTR(-ENODEV);
 	}
 
-	mutex_lock(&extcon_dev_list_lock);
-	list_for_each_entry(edev, &extcon_dev_list, entry) {
-		if (edev->dev.parent && edev->dev.parent->of_node == node) {
-			mutex_unlock(&extcon_dev_list_lock);
-			of_node_put(node);
-			return edev;
-		}
-	}
-	mutex_unlock(&extcon_dev_list_lock);
+	edev = extcon_find_edev_by_node(node);
 	of_node_put(node);
 
-	return ERR_PTR(-EPROBE_DEFER);
+	return edev;
 }
+
 #else
+
+struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
 struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev, int index)
 {
 	return ERR_PTR(-ENOSYS);
 }
+
 #endif /* CONFIG_OF */
+
+EXPORT_SYMBOL_GPL(extcon_find_edev_by_node);
 EXPORT_SYMBOL_GPL(extcon_get_edev_by_phandle);
 
 /**
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index 6d94e82c8ad9..7f033b1ea568 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -230,6 +230,7 @@ extern void devm_extcon_unregister_notifier_all(struct device *dev,
  * Following APIs get the extcon_dev from devicetree or by through extcon name.
  */
 extern struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name);
+extern struct extcon_dev *extcon_find_edev_by_node(struct device_node *node);
 extern struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev,
 						     int index);
 
@@ -283,6 +284,11 @@ static inline struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
 	return ERR_PTR(-ENODEV);
 }
 
+static inline struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)
+{
+	return ERR_PTR(-ENODEV);
+}
+
 static inline struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev,
 				int index)
 {
-- 
2.16.2

[PATCH v5 6/6] drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL

[Andrzej Hajda]

From: Maciej Purski <m.purski@samsung.com>

Currently MHL chip must be turned on permanently to detect MHL cable. It
duplicates micro-USB controller's (MUIC) functionality and consumes
unnecessary power. Lets use extcon attached to MUIC to enable MHL chip
only if it detects MHL cable.

Signed-off-by: Maciej Purski <m.purski@samsung.com>
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
v5: updated extcon API

This is rework of the patch by Maciej with following changes:
- use micro-USB port bindings to get extcon, instead of extcon property,
- fixed remove sequence,
- fixed extcon get state logic.

Code finding extcon node is hacky IMO, I guess ultimately it should be done
via some framework (maybe even extcon), or at least via helper, I hope it
can stay as is until the proper solution will be merged.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/bridge/sil-sii8620.c | 97 ++++++++++++++++++++++++++++++++++--
 1 file changed, 94 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
index 9e785b8e0ea2..62b0adabcac2 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -17,6 +17,7 @@
 
 #include <linux/clk.h>
 #include <linux/delay.h>
+#include <linux/extcon.h>
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
@@ -25,6 +26,7 @@
 #include <linux/list.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/of_graph.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 
@@ -81,6 +83,10 @@ struct sii8620 {
 	struct edid *edid;
 	unsigned int gen2_write_burst:1;
 	enum sii8620_mt_state mt_state;
+	struct extcon_dev *extcon;
+	struct notifier_block extcon_nb;
+	struct work_struct extcon_wq;
+	int cable_state;
 	struct list_head mt_queue;
 	struct {
 		int r_size;
@@ -2175,6 +2181,77 @@ static void sii8620_init_rcp_input_dev(struct sii8620 *ctx)
 	ctx->rc_dev = rc_dev;
 }
 
+static void sii8620_cable_out(struct sii8620 *ctx)
+{
+	disable_irq(to_i2c_client(ctx->dev)->irq);
+	sii8620_hw_off(ctx);
+}
+
+static void sii8620_extcon_work(struct work_struct *work)
+{
+	struct sii8620 *ctx =
+		container_of(work, struct sii8620, extcon_wq);
+	int state = extcon_get_state(ctx->extcon, EXTCON_DISP_MHL);
+
+	if (state == ctx->cable_state)
+		return;
+
+	ctx->cable_state = state;
+
+	if (state > 0)
+		sii8620_cable_in(ctx);
+	else
+		sii8620_cable_out(ctx);
+}
+
+static int sii8620_extcon_notifier(struct notifier_block *self,
+			unsigned long event, void *ptr)
+{
+	struct sii8620 *ctx =
+		container_of(self, struct sii8620, extcon_nb);
+
+	schedule_work(&ctx->extcon_wq);
+
+	return NOTIFY_DONE;
+}
+
+static int sii8620_extcon_init(struct sii8620 *ctx)
+{
+	struct extcon_dev *edev;
+	struct device_node *musb, *muic;
+	int ret;
+
+	/* get micro-USB connector node */
+	musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1);
+	/* next get micro-USB Interface Controller node */
+	muic = of_get_next_parent(musb);
+
+	if (!muic) {
+		dev_info(ctx->dev, "no extcon found, switching to 'always on' mode\n");
+		return 0;
+	}
+
+	edev = extcon_find_edev_by_node(muic);
+	of_node_put(muic);
+	if (IS_ERR(edev)) {
+		if (PTR_ERR(edev) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+		dev_err(ctx->dev, "Invalid or missing extcon\n");
+		return PTR_ERR(edev);
+	}
+
+	ctx->extcon = edev;
+	ctx->extcon_nb.notifier_call = sii8620_extcon_notifier;
+	INIT_WORK(&ctx->extcon_wq, sii8620_extcon_work);
+	ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, &ctx->extcon_nb);
+	if (ret) {
+		dev_err(ctx->dev, "failed to register notifier for MHL\n");
+		return ret;
+	}
+
+	return 0;
+}
+
 static inline struct sii8620 *bridge_to_sii8620(struct drm_bridge *bridge)
 {
 	return container_of(bridge, struct sii8620, bridge);
@@ -2307,13 +2384,20 @@ static int sii8620_probe(struct i2c_client *client,
 	if (ret)
 		return ret;
 
+	ret = sii8620_extcon_init(ctx);
+	if (ret < 0) {
+		dev_err(ctx->dev, "failed to initialize EXTCON\n");
+		return ret;
+	}
+
 	i2c_set_clientdata(client, ctx);
 
 	ctx->bridge.funcs = &sii8620_bridge_funcs;
 	ctx->bridge.of_node = dev->of_node;
 	drm_bridge_add(&ctx->bridge);
 
-	sii8620_cable_in(ctx);
+	if (!ctx->extcon)
+		sii8620_cable_in(ctx);
 
 	return 0;
 }
@@ -2322,8 +2406,15 @@ static int sii8620_remove(struct i2c_client *client)
 {
 	struct sii8620 *ctx = i2c_get_clientdata(client);
 
-	disable_irq(to_i2c_client(ctx->dev)->irq);
-	sii8620_hw_off(ctx);
+	if (ctx->extcon) {
+		extcon_unregister_notifier(ctx->extcon, EXTCON_DISP_MHL,
+					   &ctx->extcon_nb);
+		flush_work(&ctx->extcon_wq);
+		if (ctx->cable_state > 0)
+			sii8620_cable_out(ctx);
+	} else {
+		sii8620_cable_out(ctx);
+	}
 	drm_bridge_remove(&ctx->bridge);
 
 	return 0;
-- 
2.16.2

[PATCH v5 6/6] drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL

[Chanwoo Choi]

Hi,

On 2018? 02? 27? 16:11, Andrzej Hajda wrote:
> From: Maciej Purski <m.purski@samsung.com>
> 
> Currently MHL chip must be turned on permanently to detect MHL cable. It
> duplicates micro-USB controller's (MUIC) functionality and consumes
> unnecessary power. Lets use extcon attached to MUIC to enable MHL chip
> only if it detects MHL cable.
> 
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> v5: updated extcon API
> 
> This is rework of the patch by Maciej with following changes:
> - use micro-USB port bindings to get extcon, instead of extcon property,
> - fixed remove sequence,
> - fixed extcon get state logic.
> 
> Code finding extcon node is hacky IMO, I guess ultimately it should be done
> via some framework (maybe even extcon), or at least via helper, I hope it
> can stay as is until the proper solution will be merged.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/bridge/sil-sii8620.c | 97 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 94 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
> index 9e785b8e0ea2..62b0adabcac2 100644
> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> @@ -17,6 +17,7 @@
>  
>  #include <linux/clk.h>
>  #include <linux/delay.h>
> +#include <linux/extcon.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/i2c.h>
>  #include <linux/interrupt.h>
> @@ -25,6 +26,7 @@
>  #include <linux/list.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/of_graph.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
>  
> @@ -81,6 +83,10 @@ struct sii8620 {
>  	struct edid *edid;
>  	unsigned int gen2_write_burst:1;
>  	enum sii8620_mt_state mt_state;
> +	struct extcon_dev *extcon;
> +	struct notifier_block extcon_nb;
> +	struct work_struct extcon_wq;
> +	int cable_state;
>  	struct list_head mt_queue;
>  	struct {
>  		int r_size;
> @@ -2175,6 +2181,77 @@ static void sii8620_init_rcp_input_dev(struct sii8620 *ctx)
>  	ctx->rc_dev = rc_dev;
>  }
>  
> +static void sii8620_cable_out(struct sii8620 *ctx)
> +{
> +	disable_irq(to_i2c_client(ctx->dev)->irq);
> +	sii8620_hw_off(ctx);
> +}
> +
> +static void sii8620_extcon_work(struct work_struct *work)
> +{
> +	struct sii8620 *ctx =
> +		container_of(work, struct sii8620, extcon_wq);
> +	int state = extcon_get_state(ctx->extcon, EXTCON_DISP_MHL);
> +
> +	if (state == ctx->cable_state)
> +		return;
> +
> +	ctx->cable_state = state;
> +
> +	if (state > 0)
> +		sii8620_cable_in(ctx);
> +	else
> +		sii8620_cable_out(ctx);
> +}
> +
> +static int sii8620_extcon_notifier(struct notifier_block *self,
> +			unsigned long event, void *ptr)
> +{
> +	struct sii8620 *ctx =
> +		container_of(self, struct sii8620, extcon_nb);
> +
> +	schedule_work(&ctx->extcon_wq);
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static int sii8620_extcon_init(struct sii8620 *ctx)
> +{
> +	struct extcon_dev *edev;
> +	struct device_node *musb, *muic;
> +	int ret;
> +
> +	/* get micro-USB connector node */
> +	musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1);
> +	/* next get micro-USB Interface Controller node */
> +	muic = of_get_next_parent(musb);
> +
> +	if (!muic) {
> +		dev_info(ctx->dev, "no extcon found, switching to 'always on' mode\n");
> +		return 0;
> +	}
> +
> +	edev = extcon_find_edev_by_node(muic);
> +	of_node_put(muic);
> +	if (IS_ERR(edev)) {
> +		if (PTR_ERR(edev) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +		dev_err(ctx->dev, "Invalid or missing extcon\n");
> +		return PTR_ERR(edev);
> +	}
> +
> +	ctx->extcon = edev;
> +	ctx->extcon_nb.notifier_call = sii8620_extcon_notifier;
> +	INIT_WORK(&ctx->extcon_wq, sii8620_extcon_work);
> +	ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, &ctx->extcon_nb);

You better to use devm_extcon_register_notifier().

> +	if (ret) {
> +		dev_err(ctx->dev, "failed to register notifier for MHL\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static inline struct sii8620 *bridge_to_sii8620(struct drm_bridge *bridge)
>  {
>  	return container_of(bridge, struct sii8620, bridge);
> @@ -2307,13 +2384,20 @@ static int sii8620_probe(struct i2c_client *client,
>  	if (ret)
>  		return ret;
>  
> +	ret = sii8620_extcon_init(ctx);
> +	if (ret < 0) {
> +		dev_err(ctx->dev, "failed to initialize EXTCON\n");
> +		return ret;
> +	}
> +
>  	i2c_set_clientdata(client, ctx);
>  
>  	ctx->bridge.funcs = &sii8620_bridge_funcs;
>  	ctx->bridge.of_node = dev->of_node;
>  	drm_bridge_add(&ctx->bridge);
>  
> -	sii8620_cable_in(ctx);
> +	if (!ctx->extcon)
> +		sii8620_cable_in(ctx);
>  
>  	return 0;
>  }
> @@ -2322,8 +2406,15 @@ static int sii8620_remove(struct i2c_client *client)
>  {
>  	struct sii8620 *ctx = i2c_get_clientdata(client);
>  
> -	disable_irq(to_i2c_client(ctx->dev)->irq);
> -	sii8620_hw_off(ctx);
> +	if (ctx->extcon) {
> +		extcon_unregister_notifier(ctx->extcon, EXTCON_DISP_MHL,
> +					   &ctx->extcon_nb);

Don't need to unregister the notifier if using devm_extcon_register_notifier().

> +		flush_work(&ctx->extcon_wq);
> +		if (ctx->cable_state > 0)
> +			sii8620_cable_out(ctx);
> +	} else {
> +		sii8620_cable_out(ctx);
> +	}
>  	drm_bridge_remove(&ctx->bridge);
>  
>  	return 0;
> 

If you use the resource managed function (devm_extcon_register_notifier), Looks good to me.
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

[PATCH v5 6/6] drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL

[Andrzej Hajda]

On 27.02.2018 12:08, Chanwoo Choi wrote:
> Hi,
>
> On 2018? 02? 27? 16:11, Andrzej Hajda wrote:
>> From: Maciej Purski <m.purski@samsung.com>
>>
>> Currently MHL chip must be turned on permanently to detect MHL cable. It
>> duplicates micro-USB controller's (MUIC) functionality and consumes
>> unnecessary power. Lets use extcon attached to MUIC to enable MHL chip
>> only if it detects MHL cable.
>>
>> Signed-off-by: Maciej Purski <m.purski@samsung.com>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>> v5: updated extcon API
>>
>> This is rework of the patch by Maciej with following changes:
>> - use micro-USB port bindings to get extcon, instead of extcon property,
>> - fixed remove sequence,
>> - fixed extcon get state logic.
>>
>> Code finding extcon node is hacky IMO, I guess ultimately it should be done
>> via some framework (maybe even extcon), or at least via helper, I hope it
>> can stay as is until the proper solution will be merged.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  drivers/gpu/drm/bridge/sil-sii8620.c | 97 ++++++++++++++++++++++++++++++++++--
>>  1 file changed, 94 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
>> index 9e785b8e0ea2..62b0adabcac2 100644
>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
>> @@ -17,6 +17,7 @@
>>  
>>  #include <linux/clk.h>
>>  #include <linux/delay.h>
>> +#include <linux/extcon.h>
>>  #include <linux/gpio/consumer.h>
>>  #include <linux/i2c.h>
>>  #include <linux/interrupt.h>
>> @@ -25,6 +26,7 @@
>>  #include <linux/list.h>
>>  #include <linux/module.h>
>>  #include <linux/mutex.h>
>> +#include <linux/of_graph.h>
>>  #include <linux/regulator/consumer.h>
>>  #include <linux/slab.h>
>>  
>> @@ -81,6 +83,10 @@ struct sii8620 {
>>  	struct edid *edid;
>>  	unsigned int gen2_write_burst:1;
>>  	enum sii8620_mt_state mt_state;
>> +	struct extcon_dev *extcon;
>> +	struct notifier_block extcon_nb;
>> +	struct work_struct extcon_wq;
>> +	int cable_state;
>>  	struct list_head mt_queue;
>>  	struct {
>>  		int r_size;
>> @@ -2175,6 +2181,77 @@ static void sii8620_init_rcp_input_dev(struct sii8620 *ctx)
>>  	ctx->rc_dev = rc_dev;
>>  }
>>  
>> +static void sii8620_cable_out(struct sii8620 *ctx)
>> +{
>> +	disable_irq(to_i2c_client(ctx->dev)->irq);
>> +	sii8620_hw_off(ctx);
>> +}
>> +
>> +static void sii8620_extcon_work(struct work_struct *work)
>> +{
>> +	struct sii8620 *ctx =
>> +		container_of(work, struct sii8620, extcon_wq);
>> +	int state = extcon_get_state(ctx->extcon, EXTCON_DISP_MHL);
>> +
>> +	if (state == ctx->cable_state)
>> +		return;
>> +
>> +	ctx->cable_state = state;
>> +
>> +	if (state > 0)
>> +		sii8620_cable_in(ctx);
>> +	else
>> +		sii8620_cable_out(ctx);
>> +}
>> +
>> +static int sii8620_extcon_notifier(struct notifier_block *self,
>> +			unsigned long event, void *ptr)
>> +{
>> +	struct sii8620 *ctx =
>> +		container_of(self, struct sii8620, extcon_nb);
>> +
>> +	schedule_work(&ctx->extcon_wq);
>> +
>> +	return NOTIFY_DONE;
>> +}
>> +
>> +static int sii8620_extcon_init(struct sii8620 *ctx)
>> +{
>> +	struct extcon_dev *edev;
>> +	struct device_node *musb, *muic;
>> +	int ret;
>> +
>> +	/* get micro-USB connector node */
>> +	musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1);
>> +	/* next get micro-USB Interface Controller node */
>> +	muic = of_get_next_parent(musb);
>> +
>> +	if (!muic) {
>> +		dev_info(ctx->dev, "no extcon found, switching to 'always on' mode\n");
>> +		return 0;
>> +	}
>> +
>> +	edev = extcon_find_edev_by_node(muic);
>> +	of_node_put(muic);
>> +	if (IS_ERR(edev)) {
>> +		if (PTR_ERR(edev) == -EPROBE_DEFER)
>> +			return -EPROBE_DEFER;
>> +		dev_err(ctx->dev, "Invalid or missing extcon\n");
>> +		return PTR_ERR(edev);
>> +	}
>> +
>> +	ctx->extcon = edev;
>> +	ctx->extcon_nb.notifier_call = sii8620_extcon_notifier;
>> +	INIT_WORK(&ctx->extcon_wq, sii8620_extcon_work);
>> +	ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, &ctx->extcon_nb);
> You better to use devm_extcon_register_notifier().

With devm version I risk that in case of device unbind notification will
be called after .remove callback, it seems to me quite dangerous. Or am
I missing something?

Regards
Andrzej

>
>> +	if (ret) {
>> +		dev_err(ctx->dev, "failed to register notifier for MHL\n");
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static inline struct sii8620 *bridge_to_sii8620(struct drm_bridge *bridge)
>>  {
>>  	return container_of(bridge, struct sii8620, bridge);
>> @@ -2307,13 +2384,20 @@ static int sii8620_probe(struct i2c_client *client,
>>  	if (ret)
>>  		return ret;
>>  
>> +	ret = sii8620_extcon_init(ctx);
>> +	if (ret < 0) {
>> +		dev_err(ctx->dev, "failed to initialize EXTCON\n");
>> +		return ret;
>> +	}
>> +
>>  	i2c_set_clientdata(client, ctx);
>>  
>>  	ctx->bridge.funcs = &sii8620_bridge_funcs;
>>  	ctx->bridge.of_node = dev->of_node;
>>  	drm_bridge_add(&ctx->bridge);
>>  
>> -	sii8620_cable_in(ctx);
>> +	if (!ctx->extcon)
>> +		sii8620_cable_in(ctx);
>>  
>>  	return 0;
>>  }
>> @@ -2322,8 +2406,15 @@ static int sii8620_remove(struct i2c_client *client)
>>  {
>>  	struct sii8620 *ctx = i2c_get_clientdata(client);
>>  
>> -	disable_irq(to_i2c_client(ctx->dev)->irq);
>> -	sii8620_hw_off(ctx);
>> +	if (ctx->extcon) {
>> +		extcon_unregister_notifier(ctx->extcon, EXTCON_DISP_MHL,
>> +					   &ctx->extcon_nb);
> Don't need to unregister the notifier if using devm_extcon_register_notifier().
>
>> +		flush_work(&ctx->extcon_wq);
>> +		if (ctx->cable_state > 0)
>> +			sii8620_cable_out(ctx);
>> +	} else {
>> +		sii8620_cable_out(ctx);
>> +	}
>>  	drm_bridge_remove(&ctx->bridge);
>>  
>>  	return 0;
>>
> If you use the resource managed function (devm_extcon_register_notifier), Looks good to me.
> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
>

[PATCH v5 6/6] drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL

[Chanwoo Choi]

Hi,

On 2018? 02? 27? 21:05, Andrzej Hajda wrote:
> On 27.02.2018 12:08, Chanwoo Choi wrote:
>> Hi,
>>
>> On 2018? 02? 27? 16:11, Andrzej Hajda wrote:
>>> From: Maciej Purski <m.purski@samsung.com>
>>>
>>> Currently MHL chip must be turned on permanently to detect MHL cable. It
>>> duplicates micro-USB controller's (MUIC) functionality and consumes
>>> unnecessary power. Lets use extcon attached to MUIC to enable MHL chip
>>> only if it detects MHL cable.
>>>
>>> Signed-off-by: Maciej Purski <m.purski@samsung.com>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>> v5: updated extcon API
>>>
>>> This is rework of the patch by Maciej with following changes:
>>> - use micro-USB port bindings to get extcon, instead of extcon property,
>>> - fixed remove sequence,
>>> - fixed extcon get state logic.
>>>
>>> Code finding extcon node is hacky IMO, I guess ultimately it should be done
>>> via some framework (maybe even extcon), or at least via helper, I hope it
>>> can stay as is until the proper solution will be merged.
>>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>>  drivers/gpu/drm/bridge/sil-sii8620.c | 97 ++++++++++++++++++++++++++++++++++--
>>>  1 file changed, 94 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
>>> index 9e785b8e0ea2..62b0adabcac2 100644
>>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
>>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
>>> @@ -17,6 +17,7 @@
>>>  
>>>  #include <linux/clk.h>
>>>  #include <linux/delay.h>
>>> +#include <linux/extcon.h>
>>>  #include <linux/gpio/consumer.h>
>>>  #include <linux/i2c.h>
>>>  #include <linux/interrupt.h>
>>> @@ -25,6 +26,7 @@
>>>  #include <linux/list.h>
>>>  #include <linux/module.h>
>>>  #include <linux/mutex.h>
>>> +#include <linux/of_graph.h>
>>>  #include <linux/regulator/consumer.h>
>>>  #include <linux/slab.h>
>>>  
>>> @@ -81,6 +83,10 @@ struct sii8620 {
>>>  	struct edid *edid;
>>>  	unsigned int gen2_write_burst:1;
>>>  	enum sii8620_mt_state mt_state;
>>> +	struct extcon_dev *extcon;
>>> +	struct notifier_block extcon_nb;
>>> +	struct work_struct extcon_wq;
>>> +	int cable_state;
>>>  	struct list_head mt_queue;
>>>  	struct {
>>>  		int r_size;
>>> @@ -2175,6 +2181,77 @@ static void sii8620_init_rcp_input_dev(struct sii8620 *ctx)
>>>  	ctx->rc_dev = rc_dev;
>>>  }
>>>  
>>> +static void sii8620_cable_out(struct sii8620 *ctx)
>>> +{
>>> +	disable_irq(to_i2c_client(ctx->dev)->irq);
>>> +	sii8620_hw_off(ctx);
>>> +}
>>> +
>>> +static void sii8620_extcon_work(struct work_struct *work)
>>> +{
>>> +	struct sii8620 *ctx =
>>> +		container_of(work, struct sii8620, extcon_wq);
>>> +	int state = extcon_get_state(ctx->extcon, EXTCON_DISP_MHL);
>>> +
>>> +	if (state == ctx->cable_state)
>>> +		return;
>>> +
>>> +	ctx->cable_state = state;
>>> +
>>> +	if (state > 0)
>>> +		sii8620_cable_in(ctx);
>>> +	else
>>> +		sii8620_cable_out(ctx);
>>> +}
>>> +
>>> +static int sii8620_extcon_notifier(struct notifier_block *self,
>>> +			unsigned long event, void *ptr)
>>> +{
>>> +	struct sii8620 *ctx =
>>> +		container_of(self, struct sii8620, extcon_nb);
>>> +
>>> +	schedule_work(&ctx->extcon_wq);
>>> +
>>> +	return NOTIFY_DONE;
>>> +}
>>> +
>>> +static int sii8620_extcon_init(struct sii8620 *ctx)
>>> +{
>>> +	struct extcon_dev *edev;
>>> +	struct device_node *musb, *muic;
>>> +	int ret;
>>> +
>>> +	/* get micro-USB connector node */
>>> +	musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1);
>>> +	/* next get micro-USB Interface Controller node */
>>> +	muic = of_get_next_parent(musb);
>>> +
>>> +	if (!muic) {
>>> +		dev_info(ctx->dev, "no extcon found, switching to 'always on' mode\n");
>>> +		return 0;
>>> +	}
>>> +
>>> +	edev = extcon_find_edev_by_node(muic);
>>> +	of_node_put(muic);
>>> +	if (IS_ERR(edev)) {
>>> +		if (PTR_ERR(edev) == -EPROBE_DEFER)
>>> +			return -EPROBE_DEFER;
>>> +		dev_err(ctx->dev, "Invalid or missing extcon\n");
>>> +		return PTR_ERR(edev);
>>> +	}
>>> +
>>> +	ctx->extcon = edev;
>>> +	ctx->extcon_nb.notifier_call = sii8620_extcon_notifier;
>>> +	INIT_WORK(&ctx->extcon_wq, sii8620_extcon_work);
>>> +	ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, &ctx->extcon_nb);
>> You better to use devm_extcon_register_notifier().
> 
> With devm version I risk that in case of device unbind notification will
> be called after .remove callback, it seems to me quite dangerous. Or am
> I missing something?

If you use the cancel_work_sync() in remove() instead of flush_work(),
you can use the 'devm_extcon_*'.

> 
> Regards
> Andrzej
> 
>>
>>> +	if (ret) {
>>> +		dev_err(ctx->dev, "failed to register notifier for MHL\n");
>>> +		return ret;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>  static inline struct sii8620 *bridge_to_sii8620(struct drm_bridge *bridge)
>>>  {
>>>  	return container_of(bridge, struct sii8620, bridge);
>>> @@ -2307,13 +2384,20 @@ static int sii8620_probe(struct i2c_client *client,
>>>  	if (ret)
>>>  		return ret;
>>>  
>>> +	ret = sii8620_extcon_init(ctx);
>>> +	if (ret < 0) {
>>> +		dev_err(ctx->dev, "failed to initialize EXTCON\n");
>>> +		return ret;
>>> +	}
>>> +
>>>  	i2c_set_clientdata(client, ctx);
>>>  
>>>  	ctx->bridge.funcs = &sii8620_bridge_funcs;
>>>  	ctx->bridge.of_node = dev->of_node;
>>>  	drm_bridge_add(&ctx->bridge);
>>>  
>>> -	sii8620_cable_in(ctx);
>>> +	if (!ctx->extcon)
>>> +		sii8620_cable_in(ctx);
>>>  
>>>  	return 0;
>>>  }
>>> @@ -2322,8 +2406,15 @@ static int sii8620_remove(struct i2c_client *client)
>>>  {
>>>  	struct sii8620 *ctx = i2c_get_clientdata(client);
>>>  
>>> -	disable_irq(to_i2c_client(ctx->dev)->irq);
>>> -	sii8620_hw_off(ctx);
>>> +	if (ctx->extcon) {
>>> +		extcon_unregister_notifier(ctx->extcon, EXTCON_DISP_MHL,
>>> +					   &ctx->extcon_nb);
>> Don't need to unregister the notifier if using devm_extcon_register_notifier().
>>
>>> +		flush_work(&ctx->extcon_wq);
>>> +		if (ctx->cable_state > 0)
>>> +			sii8620_cable_out(ctx);
>>> +	} else {
>>> +		sii8620_cable_out(ctx);
>>> +	}
>>>  	drm_bridge_remove(&ctx->bridge);
>>>  
>>>  	return 0;
>>>
>> If you use the resource managed function (devm_extcon_register_notifier), Looks good to me.
>> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

[PATCH v5 6/6] drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL

[Andrzej Hajda]

On 27.02.2018 23:26, Chanwoo Choi wrote:
> Hi,
>
> On 2018? 02? 27? 21:05, Andrzej Hajda wrote:
>> On 27.02.2018 12:08, Chanwoo Choi wrote:
>>> Hi,
>>>
>>> On 2018? 02? 27? 16:11, Andrzej Hajda wrote:
>>>> From: Maciej Purski <m.purski@samsung.com>
>>>>
>>>> Currently MHL chip must be turned on permanently to detect MHL cable. It
>>>> duplicates micro-USB controller's (MUIC) functionality and consumes
>>>> unnecessary power. Lets use extcon attached to MUIC to enable MHL chip
>>>> only if it detects MHL cable.
>>>>
>>>> Signed-off-by: Maciej Purski <m.purski@samsung.com>
>>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>>> ---
>>>> v5: updated extcon API
>>>>
>>>> This is rework of the patch by Maciej with following changes:
>>>> - use micro-USB port bindings to get extcon, instead of extcon property,
>>>> - fixed remove sequence,
>>>> - fixed extcon get state logic.
>>>>
>>>> Code finding extcon node is hacky IMO, I guess ultimately it should be done
>>>> via some framework (maybe even extcon), or at least via helper, I hope it
>>>> can stay as is until the proper solution will be merged.
>>>>
>>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>>> ---
>>>>  drivers/gpu/drm/bridge/sil-sii8620.c | 97 ++++++++++++++++++++++++++++++++++--
>>>>  1 file changed, 94 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
>>>> index 9e785b8e0ea2..62b0adabcac2 100644
>>>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
>>>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
>>>> @@ -17,6 +17,7 @@
>>>>  
>>>>  #include <linux/clk.h>
>>>>  #include <linux/delay.h>
>>>> +#include <linux/extcon.h>
>>>>  #include <linux/gpio/consumer.h>
>>>>  #include <linux/i2c.h>
>>>>  #include <linux/interrupt.h>
>>>> @@ -25,6 +26,7 @@
>>>>  #include <linux/list.h>
>>>>  #include <linux/module.h>
>>>>  #include <linux/mutex.h>
>>>> +#include <linux/of_graph.h>
>>>>  #include <linux/regulator/consumer.h>
>>>>  #include <linux/slab.h>
>>>>  
>>>> @@ -81,6 +83,10 @@ struct sii8620 {
>>>>  	struct edid *edid;
>>>>  	unsigned int gen2_write_burst:1;
>>>>  	enum sii8620_mt_state mt_state;
>>>> +	struct extcon_dev *extcon;
>>>> +	struct notifier_block extcon_nb;
>>>> +	struct work_struct extcon_wq;
>>>> +	int cable_state;
>>>>  	struct list_head mt_queue;
>>>>  	struct {
>>>>  		int r_size;
>>>> @@ -2175,6 +2181,77 @@ static void sii8620_init_rcp_input_dev(struct sii8620 *ctx)
>>>>  	ctx->rc_dev = rc_dev;
>>>>  }
>>>>  
>>>> +static void sii8620_cable_out(struct sii8620 *ctx)
>>>> +{
>>>> +	disable_irq(to_i2c_client(ctx->dev)->irq);
>>>> +	sii8620_hw_off(ctx);
>>>> +}
>>>> +
>>>> +static void sii8620_extcon_work(struct work_struct *work)
>>>> +{
>>>> +	struct sii8620 *ctx =
>>>> +		container_of(work, struct sii8620, extcon_wq);
>>>> +	int state = extcon_get_state(ctx->extcon, EXTCON_DISP_MHL);
>>>> +
>>>> +	if (state == ctx->cable_state)
>>>> +		return;
>>>> +
>>>> +	ctx->cable_state = state;
>>>> +
>>>> +	if (state > 0)
>>>> +		sii8620_cable_in(ctx);
>>>> +	else
>>>> +		sii8620_cable_out(ctx);
>>>> +}
>>>> +
>>>> +static int sii8620_extcon_notifier(struct notifier_block *self,
>>>> +			unsigned long event, void *ptr)
>>>> +{
>>>> +	struct sii8620 *ctx =
>>>> +		container_of(self, struct sii8620, extcon_nb);
>>>> +
>>>> +	schedule_work(&ctx->extcon_wq);
>>>> +
>>>> +	return NOTIFY_DONE;
>>>> +}
>>>> +
>>>> +static int sii8620_extcon_init(struct sii8620 *ctx)
>>>> +{
>>>> +	struct extcon_dev *edev;
>>>> +	struct device_node *musb, *muic;
>>>> +	int ret;
>>>> +
>>>> +	/* get micro-USB connector node */
>>>> +	musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1);
>>>> +	/* next get micro-USB Interface Controller node */
>>>> +	muic = of_get_next_parent(musb);
>>>> +
>>>> +	if (!muic) {
>>>> +		dev_info(ctx->dev, "no extcon found, switching to 'always on' mode\n");
>>>> +		return 0;
>>>> +	}
>>>> +
>>>> +	edev = extcon_find_edev_by_node(muic);
>>>> +	of_node_put(muic);
>>>> +	if (IS_ERR(edev)) {
>>>> +		if (PTR_ERR(edev) == -EPROBE_DEFER)
>>>> +			return -EPROBE_DEFER;
>>>> +		dev_err(ctx->dev, "Invalid or missing extcon\n");
>>>> +		return PTR_ERR(edev);
>>>> +	}
>>>> +
>>>> +	ctx->extcon = edev;
>>>> +	ctx->extcon_nb.notifier_call = sii8620_extcon_notifier;
>>>> +	INIT_WORK(&ctx->extcon_wq, sii8620_extcon_work);
>>>> +	ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, &ctx->extcon_nb);
>>> You better to use devm_extcon_register_notifier().
>> With devm version I risk that in case of device unbind notification will
>> be called after .remove callback, it seems to me quite dangerous. Or am
>> I missing something?
> If you use the cancel_work_sync() in remove() instead of flush_work(),
> you can use the 'devm_extcon_*'.

cancel_work_sync() does not prevent works scheduled later from execution
[1] and this scenario is possible with devm_extcon_register_notifier()
and cancel_work_sync().
So we end up with:
??? sii8620_remove() calls cancel_work_sync()
...
??? notifier(called asynchronously) schedules sii8620_extcon_work()
...
??? notifier is removed by devm framework
??? sii8620 context is destroyed by devm framework
...
??? sii8620_extcon_work is executed on destroyed context !!! BUG !!!

For me it seems that devm_extcon_register_notifier is not safe in this case.

[1]: Since documentation was not clear I have performed live test
confirming my suspicions.

Regards
Andrzej

>
>> Regards
>> Andrzej
>>
>>>> +	if (ret) {
>>>> +		dev_err(ctx->dev, "failed to register notifier for MHL\n");
>>>> +		return ret;
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>>  static inline struct sii8620 *bridge_to_sii8620(struct drm_bridge *bridge)
>>>>  {
>>>>  	return container_of(bridge, struct sii8620, bridge);
>>>> @@ -2307,13 +2384,20 @@ static int sii8620_probe(struct i2c_client *client,
>>>>  	if (ret)
>>>>  		return ret;
>>>>  
>>>> +	ret = sii8620_extcon_init(ctx);
>>>> +	if (ret < 0) {
>>>> +		dev_err(ctx->dev, "failed to initialize EXTCON\n");
>>>> +		return ret;
>>>> +	}
>>>> +
>>>>  	i2c_set_clientdata(client, ctx);
>>>>  
>>>>  	ctx->bridge.funcs = &sii8620_bridge_funcs;
>>>>  	ctx->bridge.of_node = dev->of_node;
>>>>  	drm_bridge_add(&ctx->bridge);
>>>>  
>>>> -	sii8620_cable_in(ctx);
>>>> +	if (!ctx->extcon)
>>>> +		sii8620_cable_in(ctx);
>>>>  
>>>>  	return 0;
>>>>  }
>>>> @@ -2322,8 +2406,15 @@ static int sii8620_remove(struct i2c_client *client)
>>>>  {
>>>>  	struct sii8620 *ctx = i2c_get_clientdata(client);
>>>>  
>>>> -	disable_irq(to_i2c_client(ctx->dev)->irq);
>>>> -	sii8620_hw_off(ctx);
>>>> +	if (ctx->extcon) {
>>>> +		extcon_unregister_notifier(ctx->extcon, EXTCON_DISP_MHL,
>>>> +					   &ctx->extcon_nb);
>>> Don't need to unregister the notifier if using devm_extcon_register_notifier().
>>>
>>>> +		flush_work(&ctx->extcon_wq);
>>>> +		if (ctx->cable_state > 0)
>>>> +			sii8620_cable_out(ctx);
>>>> +	} else {
>>>> +		sii8620_cable_out(ctx);
>>>> +	}
>>>>  	drm_bridge_remove(&ctx->bridge);
>>>>  
>>>>  	return 0;
>>>>
>>> If you use the resource managed function (devm_extcon_register_notifier), Looks good to me.
>>> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>

[PATCH v5 6/6] drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL

[Chanwoo Choi]

On 2018? 02? 28? 22:44, Andrzej Hajda wrote:
> On 27.02.2018 23:26, Chanwoo Choi wrote:
>> Hi,
>>
>> On 2018? 02? 27? 21:05, Andrzej Hajda wrote:
>>> On 27.02.2018 12:08, Chanwoo Choi wrote:
>>>> Hi,
>>>>
>>>> On 2018? 02? 27? 16:11, Andrzej Hajda wrote:
>>>>> From: Maciej Purski <m.purski@samsung.com>
>>>>>
>>>>> Currently MHL chip must be turned on permanently to detect MHL cable. It
>>>>> duplicates micro-USB controller's (MUIC) functionality and consumes
>>>>> unnecessary power. Lets use extcon attached to MUIC to enable MHL chip
>>>>> only if it detects MHL cable.
>>>>>
>>>>> Signed-off-by: Maciej Purski <m.purski@samsung.com>
>>>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>>>> ---
>>>>> v5: updated extcon API
>>>>>
>>>>> This is rework of the patch by Maciej with following changes:
>>>>> - use micro-USB port bindings to get extcon, instead of extcon property,
>>>>> - fixed remove sequence,
>>>>> - fixed extcon get state logic.
>>>>>
>>>>> Code finding extcon node is hacky IMO, I guess ultimately it should be done
>>>>> via some framework (maybe even extcon), or at least via helper, I hope it
>>>>> can stay as is until the proper solution will be merged.
>>>>>
>>>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>>>> ---
>>>>>  drivers/gpu/drm/bridge/sil-sii8620.c | 97 ++++++++++++++++++++++++++++++++++--
>>>>>  1 file changed, 94 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
>>>>> index 9e785b8e0ea2..62b0adabcac2 100644
>>>>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
>>>>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
>>>>> @@ -17,6 +17,7 @@
>>>>>  
>>>>>  #include <linux/clk.h>
>>>>>  #include <linux/delay.h>
>>>>> +#include <linux/extcon.h>
>>>>>  #include <linux/gpio/consumer.h>
>>>>>  #include <linux/i2c.h>
>>>>>  #include <linux/interrupt.h>
>>>>> @@ -25,6 +26,7 @@
>>>>>  #include <linux/list.h>
>>>>>  #include <linux/module.h>
>>>>>  #include <linux/mutex.h>
>>>>> +#include <linux/of_graph.h>
>>>>>  #include <linux/regulator/consumer.h>
>>>>>  #include <linux/slab.h>
>>>>>  
>>>>> @@ -81,6 +83,10 @@ struct sii8620 {
>>>>>  	struct edid *edid;
>>>>>  	unsigned int gen2_write_burst:1;
>>>>>  	enum sii8620_mt_state mt_state;
>>>>> +	struct extcon_dev *extcon;
>>>>> +	struct notifier_block extcon_nb;
>>>>> +	struct work_struct extcon_wq;
>>>>> +	int cable_state;
>>>>>  	struct list_head mt_queue;
>>>>>  	struct {
>>>>>  		int r_size;
>>>>> @@ -2175,6 +2181,77 @@ static void sii8620_init_rcp_input_dev(struct sii8620 *ctx)
>>>>>  	ctx->rc_dev = rc_dev;
>>>>>  }
>>>>>  
>>>>> +static void sii8620_cable_out(struct sii8620 *ctx)
>>>>> +{
>>>>> +	disable_irq(to_i2c_client(ctx->dev)->irq);
>>>>> +	sii8620_hw_off(ctx);
>>>>> +}
>>>>> +
>>>>> +static void sii8620_extcon_work(struct work_struct *work)
>>>>> +{
>>>>> +	struct sii8620 *ctx =
>>>>> +		container_of(work, struct sii8620, extcon_wq);
>>>>> +	int state = extcon_get_state(ctx->extcon, EXTCON_DISP_MHL);
>>>>> +
>>>>> +	if (state == ctx->cable_state)
>>>>> +		return;
>>>>> +
>>>>> +	ctx->cable_state = state;
>>>>> +
>>>>> +	if (state > 0)
>>>>> +		sii8620_cable_in(ctx);
>>>>> +	else
>>>>> +		sii8620_cable_out(ctx);
>>>>> +}
>>>>> +
>>>>> +static int sii8620_extcon_notifier(struct notifier_block *self,
>>>>> +			unsigned long event, void *ptr)
>>>>> +{
>>>>> +	struct sii8620 *ctx =
>>>>> +		container_of(self, struct sii8620, extcon_nb);
>>>>> +
>>>>> +	schedule_work(&ctx->extcon_wq);
>>>>> +
>>>>> +	return NOTIFY_DONE;
>>>>> +}
>>>>> +
>>>>> +static int sii8620_extcon_init(struct sii8620 *ctx)
>>>>> +{
>>>>> +	struct extcon_dev *edev;
>>>>> +	struct device_node *musb, *muic;
>>>>> +	int ret;
>>>>> +
>>>>> +	/* get micro-USB connector node */
>>>>> +	musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1);
>>>>> +	/* next get micro-USB Interface Controller node */
>>>>> +	muic = of_get_next_parent(musb);
>>>>> +
>>>>> +	if (!muic) {
>>>>> +		dev_info(ctx->dev, "no extcon found, switching to 'always on' mode\n");
>>>>> +		return 0;
>>>>> +	}
>>>>> +
>>>>> +	edev = extcon_find_edev_by_node(muic);
>>>>> +	of_node_put(muic);
>>>>> +	if (IS_ERR(edev)) {
>>>>> +		if (PTR_ERR(edev) == -EPROBE_DEFER)
>>>>> +			return -EPROBE_DEFER;
>>>>> +		dev_err(ctx->dev, "Invalid or missing extcon\n");
>>>>> +		return PTR_ERR(edev);
>>>>> +	}
>>>>> +
>>>>> +	ctx->extcon = edev;
>>>>> +	ctx->extcon_nb.notifier_call = sii8620_extcon_notifier;
>>>>> +	INIT_WORK(&ctx->extcon_wq, sii8620_extcon_work);
>>>>> +	ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, &ctx->extcon_nb);
>>>> You better to use devm_extcon_register_notifier().
>>> With devm version I risk that in case of device unbind notification will
>>> be called after .remove callback, it seems to me quite dangerous. Or am
>>> I missing something?
>> If you use the cancel_work_sync() in remove() instead of flush_work(),
>> you can use the 'devm_extcon_*'.
> 
> cancel_work_sync() does not prevent works scheduled later from execution
> [1] and this scenario is possible with devm_extcon_register_notifier()
> and cancel_work_sync().
> So we end up with:
>     sii8620_remove() calls cancel_work_sync()
> ...
>     notifier(called asynchronously) schedules sii8620_extcon_work()
> ...
>     notifier is removed by devm framework
>     sii8620 context is destroyed by devm framework
> ...
>     sii8620_extcon_work is executed on destroyed context !!! BUG !!!
> 
> For me it seems that devm_extcon_register_notifier is not safe in this case.
> 
> [1]: Since documentation was not clear I have performed live test
> confirming my suspicions.

You're right. Sorry for confusion.
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

[snip]

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

[PATCH v5 0/6] dt-bindings: add bindings for USB physical connector

[Andrzej Hajda]

Hi Rob, Chanwoo, Krzysztof,


On 27.02.2018 08:11, Andrzej Hajda wrote:
> Hi,
>
> Thanks for reviews of previous iterations.
>
> This patchset introduces USB physical connector bindings, together with
> working example.
> I have removed RFC prefix - the patchset seems to be heading
> to a happy end :)
>
> v5: fixed extra parenthesis in dts, renamed extcon function.
> v4: improved binding descriptions, added missing reg in dts.
> v3: Separate binding for Samsung 11-pin connector, added full-blown USB-C
>     example.
> v2: I have addressed comments by Rob and Laurent, thanks 
>
> Changes in datail are described in the patches.
>
> Regards
> Andrzej
>
>
> Andrzej Hajda (5):
>   dt-bindings: add bindings for USB physical connector
>   dt-bindings: add bindings for Samsung micro-USB 11-pin connector
>   arm64: dts: exynos: add micro-USB connector node to TM2 platforms
>   arm64: dts: exynos: add OF graph between MHL and USB connector
>   extcon: add possibility to get extcon device by OF node
>
> Maciej Purski (1):
>   drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL

It looks like all patches received R-B or acks (I forgot add Krzysztof's
acks for dts part).
Now I have a question how to merge them.
The only functional dependency is between bridge and extcon, and from
the formal PoV bindings should be merged 1st.
I can merge it:
1. All patches via drm-misc tree.
2. All patches except dts via drm-misc, and Krzysztof will merge dts via
samsung-soc tree.

Is it OK, for all? Better ideas?

Regards
Andrzej


>
>  .../connector/samsung,usb-connector-11pin.txt      | 49 +++++++++++
>  .../bindings/connector/usb-connector.txt           | 75 +++++++++++++++++
>  .../boot/dts/exynos/exynos5433-tm2-common.dtsi     | 38 ++++++++-
>  drivers/extcon/extcon.c                            | 44 +++++++---
>  drivers/gpu/drm/bridge/sil-sii8620.c               | 97 +++++++++++++++++++++-
>  include/linux/extcon.h                             |  6 ++
>  6 files changed, 293 insertions(+), 16 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/connector/samsung,usb-connector-11pin.txt
>  create mode 100644 Documentation/devicetree/bindings/connector/usb-connector.txt
>

[PATCH v5 0/6] dt-bindings: add bindings for USB physical connector

[Krzysztof Kozlowski]

On Tue, Mar 6, 2018 at 1:53 PM, Andrzej Hajda <a.hajda@samsung.com> wrote:
> Hi Rob, Chanwoo, Krzysztof,
>
>
> On 27.02.2018 08:11, Andrzej Hajda wrote:
>> Hi,
>>
>> Thanks for reviews of previous iterations.
>>
>> This patchset introduces USB physical connector bindings, together with
>> working example.
>> I have removed RFC prefix - the patchset seems to be heading
>> to a happy end :)
>>
>> v5: fixed extra parenthesis in dts, renamed extcon function.
>> v4: improved binding descriptions, added missing reg in dts.
>> v3: Separate binding for Samsung 11-pin connector, added full-blown USB-C
>>     example.
>> v2: I have addressed comments by Rob and Laurent, thanks
>>
>> Changes in datail are described in the patches.
>>
>> Regards
>> Andrzej
>>
>>
>> Andrzej Hajda (5):
>>   dt-bindings: add bindings for USB physical connector
>>   dt-bindings: add bindings for Samsung micro-USB 11-pin connector
>>   arm64: dts: exynos: add micro-USB connector node to TM2 platforms
>>   arm64: dts: exynos: add OF graph between MHL and USB connector
>>   extcon: add possibility to get extcon device by OF node
>>
>> Maciej Purski (1):
>>   drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL
>
> It looks like all patches received R-B or acks (I forgot add Krzysztof's
> acks for dts part).
> Now I have a question how to merge them.
> The only functional dependency is between bridge and extcon, and from
> the formal PoV bindings should be merged 1st.
> I can merge it:
> 1. All patches via drm-misc tree.
> 2. All patches except dts via drm-misc, and Krzysztof will merge dts via
> samsung-soc tree.

2) option please. The DTS should always go through arm-soc because it
is independent of driver code. I'll take the DTS today.

Best regards,
Krzysztof

[PATCH v5 0/6] dt-bindings: add bindings for USB physical connector

[Chanwoo Choi]

Hi Rob and Andrzej,

On 2018? 03? 06? 21:53, Andrzej Hajda wrote:
> Hi Rob, Chanwoo, Krzysztof,
> 
> 
> On 27.02.2018 08:11, Andrzej Hajda wrote:
>> Hi,
>>
>> Thanks for reviews of previous iterations.
>>
>> This patchset introduces USB physical connector bindings, together with
>> working example.
>> I have removed RFC prefix - the patchset seems to be heading
>> to a happy end :)
>>
>> v5: fixed extra parenthesis in dts, renamed extcon function.
>> v4: improved binding descriptions, added missing reg in dts.
>> v3: Separate binding for Samsung 11-pin connector, added full-blown USB-C
>>     example.
>> v2: I have addressed comments by Rob and Laurent, thanks 
>>
>> Changes in datail are described in the patches.
>>
>> Regards
>> Andrzej
>>
>>
>> Andrzej Hajda (5):
>>   dt-bindings: add bindings for USB physical connector
>>   dt-bindings: add bindings for Samsung micro-USB 11-pin connector
>>   arm64: dts: exynos: add micro-USB connector node to TM2 platforms
>>   arm64: dts: exynos: add OF graph between MHL and USB connector
>>   extcon: add possibility to get extcon device by OF node
>>
>> Maciej Purski (1):
>>   drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL
> 
> It looks like all patches received R-B or acks (I forgot add Krzysztof's
> acks for dts part).
> Now I have a question how to merge them.
> The only functional dependency is between bridge and extcon, and from
> the formal PoV bindings should be merged 1st.
> I can merge it:
> 1. All patches via drm-misc tree.
> 2. All patches except dts via drm-misc, and Krzysztof will merge dts via
> samsung-soc tree.
> 
> Is it OK, for all? Better ideas?

Krzysztof picked the dts patches. I'll make the immutable branch based on v4.16-rc1
and apply them except for dts patchs. And I'll send the immutable branch to Rob and Andrzej.


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

[PATCH v5 0/6] dt-bindings: add bindings for USB physical connector

[Chanwoo Choi]

On 2018? 03? 07? 11:12, Chanwoo Choi wrote:
> Hi Rob and Andrzej,
> 
> On 2018? 03? 06? 21:53, Andrzej Hajda wrote:
>> Hi Rob, Chanwoo, Krzysztof,
>>
>>
>> On 27.02.2018 08:11, Andrzej Hajda wrote:
>>> Hi,
>>>
>>> Thanks for reviews of previous iterations.
>>>
>>> This patchset introduces USB physical connector bindings, together with
>>> working example.
>>> I have removed RFC prefix - the patchset seems to be heading
>>> to a happy end :)
>>>
>>> v5: fixed extra parenthesis in dts, renamed extcon function.
>>> v4: improved binding descriptions, added missing reg in dts.
>>> v3: Separate binding for Samsung 11-pin connector, added full-blown USB-C
>>>     example.
>>> v2: I have addressed comments by Rob and Laurent, thanks 
>>>
>>> Changes in datail are described in the patches.
>>>
>>> Regards
>>> Andrzej
>>>
>>>
>>> Andrzej Hajda (5):
>>>   dt-bindings: add bindings for USB physical connector
>>>   dt-bindings: add bindings for Samsung micro-USB 11-pin connector
>>>   arm64: dts: exynos: add micro-USB connector node to TM2 platforms
>>>   arm64: dts: exynos: add OF graph between MHL and USB connector
>>>   extcon: add possibility to get extcon device by OF node
>>>
>>> Maciej Purski (1):
>>>   drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL
>>
>> It looks like all patches received R-B or acks (I forgot add Krzysztof's
>> acks for dts part).
>> Now I have a question how to merge them.
>> The only functional dependency is between bridge and extcon, and from
>> the formal PoV bindings should be merged 1st.
>> I can merge it:
>> 1. All patches via drm-misc tree.
>> 2. All patches except dts via drm-misc, and Krzysztof will merge dts via
>> samsung-soc tree.
>>
>> Is it OK, for all? Better ideas?
> 
> Krzysztof picked the dts patches. I'll make the immutable branch based on v4.16-rc1
> and apply them except for dts patchs. And I'll send the immutable branch to Rob and Andrzej.
> 
> 

I made the immutable branch[1] as following: If you agree, I'll send pull request.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git/log/?h=ib-extcon-drm-dt-v4.17

Or you can make the immutable branch and send pull request to Rob and me.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

[PATCH v5 0/6] dt-bindings: add bindings for USB physical connector

[Andrzej Hajda]

Hi Chanwoo, Archit,

On 07.03.2018 05:48, Chanwoo Choi wrote:
> On 2018? 03? 07? 11:12, Chanwoo Choi wrote:
>> Hi Rob and Andrzej,
>>
>> On 2018? 03? 06? 21:53, Andrzej Hajda wrote:
>>> Hi Rob, Chanwoo, Krzysztof,
>>>
>>>
>>> On 27.02.2018 08:11, Andrzej Hajda wrote:
>>>> Hi,
>>>>
>>>> Thanks for reviews of previous iterations.
>>>>
>>>> This patchset introduces USB physical connector bindings, together with
>>>> working example.
>>>> I have removed RFC prefix - the patchset seems to be heading
>>>> to a happy end :)
>>>>
>>>> v5: fixed extra parenthesis in dts, renamed extcon function.
>>>> v4: improved binding descriptions, added missing reg in dts.
>>>> v3: Separate binding for Samsung 11-pin connector, added full-blown USB-C
>>>>     example.
>>>> v2: I have addressed comments by Rob and Laurent, thanks 
>>>>
>>>> Changes in datail are described in the patches.
>>>>
>>>> Regards
>>>> Andrzej
>>>>
>>>>
>>>> Andrzej Hajda (5):
>>>>   dt-bindings: add bindings for USB physical connector
>>>>   dt-bindings: add bindings for Samsung micro-USB 11-pin connector
>>>>   arm64: dts: exynos: add micro-USB connector node to TM2 platforms
>>>>   arm64: dts: exynos: add OF graph between MHL and USB connector
>>>>   extcon: add possibility to get extcon device by OF node
>>>>
>>>> Maciej Purski (1):
>>>>   drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL
>>> It looks like all patches received R-B or acks (I forgot add Krzysztof's
>>> acks for dts part).
>>> Now I have a question how to merge them.
>>> The only functional dependency is between bridge and extcon, and from
>>> the formal PoV bindings should be merged 1st.
>>> I can merge it:
>>> 1. All patches via drm-misc tree.
>>> 2. All patches except dts via drm-misc, and Krzysztof will merge dts via
>>> samsung-soc tree.
>>>
>>> Is it OK, for all? Better ideas?
>> Krzysztof picked the dts patches. I'll make the immutable branch based on v4.16-rc1
>> and apply them except for dts patchs. And I'll send the immutable branch to Rob and Andrzej.
>>
>>
> I made the immutable branch[1] as following: If you agree, I'll send pull request.
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git/log/?h=ib-extcon-drm-dt-v4.17
>
> Or you can make the immutable branch and send pull request to Rob and me.
>

It seems you took v5 instead of v6 version of extcon patch.

Beside it I am OK with your immutable branch, Archit is it OK to do it
this way in drm-misc?


Regards

Andrzej

[PATCH v5 0/6] dt-bindings: add bindings for USB physical connector

[Krzysztof Kozlowski]

On Wed, Mar 7, 2018 at 12:13 PM, Andrzej Hajda <a.hajda@samsung.com> wrote:
> Hi Chanwoo, Archit,
>
> On 07.03.2018 05:48, Chanwoo Choi wrote:
>> On 2018? 03? 07? 11:12, Chanwoo Choi wrote:
>>> Hi Rob and Andrzej,
>>>
>>> On 2018? 03? 06? 21:53, Andrzej Hajda wrote:
>>>> Hi Rob, Chanwoo, Krzysztof,
>>>>
>>>>
>>>> On 27.02.2018 08:11, Andrzej Hajda wrote:
>>>>> Hi,
>>>>>
>>>>> Thanks for reviews of previous iterations.
>>>>>
>>>>> This patchset introduces USB physical connector bindings, together with
>>>>> working example.
>>>>> I have removed RFC prefix - the patchset seems to be heading
>>>>> to a happy end :)
>>>>>
>>>>> v5: fixed extra parenthesis in dts, renamed extcon function.
>>>>> v4: improved binding descriptions, added missing reg in dts.
>>>>> v3: Separate binding for Samsung 11-pin connector, added full-blown USB-C
>>>>>     example.
>>>>> v2: I have addressed comments by Rob and Laurent, thanks
>>>>>
>>>>> Changes in datail are described in the patches.
>>>>>
>>>>> Regards
>>>>> Andrzej
>>>>>
>>>>>
>>>>> Andrzej Hajda (5):
>>>>>   dt-bindings: add bindings for USB physical connector
>>>>>   dt-bindings: add bindings for Samsung micro-USB 11-pin connector
>>>>>   arm64: dts: exynos: add micro-USB connector node to TM2 platforms
>>>>>   arm64: dts: exynos: add OF graph between MHL and USB connector
>>>>>   extcon: add possibility to get extcon device by OF node
>>>>>
>>>>> Maciej Purski (1):
>>>>>   drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL
>>>> It looks like all patches received R-B or acks (I forgot add Krzysztof's
>>>> acks for dts part).
>>>> Now I have a question how to merge them.
>>>> The only functional dependency is between bridge and extcon, and from
>>>> the formal PoV bindings should be merged 1st.
>>>> I can merge it:
>>>> 1. All patches via drm-misc tree.
>>>> 2. All patches except dts via drm-misc, and Krzysztof will merge dts via
>>>> samsung-soc tree.
>>>>
>>>> Is it OK, for all? Better ideas?
>>> Krzysztof picked the dts patches. I'll make the immutable branch based on v4.16-rc1
>>> and apply them except for dts patchs. And I'll send the immutable branch to Rob and Andrzej.
>>>
>>>
>> I made the immutable branch[1] as following: If you agree, I'll send pull request.
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git/log/?h=ib-extcon-drm-dt-v4.17
>>
>> Or you can make the immutable branch and send pull request to Rob and me.
>>
>
> It seems you took v5 instead of v6 version of extcon patch.

I also took v5:
https://patchwork.kernel.org/patch/10244407/
https://patchwork.kernel.org/patch/10244431/

There was no v6 in samsung-soc patchwork. Is it a problem for DTS?

BR,
Krzysztof

[PATCH v5 0/6] dt-bindings: add bindings for USB physical connector

[Andrzej Hajda]

On 07.03.2018 12:22, Krzysztof Kozlowski wrote:
> On Wed, Mar 7, 2018 at 12:13 PM, Andrzej Hajda <a.hajda@samsung.com> wrote:
>> Hi Chanwoo, Archit,
>>
>> On 07.03.2018 05:48, Chanwoo Choi wrote:
>>> On 2018? 03? 07? 11:12, Chanwoo Choi wrote:
>>>> Hi Rob and Andrzej,
>>>>
>>>> On 2018? 03? 06? 21:53, Andrzej Hajda wrote:
>>>>> Hi Rob, Chanwoo, Krzysztof,
>>>>>
>>>>>
>>>>> On 27.02.2018 08:11, Andrzej Hajda wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Thanks for reviews of previous iterations.
>>>>>>
>>>>>> This patchset introduces USB physical connector bindings, together with
>>>>>> working example.
>>>>>> I have removed RFC prefix - the patchset seems to be heading
>>>>>> to a happy end :)
>>>>>>
>>>>>> v5: fixed extra parenthesis in dts, renamed extcon function.
>>>>>> v4: improved binding descriptions, added missing reg in dts.
>>>>>> v3: Separate binding for Samsung 11-pin connector, added full-blown USB-C
>>>>>>     example.
>>>>>> v2: I have addressed comments by Rob and Laurent, thanks
>>>>>>
>>>>>> Changes in datail are described in the patches.
>>>>>>
>>>>>> Regards
>>>>>> Andrzej
>>>>>>
>>>>>>
>>>>>> Andrzej Hajda (5):
>>>>>>   dt-bindings: add bindings for USB physical connector
>>>>>>   dt-bindings: add bindings for Samsung micro-USB 11-pin connector
>>>>>>   arm64: dts: exynos: add micro-USB connector node to TM2 platforms
>>>>>>   arm64: dts: exynos: add OF graph between MHL and USB connector
>>>>>>   extcon: add possibility to get extcon device by OF node
>>>>>>
>>>>>> Maciej Purski (1):
>>>>>>   drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL
>>>>> It looks like all patches received R-B or acks (I forgot add Krzysztof's
>>>>> acks for dts part).
>>>>> Now I have a question how to merge them.
>>>>> The only functional dependency is between bridge and extcon, and from
>>>>> the formal PoV bindings should be merged 1st.
>>>>> I can merge it:
>>>>> 1. All patches via drm-misc tree.
>>>>> 2. All patches except dts via drm-misc, and Krzysztof will merge dts via
>>>>> samsung-soc tree.
>>>>>
>>>>> Is it OK, for all? Better ideas?
>>>> Krzysztof picked the dts patches. I'll make the immutable branch based on v4.16-rc1
>>>> and apply them except for dts patchs. And I'll send the immutable branch to Rob and Andrzej.
>>>>
>>>>
>>> I made the immutable branch[1] as following: If you agree, I'll send pull request.
>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git/log/?h=ib-extcon-drm-dt-v4.17
>>>
>>> Or you can make the immutable branch and send pull request to Rob and me.
>>>
>> It seems you took v5 instead of v6 version of extcon patch.
> I also took v5:
> https://patchwork.kernel.org/patch/10244407/
> https://patchwork.kernel.org/patch/10244431/
>
> There was no v6 in samsung-soc patchwork. Is it a problem for DTS?

No, v6 was only typo fix in comment, and only extcon patch has v6.

Regards
Andrzej

>
> BR,
> Krzysztof
>
>
>

[PATCH v5 0/6] dt-bindings: add bindings for USB physical connector

[Chanwoo Choi]

Hi Andrzej, Archit,

On 2018? 03? 07? 20:13, Andrzej Hajda wrote:
> Hi Chanwoo, Archit,
> 
> On 07.03.2018 05:48, Chanwoo Choi wrote:
>> On 2018? 03? 07? 11:12, Chanwoo Choi wrote:
>>> Hi Rob and Andrzej,
>>>
>>> On 2018? 03? 06? 21:53, Andrzej Hajda wrote:
>>>> Hi Rob, Chanwoo, Krzysztof,
>>>>
>>>>
>>>> On 27.02.2018 08:11, Andrzej Hajda wrote:
>>>>> Hi,
>>>>>
>>>>> Thanks for reviews of previous iterations.
>>>>>
>>>>> This patchset introduces USB physical connector bindings, together with
>>>>> working example.
>>>>> I have removed RFC prefix - the patchset seems to be heading
>>>>> to a happy end :)
>>>>>
>>>>> v5: fixed extra parenthesis in dts, renamed extcon function.
>>>>> v4: improved binding descriptions, added missing reg in dts.
>>>>> v3: Separate binding for Samsung 11-pin connector, added full-blown USB-C
>>>>>     example.
>>>>> v2: I have addressed comments by Rob and Laurent, thanks 
>>>>>
>>>>> Changes in datail are described in the patches.
>>>>>
>>>>> Regards
>>>>> Andrzej
>>>>>
>>>>>
>>>>> Andrzej Hajda (5):
>>>>>   dt-bindings: add bindings for USB physical connector
>>>>>   dt-bindings: add bindings for Samsung micro-USB 11-pin connector
>>>>>   arm64: dts: exynos: add micro-USB connector node to TM2 platforms
>>>>>   arm64: dts: exynos: add OF graph between MHL and USB connector
>>>>>   extcon: add possibility to get extcon device by OF node
>>>>>
>>>>> Maciej Purski (1):
>>>>>   drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL
>>>> It looks like all patches received R-B or acks (I forgot add Krzysztof's
>>>> acks for dts part).
>>>> Now I have a question how to merge them.
>>>> The only functional dependency is between bridge and extcon, and from
>>>> the formal PoV bindings should be merged 1st.
>>>> I can merge it:
>>>> 1. All patches via drm-misc tree.
>>>> 2. All patches except dts via drm-misc, and Krzysztof will merge dts via
>>>> samsung-soc tree.
>>>>
>>>> Is it OK, for all? Better ideas?
>>> Krzysztof picked the dts patches. I'll make the immutable branch based on v4.16-rc1
>>> and apply them except for dts patchs. And I'll send the immutable branch to Rob and Andrzej.
>>>
>>>
>> I made the immutable branch[1] as following: If you agree, I'll send pull request.
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git/log/?h=ib-extcon-drm-dt-v4.17
>>
>> Or you can make the immutable branch and send pull request to Rob and me.
>>
> 
> It seems you took v5 instead of v6 version of extcon patch.

My mistake. I picked v6 and made the new immutable branch.
After Archit confirm it, I'll send pull request.

> 
> Beside it I am OK with your immutable branch, Archit is it OK to do it
> this way in drm-misc?
> 
> 
> Regards
> 
> Andrzej
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

[PATCH v5 0/6] dt-bindings: add bindings for USB physical connector

[Andrzej Hajda]

Hi Chanwoo,

On 08.03.2018 02:52, Chanwoo Choi wrote:
> Hi Andrzej, Archit,
>
> On 2018? 03? 07? 20:13, Andrzej Hajda wrote:
>> Hi Chanwoo, Archit,
>>
>> On 07.03.2018 05:48, Chanwoo Choi wrote:
>>> On 2018? 03? 07? 11:12, Chanwoo Choi wrote:
>>>> Hi Rob and Andrzej,
>>>>
>>>> On 2018? 03? 06? 21:53, Andrzej Hajda wrote:
>>>>> Hi Rob, Chanwoo, Krzysztof,
>>>>>
>>>>>
>>>>> On 27.02.2018 08:11, Andrzej Hajda wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Thanks for reviews of previous iterations.
>>>>>>
>>>>>> This patchset introduces USB physical connector bindings, together with
>>>>>> working example.
>>>>>> I have removed RFC prefix - the patchset seems to be heading
>>>>>> to a happy end :)
>>>>>>
>>>>>> v5: fixed extra parenthesis in dts, renamed extcon function.
>>>>>> v4: improved binding descriptions, added missing reg in dts.
>>>>>> v3: Separate binding for Samsung 11-pin connector, added full-blown USB-C
>>>>>>     example.
>>>>>> v2: I have addressed comments by Rob and Laurent, thanks 
>>>>>>
>>>>>> Changes in datail are described in the patches.
>>>>>>
>>>>>> Regards
>>>>>> Andrzej
>>>>>>
>>>>>>
>>>>>> Andrzej Hajda (5):
>>>>>>   dt-bindings: add bindings for USB physical connector
>>>>>>   dt-bindings: add bindings for Samsung micro-USB 11-pin connector
>>>>>>   arm64: dts: exynos: add micro-USB connector node to TM2 platforms
>>>>>>   arm64: dts: exynos: add OF graph between MHL and USB connector
>>>>>>   extcon: add possibility to get extcon device by OF node
>>>>>>
>>>>>> Maciej Purski (1):
>>>>>>   drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL
>>>>> It looks like all patches received R-B or acks (I forgot add Krzysztof's
>>>>> acks for dts part).
>>>>> Now I have a question how to merge them.
>>>>> The only functional dependency is between bridge and extcon, and from
>>>>> the formal PoV bindings should be merged 1st.
>>>>> I can merge it:
>>>>> 1. All patches via drm-misc tree.
>>>>> 2. All patches except dts via drm-misc, and Krzysztof will merge dts via
>>>>> samsung-soc tree.
>>>>>
>>>>> Is it OK, for all? Better ideas?
>>>> Krzysztof picked the dts patches. I'll make the immutable branch based on v4.16-rc1
>>>> and apply them except for dts patchs. And I'll send the immutable branch to Rob and Andrzej.
>>>>
>>>>
>>> I made the immutable branch[1] as following: If you agree, I'll send pull request.
>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git/log/?h=ib-extcon-drm-dt-v4.17
>>>
>>> Or you can make the immutable branch and send pull request to Rob and me.
>>>
>> It seems you took v5 instead of v6 version of extcon patch.
> My mistake. I picked v6 and made the new immutable branch.
> After Archit confirm it, I'll send pull request.
>


Lets just queue all patches (except dts) via extcon branch (thanks
Daniel for advice), without making immutable branch.
It seems to be a simplest acceptable approach.

You can add my ack to sii8620 bridge patch (as bridge maintainer), to
fulfill process requirements.

Regards
Andrzej

[PATCH v5 0/6] dt-bindings: add bindings for USB physical connector

[Chanwoo Choi]

Hi Andrzej and Rob,

On 2018? 03? 09? 18:20, Andrzej Hajda wrote:
> Hi Chanwoo,
> 
> On 08.03.2018 02:52, Chanwoo Choi wrote:
>> Hi Andrzej, Archit,
>>
>> On 2018? 03? 07? 20:13, Andrzej Hajda wrote:
>>> Hi Chanwoo, Archit,
>>>
>>> On 07.03.2018 05:48, Chanwoo Choi wrote:
>>>> On 2018? 03? 07? 11:12, Chanwoo Choi wrote:
>>>>> Hi Rob and Andrzej,
>>>>>
>>>>> On 2018? 03? 06? 21:53, Andrzej Hajda wrote:
>>>>>> Hi Rob, Chanwoo, Krzysztof,
>>>>>>
>>>>>>
>>>>>> On 27.02.2018 08:11, Andrzej Hajda wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Thanks for reviews of previous iterations.
>>>>>>>
>>>>>>> This patchset introduces USB physical connector bindings, together with
>>>>>>> working example.
>>>>>>> I have removed RFC prefix - the patchset seems to be heading
>>>>>>> to a happy end :)
>>>>>>>
>>>>>>> v5: fixed extra parenthesis in dts, renamed extcon function.
>>>>>>> v4: improved binding descriptions, added missing reg in dts.
>>>>>>> v3: Separate binding for Samsung 11-pin connector, added full-blown USB-C
>>>>>>>     example.
>>>>>>> v2: I have addressed comments by Rob and Laurent, thanks 
>>>>>>>
>>>>>>> Changes in datail are described in the patches.
>>>>>>>
>>>>>>> Regards
>>>>>>> Andrzej
>>>>>>>
>>>>>>>
>>>>>>> Andrzej Hajda (5):
>>>>>>>   dt-bindings: add bindings for USB physical connector
>>>>>>>   dt-bindings: add bindings for Samsung micro-USB 11-pin connector
>>>>>>>   arm64: dts: exynos: add micro-USB connector node to TM2 platforms
>>>>>>>   arm64: dts: exynos: add OF graph between MHL and USB connector
>>>>>>>   extcon: add possibility to get extcon device by OF node
>>>>>>>
>>>>>>> Maciej Purski (1):
>>>>>>>   drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL
>>>>>> It looks like all patches received R-B or acks (I forgot add Krzysztof's
>>>>>> acks for dts part).
>>>>>> Now I have a question how to merge them.
>>>>>> The only functional dependency is between bridge and extcon, and from
>>>>>> the formal PoV bindings should be merged 1st.
>>>>>> I can merge it:
>>>>>> 1. All patches via drm-misc tree.
>>>>>> 2. All patches except dts via drm-misc, and Krzysztof will merge dts via
>>>>>> samsung-soc tree.
>>>>>>
>>>>>> Is it OK, for all? Better ideas?
>>>>> Krzysztof picked the dts patches. I'll make the immutable branch based on v4.16-rc1
>>>>> and apply them except for dts patchs. And I'll send the immutable branch to Rob and Andrzej.
>>>>>
>>>>>
>>>> I made the immutable branch[1] as following: If you agree, I'll send pull request.
>>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git/log/?h=ib-extcon-drm-dt-v4.17
>>>>
>>>> Or you can make the immutable branch and send pull request to Rob and me.
>>>>
>>> It seems you took v5 instead of v6 version of extcon patch.
>> My mistake. I picked v6 and made the new immutable branch.
>> After Archit confirm it, I'll send pull request.
>>
> 
> 
> Lets just queue all patches (except dts) via extcon branch (thanks
> Daniel for advice), without making immutable branch.
> It seems to be a simplest acceptable approach.
> 
> You can add my ack to sii8620 bridge patch (as bridge maintainer), to
> fulfill process requirements.

I added your ack to sii8620 bridge patch and
send pull request for extcon, drm bridge and device-tree.

-----------------------

The following changes since commit 7928b2cbe55b2a410a0f5c1f154610059c57b1b2:

  Linux 4.16-rc1 (2018-02-11 15:04:29 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git ib-extcon-drm-dt-v4.17

for you to fetch changes up to 688838442147d9dd94c2ef7c2c31a35cf150c5fa:

  drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL (2018-03-12 10:18:26 +0900)

----------------------------------------------------------------
Andrzej Hajda (3):
      dt-bindings: add bindings for USB physical connector
      dt-bindings: add bindings for Samsung micro-USB 11-pin connector
      extcon: add possibility to get extcon device by OF node

Maciej Purski (1):
      drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL

 .../connector/samsung,usb-connector-11pin.txt      | 49 +++++++++++
 .../bindings/connector/usb-connector.txt           | 75 +++++++++++++++++
 drivers/extcon/extcon.c                            | 44 +++++++---
 drivers/gpu/drm/bridge/sil-sii8620.c               | 97 +++++++++++++++++++++-
 include/linux/extcon.h                             |  6 ++
 5 files changed, 258 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/connector/samsung,usb-connector-11pin.txt
 create mode 100644 Documentation/devicetree/bindings/connector/usb-connector.txt

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics