linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [RESEND PATCH v3 0/4] STUSB1600 support on STM32MP15xx-DKx
@ 2020-10-29  9:58 Amelie Delaunay
  2020-10-29  9:58 ` [RESEND PATCH v3 1/4] dt-bindings: connector: add power-opmode optional property to usb-connector Amelie Delaunay
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Amelie Delaunay @ 2020-10-29  9:58 UTC (permalink / raw)
  To: Rob Herring, Greg Kroah-Hartman, Maxime Coquelin,
	Alexandre Torgue, Russell King, Heikki Krogerus
  Cc: devicetree, Amelie Delaunay, linux-usb, linux-kernel,
	linux-stm32, linux-arm-kernel

I resend a part of my series [1] as the driver patches are already
merged. Bindings are missing and also device tree and config updates.

This series adds missing bindings for Type-C power-opmode property and
STUSB160x Type-C port controllers [2].
STUSB160x driver requires to get power operation mode via device tree,
that's why this series also adds the optional DT property power-opmode
for usb-c-connector to select the power operation mode capability and
a function to convert the power operation mode string into power
operation mode value.
Tested on stm32mp157c-dk2 [3], which has a Type-C connector managed by
STUSB1600, and connected to USB OTG controller. 

[1] https://patchwork.kernel.org/project/linux-usb/list/?series=354733
[2] https://www.st.com/en/interfaces-and-transceivers/usb-type-c-and-power-delivery-controllers.html
[3] https://www.st.com/en/evaluation-tools/stm32mp157c-dk2.html

Amelie Delaunay (4):
  dt-bindings: connector: add power-opmode optional property to
    usb-connector
  dt-bindings: usb: Add DT bindings for STUSB160x Type-C controller
  ARM: dts: stm32: add STUSB1600 Type-C using I2C4 on stm32mp15xx-dkx
  ARM: multi_v7_defconfig: enable STUSB160X Type-C port controller
    support

 .../bindings/connector/usb-connector.yaml     | 18 ++++
 .../devicetree/bindings/usb/st,stusb160x.yaml | 85 +++++++++++++++++++
 arch/arm/boot/dts/stm32mp15-pinctrl.dtsi      |  7 ++
 arch/arm/boot/dts/stm32mp15xx-dkx.dtsi        | 30 +++++++
 arch/arm/configs/multi_v7_defconfig           |  2 +
 5 files changed, 142 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/st,stusb160x.yaml

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [RESEND PATCH v3 1/4] dt-bindings: connector: add power-opmode optional property to usb-connector
  2020-10-29  9:58 [RESEND PATCH v3 0/4] STUSB1600 support on STM32MP15xx-DKx Amelie Delaunay
@ 2020-10-29  9:58 ` Amelie Delaunay
  2020-10-29 15:40   ` Rob Herring
  2020-10-29  9:58 ` [RESEND PATCH v3 2/4] dt-bindings: usb: Add DT bindings for STUSB160x Type-C controller Amelie Delaunay
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Amelie Delaunay @ 2020-10-29  9:58 UTC (permalink / raw)
  To: Rob Herring, Greg Kroah-Hartman, Maxime Coquelin,
	Alexandre Torgue, Russell King, Heikki Krogerus
  Cc: devicetree, Amelie Delaunay, linux-usb, linux-kernel,
	linux-stm32, linux-arm-kernel

Power operation mode may depends on hardware design, so, add the optional
property power-opmode for usb-c connector to select the power operation
mode capability.

Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
---
 .../bindings/connector/usb-connector.yaml      | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
index 728f82db073d..200d19c60fd5 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
+++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
@@ -93,6 +93,24 @@ properties:
       - device
       - dual
 
+  power-opmode:
+    description: Determines the power operation mode that the Type C connector
+      will support and will advertise through CC pins when it has no power
+      delivery support.
+      - "default" corresponds to default USB voltage and current defined by the
+        USB 2.0 and USB 3.2 specifications, 5V 500mA for USB 2.0 ports and
+        5V 900mA or 1500mA for USB 3.2 ports in single-lane or dual-lane
+        operation respectively.
+      - "1.5A" and "3.0A", 5V 1.5A and 5V 3.0A respectively, as defined in USB
+        Type-C Cable and Connector specification, when Power Delivery is not
+        supported.
+    allOf:
+      - $ref: /schemas/types.yaml#definitions/string
+    enum:
+      - default
+      - 1.5A
+      - 3.0A
+
   # The following are optional properties for "usb-c-connector" with power
   # delivery support.
   source-pdos:
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RESEND PATCH v3 2/4] dt-bindings: usb: Add DT bindings for STUSB160x Type-C controller
  2020-10-29  9:58 [RESEND PATCH v3 0/4] STUSB1600 support on STM32MP15xx-DKx Amelie Delaunay
  2020-10-29  9:58 ` [RESEND PATCH v3 1/4] dt-bindings: connector: add power-opmode optional property to usb-connector Amelie Delaunay
@ 2020-10-29  9:58 ` Amelie Delaunay
  2020-10-29  9:58 ` [RESEND PATCH v3 3/4] ARM: dts: stm32: add STUSB1600 Type-C using I2C4 on stm32mp15xx-dkx Amelie Delaunay
  2020-10-29  9:58 ` [RESEND PATCH v3 4/4] ARM: multi_v7_defconfig: enable STUSB160X Type-C port controller support Amelie Delaunay
  3 siblings, 0 replies; 18+ messages in thread
From: Amelie Delaunay @ 2020-10-29  9:58 UTC (permalink / raw)
  To: Rob Herring, Greg Kroah-Hartman, Maxime Coquelin,
	Alexandre Torgue, Russell King, Heikki Krogerus
  Cc: devicetree, Amelie Delaunay, linux-usb, linux-kernel,
	linux-stm32, linux-arm-kernel

Add binding documentation for the STMicroelectronics STUSB160x Type-C port
controller.

Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
---
 .../devicetree/bindings/usb/st,stusb160x.yaml | 85 +++++++++++++++++++
 1 file changed, 85 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/st,stusb160x.yaml

diff --git a/Documentation/devicetree/bindings/usb/st,stusb160x.yaml b/Documentation/devicetree/bindings/usb/st,stusb160x.yaml
new file mode 100644
index 000000000000..12a996cd9405
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/st,stusb160x.yaml
@@ -0,0 +1,85 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/usb/st,stusb160x.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: STMicroelectronics STUSB160x Type-C controller bindings
+
+maintainers:
+  - Amelie Delaunay <amelie.delaunay@st.com>
+
+properties:
+  compatible:
+    enum:
+      - st,stusb1600
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  vdd-supply:
+    description: main power supply (4.1V-22V)
+
+  vsys-supply:
+    description: low power supply (3.0V-5.5V)
+
+  vconn-supply:
+    description: power supply (2.7V-5.5V) used to supply VConn on CC pin in
+      source or dual power role
+
+  connector:
+    type: object
+
+    allOf:
+      - $ref: ../connector/usb-connector.yaml
+
+    properties:
+      compatible:
+        const: usb-c-connector
+
+      power-role: true
+
+      power-opmode: true
+
+    required:
+      - compatible
+
+required:
+  - compatible
+  - reg
+  - connector
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c4 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        typec: stusb1600@28 {
+            compatible = "st,stusb1600";
+            reg = <0x28>;
+            vdd-supply = <&vbus_drd>;
+            vsys-supply = <&vdd_usb>;
+            interrupts = <11 IRQ_TYPE_EDGE_FALLING>;
+            interrupt-parent = <&gpioi>;
+
+            typec_con: connector {
+                compatible = "usb-c-connector";
+                label = "USB-C";
+                power-role = "dual";
+                power-opmode = "default";
+                data-role = "dual";
+
+                port {
+                    typec_con_ep: endpoint {
+                        remote-endpoint = <&usbotg_hs_ep>;
+                    };
+                };
+            };
+        };
+    };
+...
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RESEND PATCH v3 3/4] ARM: dts: stm32: add STUSB1600 Type-C using I2C4 on stm32mp15xx-dkx
  2020-10-29  9:58 [RESEND PATCH v3 0/4] STUSB1600 support on STM32MP15xx-DKx Amelie Delaunay
  2020-10-29  9:58 ` [RESEND PATCH v3 1/4] dt-bindings: connector: add power-opmode optional property to usb-connector Amelie Delaunay
  2020-10-29  9:58 ` [RESEND PATCH v3 2/4] dt-bindings: usb: Add DT bindings for STUSB160x Type-C controller Amelie Delaunay
@ 2020-10-29  9:58 ` Amelie Delaunay
  2020-10-29  9:58 ` [RESEND PATCH v3 4/4] ARM: multi_v7_defconfig: enable STUSB160X Type-C port controller support Amelie Delaunay
  3 siblings, 0 replies; 18+ messages in thread
From: Amelie Delaunay @ 2020-10-29  9:58 UTC (permalink / raw)
  To: Rob Herring, Greg Kroah-Hartman, Maxime Coquelin,
	Alexandre Torgue, Russell King, Heikki Krogerus
  Cc: devicetree, Amelie Delaunay, linux-usb, linux-kernel,
	linux-stm32, linux-arm-kernel

This patch adds support for STUSB1600 USB Type-C port controller, used on
I2C4 on stm32mp15xx-dkx.
The default configuration on this board, on Type-C connector, is:
- Dual Power Role (DRP), so set power-role to "dual";
- Vbus limited to 500mA, so set power-opmode to "default" (it means 500mA
  in USB 2.0).
power-opmode is used to reconfigure the STUSB1600 advertising of current
capability when its NVM is not in line with the board layout.
On stm32mp15xx-dkx, Vbus power source of STUSB1600 is 5V_VIN (so add the
vin fixed 5V regulator too). So power operation mode depends on the power
supply used. To avoid any power issues, it is better to limit Vbus to 500mA
on this board.
ALERT# is the interrupt pin of STUSB1600. It needs an external pull-up, and
signal is active low.

USB OTG controller ID and Vbus signals are not connected on stm32mp15xx-dkx
boards, so disconnection are not detected.
Without DWC2 usb-role-switch:
- if you unplug the USB cable from the Type-C port, you have to manually
disconnect the USB gadget:
echo disconnect > /sys/devices/platform/soc/49000000.usb-otg/udc/49000000.usb-otg/soft_connect
- Then you can plug the USB cable again in the Type-C port, and manually
reconnect the USB gadget:
echo connect > /sys/devices/platform/soc/49000000.usb-otg/udc/49000000.usb-otg/soft_connect
With DWC2 usb-role-switch, USB gadget is dynamically disconnected or connected.

Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
---
 arch/arm/boot/dts/stm32mp15-pinctrl.dtsi |  7 ++++++
 arch/arm/boot/dts/stm32mp15xx-dkx.dtsi   | 30 ++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/arch/arm/boot/dts/stm32mp15-pinctrl.dtsi b/arch/arm/boot/dts/stm32mp15-pinctrl.dtsi
index d84686e00370..d2e9e7ac3336 100644
--- a/arch/arm/boot/dts/stm32mp15-pinctrl.dtsi
+++ b/arch/arm/boot/dts/stm32mp15-pinctrl.dtsi
@@ -1591,6 +1591,13 @@
 		};
 	};
 
