Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 24 Oct 2022 09:54:46 -0500
From:   Rob Herring <robh@kernel.org>
To:     Wayne Chang <waynec@nvidia.com>
Cc:     gregkh@linuxfoundation.org, krzysztof.kozlowski+dt@linaro.org,
        treding@nvidia.com, jonathanh@nvidia.com, thierry.reding@gmail.com,
        heikki.krogerus@linux.intel.com, ajayg@nvidia.com, kishon@ti.com,
        vkoul@kernel.org, p.zabel@pengutronix.de, balbi@kernel.org,
        mathias.nyman@intel.com, jckuo@nvidia.com,
        linux-usb@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, singhanc@nvidia.com,
        linux-i2c@vger.kernel.org, linux-phy@lists.infradead.org,
        linux-tegra@vger.kernel.org
Subject: Re: [PATCH 02/11] dt-bindings: usb: Add NVIDIA Tegra XUSB host
 controller binding
Message-ID: <20221024145446.GA1763588-robh@kernel.org>
References: <20221024074128.1113554-1-waynec@nvidia.com>
 <20221024074128.1113554-3-waynec@nvidia.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221024074128.1113554-3-waynec@nvidia.com>
Precedence: bulk

On Mon, Oct 24, 2022 at 03:41:19PM +0800, Wayne Chang wrote:
> Add device-tree binding documentation for the XUSB host controller present
> on Tegra194 and Tegra234 SoC. This controller supports the USB 3.1
> specification.
> 
> Signed-off-by: Wayne Chang <waynec@nvidia.com>
> ---
>  .../bindings/usb/nvidia,tegra-xhci.yaml       | 213 ++++++++++++++++++
>  1 file changed, 213 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/nvidia,tegra-xhci.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra-xhci.yaml b/Documentation/devicetree/bindings/usb/nvidia,tegra-xhci.yaml
> new file mode 100644
> index 000000000000..d261a419a04f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/nvidia,tegra-xhci.yaml
> @@ -0,0 +1,213 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/usb/nvidia,tegra-xhci.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Device tree binding for NVIDIA Tegra XUSB host controller

Drop 'Device tree binding for '

> +
> +description:
> +  The Tegra XHCI controller supports both USB 2.0 HighSpeed/FullSpeed and
> +  USB 3.1 SuperSpeed protocols.
> +
> +maintainers:
> +  - Wayne Chang <waynec@nvidia.com>
> +

Ref to usb-xhci.yaml? Or usb-hcd.yaml.

> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - nvidia,tegra194-xusb # For Tegra194
> +          - nvidia,tegra234-xusb # For Tegra234

The comment is kind of redundant.

> +
> +  reg:
> +    minItems: 2
> +    items:
> +      - description: XUSB host controller registers
> +      - description: XUSB host PCI Config registers
> +      - description: XUSB host bar2 registers

Drop 'XUSB host '

> +
> +  reg-names:
> +    minItems: 2
> +    items:
> +      - const: hcd
> +      - const: fpci
> +      - const: bar2
> +
> +  interrupts:
> +    items:
> +      - description: Must contain the XUSB host interrupt.
> +      - description: Must contain the XUSB mbox interrupt.

Drop 'Must contain the '

> +
> +  clocks:
> +    items:
> +      - description: Clock to enable core XUSB host clock.
> +      - description: Clock to enable XUSB falcon clock.
> +      - description: Clock to enable XUSB super speed clock.
> +      - description: Clock to enable XUSB super speed dev clock.
> +      - description: Clock to enable XUSB high speed dev clock.
> +      - description: Clock to enable XUSB full speed dev clock.
> +      - description: Clock to enable XUSB UTMI PLL clock.
> +      - description: Clock to enable core XUSB dev clock.
> +      - description: Clock to enable XUSB PLLE clock.

Drop 'Clock to enable '

> +
> +  clock-names:
> +    items:
> +      - const: xusb_host
> +      - const: xusb_falcon_src
> +      - const: xusb_ss
> +      - const: xusb_ss_src
> +      - const: xusb_hs_src
> +      - const: xusb_fs_src
> +      - const: pll_u_480m
> +      - const: clk_m
> +      - const: pll_e
> +
> +  interconnects:
> +    items:
> +      - description: memory read client
> +      - description: memory write client
> +
> +  interconnect-names:
> +    items:
> +      - const: dma-mem # read
> +      - const: write
> +
> +  iommus:
> +    maxItems: 1
> +
> +  power-domains:
> +    items:
> +      - description: XUSBC(host) power-domain
> +      - description: XUSBA(superspeed) power-domain
> +
> +  power-domain-names:
> +    items:
> +      - const: xusb_host
> +      - const: xusb_ss

