diff mbox series

[v5,2/6] dt-bindings: usb: Add Qualcomm PMIC type C controller dt-binding

Message ID 20200703015102.27295-3-wcheng@codeaurora.org
State Changes Requested, archived
Headers show
Series Introduce PMIC based USB type C detection | expand

Checks

Context Check Description
robh/checkpatch warning total: 0 errors, 1 warnings, 130 lines checked
robh/dt-meta-schema success

Commit Message

Wesley Cheng July 3, 2020, 1:50 a.m. UTC
Introduce the dt-binding for enabling USB type C orientation and role
detection using the PM8150B.  The driver will be responsible for receiving
the interrupt at a state change on the CC lines, reading the orientation/role,
and communicating this information to the remote clients, which can include
a role switch node and a type C switch.

Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>
---
 .../bindings/usb/qcom,pmic-typec.yaml         | 130 ++++++++++++++++++
 1 file changed, 130 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml

Comments

Rob Herring July 9, 2020, 10:46 p.m. UTC | #1
On Thu, Jul 02, 2020 at 06:50:58PM -0700, Wesley Cheng wrote:
> Introduce the dt-binding for enabling USB type C orientation and role
> detection using the PM8150B.  The driver will be responsible for receiving
> the interrupt at a state change on the CC lines, reading the orientation/role,
> and communicating this information to the remote clients, which can include
> a role switch node and a type C switch.
> 
> Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>
> ---
>  .../bindings/usb/qcom,pmic-typec.yaml         | 130 ++++++++++++++++++
>  1 file changed, 130 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml b/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml
> new file mode 100644
> index 000000000000..735b1f74664b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml
> @@ -0,0 +1,130 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/usb/qcom,pmic-typec.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Qualcomm PMIC based USB type C Detection Driver
> +
> +maintainers:
> +  - Wesley Cheng <wcheng@codeaurora.org>
> +
> +description: |
> +  Qualcomm PMIC Type C Detect
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,pm8150b-usb-typec
> +
> +  reg:
> +    maxItems: 1
> +    description: Type C base address
> +
> +  interrupts:
> +    maxItems: 1
> +    description: CC change interrupt from PMIC
> +
> +  connector:
> +    description: Connector type for remote endpoints
> +    type: object
> +    $ref: /schemas/connector/usb-connector.yaml#

Why is all the connector schema duplicated here? You only need things 
that are further constrained like 'compatible'.

> +
> +    properties:
> +      compatible:
> +        enum:
> +          - usb-c-connector
> +
> +      power-role:
> +       enum:
> +         - dual
> +         - source
> +         - sink
> +
> +      data-role:
> +        enum:
> +          - dual
> +          - host
> +          - device
> +
> +      ports:
> +        description: Remote endpoint connections
> +        type: object
> +
> +        properties:
> +          port@0:
> +            description: Remote endpoints for the High Speed path
> +            type: object
> +
> +          port@1:
> +            description: Remote endpoints for the Super Speed path
> +            type: object
> +
> +            properties:
> +              endpoint@0:
> +                description: Connection to USB type C mux node
> +                type: object
> +
> +                properties:
> +                  remote-endpoint:
> +                    maxItems: 1

'remote-endpoint' in not an array.

> +                    description: Node reference to the type C mux
> +
> +              endpoint@1:
> +                description: Connection to role switch node
> +                type: object
> +
> +                properties:
> +                  remote-endpoint:
> +                    maxItems: 1
> +                    description: Node reference to the role switch node

So USB-SS data can come from 'type C mux' or 'role switch node'? That 
seems odd.

Rob
Wesley Cheng July 10, 2020, 3:08 a.m. UTC | #2
On 7/9/2020 3:46 PM, Rob Herring wrote:
> 
> Why is all the connector schema duplicated here? You only need things 
> that are further constrained like 'compatible'.
> 

Hi Rob,

Most of the properties in this dt-binding are going to be constrained by
the definitions/values specified by usb-connector.  I can add individual
references to each property, such as compatible, power-role, data-role
and ports, if that is the recommended approach.

> 
> 'remote-endpoint' in not an array.
> 

Agreed, I have removed the maxItems parameter.

> 
> So USB-SS data can come from 'type C mux' or 'role switch node'? That 
> seems odd.
> 

This was one of the interpretations, which might work with the current
usb-connector model.  From the previous block diagram I shared, we can
see that the SS path has two potential "endpoints," one to the mux and
another to the USB controller on the SOC.

