Message ID | 5f81187b-0558-3815-051b-e40685fd047a@ti.com |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | [v2] dt-bindings: usb: ti,keystone-dwc3.yaml: Improve schema | expand |
Context | Check | Description |
---|---|---|
robh/checkpatch | warning | total: 0 errors, 1 warnings, 101 lines checked |
On Fri, Jun 05, 2020 at 10:52:15AM +0300, Roger Quadros wrote: > There were some review comments after the patch was integrated. > Address those. > > Fixes: 1883a934e156 ("dt-bindings: usb: convert keystone-usb.txt to YAML") > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > > Changelog: > v2 > - don't use quotes for enum/const string > - use phandle instead of phandle-array for phys > - add maxItems for phy-names > > .../bindings/usb/ti,keystone-dwc3.yaml | 50 ++++++++++++++----- > 1 file changed, 37 insertions(+), 13 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml > index f127535feb0b..394e47d2f5d7 100644 > --- a/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml > +++ b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml > @@ -11,64 +11,88 @@ maintainers: > properties: > compatible: > - oneOf: > - - const: "ti,keystone-dwc3" > - - const: "ti,am654-dwc3" > + items: > + - enum: > + - ti,keystone-dwc3 > + - ti,am654-dwc3 > reg: > maxItems: 1 > - description: Address and length of the register set for the USB subsystem on > - the SOC. > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 1 > + > + ranges: true blank line > interrupts: > maxItems: 1 > - description: The irq number of this device that is used to interrupt the MPU. > - > clocks: > + $ref: /schemas/types.yaml#definitions/phandle-array Common property. It already has a type and doesn't need to be redefined here. Just "maxItems: 1" if it is a single clock is enough, or an 'items' list for each entry if more than 1. > description: Clock ID for USB functional clock. Drop. > + assigned-clocks: > + $ref: /schemas/types.yaml#definitions/phandle-array > + > + assigned-clock-parents: > + $ref: /schemas/types.yaml#definitions/phandle-array > + > power-domains: > + $ref: /schemas/types.yaml#definitions/phandle-array Same as 'clocks'. > 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 > phys: > + $ref: /schemas/types.yaml#/definitions/phandle Same as 'clocks'. > description: > PHY specifier for the USB3.0 PHY. Some SoCs need the USB3.0 PHY > to be turned on before the controller. > Documentation/devicetree/bindings/phy/phy-bindings.txt > phy-names: > + maxItems: 1 > items: > - - const: "usb3-phy" > + - const: usb3-phy Don't need maxItems as that's implied by the length of 'items'. > + > + dma-coherent: true > - dwc3: > + dma-ranges: true > + > +patternProperties: > + "usb@[a-f0-9]+$": > + type: object > description: This is the node representing the DWC3 controller instance > Documentation/devicetree/bindings/usb/dwc3.txt > required: > - compatible > - reg > + - "#address-cells" > + - "#size-cells" > + - ranges > - interrupts > - - clocks > + > +additionalProperties: false > examples: > - | > #include <dt-bindings/interrupt-controller/arm-gic.h> > - usb: usb@2680000 { > + dwc3@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 { > + usb@2690000 { > compatible = "synopsys,dwc3"; > reg = <0x2690000 0x70000>; > interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>; > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > >
Hi, On 15/06/2020 20:41, Rob Herring wrote: > On Fri, Jun 05, 2020 at 10:52:15AM +0300, Roger Quadros wrote: >> There were some review comments after the patch was integrated. >> Address those. >> >> Fixes: 1883a934e156 ("dt-bindings: usb: convert keystone-usb.txt to YAML") >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> >> Changelog: >> v2 >> - don't use quotes for enum/const string >> - use phandle instead of phandle-array for phys >> - add maxItems for phy-names >> >> .../bindings/usb/ti,keystone-dwc3.yaml | 50 ++++++++++++++----- >> 1 file changed, 37 insertions(+), 13 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml >> index f127535feb0b..394e47d2f5d7 100644 >> --- a/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml >> +++ b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml >> @@ -11,64 +11,88 @@ maintainers: >> properties: >> compatible: >> - oneOf: >> - - const: "ti,keystone-dwc3" >> - - const: "ti,am654-dwc3" >> + items: >> + - enum: >> + - ti,keystone-dwc3 >> + - ti,am654-dwc3 >> reg: >> maxItems: 1 >> - description: Address and length of the register set for the USB subsystem on >> - the SOC. >> + >> + '#address-cells': >> + const: 1 >> + >> + '#size-cells': >> + const: 1 >> + >> + ranges: true > > blank line There is a blank line before interrupts. > >> interrupts: >> maxItems: 1 >> - description: The irq number of this device that is used to interrupt the MPU. >> - >> clocks: >> + $ref: /schemas/types.yaml#definitions/phandle-array > > Common property. It already has a type and doesn't need to be > redefined here. > > Just "maxItems: 1" if it is a single clock is enough, or an 'items' list > for each entry if more than 1. Some nodes need 1 and some need 2. So "minItems: 1" and "maxItems: 2"? The driver doesn't really care, and relies of device/clock framework to just set the right clock parent. In this case do I need to add items list? cheers, -roger > >> description: Clock ID for USB functional clock. > > Drop. > >> + assigned-clocks: >> + $ref: /schemas/types.yaml#definitions/phandle-array > > >> + >> + assigned-clock-parents: >> + $ref: /schemas/types.yaml#definitions/phandle-array >> + >> power-domains: >> + $ref: /schemas/types.yaml#definitions/phandle-array > > Same as 'clocks'. > >> 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 >> phys: >> + $ref: /schemas/types.yaml#/definitions/phandle > > Same as 'clocks'. > >> description: >> PHY specifier for the USB3.0 PHY. Some SoCs need the USB3.0 PHY >> to be turned on before the controller. >> Documentation/devicetree/bindings/phy/phy-bindings.txt >> phy-names: >> + maxItems: 1 >> items: >> - - const: "usb3-phy" >> + - const: usb3-phy > > Don't need maxItems as that's implied by the length of 'items'. > >> + >> + dma-coherent: true >> - dwc3: >> + dma-ranges: true >> + >> +patternProperties: >> + "usb@[a-f0-9]+$": >> + type: object >> description: This is the node representing the DWC3 controller instance >> Documentation/devicetree/bindings/usb/dwc3.txt >> required: >> - compatible >> - reg >> + - "#address-cells" >> + - "#size-cells" >> + - ranges >> - interrupts >> - - clocks >> + >> +additionalProperties: false >> examples: >> - | >> #include <dt-bindings/interrupt-controller/arm-gic.h> >> - usb: usb@2680000 { >> + dwc3@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 { >> + usb@2690000 { >> compatible = "synopsys,dwc3"; >> reg = <0x2690000 0x70000>; >> interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>; >> -- >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. >> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki >> >>
diff --git a/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml index f127535feb0b..394e47d2f5d7 100644 --- a/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml +++ b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml @@ -11,64 +11,88 @@ maintainers: properties: compatible: - oneOf: - - const: "ti,keystone-dwc3" - - const: "ti,am654-dwc3" + items: + - enum: + - ti,keystone-dwc3 + - ti,am654-dwc3 reg: maxItems: 1 - description: Address and length of the register set for the USB subsystem on - the SOC. + + '#address-cells': + const: 1 + + '#size-cells': + const: 1 + + ranges: true interrupts: maxItems: 1 - description: The irq number of this device that is used to interrupt the MPU. - clocks: + $ref: /schemas/types.yaml#definitions/phandle-array description: Clock ID for USB functional clock. + assigned-clocks: + $ref: /schemas/types.yaml#definitions/phandle-array + + assigned-clock-parents: + $ref: /schemas/types.yaml#definitions/phandle-array + power-domains: + $ref: /schemas/types.yaml#definitions/phandle-array 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 phys: + $ref: /schemas/types.yaml#/definitions/phandle description: PHY specifier for the USB3.0 PHY. Some SoCs need the USB3.0 PHY to be turned on before the controller. Documentation/devicetree/bindings/phy/phy-bindings.txt phy-names: + maxItems: 1 items: - - const: "usb3-phy" + - const: usb3-phy + + dma-coherent: true - dwc3: + dma-ranges: true + +patternProperties: + "usb@[a-f0-9]+$": + type: object description: This is the node representing the DWC3 controller instance Documentation/devicetree/bindings/usb/dwc3.txt required: - compatible - reg + - "#address-cells" + - "#size-cells" + - ranges - interrupts - - clocks + +additionalProperties: false examples: - | #include <dt-bindings/interrupt-controller/arm-gic.h> - usb: usb@2680000 { + dwc3@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 { + usb@2690000 { compatible = "synopsys,dwc3"; reg = <0x2690000 0x70000>; interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
There were some review comments after the patch was integrated. Address those. Fixes: 1883a934e156 ("dt-bindings: usb: convert keystone-usb.txt to YAML") Signed-off-by: Roger Quadros <rogerq@ti.com> --- Changelog: v2 - don't use quotes for enum/const string - use phandle instead of phandle-array for phys - add maxItems for phy-names .../bindings/usb/ti,keystone-dwc3.yaml | 50 ++++++++++++++----- 1 file changed, 37 insertions(+), 13 deletions(-)