[PATCH 0/4] arm64: qcom: sm8550: retrieve USB-C port orientation from gpio

[PATCH 0/4] arm64: qcom: sm8550: retrieve USB-C port orientation from gpio

[Neil Armstrong]

This patchset is derived from [1], with only the GPIO orientation.

On the SM8550 platform, the PMIC Glink firmware doesn't emit
ALTMODE events for all USB-C port changes like it was done
for older platforms (like SM8450).
This means we only have a valid orientation when an Altmode
compliant device is inserted, but not for all the other devices,
including USB cables, dongles and non-altmode Hubs.

But the actual orientation is shared by the PM8550B by
a signal called CC_OUT which can be read on a gpio of the SM8550
SoC.

Let's add support for this feature by using the UCSI PMIC
Glink driver events to read and dispatch the orientation
to the USB-C connector devices on the DT graph.

While everybody would prefer having a proper PMIC Glink event
for an orientation change, this is not implemented and not
planned for future platforms either.

[1] https://lore.kernel.org/all/20230601-topic-sm8550-upstream-type-c-v3-0-22c9973012b6@linaro.org/

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
Neil Armstrong (4):
      dt-bindings: soc: qcom: qcom,pmic-glink: add a gpio used to determine the Type-C port plug orientation
      usb: ucsi: glink: use the connector orientation GPIO to provide switch events
      arm64: dts: qcom: sm8550-mtp: add orientation gpio
      arm64: dts: qcom: sm8550-qrd: add orientation gpio

 .../bindings/soc/qcom/qcom,pmic-glink.yaml         | 18 ++++++++
 arch/arm64/boot/dts/qcom/sm8550-mtp.dts            |  1 +
 arch/arm64/boot/dts/qcom/sm8550-qrd.dts            |  1 +
 drivers/usb/typec/ucsi/ucsi_glink.c                | 54 +++++++++++++++++++++-
 4 files changed, 73 insertions(+), 1 deletion(-)
---
base-commit: fb4327106e5250ee360d0d8b056c1eef7eeb9a98
change-id: 20230804-topic-sm8550-upstream-type-c-orientation-0bbbb360d3f7

Best regards,
-- 
Neil Armstrong <neil.armstrong@linaro.org>

[PATCH 1/4] dt-bindings: soc: qcom: qcom,pmic-glink: add a gpio used to determine the Type-C port plug orientation

[Neil Armstrong]

On SM8450 and SM8550 based platforms, the Type-C plug orientation is given on a
GPIO line set by the PMIC.

Document this optional Type-C connector property, and take the
assumption an active level represents an inverted/flipped orientation.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml  | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
index bceb479f74c5..1b0a00b19a54 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
@@ -35,6 +35,11 @@ properties:
   '#size-cells':
     const: 0
 
+  orientation-gpios:
+    description: An input gpio for Type-C connector orientation, used to detect orientation
+      of the Type-C connector. GPIO active level means "CC2" or Reversed/Flipped orientation.
+    maxItems: 1
+
 patternProperties:
   '^connector@\d$':
     $ref: /schemas/connector/usb-connector.yaml#
@@ -44,6 +49,19 @@ patternProperties:
 required:
   - compatible
 
+allOf:
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              enum:
+                - qcom,sm8450-pmic-glink
+                - qcom,sm8550-pmic-glink
+    then:
+      properties:
+        orientation-gpios: false
+
 additionalProperties: false
 
 examples:

-- 
2.34.1

[PATCH 2/4] usb: ucsi: glink: use the connector orientation GPIO to provide switch events

[Neil Armstrong]

On SM8550, the non-altmode orientation is not given anymore within
altmode events, even with USB SVIDs events.

On the other side, the Type-C connector orientation is correctly
reported by a signal from the PMIC.

