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

Krzysztof Kozlowski
2022-07-12T21:16:28Z

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?

> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + usb {
> + dr_mode = "host";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + /* 2.0 hub on port 1 */
> + hub_2_0: hub@1 {
> + compatible = "usb451,8142";
> + reg = <1>;
> + peer-hub = <&hub_3_0>;
> + reset-gpio = <&gpio1 11 GPIO_ACTIVE_LOW>;

reset-gpios

Best regards,
Krzysztof