On Mon, Jun 27, 2022 at 06:36:53PM +0530, Harsh Agarwal wrote:
On 6/10/2022 10:52 PM, Rob Herring wrote:
On Fri, Jun 10, 2022 at 05:25:25PM +0530, Harsh Agarwal wrote:Hi Rob,
On 6/9/2022 9:08 PM, Rob Herring wrote:Because someone soldered a hub on the board and then connected extra
On Wed, Jun 08, 2022 at 11:06:25PM +0530, Harsh Agarwal wrote:Thanks for the review Rob.
Added support for multiport, mport, num_usb2_phy and num_usb3_phyAgain, I don't think this is going to play well if you need to describe
properties. These properties are used to support devices having
a multiport controller.
Signed-off-by: Harsh Agarwal <quic_harshq@xxxxxxxxxxx>
---
.../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:
USB devices in your DT. For example, a USB hub with additional DT
properties.
Can you please explain why would one want to describe a USB hub in device
tree ?
things like resets, GPIOs, supplies which are all outside of standard
USB. It's quite common...
There's some flavors of Beagle boards that have a USB ethernet on board.
Guess what, they skipped out on a eeprom and so the device and a MAC
address has to be described in DT (if you want a stable MAC addr).
IF USB hub is attached to a root port , it would be enumerated by the SW. IIt won't be enumerated by the SW if it has to be powered on first using
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.
non-standard resources.
Do you feel we still might have a problem with multiport node ?A board design could have a hub or device on any or all your ports.
It is replaced by 'phys'. Any new user should use 'phys'.Do you mean "usb-phy" is deprecated ?+ description:This is deprecated. Why are you adding it?
+ 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
Internally we use usb-phy with our downstream GLUE driverUpstream does not care about that.
You are doing string parsing anyways to get the child nodes andWith the above method we would have to do some kind of string parsing on the+ - required:Other multi port USB hosts just have a list of phys. Why can't you just
+ - phys
+ - phy-names
use phy-names to identify each phy:
phy-names = "port0-hs", "port0-ss", "port1-hs", "port1-ss", "port2-hs",
"port3-hs";
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).
properties.
So one challenge here is with the "usb-phy". There we directly define theThe schema can and should enforce that you have the proper strings.
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.
Apologies for replying late.
I get your concern. Yes we can remove the "multiport" node and instead
define the
USB phy phandles all in one place. Still I would need to add support for
both generic-phy and
usb-phy framework as downstream many vendors are using "usb-phy" and it's
supported by ACK as well.
Upstream is not concerned with downstream. The generic PHY has replaced
usb-phy for many years now.
Furthermore, if downstream was using documented bindings, then we
wouldn't be having this conversation.
Thanks for the review. How about we do the following:This would not regress anything with Generic PHY.
@Greg can you please comment as ACK has support for usb-phy framework.
Now coming to implementation, let's consider a 4 port USB multiport
controller having
4 HS PHYs and 2 SS PHYs. We can have two approaches here
#1 -> If we could mandate using "HS" or "SS" as substring in
phy-names or usb-phy, then we can calculate number of HS and SS phy and also
get
corresponding PHY nodes. Only concern here is that downstream vendors might
need
to change their existing usb-phy names and add proper substring if they are
not doing so ;
phy = <&usb-hs-phy>,<&usb-ss-phy>,
<&usb-hs-phy1>, <&usb-ss-phy1>,
<&usb-hs-phy2>, <&usb-hs-phy3>;
phy-names = "port0-hs", "port0-ss", "port1-hs", "port1-ss", "port2-hs",
"port3-hs";
OR
#2-> We could mandate defining the USB phy in HS - SS pairs.
For ports that has only HS PHY, we would need to define usb_nop_phy in SS
place.
Then we can calculate the number of HS & SS phys and get corresponding
PHY nodes by using simple fact that HS phy would be defined at odd places &
SS phy defined at even. Here substrings are not mandated.
phy = <&usb-hs-phy>,<&usb-qmp-phy>,
<&usb-hs-phy1>, <&usb-qmp-phy1>,
<&usb-hs-phy2>, <&usb_nop_phy>
<&usb-hs-phy3>, <&usb_nop_phy>;
phy-names = "port0-hs", "port0-ss",
"port1-hs", "port1-ss",
"port2-hs", "usb-nop",
"port3-hs", "usb-nop";
The whole reason for -names is to avoid something like this with filler
entries. So I prefer #1 as I suggested.
Rob