Re: [PATCH 1/4] dt-bindings: phy: rockchip,inno-usb2phy: add port property

[Rob Herring <robh@kernel.org>]

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