All of
 help / color / mirror / Atom feed
From: Harsh Agarwal <>
To: Rob Herring <>
Cc: Greg Kroah-Hartman <>,
	Philipp Zabel <>,
	Krzysztof Kozlowski <>,
	Felipe Balbi <>,
	"Bjorn Andersson" <>,
	<>, <>,
	<>, <>,
	<>, <>,
Subject: Re: [PATCH v3 1/3] dt-bindings: usb: dwc3: Add support for multiport related properties
Date: Fri, 10 Jun 2022 17:25:25 +0530	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On 6/9/2022 9:08 PM, Rob Herring wrote:
> On Wed, Jun 08, 2022 at 11:06:25PM +0530, Harsh Agarwal wrote:
>> Added support for multiport, mport, num_usb2_phy and num_usb3_phy
>> properties. These properties are used to support devices having
>> a multiport controller.
>> Signed-off-by: Harsh Agarwal <>
>> ---
>>   .../devicetree/bindings/usb/snps,dwc3.yaml         | 53 ++++++++++++++++++++++
>>   1 file changed, 53 insertions(+)
>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> index d41265b..9332fa2 100644
>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> @@ -343,6 +343,32 @@ properties:
>>         This port is used with the 'usb-role-switch' property  to connect the
>>         dwc3 to type C connector.
>> +  multiport:
> Again, I don't think this is going to play well if you need to describe
> USB devices in your DT. For example, a USB hub with additional DT
> properties.
Thanks for the review Rob.
Can you please explain why would one want to describe a USB hub in 
device tree ?
IF USB hub is attached to a root port , it would be enumerated by the 
SW. I am not clear how DT is coming
into picture. Even if there was a scenario to add DT properties for a 
hub, then this multiport node would be like a nop
as it just helps us to get the PHY phandles in a proper way.
Do you feel we still might have a problem with multiport node ?
>> +    description:
>> +      If a single USB controller supports multiple ports, then it's referred to as
>> +      a multiport controller. Each port of the multiport controller can support
>> +      either High Speed or Super Speed or both and have their own PHY phandles. Each
>> +      port is represented by "mport" node and all the "mport" nodes are grouped
>> +      together inside the "multiport" node where individual "mport" node defines the
>> +      PHYs supported by that port.
>> +
>> +  num_usb2_phy:
>> +    description: Total number of HS-PHYs defined by the multiport controller.
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> +  num_usb3_phy:
>> +    description: Total number of SS-PHYs defined by the multiport controller.
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> +  mport:
>> +    description: Each mport node represents one port of the multiport controller.
>> +    oneOf:
>> +      - required:
>> +        - usb-phy
> This is deprecated. Why are you adding it?
Do you mean "usb-phy" is deprecated ?
Internally we use usb-phy with our downstream GLUE driver
>> +      - required:
>> +        - phys
>> +        - phy-names
> Other multi port USB hosts just have a list of phys. Why can't you just
> use phy-names to identify each phy:
> phy-names = "port0-hs", "port0-ss", "port1-hs", "port1-ss", "port2-hs",
>    "port3-hs";
With the above method we would have to do some kind of string parsing on 
the phy-names to get the HS and SS PHYs as we need to cater to different 
combinations of Ports ( some support HS+SS , other supports SS only).
So one challenge here is with the "usb-phy". There we directly define 
the phy phandles and that might/might-not have proper sub-strings. eg 
USB_QMP_PHY . So extracting PHYS could be tricky if the phy-handle does 
not have proper substring like "SS" "HS" etc.
We cannot break existing implementation and so we thought of going with 
the "multiport" node approach, listing below some flexibility :

1. Better representation of the PHYs and it's relation with a port.
2. Here for each port we pick the first PHY as HS and 2nd PHY as SS as 
we have been doing traditionally.
So for "usb-phy" we need not care how the PHY handles are named.

3. It's future proof incase we need to add additional properties 
specific to a port. We can just add those properties inside MP_1 or MP_2 
Though nothing like this has yet been implemented.

Also agree that there are multiple ways to approach this problem and 
that's why we had RFC tag earlier to get feedback on the same.
> Rob

  reply	other threads:[~2022-06-10 11:55 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-08 17:36 [PATCH v3 0/3] Add support for multiport controller Harsh Agarwal
2022-06-08 17:36 ` [PATCH v3 1/3] dt-bindings: usb: dwc3: Add support for multiport related properties Harsh Agarwal
2022-06-08 21:48   ` Rob Herring
2022-06-09 15:38   ` Rob Herring
2022-06-10 11:55     ` Harsh Agarwal [this message]
2022-06-10 17:22       ` Rob Herring
2022-06-27 13:06         ` Harsh Agarwal
2022-07-06 22:09           ` Rob Herring
2022-11-18  9:01             ` Krishna Kurapati PSSNV
2022-06-08 17:36 ` [PATCH v3 2/3] usb: phy: Add devm_of_usb_get_phy_by_phandle Harsh Agarwal
2022-06-08 17:36 ` [PATCH v3 3/3] usb: dwc3: Refactor PHY logic to support Multiport Controller Harsh Agarwal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.