Re: [PATCH 1/3] dt-bindings: usb: Add binding for TI USB8041 hub controller

[Matthias Kaehlcke <mka@chromium.org>]

On Tue, Jul 12, 2022 at 11:16:21PM +0200, Krzysztof Kozlowski wrote:
> On 12/07/2022 17:06, Alexander Stein wrote:
> > The TI USB8041 is a USB 3.0 hub controller with 4 ports.
> > 
> > This initial version of the binding only describes USB related aspects
> > of the USB8041, it does not cover the option of connecting the controller
> > as an i2c slave.
> > 
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > ---
> > Well, this is essentially a ripoff of
> > Documentation/devicetree/bindings/usb/realtek,rts5411.yaml with USB IDs
> > replaced, reset-gpio added and example adjusted.
> > IMHO this should be merged together with realtek,rts5411.yaml. Is it ok
> > to rename bindings files? I guess a common onboard-usb-hub.yaml matching
> > the driver seens reasonable. Any recommendations how to proceed?
> > 
> >  .../devicetree/bindings/usb/ti,usb8041.yaml   | 69 +++++++++++++++++++
> >  1 file changed, 69 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/usb/ti,usb8041.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/ti,usb8041.yaml b/Documentation/devicetree/bindings/usb/ti,usb8041.yaml
> > new file mode 100644
> > index 000000000000..9a49d60527b1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/ti,usb8041.yaml
> > @@ -0,0 +1,69 @@
> > +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/usb/ti,usb8041.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Binding for the TI USB8041 USB 3.0 hub controller
> > +
> > +maintainers:
> > +  - Matthias Kaehlcke <mka@chromium.org>
> > +
> > +allOf:
> > +  - $ref: usb-device.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    items:
> 
> No items. It's just one item.
> 
> > +      - enum:
> > +          - usb451,8140
> > +          - usb451,8142
> > +
> > +  reg: true
> > +
> > +  reset-gpio:
> 
> reset-gpios
> 
> > +    maxItems: 1
> > +    description:
> > +      GPIO specifier for GSRT# pin.
> 
> Combine maxItems and above into:
> items:
>  - description: GPIO specifier for GSRT# pin.
> 
> > +
> > +  vdd-supply:
> > +    description:
> > +      phandle to the regulator that provides power to the hub.
> 
> s/phandle to the regulator that provides//
> and create some nice sentence from left-over, like "VDD power supply to
> the hub"
> 
> 
> > +
> > +  peer-hub:
> > +    $ref: '/schemas/types.yaml#/definitions/phandle'
> 
> No quotes.
> 
> > +    description:
> > +      phandle to the peer hub on the controller.
> > +
> > +required:
> > +  - peer-hub
> > +  - compatible
> > +  - reg
> 
> Messed order. Use same as they appear in properties, so
> compatible+reg+peer-hub.
> 
> But another question - why "peer-hub"? I remember some discussion about
> naming, so was peer preferred over companion?

Yes, Alan Stern pointed out that 'companion' can be confusing in the context
of USB:

  What do you mean by "companion hub"?  I think you are using the wrong
  word here.  If you're talking about the relation between the two logical
  hubs (one attached to the SuperSpeed bus and one attached to the
  Low/Full/High-speed bus) within a physical USB-3 hub, the correct term
  for this is "peer".  See the existing usages in hub.h, hub.c, and
  port.c.

  "Companion" refers to something completely different (i.e., the UHCI or
  OHCI controllers that handle Low/Full-speed connections on behalf of a
  High-speed EHCI controller).

https://patchwork.kernel.org/comment/24912563/