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

[Roger Quadros <rogerq@ti.com>]

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@ti.com>
>> ---
>>   .../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@ti.com>
>> +
>> +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