Another design consideration is when the device supports the "Audio
Adapter Accessory Mode."  The audio accessory is mentioned in the type C
spec as an adapter which will utilize the USB D+/- lines for audio
output.  So now, you could potentially have something like below:

				   _______		 _______
                            ______|FUSB302|		|SOC	|
			   |	  |Type C |		|	|
			   |      |Cntrl  |__I2C_______	|	|
			   |	  |_______|		|	|
 ___                       |       			|	|
|   |______ CC1/2 _________|				|	|
|   |				   _______		|	|
|   |			     ____*|Charger|		|	|
|   |			    |	  |HW     |		|	|
|   |			    |     |_______|		|	|
|   |			    |	   ________      	|	|
|   |______ HS DP/DM _______|____*|MAX20328|_HS D+/D-__	|	|
|   |				  |________|__Audio Out_|	|
|   |				   ________		|	|
|   |______ SS RX/TX1 __________**|FUSB304 |_SS RX/TX**_|	|
|   |______ SS RX/TX2 ____________|USB Mux |		|_______|
|   |                             |________|
|   |
|___|

With this kind of device, it would make sense to have multiple endpoints
per path, which can be represented like the following:

connector {
        compatible = "usb-c-connector";
	...
        ports {
        	port@0 {
			...
			//Charger endpoint*
			usb_con_hs_chg: endpoint@0 {
				reg = <0>;
				remote-endpoint = <&max77865_usbc_hs>;
			};
			//Audio accessory adapter mux*
			usb_con_hs_audio: endpoint@1 {
				reg = <1>;
				remote-endpoint = <&analog_audio>;
			};
                };
		port@1 {
			...
			//USB3 lane mux**
			usb3_data_ss: endpoint@0 {
				reg = <0>;
				remote-endpoint = <&qmp_ss_mux>;
			};
			//USB3 SOC controller**
			usb3_role: endpoint@1 {
				reg = <1>;
				remote-endpoint = <&dwc3_drd_switch>;
			};
		};
	};
};

Thanks
Wesley Cheng
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml b/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml
new file mode 100644
index 000000000000..735b1f74664b
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml
@@ -0,0 +1,130 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/usb/qcom,pmic-typec.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Qualcomm PMIC based USB type C Detection Driver
+
+maintainers:
+  - Wesley Cheng <wcheng@codeaurora.org>
+
+description: |
+  Qualcomm PMIC Type C Detect
+
+properties:
+  compatible:
+    enum:
+      - qcom,pm8150b-usb-typec
+
+  reg:
+    maxItems: 1
+    description: Type C base address
+
+  interrupts:
+    maxItems: 1
+    description: CC change interrupt from PMIC
+
+  connector:
+    description: Connector type for remote endpoints
+    type: object
+    $ref: /schemas/connector/usb-connector.yaml#
+
+    properties:
+      compatible:
+        enum:
+          - usb-c-connector
+
+      power-role:
+       enum:
+         - dual
+         - source
+         - sink
+
+      data-role:
+        enum:
+          - dual
+          - host
+          - device
+
+      ports:
+        description: Remote endpoint connections
+        type: object
+
+        properties:
+          port@0:
+            description: Remote endpoints for the High Speed path
+            type: object
+
+          port@1:
+            description: Remote endpoints for the Super Speed path
+            type: object
+
+            properties:
+              endpoint@0:
+                description: Connection to USB type C mux node
+                type: object
+
+                properties:
+                  remote-endpoint:
+                    maxItems: 1
+                    description: Node reference to the type C mux
+
+              endpoint@1:
+                description: Connection to role switch node
+                type: object
+
+                properties:
+                  remote-endpoint:
+                    maxItems: 1
+                    description: Node reference to the role switch node
+
+    required:
+      - compatible
+
+required:
+  - compatible
+  - interrupts
+  - connector
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    pm8150b {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        pm8150b_typec: typec@1500 {
+            compatible = "qcom,pm8150b-usb-typec";
+            reg = <0x1500>;
+            interrupts = <0x2 0x15 0x5 IRQ_TYPE_EDGE_RISING>;
+
+            connector {
+                compatible = "usb-c-connector";
+                power-role = "dual";
+                data-role = "dual";
+                ports {
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+                    port@0 {
+                        reg = <0>;
+                    };
+                    port@1 {
+                        reg = <1>;
+                        #address-cells = <1>;
+                        #size-cells = <0>;
+                        usb3_data_ss: endpoint@0 {
+                            reg = <0>;
+                            remote-endpoint = <&qmp_ss_mux>;
+                        };
+                        usb3_role: endpoint@1 {
+                            reg = <1>;
+                            remote-endpoint = <&dwc3_drd_switch>;
+                        };
+                    };
+                };
+            };
+        };
+    };
+...