Take this gpio signal when we detect some Type-C port activity
to notify any Type-C switches tied to the Type-C port connectors.

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/usb/typec/ucsi/ucsi_glink.c | 54 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
index bb1854b3311d..db6e248f8208 100644
--- a/drivers/usb/typec/ucsi/ucsi_glink.c
+++ b/drivers/usb/typec/ucsi/ucsi_glink.c
@@ -8,9 +8,13 @@
 #include <linux/mutex.h>
 #include <linux/property.h>
 #include <linux/soc/qcom/pdr.h>
+#include <linux/usb/typec_mux.h>
+#include <linux/gpio/consumer.h>
 #include <linux/soc/qcom/pmic_glink.h>
 #include "ucsi.h"
 
+#define PMIC_GLINK_MAX_PORTS	2
+
 #define UCSI_BUF_SIZE                   48
 
 #define MSG_TYPE_REQ_RESP               1
@@ -52,6 +56,9 @@ struct ucsi_notify_ind_msg {
 struct pmic_glink_ucsi {
 	struct device *dev;
 
+	struct gpio_desc *port_orientation[PMIC_GLINK_MAX_PORTS];
+	struct typec_switch *port_switch[PMIC_GLINK_MAX_PORTS];
+
 	struct pmic_glink_client *client;
 
 	struct ucsi *ucsi;
@@ -220,8 +227,20 @@ static void pmic_glink_ucsi_notify(struct work_struct *work)
 	}
 
 	con_num = UCSI_CCI_CONNECTOR(cci);
-	if (con_num)
+	if (con_num) {
+		if (con_num < PMIC_GLINK_MAX_PORTS &&
+		    ucsi->port_orientation[con_num - 1]) {
+			int orientation = gpiod_get_value(ucsi->port_orientation[con_num - 1]);
+
+			if (orientation >= 0) {
+				typec_switch_set(ucsi->port_switch[con_num - 1],
+						 orientation ? TYPEC_ORIENTATION_REVERSE
+							     : TYPEC_ORIENTATION_NORMAL);
+			}
+		}
+
 		ucsi_connector_change(ucsi->ucsi, con_num);
+	}
 
 	if (ucsi->sync_pending && cci & UCSI_CCI_BUSY) {
 		ucsi->sync_val = -EBUSY;
@@ -282,6 +301,7 @@ static int pmic_glink_ucsi_probe(struct auxiliary_device *adev,
 {
 	struct pmic_glink_ucsi *ucsi;
 	struct device *dev = &adev->dev;
+	struct fwnode_handle *fwnode;
 	int ret;
 
 	ucsi = devm_kzalloc(dev, sizeof(*ucsi), GFP_KERNEL);
@@ -309,6 +329,38 @@ static int pmic_glink_ucsi_probe(struct auxiliary_device *adev,
 
 	ucsi_set_drvdata(ucsi->ucsi, ucsi);
 
+	device_for_each_child_node(dev, fwnode) {
+		struct gpio_desc *desc;
+		u32 port;
+
+		ret = fwnode_property_read_u32(fwnode, "reg", &port);
+		if (ret < 0) {
+			dev_err(dev, "missing reg property of %pOFn\n", fwnode);
+			return ret;
+		}
+
+		if (port >= PMIC_GLINK_MAX_PORTS) {
+			dev_warn(dev, "invalid connector number, ignoring\n");
+			continue;
+		}
+
+		desc = devm_gpiod_get_index_optional(&adev->dev, "orientation", port, GPIOD_IN);
+
+		/* If GPIO isn't found, continue */
+		if (!desc)
+			continue;
+
+		if (IS_ERR(desc))
+			return dev_err_probe(dev, PTR_ERR(desc),
+					     "unable to acquire orientation gpio\n");
+		ucsi->port_orientation[port] = desc;
+
+		ucsi->port_switch[port] = fwnode_typec_switch_get(fwnode);
+		if (IS_ERR(ucsi->port_switch[port]))
+			return dev_err_probe(dev, PTR_ERR(ucsi->port_switch[port]),
+					"failed to acquire orientation-switch\n");
+	}
+
 	ucsi->client = devm_pmic_glink_register_client(dev,
 						       PMIC_GLINK_OWNER_USBC,
 						       pmic_glink_ucsi_callback,

-- 
2.34.1

[PATCH 3/4] arm64: dts: qcom: sm8550-mtp: add orientation gpio

[Neil Armstrong]

Specify orientation GPIO in the PMIC GLINK node.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8550-mtp.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8550-mtp.dts b/arch/arm64/boot/dts/qcom/sm8550-mtp.dts
index 39a43db26ea1..4fef4eef6575 100644
--- a/arch/arm64/boot/dts/qcom/sm8550-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sm8550-mtp.dts
@@ -58,6 +58,7 @@ pmic-glink {
 		compatible = "qcom,sm8550-pmic-glink", "qcom,pmic-glink";
 		#address-cells = <1>;
 		#size-cells = <0>;
+		orientation-gpios = <&tlmm 11 GPIO_ACTIVE_HIGH>;
 
 		connector@0 {
 			compatible = "usb-c-connector";

-- 
2.34.1

[PATCH 4/4] arm64: dts: qcom: sm8550-qrd: add orientation gpio

[Neil Armstrong]

Specify orientation GPIO to the PMIC GLINK node.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
index 2c09ce8aeafd..950dfbf73410 100644
--- a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
+++ b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
@@ -75,6 +75,7 @@ pmic-glink {
 		compatible = "qcom,sm8550-pmic-glink", "qcom,pmic-glink";
 		#address-cells = <1>;
 		#size-cells = <0>;
+		orientation-gpios = <&tlmm 11 GPIO_ACTIVE_HIGH>;
 
 		connector@0 {
 			compatible = "usb-c-connector";

-- 
2.34.1

Re: [PATCH 1/4] dt-bindings: soc: qcom: qcom,pmic-glink: add a gpio used to determine the Type-C port plug orientation

[Bjorn Andersson]

On Fri, Aug 04, 2023 at 03:50:07PM +0200, Neil Armstrong wrote:
> On SM8450 and SM8550 based platforms, the Type-C plug orientation is given on a
> GPIO line set by the PMIC.
> 
> Document this optional Type-C connector property, and take the
> assumption an active level represents an inverted/flipped orientation.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  .../devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml  | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> index bceb479f74c5..1b0a00b19a54 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> @@ -35,6 +35,11 @@ properties:
>    '#size-cells':
>      const: 0
>  
> +  orientation-gpios:
> +    description: An input gpio for Type-C connector orientation, used to detect orientation
> +      of the Type-C connector. GPIO active level means "CC2" or Reversed/Flipped orientation.

This is a per-connector/port property, as such be part of the connector
sub nodes rather than as a property of the shared pmic_glink entity.

Regards,
Bjorn

> +    maxItems: 1
> +
>  patternProperties:
>    '^connector@\d$':
>      $ref: /schemas/connector/usb-connector.yaml#
> @@ -44,6 +49,19 @@ patternProperties:
>  required:
>    - compatible
>  
> +allOf:
> +  - if:
> +      not:
> +        properties:
> +          compatible:
> +            contains:
> +              enum:
> +                - qcom,sm8450-pmic-glink
> +                - qcom,sm8550-pmic-glink
> +    then:
> +      properties:
> +        orientation-gpios: false
> +
>  additionalProperties: false
>  
>  examples:
> 
> -- 
> 2.34.1
> 

Re: [PATCH 1/4] dt-bindings: soc: qcom: qcom,pmic-glink: add a gpio used to determine the Type-C port plug orientation

[Neil Armstrong]

On 04/08/2023 18:03, Bjorn Andersson wrote:
> On Fri, Aug 04, 2023 at 03:50:07PM +0200, Neil Armstrong wrote:
>> On SM8450 and SM8550 based platforms, the Type-C plug orientation is given on a
>> GPIO line set by the PMIC.
>>
>> Document this optional Type-C connector property, and take the
>> assumption an active level represents an inverted/flipped orientation.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>>   .../devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml  | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
>> index bceb479f74c5..1b0a00b19a54 100644
>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
>> @@ -35,6 +35,11 @@ properties:
>>     '#size-cells':
>>       const: 0
>>   
>> +  orientation-gpios:
>> +    description: An input gpio for Type-C connector orientation, used to detect orientation
>> +      of the Type-C connector. GPIO active level means "CC2" or Reversed/Flipped orientation.
> 
> This is a per-connector/port property, as such be part of the connector
> sub nodes rather than as a property of the shared pmic_glink entity.

This has been rejected by Rob, Dmitry & Krzysztof in:
https://lore.kernel.org/all/0fbf55e7-2140-751d-5347-f907a46ef78c@linaro.org/
&
https://lore.kernel.org/all/20230608154751.GA2750742-robh@kernel.org/

Indeed the "GPIO" is not part of the physical USB-C connector, but a property,
and it's not part of the PMIC GLINK firmware either, so ?

The PMIC function which generates this signal is handled by the PMIC GLINK
firmware, so this representation is the most accurate.

Neil

> 
> Regards,
> Bjorn
> 
>> +    maxItems: 1
>> +
>>   patternProperties:
>>     '^connector@\d$':
>>       $ref: /schemas/connector/usb-connector.yaml#
>> @@ -44,6 +49,19 @@ patternProperties:
>>   required:
>>     - compatible
>>   
>> +allOf:
>> +  - if:
>> +      not:
>> +        properties:
>> +          compatible:
>> +            contains:
>> +              enum:
>> +                - qcom,sm8450-pmic-glink
>> +                - qcom,sm8550-pmic-glink
>> +    then:
>> +      properties:
>> +        orientation-gpios: false
>> +
>>   additionalProperties: false
>>   
>>   examples:
>>
>> -- 
>> 2.34.1
>>

Re: [PATCH 1/4] dt-bindings: soc: qcom: qcom,pmic-glink: add a gpio used to determine the Type-C port plug orientation

[Bjorn Andersson]

On Mon, Aug 14, 2023 at 10:09:41AM +0200, Neil Armstrong wrote:
> On 04/08/2023 18:03, Bjorn Andersson wrote:
> > On Fri, Aug 04, 2023 at 03:50:07PM +0200, Neil Armstrong wrote:
> > > On SM8450 and SM8550 based platforms, the Type-C plug orientation is given on a
> > > GPIO line set by the PMIC.
> > > 
> > > Document this optional Type-C connector property, and take the
> > > assumption an active level represents an inverted/flipped orientation.
> > > 
> > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> > > ---
> > >   .../devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml  | 18 ++++++++++++++++++
> > >   1 file changed, 18 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> > > index bceb479f74c5..1b0a00b19a54 100644
> > > --- a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> > > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> > > @@ -35,6 +35,11 @@ properties:
> > >     '#size-cells':
> > >       const: 0
> > > +  orientation-gpios:
> > > +    description: An input gpio for Type-C connector orientation, used to detect orientation
> > > +      of the Type-C connector. GPIO active level means "CC2" or Reversed/Flipped orientation.
> > 
> > This is a per-connector/port property, as such be part of the connector
> > sub nodes rather than as a property of the shared pmic_glink entity.
> 
> This has been rejected by Rob, Dmitry & Krzysztof in:
> https://lore.kernel.org/all/0fbf55e7-2140-751d-5347-f907a46ef78c@linaro.org/
> &
> https://lore.kernel.org/all/20230608154751.GA2750742-robh@kernel.org/
> 
> Indeed the "GPIO" is not part of the physical USB-C connector, but a property,
> and it's not part of the PMIC GLINK firmware either, so ?
> 
> The PMIC function which generates this signal is handled by the PMIC GLINK
> firmware, so this representation is the most accurate.
> 

I don't think we reached a conclusion on the discussion that followed.
If this indeed is the path forward, please document clearly how the next
person should proceed when the need for handling multiple ports with
this scheme arise.

Thanks,
Bjorn

> Neil
> 
> > 
> > Regards,
> > Bjorn
> > 
> > > +    maxItems: 1
> > > +
> > >   patternProperties:
> > >     '^connector@\d$':
> > >       $ref: /schemas/connector/usb-connector.yaml#
> > > @@ -44,6 +49,19 @@ patternProperties:
> > >   required:
> > >     - compatible
> > > +allOf:
> > > +  - if:
> > > +      not:
> > > +        properties:
> > > +          compatible:
> > > +            contains:
> > > +              enum:
> > > +                - qcom,sm8450-pmic-glink
> > > +                - qcom,sm8550-pmic-glink
> > > +    then:
> > > +      properties:
> > > +        orientation-gpios: false
> > > +
> > >   additionalProperties: false
> > >   examples:
> > > 
> > > -- 
> > > 2.34.1
> > > 
> 

Re: [PATCH 1/4] dt-bindings: soc: qcom: qcom,pmic-glink: add a gpio used to determine the Type-C port plug orientation

[Neil Armstrong]

On 20/09/2023 18:50, Bjorn Andersson wrote:
> On Mon, Aug 14, 2023 at 10:09:41AM +0200, Neil Armstrong wrote:
>> On 04/08/2023 18:03, Bjorn Andersson wrote:
>>> On Fri, Aug 04, 2023 at 03:50:07PM +0200, Neil Armstrong wrote:
>>>> On SM8450 and SM8550 based platforms, the Type-C plug orientation is given on a
>>>> GPIO line set by the PMIC.
>>>>
>>>> Document this optional Type-C connector property, and take the
>>>> assumption an active level represents an inverted/flipped orientation.
>>>>
>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>> ---
>>>>    .../devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml  | 18 ++++++++++++++++++
>>>>    1 file changed, 18 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
>>>> index bceb479f74c5..1b0a00b19a54 100644
>>>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
>>>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
>>>> @@ -35,6 +35,11 @@ properties:
>>>>      '#size-cells':
>>>>        const: 0
>>>> +  orientation-gpios:
>>>> +    description: An input gpio for Type-C connector orientation, used to detect orientation
>>>> +      of the Type-C connector. GPIO active level means "CC2" or Reversed/Flipped orientation.
>>>
>>> This is a per-connector/port property, as such be part of the connector
>>> sub nodes rather than as a property of the shared pmic_glink entity.
>>
>> This has been rejected by Rob, Dmitry & Krzysztof in:
>> https://lore.kernel.org/all/0fbf55e7-2140-751d-5347-f907a46ef78c@linaro.org/
>> &
>> https://lore.kernel.org/all/20230608154751.GA2750742-robh@kernel.org/
>>
>> Indeed the "GPIO" is not part of the physical USB-C connector, but a property,
>> and it's not part of the PMIC GLINK firmware either, so ?
>>
>> The PMIC function which generates this signal is handled by the PMIC GLINK
>> firmware, so this representation is the most accurate.
>>
> 
> I don't think we reached a conclusion on the discussion that followed.
> If this indeed is the path forward, please document clearly how the next
> person should proceed when the need for handling multiple ports with
> this scheme arise.

Sure, I'll explicit this is an array of gpios, one for each port, I thought I wrote
it but seems I forgot to add it to the description.

Neil

> 
> Thanks,
> Bjorn
> 
>> Neil
>>
>>>
>>> Regards,
>>> Bjorn
>>>
>>>> +    maxItems: 1
>>>> +
>>>>    patternProperties:
>>>>      '^connector@\d$':
>>>>        $ref: /schemas/connector/usb-connector.yaml#
>>>> @@ -44,6 +49,19 @@ patternProperties:
>>>>    required:
>>>>      - compatible
>>>> +allOf:
>>>> +  - if:
>>>> +      not:
>>>> +        properties:
>>>> +          compatible:
>>>> +            contains:
>>>> +              enum:
>>>> +                - qcom,sm8450-pmic-glink
>>>> +                - qcom,sm8550-pmic-glink
>>>> +    then:
>>>> +      properties:
>>>> +        orientation-gpios: false
>>>> +
>>>>    additionalProperties: false
>>>>    examples:
>>>>
>>>> -- 
>>>> 2.34.1
>>>>
>>