+	stusb1600_pins_a: stusb1600-0 {
+		pins {
+			pinmux = <STM32_PINMUX('I', 11, ANALOG)>;
+			bias-pull-up;
+		};
+	};
+
 	uart4_pins_a: uart4-0 {
 		pins1 {
 			pinmux = <STM32_PINMUX('G', 11, AF6)>; /* UART4_TX */
diff --git a/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi b/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi
index 93398cfae97e..01fa571584a4 100644
--- a/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi
+++ b/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi
@@ -238,6 +238,30 @@
 	/delete-property/dmas;
 	/delete-property/dma-names;
 
+	stusb1600@28 {
+		compatible = "st,stusb1600";
+		reg = <0x28>;
+		interrupts = <11 IRQ_TYPE_EDGE_FALLING>;
+		interrupt-parent = <&gpioi>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&stusb1600_pins_a>;
+		status = "okay";
+		vdd-supply = <&vin>;
+
+		connector {
+			compatible = "usb-c-connector";
+			label = "USB-C";
+			power-role = "dual";
+			power-opmode = "default";
+
+			port {
+				con_usbotg_hs_ep: endpoint {
+					remote-endpoint = <&usbotg_hs_ep>;
+				};
+			};
+		};
+	};
+
 	pmic: stpmic@33 {
 		compatible = "st,stpmic1";
 		reg = <0x33>;
@@ -648,6 +672,12 @@
 	phy-names = "usb2-phy";
 	usb-role-switch;
 	status = "okay";
+
+	port {
+		usbotg_hs_ep: endpoint {
+			remote-endpoint = <&con_usbotg_hs_ep>;
+		};
+	};
 };
 
 &usbphyc {
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RESEND PATCH v3 4/4] ARM: multi_v7_defconfig: enable STUSB160X Type-C port controller support
  2020-10-29  9:58 [RESEND PATCH v3 0/4] STUSB1600 support on STM32MP15xx-DKx Amelie Delaunay
                   ` (2 preceding siblings ...)
  2020-10-29  9:58 ` [RESEND PATCH v3 3/4] ARM: dts: stm32: add STUSB1600 Type-C using I2C4 on stm32mp15xx-dkx Amelie Delaunay
@ 2020-10-29  9:58 ` Amelie Delaunay
  3 siblings, 0 replies; 18+ messages in thread
From: Amelie Delaunay @ 2020-10-29  9:58 UTC (permalink / raw)
  To: Rob Herring, Greg Kroah-Hartman, Maxime Coquelin,
	Alexandre Torgue, Russell King, Heikki Krogerus
  Cc: devicetree, Amelie Delaunay, linux-usb, linux-kernel,
	linux-stm32, linux-arm-kernel

Enable support for the STMicroelectronics STUSB160X USB Type-C port
controller driver by turning on CONFIG_TYPEC and CONFIG_TYPEC_STUSB160X as
modules.

Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
---
 arch/arm/configs/multi_v7_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index e731cdf7c88c..41d0def64ce6 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -828,6 +828,8 @@ CONFIG_USB_CONFIGFS_F_HID=y
 CONFIG_USB_CONFIGFS_F_UVC=y
 CONFIG_USB_CONFIGFS_F_PRINTER=y
 CONFIG_USB_ETH=m
+CONFIG_TYPEC=m
+CONFIG_TYPEC_STUSB160X=m
 CONFIG_MMC=y
 CONFIG_MMC_BLOCK_MINORS=16
 CONFIG_MMC_ARMMMCI=y
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [RESEND PATCH v3 1/4] dt-bindings: connector: add power-opmode optional property to usb-connector
  2020-10-29  9:58 ` [RESEND PATCH v3 1/4] dt-bindings: connector: add power-opmode optional property to usb-connector Amelie Delaunay
@ 2020-10-29 15:40   ` Rob Herring
  2020-10-29 16:49     ` Amelie DELAUNAY
  0 siblings, 1 reply; 18+ messages in thread
From: Rob Herring @ 2020-10-29 15:40 UTC (permalink / raw)
  To: Amelie Delaunay
  Cc: devicetree, Heikki Krogerus, Alexandre Torgue,
	Greg Kroah-Hartman, linux-usb, Russell King, linux-kernel,
	Maxime Coquelin, linux-stm32, linux-arm-kernel

On Thu, Oct 29, 2020 at 10:58:03AM +0100, Amelie Delaunay wrote:
> Power operation mode may depends on hardware design, so, add the optional
> property power-opmode for usb-c connector to select the power operation
> mode capability.
> 
> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
> ---
>  .../bindings/connector/usb-connector.yaml      | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> index 728f82db073d..200d19c60fd5 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> @@ -93,6 +93,24 @@ properties:
>        - device
>        - dual
>  
> +  power-opmode:

I've acked this version:

https://lore.kernel.org/r/20201020093627.256885-2-badhri@google.com

Please ack it if you are okay with it.

Rob


> +    description: Determines the power operation mode that the Type C connector
> +      will support and will advertise through CC pins when it has no power
> +      delivery support.
> +      - "default" corresponds to default USB voltage and current defined by the
> +        USB 2.0 and USB 3.2 specifications, 5V 500mA for USB 2.0 ports and
> +        5V 900mA or 1500mA for USB 3.2 ports in single-lane or dual-lane
> +        operation respectively.
> +      - "1.5A" and "3.0A", 5V 1.5A and 5V 3.0A respectively, as defined in USB
> +        Type-C Cable and Connector specification, when Power Delivery is not
> +        supported.
> +    allOf:
> +      - $ref: /schemas/types.yaml#definitions/string
> +    enum:
> +      - default
> +      - 1.5A
> +      - 3.0A
> +
>    # The following are optional properties for "usb-c-connector" with power
>    # delivery support.
>    source-pdos:
> -- 
> 2.17.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RESEND PATCH v3 1/4] dt-bindings: connector: add power-opmode optional property to usb-connector
  2020-10-29 15:40   ` Rob Herring
@ 2020-10-29 16:49     ` Amelie DELAUNAY
  2020-10-30  1:54       ` Jun Li
  2020-10-30 14:29       ` Rob Herring
  0 siblings, 2 replies; 18+ messages in thread
From: Amelie DELAUNAY @ 2020-10-29 16:49 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Heikki Krogerus, Alexandre Torgue,
	Greg Kroah-Hartman, linux-usb, Russell King, linux-kernel,
	Maxime Coquelin, linux-stm32, linux-arm-kernel



On 10/29/20 4:40 PM, Rob Herring wrote:
> On Thu, Oct 29, 2020 at 10:58:03AM +0100, Amelie Delaunay wrote:
>> Power operation mode may depends on hardware design, so, add the optional
>> property power-opmode for usb-c connector to select the power operation
>> mode capability.
>>
>> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
>> ---
>>   .../bindings/connector/usb-connector.yaml      | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> index 728f82db073d..200d19c60fd5 100644
>> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> @@ -93,6 +93,24 @@ properties:
>>         - device
>>         - dual
>>   
>> +  power-opmode:
> 
> I've acked this version:
> 
> https://lore.kernel.org/r/20201020093627.256885-2-badhri@google.com
>

frs is used for Fast Role Swap defined in USB PD spec.
I understand it allows to get the same information but I'm wondering why 
the property name is limited to -frs- in this case. What about a 
non-power delivery USB-C connector ?

Moreover, power-opmode property support is already merged in typec class:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/typec/class.c?h=v5.10-rc1&id=12f3467b0d28369d3add7a0deb65fdac9b503c90
and stusb160x driver uses it :(

So, do I need to modify stusb160x driver (and bindings) to take into 
account this USB PD specific property?

Regards,
Amelie

> Please ack it if you are okay with it.
> 
> Rob
> 
> 
>> +    description: Determines the power operation mode that the Type C connector
>> +      will support and will advertise through CC pins when it has no power
>> +      delivery support.
>> +      - "default" corresponds to default USB voltage and current defined by the
>> +        USB 2.0 and USB 3.2 specifications, 5V 500mA for USB 2.0 ports and
>> +        5V 900mA or 1500mA for USB 3.2 ports in single-lane or dual-lane
>> +        operation respectively.
>> +      - "1.5A" and "3.0A", 5V 1.5A and 5V 3.0A respectively, as defined in USB
>> +        Type-C Cable and Connector specification, when Power Delivery is not
>> +        supported.
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#definitions/string
>> +    enum:
>> +      - default
>> +      - 1.5A
>> +      - 3.0A
>> +
>>     # The following are optional properties for "usb-c-connector" with power
>>     # delivery support.
>>     source-pdos:
>> -- 
>> 2.17.1
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RESEND PATCH v3 1/4] dt-bindings: connector: add power-opmode optional property to usb-connector
  2020-10-29 16:49     ` Amelie DELAUNAY
@ 2020-10-30  1:54       ` Jun Li
  2020-10-30 14:32         ` Rob Herring
  2020-10-30 14:29       ` Rob Herring
  1 sibling, 1 reply; 18+ messages in thread
From: Jun Li @ 2020-10-30  1:54 UTC (permalink / raw)
  To: Amelie DELAUNAY
  Cc: Rob Herring, Heikki Krogerus, Alexandre Torgue,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Greg Kroah-Hartman, Linux USB List, Russell King, lkml,
	Maxime Coquelin, linux-stm32, linux-arm-kernel

Amelie DELAUNAY <amelie.delaunay@st.com> 于2020年10月30日周五 上午12:52写道:
>
>
>
> On 10/29/20 4:40 PM, Rob Herring wrote:
> > On Thu, Oct 29, 2020 at 10:58:03AM +0100, Amelie Delaunay wrote:
> >> Power operation mode may depends on hardware design, so, add the optional
> >> property power-opmode for usb-c connector to select the power operation
> >> mode capability.
> >>
> >> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
> >> ---
> >>   .../bindings/connector/usb-connector.yaml      | 18 ++++++++++++++++++
> >>   1 file changed, 18 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> >> index 728f82db073d..200d19c60fd5 100644
> >> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> >> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> >> @@ -93,6 +93,24 @@ properties:
> >>         - device
> >>         - dual
> >>
> >> +  power-opmode:
> >
> > I've acked this version:
> >
> > https://lore.kernel.org/r/20201020093627.256885-2-badhri@google.com

That is a different property only for FRS.

> >
>
> frs is used for Fast Role Swap defined in USB PD spec.
> I understand it allows to get the same information but I'm wondering why
> the property name is limited to -frs- in this case. What about a
> non-power delivery USB-C connector ?

It's only for FRS, FRS is in the scope of power delivery.

>
> Moreover, power-opmode property support is already merged in typec class:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/typec/class.c?h=v5.10-rc1&id=12f3467b0d28369d3add7a0deb65fdac9b503c90
> and stusb160x driver uses it :(
>
> So, do I need to modify stusb160x driver (and bindings) to take into
> account this USB PD specific property?

Only Type-C w/o PD need this "power-opmode" property, so this
property is still required.

Li Jun

>
> Regards,
> Amelie
>
> > Please ack it if you are okay with it.
> >
> > Rob
> >
> >
> >> +    description: Determines the power operation mode that the Type C connector
> >> +      will support and will advertise through CC pins when it has no power
> >> +      delivery support.
> >> +      - "default" corresponds to default USB voltage and current defined by the
> >> +        USB 2.0 and USB 3.2 specifications, 5V 500mA for USB 2.0 ports and
> >> +        5V 900mA or 1500mA for USB 3.2 ports in single-lane or dual-lane
> >> +        operation respectively.
> >> +      - "1.5A" and "3.0A", 5V 1.5A and 5V 3.0A respectively, as defined in USB
> >> +        Type-C Cable and Connector specification, when Power Delivery is not
> >> +        supported.
> >> +    allOf:
> >> +      - $ref: /schemas/types.yaml#definitions/string
> >> +    enum:
> >> +      - default
> >> +      - 1.5A
> >> +      - 3.0A
> >> +
> >>     # The following are optional properties for "usb-c-connector" with power
> >>     # delivery support.
> >>     source-pdos:
> >> --
> >> 2.17.1
> >>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RESEND PATCH v3 1/4] dt-bindings: connector: add power-opmode optional property to usb-connector
  2020-10-29 16:49     ` Amelie DELAUNAY
  2020-10-30  1:54       ` Jun Li
@ 2020-10-30 14:29       ` Rob Herring
  2020-10-30 15:27         ` Amelie DELAUNAY
  1 sibling, 1 reply; 18+ messages in thread
From: Rob Herring @ 2020-10-30 14:29 UTC (permalink / raw)
  To: Amelie DELAUNAY
  Cc: devicetree, Heikki Krogerus, Alexandre Torgue,
	Greg Kroah-Hartman, Linux USB List, Russell King, linux-kernel,
	Maxime Coquelin, moderated list:ARM/STM32 ARCHITECTURE,
	linux-arm-kernel

On Thu, Oct 29, 2020 at 11:49 AM Amelie DELAUNAY <amelie.delaunay@st.com> wrote:
>
>
>
> On 10/29/20 4:40 PM, Rob Herring wrote:
> > On Thu, Oct 29, 2020 at 10:58:03AM +0100, Amelie Delaunay wrote:
> >> Power operation mode may depends on hardware design, so, add the optional
> >> property power-opmode for usb-c connector to select the power operation
> >> mode capability.
> >>
> >> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
> >> ---
> >>   .../bindings/connector/usb-connector.yaml      | 18 ++++++++++++++++++
> >>   1 file changed, 18 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> >> index 728f82db073d..200d19c60fd5 100644
> >> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> >> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> >> @@ -93,6 +93,24 @@ properties:
> >>         - device
> >>         - dual
> >>
> >> +  power-opmode:
> >
> > I've acked this version:
> >
> > https://lore.kernel.org/r/20201020093627.256885-2-badhri@google.com
> >
>
> frs is used for Fast Role Swap defined in USB PD spec.
> I understand it allows to get the same information but I'm wondering why
> the property name is limited to -frs- in this case. What about a
> non-power delivery USB-C connector ?

I've got no idea. The folks that know USB-C and PD details need to get
together and work all this out. To me, it looks like the same thing...

And it's not just this, but the stream of USB-C additions that trickle in.

> Moreover, power-opmode property support is already merged in typec class:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/typec/class.c?h=v5.10-rc1&id=12f3467b0d28369d3add7a0deb65fdac9b503c90
> and stusb160x driver uses it :(
>
> So, do I need to modify stusb160x driver (and bindings) to take into
> account this USB PD specific property?

If not documented, then it's not an ABI, so yes.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RESEND PATCH v3 1/4] dt-bindings: connector: add power-opmode optional property to usb-connector
  2020-10-30  1:54       ` Jun Li
@ 2020-10-30 14:32         ` Rob Herring
  0 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2020-10-30 14:32 UTC (permalink / raw)
  To: Jun Li
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Amelie DELAUNAY, Heikki Krogerus, Alexandre Torgue,
	Greg Kroah-Hartman, Linux USB List, Russell King, lkml,
	Maxime Coquelin, moderated list:ARM/STM32 ARCHITECTURE,
	linux-arm-kernel

On Thu, Oct 29, 2020 at 8:55 PM Jun Li <lijun.kernel@gmail.com> wrote:
>
> Amelie DELAUNAY <amelie.delaunay@st.com> 于2020年10月30日周五 上午12:52写道:
> >
> >
> >
> > On 10/29/20 4:40 PM, Rob Herring wrote:
> > > On Thu, Oct 29, 2020 at 10:58:03AM +0100, Amelie Delaunay wrote:
> > >> Power operation mode may depends on hardware design, so, add the optional
> > >> property power-opmode for usb-c connector to select the power operation
> > >> mode capability.
> > >>
> > >> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
> > >> ---
> > >>   .../bindings/connector/usb-connector.yaml      | 18 ++++++++++++++++++
> > >>   1 file changed, 18 insertions(+)
> > >>
> > >> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > >> index 728f82db073d..200d19c60fd5 100644
> > >> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > >> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > >> @@ -93,6 +93,24 @@ properties:
> > >>         - device
> > >>         - dual
> > >>
> > >> +  power-opmode:
> > >
> > > I've acked this version:
> > >
> > > https://lore.kernel.org/r/20201020093627.256885-2-badhri@google.com
>
> That is a different property only for FRS.
>
> > >
> >
> > frs is used for Fast Role Swap defined in USB PD spec.
> > I understand it allows to get the same information but I'm wondering why
> > the property name is limited to -frs- in this case. What about a
> > non-power delivery USB-C connector ?
>
> It's only for FRS, FRS is in the scope of power delivery.
>
> >
> > Moreover, power-opmode property support is already merged in typec class:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/typec/class.c?h=v5.10-rc1&id=12f3467b0d28369d3add7a0deb65fdac9b503c90
> > and stusb160x driver uses it :(
> >
> > So, do I need to modify stusb160x driver (and bindings) to take into
> > account this USB PD specific property?
>
> Only Type-C w/o PD need this "power-opmode" property, so this
> property is still required.

Yet we have the same set of values. So there's something common...

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RESEND PATCH v3 1/4] dt-bindings: connector: add power-opmode optional property to usb-connector
  2020-10-30 14:29       ` Rob Herring
@ 2020-10-30 15:27         ` Amelie DELAUNAY
  2020-11-04 21:08           ` Rob Herring
  0 siblings, 1 reply; 18+ messages in thread
From: Amelie DELAUNAY @ 2020-10-30 15:27 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Heikki Krogerus, Alexandre Torgue,
	Greg Kroah-Hartman, Linux USB List, Russell King, linux-kernel,
	Maxime Coquelin, moderated list:ARM/STM32 ARCHITECTURE,
	linux-arm-kernel



On 10/30/20 3:29 PM, Rob Herring wrote:
> On Thu, Oct 29, 2020 at 11:49 AM Amelie DELAUNAY <amelie.delaunay@st.com> wrote:
>>
>>
>>
>> On 10/29/20 4:40 PM, Rob Herring wrote:
>>> On Thu, Oct 29, 2020 at 10:58:03AM +0100, Amelie Delaunay wrote:
>>>> Power operation mode may depends on hardware design, so, add the optional
>>>> property power-opmode for usb-c connector to select the power operation
>>>> mode capability.
>>>>
>>>> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
>>>> ---
>>>>    .../bindings/connector/usb-connector.yaml      | 18 ++++++++++++++++++
>>>>    1 file changed, 18 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>>> index 728f82db073d..200d19c60fd5 100644
>>>> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>>> @@ -93,6 +93,24 @@ properties:
>>>>          - device
>>>>          - dual
>>>>
>>>> +  power-opmode:
>>>
>>> I've acked this version:
>>>
>>> https://lore.kernel.org/r/20201020093627.256885-2-badhri@google.com
>>>
>>
>> frs is used for Fast Role Swap defined in USB PD spec.
>> I understand it allows to get the same information but I'm wondering why
>> the property name is limited to -frs- in this case. What about a
>> non-power delivery USB-C connector ?
> 
> I've got no idea. The folks that know USB-C and PD details need to get
> together and work all this out. To me, it looks like the same thing...
> 

It looks but...

The purpose of power-opmode property is to configure the USB-C 
controllers, especially the non-PD USB-C controllers to determine the 
power operation mode that the Type C connector will support and will 
advertise through CC pins when it has no power delivery support, 
whatever the power role: Sink, Source or Dual
The management of the property is the same that data-role and power-role 
properties, and done by USB Type-C Connector Class.

new-source-frs-typec-current specifies initial current capability of the 
new source when vSafe5V is applied during PD3.0 Fast Role Swap. So here, 
this property is not applied at usb-c controller configuration level, 
but during PD Fast Role Swap, so when the Sink become the Source.
Moreover, the related driver code says FRS can only be supported by DRP 
ports. So new-source-frs-typec-current property, in addition to being 
specific to PD, is also dedicated to DRP usb-c controller.
The property is managed by Type-C Port Controller Manager for PD.

> And it's not just this, but the stream of USB-C additions that trickle in.
> 
>> Moreover, power-opmode property support is already merged in typec class:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/typec/class.c?h=v5.10-rc1&id=12f3467b0d28369d3add7a0deb65fdac9b503c90
>> and stusb160x driver uses it :(
>>
>> So, do I need to modify stusb160x driver (and bindings) to take into
>> account this USB PD specific property?
> 
> If not documented, then it's not an ABI, so yes.

I have tried to document it since months ago
v1: https://lkml.org/lkml/2020/6/15/927
v2: https://lkml.org/lkml/2020/7/23/445 integrating your remarks
v2 RESENT: https://lkml.org/lkml/2020/9/2/174
v3: https://lkml.org/lkml/2020/9/24/306 integrated Li Jun remarks

Regards,
Amelie

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RESEND PATCH v3 1/4] dt-bindings: connector: add power-opmode optional property to usb-connector
  2020-10-30 15:27         ` Amelie DELAUNAY
@ 2020-11-04 21:08           ` Rob Herring
  2020-11-05 11:36             ` Amelie DELAUNAY
  0 siblings, 1 reply; 18+ messages in thread
From: Rob Herring @ 2020-11-04 21:08 UTC (permalink / raw)
  To: Amelie DELAUNAY
  Cc: devicetree, Heikki Krogerus, Alexandre Torgue,
	Greg Kroah-Hartman, Linux USB List, Russell King, linux-kernel,
	Maxime Coquelin, moderated list:ARM/STM32 ARCHITECTURE,
	linux-arm-kernel

On Fri, Oct 30, 2020 at 04:27:14PM +0100, Amelie DELAUNAY wrote:
> 
> 
> On 10/30/20 3:29 PM, Rob Herring wrote:
> > On Thu, Oct 29, 2020 at 11:49 AM Amelie DELAUNAY <amelie.delaunay@st.com> wrote:
> > > 
> > > 
> > > 
> > > On 10/29/20 4:40 PM, Rob Herring wrote:
> > > > On Thu, Oct 29, 2020 at 10:58:03AM +0100, Amelie Delaunay wrote:
> > > > > Power operation mode may depends on hardware design, so, add the optional
> > > > > property power-opmode for usb-c connector to select the power operation
> > > > > mode capability.
> > > > > 
> > > > > Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
> > > > > ---
> > > > >    .../bindings/connector/usb-connector.yaml      | 18 ++++++++++++++++++
> > > > >    1 file changed, 18 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > > > index 728f82db073d..200d19c60fd5 100644
> > > > > --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > > > +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > > > @@ -93,6 +93,24 @@ properties:
> > > > >          - device
> > > > >          - dual
> > > > > 
> > > > > +  power-opmode:
> > > > 
> > > > I've acked this version:
> > > > 
> > > > https://lore.kernel.org/r/20201020093627.256885-2-badhri@google.com
> > > > 
> > > 
> > > frs is used for Fast Role Swap defined in USB PD spec.
> > > I understand it allows to get the same information but I'm wondering why
> > > the property name is limited to -frs- in this case. What about a
> > > non-power delivery USB-C connector ?
> > 
> > I've got no idea. The folks that know USB-C and PD details need to get
> > together and work all this out. To me, it looks like the same thing...
> > 
> 
> It looks but...
> 
> The purpose of power-opmode property is to configure the USB-C controllers,
> especially the non-PD USB-C controllers to determine the power operation
> mode that the Type C connector will support and will advertise through CC
> pins when it has no power delivery support, whatever the power role: Sink,
> Source or Dual
> The management of the property is the same that data-role and power-role
> properties, and done by USB Type-C Connector Class.
> 
> new-source-frs-typec-current specifies initial current capability of the new
> source when vSafe5V is applied during PD3.0 Fast Role Swap. So here, this
> property is not applied at usb-c controller configuration level, but during
> PD Fast Role Swap, so when the Sink become the Source.
> Moreover, the related driver code says FRS can only be supported by DRP
> ports. So new-source-frs-typec-current property, in addition to being
> specific to PD, is also dedicated to DRP usb-c controller.
> The property is managed by Type-C Port Controller Manager for PD.

But it's the same set of possible values, right? So we can align the 
values at least.

Can we align the names in some way? power-opmode and frs-source-opmode 
or ??

Are these 2 properties mutually exclusive? If so, that should be 
captured.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RESEND PATCH v3 1/4] dt-bindings: connector: add power-opmode optional property to usb-connector
  2020-11-04 21:08           ` Rob Herring
@ 2020-11-05 11:36             ` Amelie DELAUNAY
  2020-11-05 12:23               ` Jun Li
  0 siblings, 1 reply; 18+ messages in thread
From: Amelie DELAUNAY @ 2020-11-05 11:36 UTC (permalink / raw)
  To: Rob Herring, Heikki Krogerus, Badhri Jagan Sridharan, Jun Li
  Cc: devicetree, Alexandre Torgue, Greg Kroah-Hartman, Linux USB List,
	linux-kernel, Russell King, Maxime Coquelin,
	moderated list:ARM/STM32 ARCHITECTURE, linux-arm-kernel

On 11/4/20 10:08 PM, Rob Herring wrote:
> On Fri, Oct 30, 2020 at 04:27:14PM +0100, Amelie DELAUNAY wrote:
>>
>>
>> On 10/30/20 3:29 PM, Rob Herring wrote:
>>> On Thu, Oct 29, 2020 at 11:49 AM Amelie DELAUNAY <amelie.delaunay@st.com> wrote:
>>>>
>>>>
>>>>
>>>> On 10/29/20 4:40 PM, Rob Herring wrote:
>>>>> On Thu, Oct 29, 2020 at 10:58:03AM +0100, Amelie Delaunay wrote:
>>>>>> Power operation mode may depends on hardware design, so, add the optional
>>>>>> property power-opmode for usb-c connector to select the power operation
>>>>>> mode capability.
>>>>>>
>>>>>> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
>>>>>> ---
>>>>>>     .../bindings/connector/usb-connector.yaml      | 18 ++++++++++++++++++
>>>>>>     1 file changed, 18 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>>>>> index 728f82db073d..200d19c60fd5 100644
>>>>>> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>>>>> @@ -93,6 +93,24 @@ properties:
>>>>>>           - device
>>>>>>           - dual
>>>>>>
>>>>>> +  power-opmode:
>>>>>
>>>>> I've acked this version:
>>>>>
>>>>> https://lore.kernel.org/r/20201020093627.256885-2-badhri@google.com
>>>>>
>>>>
>>>> frs is used for Fast Role Swap defined in USB PD spec.
>>>> I understand it allows to get the same information but I'm wondering why
>>>> the property name is limited to -frs- in this case. What about a
>>>> non-power delivery USB-C connector ?
>>>
>>> I've got no idea. The folks that know USB-C and PD details need to get
>>> together and work all this out. To me, it looks like the same thing...
>>>
>>
>> It looks but...
>>
>> The purpose of power-opmode property is to configure the USB-C controllers,
>> especially the non-PD USB-C controllers to determine the power operation
>> mode that the Type C connector will support and will advertise through CC
>> pins when it has no power delivery support, whatever the power role: Sink,
>> Source or Dual
>> The management of the property is the same that data-role and power-role
>> properties, and done by USB Type-C Connector Class.
>>
>> new-source-frs-typec-current specifies initial current capability of the new
>> source when vSafe5V is applied during PD3.0 Fast Role Swap. So here, this
>> property is not applied at usb-c controller configuration level, but during
>> PD Fast Role Swap, so when the Sink become the Source.
>> Moreover, the related driver code says FRS can only be supported by DRP
>> ports. So new-source-frs-typec-current property, in addition to being
>> specific to PD, is also dedicated to DRP usb-c controller.
>> The property is managed by Type-C Port Controller Manager for PD.
> 
> But it's the same set of possible values, right? So we can align the
> values at least.
> 

USB Power Delivery FRS values are defined in 
include/dt-bindings/usb/pd.h to fit with drivers/usb/typec/tcpm/tcpm.c 
frs_typec_current enum.

USB-C power operation mode values are defined in 
include/linux/usb/typec.h with typec_pwr_opmode enum and matching with 
string values of typec_pwr_opmodes tab.

USB PD requires USB-C.
USB-C doesn't requires USB PD.

drivers/usb/typec/tcpm/tcpm.c already used typec_pwr_opmode values.

USB PD specification Table 6-14 Fixed Supply PDO says:
Fast Role Swap required USB Type-C Current (see also [USB Type-C 2.0]):
Value | Description
  00b  | Fast Swap not supported (default)
  01b  | Default USB Power
  10b  | 1.5A @ 5V
  11b  | 3.0A @ 5V

Note the *see also USB Type-C 2.0*.

USB Type-C specification 4.6.2.1 USB Type-C Current says:
The USB Type-C connector uses CC pins for configuration including an 
ability for a Source to advertise to its port partner (Sink) the amount 
of current it shall supply:
• Default is the as-configured for high-power operation current value as 
defined by the USB Specification (500 mA for USB 2.0 ports; 900 mA or 
1,500 mA for USB 3.2 ports in single-lane or dual-lane operation, 
respectively)
• 1.5 A
• 3.0 A

> Can we align the names in some way? power-opmode and frs-source-opmode
> or ??
> 

I let USB PD specialists answer.

*frs* property fits with USB PD specification, so with USB PD protocol.
*power-opmode fits with USB Type-C specification, so with USB-C hardware 
support.

> Are these 2 properties mutually exclusive? If so, that should be
> captured.

FRS is specific to products with Power Delivery Support.

power-opmode is dedicated to products with USB-C connector support.

Regards,
Amelie

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RESEND PATCH v3 1/4] dt-bindings: connector: add power-opmode optional property to usb-connector
  2020-11-05 11:36             ` Amelie DELAUNAY
@ 2020-11-05 12:23               ` Jun Li
  2020-11-05 15:17                 ` Amelie DELAUNAY
  2020-11-05 15:55                 ` Rob Herring
  0 siblings, 2 replies; 18+ messages in thread
From: Jun Li @ 2020-11-05 12:23 UTC (permalink / raw)
  To: Amelie DELAUNAY
  Cc: Rob Herring, Heikki Krogerus, Alexandre Torgue,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Greg Kroah-Hartman, Linux USB List, Russell King, linux-kernel,
	Badhri Jagan Sridharan, Maxime Coquelin,
	moderated list:ARM/STM32 ARCHITECTURE, linux-arm-kernel

Amelie DELAUNAY <amelie.delaunay@st.com> 于2020年11月5日周四 下午7:36写道:
>
> On 11/4/20 10:08 PM, Rob Herring wrote:
> > On Fri, Oct 30, 2020 at 04:27:14PM +0100, Amelie DELAUNAY wrote:
> >>
> >>
> >> On 10/30/20 3:29 PM, Rob Herring wrote:
> >>> On Thu, Oct 29, 2020 at 11:49 AM Amelie DELAUNAY <amelie.delaunay@st.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 10/29/20 4:40 PM, Rob Herring wrote:
> >>>>> On Thu, Oct 29, 2020 at 10:58:03AM +0100, Amelie Delaunay wrote:
> >>>>>> Power operation mode may depends on hardware design, so, add the optional
> >>>>>> property power-opmode for usb-c connector to select the power operation
> >>>>>> mode capability.
> >>>>>>
> >>>>>> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
> >>>>>> ---
> >>>>>>     .../bindings/connector/usb-connector.yaml      | 18 ++++++++++++++++++
> >>>>>>     1 file changed, 18 insertions(+)
> >>>>>>
> >>>>>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> >>>>>> index 728f82db073d..200d19c60fd5 100644
> >>>>>> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> >>>>>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> >>>>>> @@ -93,6 +93,24 @@ properties:
> >>>>>>           - device
> >>>>>>           - dual
> >>>>>>
> >>>>>> +  power-opmode:
> >>>>>
> >>>>> I've acked this version:
> >>>>>
> >>>>> https://lore.kernel.org/r/20201020093627.256885-2-badhri@google.com
> >>>>>
> >>>>
> >>>> frs is used for Fast Role Swap defined in USB PD spec.
> >>>> I understand it allows to get the same information but I'm wondering why
> >>>> the property name is limited to -frs- in this case. What about a
> >>>> non-power delivery USB-C connector ?
> >>>
> >>> I've got no idea. The folks that know USB-C and PD details need to get
> >>> together and work all this out. To me, it looks like the same thing...
> >>>
> >>
> >> It looks but...
> >>
> >> The purpose of power-opmode property is to configure the USB-C controllers,
> >> especially the non-PD USB-C controllers to determine the power operation
> >> mode that the Type C connector will support and will advertise through CC
> >> pins when it has no power delivery support, whatever the power role: Sink,
> >> Source or Dual
> >> The management of the property is the same that data-role and power-role
> >> properties, and done by USB Type-C Connector Class.
> >>
> >> new-source-frs-typec-current specifies initial current capability of the new
> >> source when vSafe5V is applied during PD3.0 Fast Role Swap. So here, this
> >> property is not applied at usb-c controller configuration level, but during
> >> PD Fast Role Swap, so when the Sink become the Source.
> >> Moreover, the related driver code says FRS can only be supported by DRP
> >> ports. So new-source-frs-typec-current property, in addition to being
> >> specific to PD, is also dedicated to DRP usb-c controller.
> >> The property is managed by Type-C Port Controller Manager for PD.
> >
> > But it's the same set of possible values, right? So we can align the
> > values at least.
> >
>
> USB Power Delivery FRS values are defined in
> include/dt-bindings/usb/pd.h

I think this can be changed if both can be aligned.

>to fit with drivers/usb/typec/tcpm/tcpm.c
> frs_typec_current enum.
>
> USB-C power operation mode values are defined in
> include/linux/usb/typec.h with typec_pwr_opmode enum and matching with
> string values of typec_pwr_opmodes tab.
>
> USB PD requires USB-C.
> USB-C doesn't requires USB PD.
>
> drivers/usb/typec/tcpm/tcpm.c already used typec_pwr_opmode values.
>
> USB PD specification Table 6-14 Fixed Supply PDO says:
> Fast Role Swap required USB Type-C Current (see also [USB Type-C 2.0]):
> Value | Description
>   00b  | Fast Swap not supported (default)
>   01b  | Default USB Power
>   10b  | 1.5A @ 5V
>   11b  | 3.0A @ 5V

This is the value in PDO of sink, the FRS property value(or after translated)
actually is used to compare with above value.

So I think both properties can share the same "value", maybe string
like below

  10 static const char * const typec_pwr_opmodes[] = {
  11         [TYPEC_PWR_MODE_USB]    = "default",
  12         [TYPEC_PWR_MODE_1_5A]   = "1.5A",
  13         [TYPEC_PWR_MODE_3_0A]   = "3.0A",

>
> Note the *see also USB Type-C 2.0*.
>
> USB Type-C specification 4.6.2.1 USB Type-C Current says:
> The USB Type-C connector uses CC pins for configuration including an
> ability for a Source to advertise to its port partner (Sink) the amount
> of current it shall supply:
> • Default is the as-configured for high-power operation current value as
> defined by the USB Specification (500 mA for USB 2.0 ports; 900 mA or
> 1,500 mA for USB 3.2 ports in single-lane or dual-lane operation,
> respectively)
> • 1.5 A
> • 3.0 A
>
> > Can we align the names in some way? power-opmode and frs-source-opmode
> > or ??

how about typec-power-opmode and frs-new-source-opmode

> >
>
> I let USB PD specialists answer.
>
> *frs* property fits with USB PD specification, so with USB PD protocol.
> *power-opmode fits with USB Type-C specification, so with USB-C hardware
> support.
>
> > Are these 2 properties mutually exclusive?

I think yes.

thanks
Li Jun
>> If so, that should be
> > captured.
>
> FRS is specific to products with Power Delivery Support.
>
> power-opmode is dedicated to products with USB-C connector support.
>
> Regards,
> Amelie

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RESEND PATCH v3 1/4] dt-bindings: connector: add power-opmode optional property to usb-connector
  2020-11-05 12:23               ` Jun Li
@ 2020-11-05 15:17                 ` Amelie DELAUNAY
  2020-11-05 15:55                 ` Rob Herring
  1 sibling, 0 replies; 18+ messages in thread
From: Amelie DELAUNAY @ 2020-11-05 15:17 UTC (permalink / raw)
  To: Jun Li
  Cc: Rob Herring, Heikki Krogerus, Alexandre Torgue,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Greg Kroah-Hartman, Linux USB List, Russell King, linux-kernel,
	Badhri Jagan Sridharan, Maxime Coquelin,
	moderated list:ARM/STM32 ARCHITECTURE, linux-arm-kernel

On 11/5/20 1:23 PM, Jun Li wrote:
> Amelie DELAUNAY <amelie.delaunay@st.com> 于2020年11月5日周四 下午7:36写道:
>>
>> On 11/4/20 10:08 PM, Rob Herring wrote:
>>> On Fri, Oct 30, 2020 at 04:27:14PM +0100, Amelie DELAUNAY wrote:
>>>>
>>>>
>>>> On 10/30/20 3:29 PM, Rob Herring wrote:
>>>>> On Thu, Oct 29, 2020 at 11:49 AM Amelie DELAUNAY <amelie.delaunay@st.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 10/29/20 4:40 PM, Rob Herring wrote:
>>>>>>> On Thu, Oct 29, 2020 at 10:58:03AM +0100, Amelie Delaunay wrote:
>>>>>>>> Power operation mode may depends on hardware design, so, add the optional
>>>>>>>> property power-opmode for usb-c connector to select the power operation
>>>>>>>> mode capability.
>>>>>>>>
>>>>>>>> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
>>>>>>>> ---
>>>>>>>>      .../bindings/connector/usb-connector.yaml      | 18 ++++++++++++++++++
>>>>>>>>      1 file changed, 18 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>>>>>>> index 728f82db073d..200d19c60fd5 100644
>>>>>>>> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>>>>>>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>>>>>>> @@ -93,6 +93,24 @@ properties:
>>>>>>>>            - device
>>>>>>>>            - dual
>>>>>>>>
>>>>>>>> +  power-opmode:
>>>>>>>
>>>>>>> I've acked this version:
>>>>>>>
>>>>>>> https://lore.kernel.org/r/20201020093627.256885-2-badhri@google.com
>>>>>>>
>>>>>>
>>>>>> frs is used for Fast Role Swap defined in USB PD spec.
>>>>>> I understand it allows to get the same information but I'm wondering why
>>>>>> the property name is limited to -frs- in this case. What about a
>>>>>> non-power delivery USB-C connector ?
>>>>>
>>>>> I've got no idea. The folks that know USB-C and PD details need to get
>>>>> together and work all this out. To me, it looks like the same thing...
>>>>>
>>>>
>>>> It looks but...
>>>>
>>>> The purpose of power-opmode property is to configure the USB-C controllers,
>>>> especially the non-PD USB-C controllers to determine the power operation
>>>> mode that the Type C connector will support and will advertise through CC
>>>> pins when it has no power delivery support, whatever the power role: Sink,
>>>> Source or Dual
>>>> The management of the property is the same that data-role and power-role
>>>> properties, and done by USB Type-C Connector Class.
>>>>
>>>> new-source-frs-typec-current specifies initial current capability of the new
>>>> source when vSafe5V is applied during PD3.0 Fast Role Swap. So here, this
>>>> property is not applied at usb-c controller configuration level, but during
>>>> PD Fast Role Swap, so when the Sink become the Source.
>>>> Moreover, the related driver code says FRS can only be supported by DRP
>>>> ports. So new-source-frs-typec-current property, in addition to being
>>>> specific to PD, is also dedicated to DRP usb-c controller.
>>>> The property is managed by Type-C Port Controller Manager for PD.
>>>
>>> But it's the same set of possible values, right? So we can align the
>>> values at least.
>>>
>>
>> USB Power Delivery FRS values are defined in
>> include/dt-bindings/usb/pd.h
> 
> I think this can be changed if both can be aligned.
> 
>> to fit with drivers/usb/typec/tcpm/tcpm.c
>> frs_typec_current enum.
>>
>> USB-C power operation mode values are defined in
>> include/linux/usb/typec.h with typec_pwr_opmode enum and matching with
>> string values of typec_pwr_opmodes tab.
>>
>> USB PD requires USB-C.
>> USB-C doesn't requires USB PD.
>>
>> drivers/usb/typec/tcpm/tcpm.c already used typec_pwr_opmode values.
>>
>> USB PD specification Table 6-14 Fixed Supply PDO says:
>> Fast Role Swap required USB Type-C Current (see also [USB Type-C 2.0]):
>> Value | Description
>>    00b  | Fast Swap not supported (default)
>>    01b  | Default USB Power
>>    10b  | 1.5A @ 5V
>>    11b  | 3.0A @ 5V
> 
> This is the value in PDO of sink, the FRS property value(or after translated)
> actually is used to compare with above value.
> 
> So I think both properties can share the same "value", maybe string
> like below
> 
>    10 static const char * const typec_pwr_opmodes[] = {
>    11         [TYPEC_PWR_MODE_USB]    = "default",
>    12         [TYPEC_PWR_MODE_1_5A]   = "1.5A",
>    13         [TYPEC_PWR_MODE_3_0A]   = "3.0A",
> 
>>
>> Note the *see also USB Type-C 2.0*.
>>
>> USB Type-C specification 4.6.2.1 USB Type-C Current says:
>> The USB Type-C connector uses CC pins for configuration including an
>> ability for a Source to advertise to its port partner (Sink) the amount
>> of current it shall supply:
>> • Default is the as-configured for high-power operation current value as
>> defined by the USB Specification (500 mA for USB 2.0 ports; 900 mA or
>> 1,500 mA for USB 3.2 ports in single-lane or dual-lane operation,
>> respectively)
>> • 1.5 A
>> • 3.0 A
>>
>>> Can we align the names in some way? power-opmode and frs-source-opmode
>>> or ??
> 
> how about typec-power-opmode and frs-new-source-opmode
> 

I agree with typec-power-opmode. And with string values. This way, 
typec_find_pwr_opmode is still usable to translate into TYPEC defines.

>>>
>>
>> I let USB PD specialists answer.
>>
>> *frs* property fits with USB PD specification, so with USB PD protocol.
>> *power-opmode fits with USB Type-C specification, so with USB-C hardware
>> support.
>>
>>> Are these 2 properties mutually exclusive?
> 
> I think yes.
> 
> thanks
> Li Jun
>>> If so, that should be
>>> captured.
>>
>> FRS is specific to products with Power Delivery Support.
>>
>> power-opmode is dedicated to products with USB-C connector support.
>>
>> Regards,
>> Amelie

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RESEND PATCH v3 1/4] dt-bindings: connector: add power-opmode optional property to usb-connector
  2020-11-05 12:23               ` Jun Li
  2020-11-05 15:17                 ` Amelie DELAUNAY
@ 2020-11-05 15:55                 ` Rob Herring
  2020-11-06  3:10                   ` Badhri Jagan Sridharan
  1 sibling, 1 reply; 18+ messages in thread
From: Rob Herring @ 2020-11-05 15:55 UTC (permalink / raw)
  To: Jun Li
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Amelie DELAUNAY, Heikki Krogerus, Alexandre Torgue,
	Greg Kroah-Hartman, Linux USB List, Russell King, linux-kernel,
	Badhri Jagan Sridharan, Maxime Coquelin,
	moderated list:ARM/STM32 ARCHITECTURE, linux-arm-kernel

On Thu, Nov 5, 2020 at 6:24 AM Jun Li <lijun.kernel@gmail.com> wrote:
>
> Amelie DELAUNAY <amelie.delaunay@st.com> 于2020年11月5日周四 下午7:36写道:
> >
> > On 11/4/20 10:08 PM, Rob Herring wrote:
> > > On Fri, Oct 30, 2020 at 04:27:14PM +0100, Amelie DELAUNAY wrote:
> > >>
> > >>
> > >> On 10/30/20 3:29 PM, Rob Herring wrote:
> > >>> On Thu, Oct 29, 2020 at 11:49 AM Amelie DELAUNAY <amelie.delaunay@st.com> wrote:
> > >>>>
> > >>>>
> > >>>>
> > >>>> On 10/29/20 4:40 PM, Rob Herring wrote:
> > >>>>> On Thu, Oct 29, 2020 at 10:58:03AM +0100, Amelie Delaunay wrote:
> > >>>>>> Power operation mode may depends on hardware design, so, add the optional
> > >>>>>> property power-opmode for usb-c connector to select the power operation
> > >>>>>> mode capability.
> > >>>>>>
> > >>>>>> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
> > >>>>>> ---
> > >>>>>>     .../bindings/connector/usb-connector.yaml      | 18 ++++++++++++++++++
> > >>>>>>     1 file changed, 18 insertions(+)
> > >>>>>>
> > >>>>>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > >>>>>> index 728f82db073d..200d19c60fd5 100644
> > >>>>>> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > >>>>>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > >>>>>> @@ -93,6 +93,24 @@ properties:
> > >>>>>>           - device
> > >>>>>>           - dual
> > >>>>>>
> > >>>>>> +  power-opmode:
> > >>>>>
> > >>>>> I've acked this version:
> > >>>>>
> > >>>>> https://lore.kernel.org/r/20201020093627.256885-2-badhri@google.com
> > >>>>>
> > >>>>
> > >>>> frs is used for Fast Role Swap defined in USB PD spec.
> > >>>> I understand it allows to get the same information but I'm wondering why
> > >>>> the property name is limited to -frs- in this case. What about a
> > >>>> non-power delivery USB-C connector ?
> > >>>
> > >>> I've got no idea. The folks that know USB-C and PD details need to get
> > >>> together and work all this out. To me, it looks like the same thing...
> > >>>
> > >>
> > >> It looks but...
> > >>
> > >> The purpose of power-opmode property is to configure the USB-C controllers,
> > >> especially the non-PD USB-C controllers to determine the power operation
> > >> mode that the Type C connector will support and will advertise through CC
> > >> pins when it has no power delivery support, whatever the power role: Sink,
> > >> Source or Dual
> > >> The management of the property is the same that data-role and power-role
> > >> properties, and done by USB Type-C Connector Class.
> > >>
> > >> new-source-frs-typec-current specifies initial current capability of the new
> > >> source when vSafe5V is applied during PD3.0 Fast Role Swap. So here, this
> > >> property is not applied at usb-c controller configuration level, but during
> > >> PD Fast Role Swap, so when the Sink become the Source.
> > >> Moreover, the related driver code says FRS can only be supported by DRP
> > >> ports. So new-source-frs-typec-current property, in addition to being
> > >> specific to PD, is also dedicated to DRP usb-c controller.
> > >> The property is managed by Type-C Port Controller Manager for PD.
> > >
> > > But it's the same set of possible values, right? So we can align the
> > > values at least.
> > >
> >
> > USB Power Delivery FRS values are defined in
> > include/dt-bindings/usb/pd.h
>
> I think this can be changed if both can be aligned.
>
> >to fit with drivers/usb/typec/tcpm/tcpm.c
> > frs_typec_current enum.
> >
> > USB-C power operation mode values are defined in
> > include/linux/usb/typec.h with typec_pwr_opmode enum and matching with
> > string values of typec_pwr_opmodes tab.
> >
> > USB PD requires USB-C.
> > USB-C doesn't requires USB PD.
> >
> > drivers/usb/typec/tcpm/tcpm.c already used typec_pwr_opmode values.
> >
> > USB PD specification Table 6-14 Fixed Supply PDO says:
> > Fast Role Swap required USB Type-C Current (see also [USB Type-C 2.0]):
> > Value | Description
> >   00b  | Fast Swap not supported (default)
> >   01b  | Default USB Power
> >   10b  | 1.5A @ 5V
> >   11b  | 3.0A @ 5V
>
> This is the value in PDO of sink, the FRS property value(or after translated)
> actually is used to compare with above value.
>
> So I think both properties can share the same "value", maybe string
> like below
>
>   10 static const char * const typec_pwr_opmodes[] = {
>   11         [TYPEC_PWR_MODE_USB]    = "default",
>   12         [TYPEC_PWR_MODE_1_5A]   = "1.5A",
>   13         [TYPEC_PWR_MODE_3_0A]   = "3.0A",
>
> >
> > Note the *see also USB Type-C 2.0*.
> >
> > USB Type-C specification 4.6.2.1 USB Type-C Current says:
> > The USB Type-C connector uses CC pins for configuration including an
> > ability for a Source to advertise to its port partner (Sink) the amount
> > of current it shall supply:
> > • Default is the as-configured for high-power operation current value as
> > defined by the USB Specification (500 mA for USB 2.0 ports; 900 mA or
> > 1,500 mA for USB 3.2 ports in single-lane or dual-lane operation,
> > respectively)
> > • 1.5 A
> > • 3.0 A
> >
> > > Can we align the names in some way? power-opmode and frs-source-opmode
> > > or ??
>
> how about typec-power-opmode and frs-new-source-opmode

Sure.

>
> > >
> >
> > I let USB PD specialists answer.
> >
> > *frs* property fits with USB PD specification, so with USB PD protocol.
> > *power-opmode fits with USB Type-C specification, so with USB-C hardware
> > support.
> >
> > > Are these 2 properties mutually exclusive?
>
> I think yes.

This should work to express that:

allOf:
  - not:
      required:
        - typec-power-opmode
        - frs-new-source-opmode

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RESEND PATCH v3 1/4] dt-bindings: connector: add power-opmode optional property to usb-connector
  2020-11-05 15:55                 ` Rob Herring
@ 2020-11-06  3:10                   ` Badhri Jagan Sridharan
  2020-11-06  8:03                     ` Amelie DELAUNAY
  0 siblings, 1 reply; 18+ messages in thread
From: Badhri Jagan Sridharan @ 2020-11-06  3:10 UTC (permalink / raw)
  To: Rob Herring
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Amelie DELAUNAY, Heikki Krogerus, Alexandre Torgue,
	Greg Kroah-Hartman, Linux USB List, Russell King, linux-kernel,
	Jun Li, Maxime Coquelin, moderated list:ARM/STM32 ARCHITECTURE,
	linux-arm-kernel

Hi Rob and Amelie,

With regards to discussions in this thread,
For https://lore.kernel.org/r/20201020093627.256885-2-badhri@google.com,
I can send in a patch to update the new-source-frs-typec-current property.
Amelie, If you are already planning to send that I am fine with that as well.
Let me know !

To summarize the changes for new-source-frs-typec-current would be,
1. Rename to frs-new-source-opmode
2. Use string values instead of u32 similar to typec-power-opmode.
Are these correct ?

Thanks,
Badhri

On Thu, Nov 5, 2020 at 7:55 AM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Nov 5, 2020 at 6:24 AM Jun Li <lijun.kernel@gmail.com> wrote:
> >
> > Amelie DELAUNAY <amelie.delaunay@st.com> 于2020年11月5日周四 下午7:36写道:
> > >
> > > On 11/4/20 10:08 PM, Rob Herring wrote:
> > > > On Fri, Oct 30, 2020 at 04:27:14PM +0100, Amelie DELAUNAY wrote:
> > > >>
> > > >>
> > > >> On 10/30/20 3:29 PM, Rob Herring wrote:
> > > >>> On Thu, Oct 29, 2020 at 11:49 AM Amelie DELAUNAY <amelie.delaunay@st.com> wrote:
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>> On 10/29/20 4:40 PM, Rob Herring wrote:
> > > >>>>> On Thu, Oct 29, 2020 at 10:58:03AM +0100, Amelie Delaunay wrote:
> > > >>>>>> Power operation mode may depends on hardware design, so, add the optional
> > > >>>>>> property power-opmode for usb-c connector to select the power operation
> > > >>>>>> mode capability.
> > > >>>>>>
> > > >>>>>> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
> > > >>>>>> ---
> > > >>>>>>     .../bindings/connector/usb-connector.yaml      | 18 ++++++++++++++++++
> > > >>>>>>     1 file changed, 18 insertions(+)
> > > >>>>>>
> > > >>>>>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > >>>>>> index 728f82db073d..200d19c60fd5 100644
> > > >>>>>> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > >>>>>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > >>>>>> @@ -93,6 +93,24 @@ properties:
> > > >>>>>>           - device
> > > >>>>>>           - dual
> > > >>>>>>
> > > >>>>>> +  power-opmode:
> > > >>>>>
> > > >>>>> I've acked this version:
> > > >>>>>
> > > >>>>> https://lore.kernel.org/r/20201020093627.256885-2-badhri@google.com
> > > >>>>>
> > > >>>>
> > > >>>> frs is used for Fast Role Swap defined in USB PD spec.
> > > >>>> I understand it allows to get the same information but I'm wondering why
> > > >>>> the property name is limited to -frs- in this case. What about a
> > > >>>> non-power delivery USB-C connector ?
> > > >>>
> > > >>> I've got no idea. The folks that know USB-C and PD details need to get
> > > >>> together and work all this out. To me, it looks like the same thing...
> > > >>>
> > > >>
> > > >> It looks but...
> > > >>
> > > >> The purpose of power-opmode property is to configure the USB-C controllers,
> > > >> especially the non-PD USB-C controllers to determine the power operation
> > > >> mode that the Type C connector will support and will advertise through CC
> > > >> pins when it has no power delivery support, whatever the power role: Sink,
> > > >> Source or Dual
> > > >> The management of the property is the same that data-role and power-role
> > > >> properties, and done by USB Type-C Connector Class.
> > > >>
> > > >> new-source-frs-typec-current specifies initial current capability of the new
> > > >> source when vSafe5V is applied during PD3.0 Fast Role Swap. So here, this
> > > >> property is not applied at usb-c controller configuration level, but during
> > > >> PD Fast Role Swap, so when the Sink become the Source.
> > > >> Moreover, the related driver code says FRS can only be supported by DRP
> > > >> ports. So new-source-frs-typec-current property, in addition to being
> > > >> specific to PD, is also dedicated to DRP usb-c controller.
> > > >> The property is managed by Type-C Port Controller Manager for PD.
> > > >
> > > > But it's the same set of possible values, right? So we can align the
> > > > values at least.
> > > >
> > >
> > > USB Power Delivery FRS values are defined in
> > > include/dt-bindings/usb/pd.h
> >
> > I think this can be changed if both can be aligned.
> >
> > >to fit with drivers/usb/typec/tcpm/tcpm.c
> > > frs_typec_current enum.
> > >
> > > USB-C power operation mode values are defined in
> > > include/linux/usb/typec.h with typec_pwr_opmode enum and matching with
> > > string values of typec_pwr_opmodes tab.
> > >
> > > USB PD requires USB-C.
> > > USB-C doesn't requires USB PD.
> > >
> > > drivers/usb/typec/tcpm/tcpm.c already used typec_pwr_opmode values.
> > >
> > > USB PD specification Table 6-14 Fixed Supply PDO says:
> > > Fast Role Swap required USB Type-C Current (see also [USB Type-C 2.0]):
> > > Value | Description
> > >   00b  | Fast Swap not supported (default)
> > >   01b  | Default USB Power
> > >   10b  | 1.5A @ 5V
> > >   11b  | 3.0A @ 5V
> >
> > This is the value in PDO of sink, the FRS property value(or after translated)
> > actually is used to compare with above value.
> >
> > So I think both properties can share the same "value", maybe string
> > like below
> >
> >   10 static const char * const typec_pwr_opmodes[] = {
> >   11         [TYPEC_PWR_MODE_USB]    = "default",
> >   12         [TYPEC_PWR_MODE_1_5A]   = "1.5A",
> >   13         [TYPEC_PWR_MODE_3_0A]   = "3.0A",
> >
> > >
> > > Note the *see also USB Type-C 2.0*.
> > >
> > > USB Type-C specification 4.6.2.1 USB Type-C Current says:
> > > The USB Type-C connector uses CC pins for configuration including an
> > > ability for a Source to advertise to its port partner (Sink) the amount
> > > of current it shall supply:
> > > • Default is the as-configured for high-power operation current value as
> > > defined by the USB Specification (500 mA for USB 2.0 ports; 900 mA or
> > > 1,500 mA for USB 3.2 ports in single-lane or dual-lane operation,
> > > respectively)
> > > • 1.5 A
> > > • 3.0 A
> > >
> > > > Can we align the names in some way? power-opmode and frs-source-opmode
> > > > or ??
> >
> > how about typec-power-opmode and frs-new-source-opmode
>
> Sure.
>
> >
> > > >
> > >
> > > I let USB PD specialists answer.
> > >
> > > *frs* property fits with USB PD specification, so with USB PD protocol.
> > > *power-opmode fits with USB Type-C specification, so with USB-C hardware
> > > support.
> > >
> > > > Are these 2 properties mutually exclusive?
> >
> > I think yes.
>
> This should work to express that:
>
> allOf:
>   - not:
>       required:
>         - typec-power-opmode
>         - frs-new-source-opmode
>
> Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RESEND PATCH v3 1/4] dt-bindings: connector: add power-opmode optional property to usb-connector
  2020-11-06  3:10                   ` Badhri Jagan Sridharan
@ 2020-11-06  8:03                     ` Amelie DELAUNAY
  0 siblings, 0 replies; 18+ messages in thread
From: Amelie DELAUNAY @ 2020-11-06  8:03 UTC (permalink / raw)
  To: Badhri Jagan Sridharan, Rob Herring
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Heikki Krogerus, Alexandre Torgue, Greg Kroah-Hartman,
	Linux USB List, Russell King, linux-kernel, Jun Li,
	Maxime Coquelin, moderated list:ARM/STM32 ARCHITECTURE,
	linux-arm-kernel

Hi Badhri,

On 11/6/20 4:10 AM, Badhri Jagan Sridharan wrote:
> Hi Rob and Amelie,
> 
> With regards to discussions in this thread,
> For https://lore.kernel.org/r/20201020093627.256885-2-badhri@google.com,
> I can send in a patch to update the new-source-frs-typec-current property.
> Amelie, If you are already planning to send that I am fine with that as well.
> Let me know !
> 
> To summarize the changes for new-source-frs-typec-current would be,
> 1. Rename to frs-new-source-opmode
> 2. Use string values instead of u32 similar to typec-power-opmode.
> Are these correct ?


You can send patches to rename the new-source-frq-typec-current property 
into frs-new-source-opmode, and update the tcpm as well. You can use the 
typec_find_pwr_opmode(), it will return
enum typec_pwr_opmode {
	TYPEC_PWR_MODE_USB,
	TYPEC_PWR_MODE_1_5A,
	TYPEC_PWR_MODE_3_0A,
	TYPEC_PWR_MODE_PD,
};
Then you have to translate it for FRS.

I'll send a new version of my series to document typec-power-opmode and 
update stusb160x driver and stm32mp15xx-dkx device tree accordingly.

Thanks,
Amelie

> 
> Thanks,
> Badhri
> 
> On Thu, Nov 5, 2020 at 7:55 AM Rob Herring <robh@kernel.org> wrote:
>>
>> On Thu, Nov 5, 2020 at 6:24 AM Jun Li <lijun.kernel@gmail.com> wrote:
>>>
>>> Amelie DELAUNAY <amelie.delaunay@st.com> 于2020年11月5日周四 下午7:36写道:
>>>>
>>>> On 11/4/20 10:08 PM, Rob Herring wrote:
>>>>> On Fri, Oct 30, 2020 at 04:27:14PM +0100, Amelie DELAUNAY wrote:
>>>>>>
>>>>>>
>>>>>> On 10/30/20 3:29 PM, Rob Herring wrote:
>>>>>>> On Thu, Oct 29, 2020 at 11:49 AM Amelie DELAUNAY <amelie.delaunay@st.com> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 10/29/20 4:40 PM, Rob Herring wrote:
>>>>>>>>> On Thu, Oct 29, 2020 at 10:58:03AM +0100, Amelie Delaunay wrote:
>>>>>>>>>> Power operation mode may depends on hardware design, so, add the optional
>>>>>>>>>> property power-opmode for usb-c connector to select the power operation
>>>>>>>>>> mode capability.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
>>>>>>>>>> ---
>>>>>>>>>>      .../bindings/connector/usb-connector.yaml      | 18 ++++++++++++++++++
>>>>>>>>>>      1 file changed, 18 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>>>>>>>>> index 728f82db073d..200d19c60fd5 100644
>>>>>>>>>> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>>>>>>>>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>>>>>>>>> @@ -93,6 +93,24 @@ properties:
>>>>>>>>>>            - device
>>>>>>>>>>            - dual
>>>>>>>>>>
>>>>>>>>>> +  power-opmode:
>>>>>>>>>
>>>>>>>>> I've acked this version:
>>>>>>>>>
>>>>>>>>> https://lore.kernel.org/r/20201020093627.256885-2-badhri@google.com
>>>>>>>>>
>>>>>>>>
>>>>>>>> frs is used for Fast Role Swap defined in USB PD spec.
>>>>>>>> I understand it allows to get the same information but I'm wondering why
>>>>>>>> the property name is limited to -frs- in this case. What about a
>>>>>>>> non-power delivery USB-C connector ?
>>>>>>>
>>>>>>> I've got no idea. The folks that know USB-C and PD details need to get
>>>>>>> together and work all this out. To me, it looks like the same thing...
>>>>>>>
>>>>>>
>>>>>> It looks but...
>>>>>>
>>>>>> The purpose of power-opmode property is to configure the USB-C controllers,
>>>>>> especially the non-PD USB-C controllers to determine the power operation
>>>>>> mode that the Type C connector will support and will advertise through CC
>>>>>> pins when it has no power delivery support, whatever the power role: Sink,
>>>>>> Source or Dual
>>>>>> The management of the property is the same that data-role and power-role
>>>>>> properties, and done by USB Type-C Connector Class.
>>>>>>
>>>>>> new-source-frs-typec-current specifies initial current capability of the new
>>>>>> source when vSafe5V is applied during PD3.0 Fast Role Swap. So here, this
>>>>>> property is not applied at usb-c controller configuration level, but during
>>>>>> PD Fast Role Swap, so when the Sink become the Source.
>>>>>> Moreover, the related driver code says FRS can only be supported by DRP
>>>>>> ports. So new-source-frs-typec-current property, in addition to being
>>>>>> specific to PD, is also dedicated to DRP usb-c controller.
>>>>>> The property is managed by Type-C Port Controller Manager for PD.
>>>>>
>>>>> But it's the same set of possible values, right? So we can align the
>>>>> values at least.
>>>>>
>>>>
>>>> USB Power Delivery FRS values are defined in
>>>> include/dt-bindings/usb/pd.h
>>>
>>> I think this can be changed if both can be aligned.
>>>
>>>> to fit with drivers/usb/typec/tcpm/tcpm.c
>>>> frs_typec_current enum.
>>>>
>>>> USB-C power operation mode values are defined in
>>>> include/linux/usb/typec.h with typec_pwr_opmode enum and matching with
>>>> string values of typec_pwr_opmodes tab.
>>>>
>>>> USB PD requires USB-C.
>>>> USB-C doesn't requires USB PD.
>>>>
>>>> drivers/usb/typec/tcpm/tcpm.c already used typec_pwr_opmode values.
>>>>
>>>> USB PD specification Table 6-14 Fixed Supply PDO says:
>>>> Fast Role Swap required USB Type-C Current (see also [USB Type-C 2.0]):
>>>> Value | Description
>>>>    00b  | Fast Swap not supported (default)
>>>>    01b  | Default USB Power
>>>>    10b  | 1.5A @ 5V
>>>>    11b  | 3.0A @ 5V
>>>
>>> This is the value in PDO of sink, the FRS property value(or after translated)
>>> actually is used to compare with above value.
>>>
>>> So I think both properties can share the same "value", maybe string
>>> like below
>>>
>>>    10 static const char * const typec_pwr_opmodes[] = {
>>>    11         [TYPEC_PWR_MODE_USB]    = "default",
>>>    12         [TYPEC_PWR_MODE_1_5A]   = "1.5A",
>>>    13         [TYPEC_PWR_MODE_3_0A]   = "3.0A",
>>>
>>>>
>>>> Note the *see also USB Type-C 2.0*.
>>>>
>>>> USB Type-C specification 4.6.2.1 USB Type-C Current says:
>>>> The USB Type-C connector uses CC pins for configuration including an
>>>> ability for a Source to advertise to its port partner (Sink) the amount
>>>> of current it shall supply:
>>>> • Default is the as-configured for high-power operation current value as
>>>> defined by the USB Specification (500 mA for USB 2.0 ports; 900 mA or
>>>> 1,500 mA for USB 3.2 ports in single-lane or dual-lane operation,
>>>> respectively)
>>>> • 1.5 A
>>>> • 3.0 A
>>>>
>>>>> Can we align the names in some way? power-opmode and frs-source-opmode
>>>>> or ??
>>>
>>> how about typec-power-opmode and frs-new-source-opmode
>>
>> Sure.
>>
>>>
>>>>>
>>>>
>>>> I let USB PD specialists answer.
>>>>
>>>> *frs* property fits with USB PD specification, so with USB PD protocol.
>>>> *power-opmode fits with USB Type-C specification, so with USB-C hardware
>>>> support.
>>>>
>>>>> Are these 2 properties mutually exclusive?
>>>
>>> I think yes.
>>
>> This should work to express that:
>>
>> allOf:
>>    - not:
>>        required:
>>          - typec-power-opmode
>>          - frs-new-source-opmode
>>
>> Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2020-11-06  8:05 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-29  9:58 [RESEND PATCH v3 0/4] STUSB1600 support on STM32MP15xx-DKx Amelie Delaunay
2020-10-29  9:58 ` [RESEND PATCH v3 1/4] dt-bindings: connector: add power-opmode optional property to usb-connector Amelie Delaunay
2020-10-29 15:40   ` Rob Herring
2020-10-29 16:49     ` Amelie DELAUNAY
2020-10-30  1:54       ` Jun Li
2020-10-30 14:32         ` Rob Herring
2020-10-30 14:29       ` Rob Herring
2020-10-30 15:27         ` Amelie DELAUNAY
2020-11-04 21:08           ` Rob Herring
2020-11-05 11:36             ` Amelie DELAUNAY
2020-11-05 12:23               ` Jun Li
2020-11-05 15:17                 ` Amelie DELAUNAY
2020-11-05 15:55                 ` Rob Herring
2020-11-06  3:10                   ` Badhri Jagan Sridharan
2020-11-06  8:03                     ` Amelie DELAUNAY
2020-10-29  9:58 ` [RESEND PATCH v3 2/4] dt-bindings: usb: Add DT bindings for STUSB160x Type-C controller Amelie Delaunay
2020-10-29  9:58 ` [RESEND PATCH v3 3/4] ARM: dts: stm32: add STUSB1600 Type-C using I2C4 on stm32mp15xx-dkx Amelie Delaunay
2020-10-29  9:58 ` [RESEND PATCH v3 4/4] ARM: multi_v7_defconfig: enable STUSB160X Type-C port controller support Amelie Delaunay

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).