linux-kernel - Re: [PATCH 1/2] dt-bindings: pwm: convert atmel pwm to json-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: <f4d97269-c2c0-fb61-08e3-3b3396db5b1c@microchip.com>
Date:   Mon, 7 Mar 2022 14:05:53 +0000
From:   <Sergiu.Moga@...rochip.com>
To:     <krzysztof.kozlowski@...onical.com>,
        <Claudiu.Beznea@...rochip.com>, <thierry.reding@...il.com>,
        <u.kleine-koenig@...gutronix.de>, <lee.jones@...aro.org>,
        <robh+dt@...nel.org>, <Nicolas.Ferre@...rochip.com>,
        <alexandre.belloni@...tlin.com>
CC:     <linux-arm-kernel@...ts.infradead.org>,
        <linux-pwm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] dt-bindings: pwm: convert atmel pwm to json-schema

On 07.03.2022 15:22, Krzysztof Kozlowski wrote:
> On 07/03/2022 14:03, Sergiu Moga wrote:
>> Convert PWM binding for Atmel/Microchip SoCs to Device Tree Schema
>> format.
>>
>> Signed-off-by: Sergiu Moga <sergiu.moga@...rochip.com>
>> ---
>>   .../devicetree/bindings/pwm/atmel-pwm.txt     | 35 -----------
>>   .../devicetree/bindings/pwm/atmel-pwm.yaml    | 62 +++++++++++++++++++
>>   2 files changed, 62 insertions(+), 35 deletions(-)
>>   delete mode 100644 Documentation/devicetree/bindings/pwm/atmel-pwm.txt
>>   create mode 100644 Documentation/devicetree/bindings/pwm/atmel-pwm.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/pwm/atmel-pwm.txt b/Documentation/devicetree/bindings/pwm/atmel-pwm.txt
>> deleted file mode 100644
>> index fbb5325be1f0..000000000000
>> --- a/Documentation/devicetree/bindings/pwm/atmel-pwm.txt
> I would prefer for new files to switch to common DT conversion:
> vendor,name.yaml
>
> You have there another PWM bindings for Atmel, so the name should not be
> just "pwm" but something more specific, maybe "at91sam-pwm"?
>
>
Understood, yes, it does make more sense like this. Thank you for the 
suggestion, I will rename the file to `atmel,at91sam-pwm.yaml`.

>> +++ /dev/null
>> @@ -1,35 +0,0 @@
>> -Atmel PWM controller
>> -
>> -Required properties:
>> -  - compatible: should be one of:
>> -    - "atmel,at91sam9rl-pwm"
>> -    - "atmel,sama5d3-pwm"
>> -    - "atmel,sama5d2-pwm"
>> -    - "microchip,sam9x60-pwm"
>> -  - reg: physical base address and length of the controller's registers
>> -  - #pwm-cells: Should be 3. See pwm.yaml in this directory for a
>> -    description of the cells format.
>> -
>> -Example:
>> -
>> -     pwm0: pwm@...34000 {
>> -             compatible = "atmel,at91sam9rl-pwm";
>> -             reg = <0xf8034000 0x400>;
>> -             #pwm-cells = <3>;
>> -     };
>> -
>> -     pwmleds {
>> -             compatible = "pwm-leds";
>> -
>> -             d1 {
>> -                     label = "d1";
>> -                     pwms = <&pwm0 3 5000 0>
>> -                     max-brightness = <255>;
>> -             };
>> -
>> -             d2 {
>> -                     label = "d2";
>> -                     pwms = <&pwm0 1 5000 1>
>> -                     max-brightness = <255>;
>> -             };
>> -     };
>> diff --git a/Documentation/devicetree/bindings/pwm/atmel-pwm.yaml b/Documentation/devicetree/bindings/pwm/atmel-pwm.yaml
>> new file mode 100644
>> index 000000000000..f5378a2226bd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pwm/atmel-pwm.yaml
>> @@ -0,0 +1,62 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +# Copyright (C) 2022 Microchip Technology, Inc. and its subsidiaries
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/pwm/atmel-pwm.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Atmel PWM controller
> title: Atmel/Microchip PWM controller
>
>> +
>> +maintainers:
>> +  - Claudiu Beznea <claudiu.beznea@...rochip.com>
>> +
> allOf including pwm.yaml
>
Noted.

>> +properties:
>> +  compatible:
>> +    enum:
>> +      - atmel,at91sam9rl-pwm
>> +      - atmel,sama5d3-pwm
>> +      - atmel,sama5d2-pwm
>> +      - microchip,sam9x60-pwm
>> +
>> +  reg:
>> +    description: |
>> +      Physical base address and length of the controller's
>> +      registers
> Skip description, it's obvious.
>
>> +    maxItems: 1
>> +
>> +  "#pwm-cells":
>> +    description: |
>> +      Should be 3. See pwm.yaml in this directory for a
>> +      description of the cells format.
> Skip description, it's obvious.
>
>> +    const: 3
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - "#pwm-cells"
> Skip pwm-cells, required by pwm.yaml.
>
Understood, will remove it in the next version along with the unnecessary descriptions.

>> +
>> +additionalProperties: true
>> +
>> +examples:
>> +  - |
>> +        pwm0: pwm@...34000 {
>> +                compatible = "atmel,at91sam9rl-pwm";
>> +                reg = <0xf8034000 0x400>;
>> +                #pwm-cells = <3>;
>> +        };
>> +
>> +        pwmleds {
> Skip this node. No actual benefit. All clients are the same.
>
Understood. I will include `pwm0` only.

>> +                compatible = "pwm-leds";
>> +
>> +                led-1 {
>> +                        label = "led-1";
>> +                        pwms = <&pwm0 3 5000 0>;
>> +                        max-brightness = <255>;
>> +                };
>> +
>> +                led-2 {
>> +                        label = "led-2";
>> +                        pwms = <&pwm0 1 5000 1>;
>> +                        max-brightness = <255>;
>> +                };
>> +        };
>
> Best regards,
> Krzysztof

Thank you for the detailed feedback.

Sergiu