| Message ID | 20250407-rk3576-sige5-usb-v1-1-67eec166f82f@collabora.com |
|---|---|
| State | New |
| Headers | show |
| Series | RK3576 USB Enablement | expand |
On Mon, Apr 07, 2025 at 08:09:14PM +0200, Nicolas Frattaroli wrote: > USB connectors like to have OF graph connections to high-speed related > nodes to do various things. In the case of the RK3576, we can make use > of a port in the usb2 PHY to detect whether the OTG controller is > connected to a type C port and apply some special behaviour accordingly. > > The usefulness of having different bits of a fully functioning USB stack > point to each other is more general though, and not constrained to > RK3576 at all, even for this use-case. > > Add a port property to the binding. > > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com> > --- > Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml > index 6a7ef556414cebad63c10de754778f84fd4486ee..3a662bfc353250a8ad9386ebb5575d1e84c1b5ba 100644 > --- a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml > +++ b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml > @@ -78,6 +78,11 @@ properties: > When set the driver will request its phandle as one companion-grf > for some special SoCs (e.g rv1108). > > + port: > + $ref: /schemas/graph.yaml#/properties/port > + description: > + A port node to link the PHY to a USB connector's "high-speed" port. I don't think this is correct. The HS port of the connector goes to the controller. The controller has the link to the phy. If the PHY is also what handles USB-C muxing or orientation switching, then it might have ports, but then it needs input and output ports. Rob
On Thursday, 10 April 2025 23:11:23 Central European Summer Time Rob Herring wrote: > On Mon, Apr 07, 2025 at 08:09:14PM +0200, Nicolas Frattaroli wrote: > > USB connectors like to have OF graph connections to high-speed related > > nodes to do various things. In the case of the RK3576, we can make use > > of a port in the usb2 PHY to detect whether the OTG controller is > > connected to a type C port and apply some special behaviour accordingly. > > > > The usefulness of having different bits of a fully functioning USB stack > > point to each other is more general though, and not constrained to > > RK3576 at all, even for this use-case. > > > > Add a port property to the binding. > > > > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com> > > --- > > Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml > > index 6a7ef556414cebad63c10de754778f84fd4486ee..3a662bfc353250a8ad9386ebb5575d1e84c1b5ba 100644 > > --- a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml > > +++ b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml > > @@ -78,6 +78,11 @@ properties: > > When set the driver will request its phandle as one companion-grf > > for some special SoCs (e.g rv1108). > > > > + port: > > + $ref: /schemas/graph.yaml#/properties/port > > + description: > > + A port node to link the PHY to a USB connector's "high-speed" port. > > I don't think this is correct. The HS port of the connector goes to the > controller. The controller has the link to the phy. > > If the PHY is also what handles USB-C muxing or orientation switching, > then it might have ports, but then it needs input and output ports. > > Rob > Hi Rob, thank you for the quick response. I've feared this would be the case, but chose to go ahead with this solution anyway because I'm not super stoked about the alternatives I can think of. The problem is that I need to go from the USB PHY node to the USB connector somehow, but there's no way I can see to get from the PHY node to the USB2 controller it's connected to, unless I'm missing something obvious. So I see two alternatives: 1. Extend the usb connector binding to add additional ports for PHYs that handle vbus detection or something, then extend either the inno2 binding or a more general usb PHY binding to add that port definition. 2. Revert to what the downstream vendor kernel does and simply add a boolean flag property to the inno usb2phy binding that tells it whether it's connected to a USB-C port and should therefore expect vbusdet to remain high. Let me know if there's any better alternatives I missed. If there's some OF driver function to enumerate all controllers a PHY is referenced by, then that would probably work as well, allowing me to just point the HS port to the controller instead as intended. If no better solutions exist then I'm partial to 2. While it makes writing device trees a little more error prone and I don't like vendor properties, it means we don't set the "all USB-C ports will always have vbusdet pulled high" in stone, a claim that I am not 100% confident in. Kind regards, Nicolas Frattaroli
On Fri, Apr 11, 2025 at 04:31:38PM +0200, Nicolas Frattaroli wrote: > On Thursday, 10 April 2025 23:11:23 Central European Summer Time Rob Herring wrote: > > On Mon, Apr 07, 2025 at 08:09:14PM +0200, Nicolas Frattaroli wrote: > > > USB connectors like to have OF graph connections to high-speed related > > > nodes to do various things. In the case of the RK3576, we can make use > > > of a port in the usb2 PHY to detect whether the OTG controller is > > > connected to a type C port and apply some special behaviour accordingly. > > > > > > The usefulness of having different bits of a fully functioning USB stack > > > point to each other is more general though, and not constrained to > > > RK3576 at all, even for this use-case. > > > > > > Add a port property to the binding. > > > > > > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com> > > > --- > > > Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml > > > index 6a7ef556414cebad63c10de754778f84fd4486ee..3a662bfc353250a8ad9386ebb5575d1e84c1b5ba 100644 > > > --- a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml > > > +++ b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml > > > @@ -78,6 +78,11 @@ properties: > > > When set the driver will request its phandle as one companion-grf > > > for some special SoCs (e.g rv1108). > > > > > > + port: > > > + $ref: /schemas/graph.yaml#/properties/port > > > + description: > > > + A port node to link the PHY to a USB connector's "high-speed" port. > > > > I don't think this is correct. The HS port of the connector goes to the > > controller. The controller has the link to the phy. > > > > If the PHY is also what handles USB-C muxing or orientation switching, > > then it might have ports, but then it needs input and output ports. > > > > Rob > > > > Hi Rob, > > thank you for the quick response. > > I've feared this would be the case, but chose to go ahead with this solution > anyway because I'm not super stoked about the alternatives I can think of. The > problem is that I need to go from the USB PHY node to the USB connector somehow, > but there's no way I can see to get from the PHY node to the USB2 controller > it's connected to, unless I'm missing something obvious. > > So I see two alternatives: > 1. Extend the usb connector binding to add additional ports for PHYs that handle > vbus detection or something, then extend either the inno2 binding or a more > general usb PHY binding to add that port definition. > 2. Revert to what the downstream vendor kernel does and simply add a boolean > flag property to the inno usb2phy binding that tells it whether it's > connected to a USB-C port and should therefore expect vbusdet to remain high. > > Let me know if there's any better alternatives I missed. If there's some OF > driver function to enumerate all controllers a PHY is referenced by, then that > would probably work as well, allowing me to just point the HS port to the > controller instead as intended. The building blocks are there. You can iterate over nodes with 'phys' with for_each_node_with_property(), then on each entry in 'phys' check if it matches your node. Then you need to iterate over the ports to check for connection to usb-c-connector. of_graph_get_remote_port_parent() will help you there. Not terribly efficient, but you're only doing it once. Another option is extend phy modes to distinguish USB-C or not. Then you can set the mode either with the 'phy-mode' property or in phy cells. Though if you have to add a cell, that's an ABI change (not sure if the existing kernel would accept another cell). I rather see the kernel use the information that's already there rather than have 2 sources of information. Rob
diff --git a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml index 6a7ef556414cebad63c10de754778f84fd4486ee..3a662bfc353250a8ad9386ebb5575d1e84c1b5ba 100644 --- a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml +++ b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml @@ -78,6 +78,11 @@ properties: When set the driver will request its phandle as one companion-grf for some special SoCs (e.g rv1108). + port: + $ref: /schemas/graph.yaml#/properties/port + description: + A port node to link the PHY to a USB connector's "high-speed" port. + host-port: type: object additionalProperties: false
USB connectors like to have OF graph connections to high-speed related nodes to do various things. In the case of the RK3576, we can make use of a port in the usb2 PHY to detect whether the OTG controller is connected to a type C port and apply some special behaviour accordingly. The usefulness of having different bits of a fully functioning USB stack point to each other is more general though, and not constrained to RK3576 at all, even for this use-case. Add a port property to the binding. Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com> --- Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml | 5 +++++ 1 file changed, 5 insertions(+)