linux-kernel - Re: [PATCH v2] dt-bindings: usb: ti,keystone-dwc3.yaml: Improve schema

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b7dc1ee0-36a8-259f-391e-61c256623301@ti.com>
Date:   Wed, 24 Jun 2020 13:26:48 +0300
From:   Roger Quadros <rogerq@...com>
To:     Rob Herring <robh@...nel.org>
CC:     <balbi@...nel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] dt-bindings: usb: ti,keystone-dwc3.yaml: Improve
 schema

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@...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@...0000 {
>> +    dwc3@...0000 {
>>         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@...0000 {
>> +      usb@...0000 {
>>           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
>>
>>

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