Drop 'xusb_'.

> +
> +  nvidia,xusb-padctl:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      phandle to the XUSB pad controller that is used to configure the USB pads
> +      used by the XUDC controller.
> +
> +  phys:
> +    minItems: 1
> +    maxItems: 8
> +    description:
> +      Must contain an entry for each entry in phy-names.
> +      See ../phy/phy-bindings.txt for details.

Drop description.

> +
> +  phy-names:
> +    minItems: 1
> +    maxItems: 8
> +    items:
> +      anyOf:
> +        - const: usb2-0
> +        - const: usb2-1
> +        - const: usb2-2
> +        - const: usb2-3
> +        - const: usb3-0
> +        - const: usb3-1
> +        - const: usb3-2
> +        - const: usb3-3
> +
> +  dma-coherent:
> +    type: boolean
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - power-domains
> +  - power-domain-names
> +  - nvidia,xusb-padctl
> +  - phys
> +  - phy-names
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - nvidia,tegra194-xusb
> +    then:
> +      properties:
> +        reg:
> +          minItems: 2
> +        reg-names:
> +          minItems: 2
> +        clocks:
> +          minItems: 9
> +        clock-names:
> +          minItems: 9
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - nvidia,tegra234-xusb
> +    then:
> +      properties:
> +        reg:
> +          minItems: 3
> +        reg-names:
> +          minItems: 3
> +        clocks:
> +          minItems: 9
> +        clock-names:
> +          minItems: 9

Same number of items, why are clocks in the if/then?

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/tegra234-gpio.h>
> +    #include <dt-bindings/clock/tegra234-clock.h>
> +    #include <dt-bindings/memory/tegra234-mc.h>
> +    #include <dt-bindings/power/tegra234-powergate.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    usb@3610000 {
> +      compatible = "nvidia,tegra234-xusb";
> +      reg = <0x03610000 0x40000>,
> +            <0x03600000 0x10000>,
> +            <0x03650000 0x10000>;
> +      reg-names = "hcd", "fpci", "bar2";
> +
> +      interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>,
> +             <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;
> +
> +      clocks = <&bpmp TEGRA234_CLK_XUSB_CORE_HOST>,
> +         <&bpmp TEGRA234_CLK_XUSB_FALCON>,
> +         <&bpmp TEGRA234_CLK_XUSB_CORE_SS>,
> +         <&bpmp TEGRA234_CLK_XUSB_SS>,
> +         <&bpmp TEGRA234_CLK_CLK_M>,
> +         <&bpmp TEGRA234_CLK_XUSB_FS>,
> +         <&bpmp TEGRA234_CLK_UTMIP_PLL>,
> +         <&bpmp TEGRA234_CLK_CLK_M>,
> +         <&bpmp TEGRA234_CLK_PLLE>;
> +      clock-names = "xusb_host", "xusb_falcon_src",
> +              "xusb_ss", "xusb_ss_src", "xusb_hs_src",
> +              "xusb_fs_src", "pll_u_480m", "clk_m",
> +              "pll_e";
> +      interconnects = <&mc TEGRA234_MEMORY_CLIENT_XUSB_HOSTR &emc>,
> +          <&mc TEGRA234_MEMORY_CLIENT_XUSB_HOSTW &emc>;
> +      interconnect-names = "dma-mem", "write";
> +      iommus = <&smmu_niso1 TEGRA234_SID_XUSB_HOST>;
> +
> +      power-domains = <&bpmp TEGRA234_POWER_DOMAIN_XUSBC>,
> +          <&bpmp TEGRA234_POWER_DOMAIN_XUSBA>;
> +      power-domain-names = "xusb_host", "xusb_ss";
> +
> +      nvidia,xusb-padctl = <&xusb_padctl>;
> +
> +      phys =  <&pad_lanes_usb2_0>;
> +      phy-names = "usb2-0";
> +
> +    };
> -- 
> 2.25.1
> 
>