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

2020-07-09 Thread Wesley Cheng


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 = <_usbc_hs>;
};
//Audio accessory adapter mux*
usb_con_hs_audio: endpoint@1 {
reg = <1>;
remote-endpoint = <_audio>;
};
};
port@1 {
...
//USB3 lane mux**
usb3_data_ss: endpoint@0 {
reg = <0>;
remote-endpoint = <_ss_mux>;
};
//USB3 SOC controller**
usb3_role: endpoint@1 {
reg = <1>;
remote-endpoint = <_drd_switch>;
};
};
};
};

Thanks
Wesley Cheng

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

2020-07-09 Thread Rob Herring
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 
> ---
>  .../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 ..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 
> +
> +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