Re: [PATCH 1/3] dt-bindings: usb: convert keystone-usb.txt to YAML

[Roger Quadros]

Rob,

Thanks for the review. Since this patch was already picked up I will
address the issues in a follow up patch.

cheers,
-roger

On 27/05/2020 04:37, Rob Herring wrote:
> On Wed, May 13, 2020 at 04:07:07PM +0300, Roger Quadros wrote:
> > Convert keystone-usb documentation to YAML format.
> >
> > Signed-off-by: Roger Quadros <rogerq@xxxxxx>
> > ---
> >   .../devicetree/bindings/usb/keystone-usb.txt  | 56 ----------------
> >   .../bindings/usb/ti,keystone-dwc3.yaml        | 67 +++++++++++++++++++
> >   2 files changed, 67 insertions(+), 56 deletions(-)
> >   delete mode 100644 Documentation/devicetree/bindings/usb/keystone-usb.txt
> >   create mode 100644 Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/usb/keystone-usb.txt b/Documentation/devicetree/bindings/usb/keystone-usb.txt
> > deleted file mode 100644
> > index 77df82e36138..000000000000
> > --- a/Documentation/devicetree/bindings/usb/keystone-usb.txt
> > +++ /dev/null
> > @@ -1,56 +0,0 @@
> > -TI Keystone Soc USB Controller
> > -
> > -DWC3 GLUE
> > -
> > -Required properties:
> > - - compatible: should be
> > -		"ti,keystone-dwc3" for Keystone 2 SoCs
> > -		"ti,am654-dwc3" for AM654 SoC
> > - - #address-cells, #size-cells : should be '1' if the device has sub-nodes
> > -   with 'reg' property.
> > - - reg : Address and length of the register set for the USB subsystem on
> > -   the SOC.
> > - - interrupts : The irq number of this device that is used to interrupt the
> > -   MPU.
> > - - ranges: allows valid 1:1 translation between child's address space and
> > -   parent's address space.
> > -
> > -SoC-specific Required Properties:
> > -The following are mandatory properties for Keystone 2 66AK2HK, 66AK2L and 66AK2E
> > -SoCs only:
> > -
> > -- clocks:		Clock ID for USB functional clock.
> > -- clock-names:		Must be "usb".
> > -
> > -
> > -The following are mandatory properties for 66AK2G and AM654:
> > -
> > -- power-domains:	Should contain a phandle to a PM domain provider node
> > -			and an args specifier containing the USB device id
> > -			value. This property is as per the binding,
> > -			Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
> > -
> > -Sub-nodes:
> > -The dwc3 core should be added as subnode to Keystone DWC3 glue.
> > -- dwc3 :
> > -   The binding details of dwc3 can be found in:
> > -   Documentation/devicetree/bindings/usb/dwc3.txt
> > -
> > -Example:
> > -	usb: usb@2680000 {
> > -		compatible = "ti,keystone-dwc3";
> > -		#address-cells = <1>;
> > -		#size-cells = <1>;
> > -		reg = <0x2680000 0x10000>;
> > -		clocks = <&clkusb>;
> > -		clock-names = "usb";
> > -		interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
> > -		ranges;
> > -
> > -		dwc3@2690000 {
> > -			compatible = "synopsys,dwc3";
> > -			reg = <0x2690000 0x70000>;
> > -			interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
> > -			usb-phy = <&usb_phy>, <&usb_phy>;
> > -		};
> > -	};
> > diff --git a/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
> > new file mode 100644
> > index 000000000000..14d2fe329b93
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
> > @@ -0,0 +1,67 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/usb/ti,keystone-dwc3.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: TI Keystone Soc USB Controller
> > +
> > +maintainers:
> > +  - Roger Quadros <rogerq@xxxxxx>
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - const: "ti,keystone-dwc3"
> > +      - const: "ti,am654-dwc3"
>
> Use enum rather than oneOf+const.
>
> > +
> > +  reg:
> > +    maxItems: 1
> > +    description: Address and length of the register set for the USB subsystem on
> > +      the SOC.
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +    description: The irq number of this device that is used to interrupt the MPU.
>
> No need for genericish descriptions when a single item.
>
> > +
> > +
> > +  clocks:
> > +    description: Clock ID for USB functional clock.
>
> How many?
>
> > +
> > +  power-domains:
> > +    description: Should contain a phandle to a PM domain provider node
> > +      and an args specifier containing the USB device id
> > +      value. This property is as per the binding,
> > +      Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>
> How many?
>
> > +
> > +  dwc3:
>
> This doesn't work because there's a unit address. You need a pattern.
>
> > +    description: This is the node representing the DWC3 controller instance
> > +      Documentation/devicetree/bindings/usb/dwc3.txt
>
> type: object
>
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
>
> additionalProperties: false
>
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    usb: usb@2680000 {
> > +      compatible = "ti,keystone-dwc3";
> > +      #address-cells = <1>;
> > +      #size-cells = <1>;
>
> These have to be documented.
>
> > +      reg = <0x2680000 0x10000>;
> > +      clocks = <&clkusb>;
> > +      clock-names = "usb";
> > +      interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
> > +      ranges;
>
> This too.
>
> > +
> > +      dwc3@2690000 {
> > +        compatible = "synopsys,dwc3";
> > +        reg = <0x2690000 0x70000>;
> > +        interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
> > +        usb-phy = <&usb_phy>, <&usb_phy>;
> > +      };
> > +    };
> > --
> > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki