| 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
| ||
|
Message-ID: <1aa27ad3-d6a3-6ef1-5e56-36697583988c@ncr.com>
Date: Tue, 7 Feb 2023 09:49:53 +0000
From: "Leonard, Niall" <Niall.Leonard@....com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
Linus Walleij <linus.walleij@...aro.org>,
Bartosz Golaszewski <brgl@...ev.pl>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>
CC: "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] dt-bindings: improve wb,mbl-gpio binding
documentation.
On 01/02/2023 08:27, Krzysztof Kozlowski wrote:
> *External Message* - Use caution before opening links or attachments
>
> On 31/01/2023 14:49, Niall Leonard via B4 Submission Endpoint wrote:
>> From: Niall Leonard <nl250060@....com>
>>
>> Convert existing wd,mbl-gpio binding documentation to YAML and add
>> new optional propery "no-input".
>
> Subject: drop full stop
>
> Use subject prefixes matching the subsystem (which you can get for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching).
>
> Subject: improve is vague. Instead: convert to DT schema
>
>
>>
>> Signed-off-by: Niall Leonard <nl250060@....com>
>> ---
>> .../devicetree/bindings/gpio/wd,mbl-gpio.txt | 38 -----------
>> .../devicetree/bindings/gpio/wd,mbl-gpio.yaml | 78 ++++++++++++++++++++++
>> 2 files changed, 78 insertions(+), 38 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt b/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt
>> deleted file mode 100644
>> index 038c3a6a1f4d..000000000000
>> --- a/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt
>> +++ /dev/null
>> @@ -1,38 +0,0 @@
>> -Bindings for the Western Digital's MyBook Live memory-mapped GPIO controllers.
>> -
>> -The Western Digital MyBook Live has two memory-mapped GPIO controllers.
>> -Both GPIO controller only have a single 8-bit data register, where GPIO
>> -state can be read and/or written.
>> -
>> -Required properties:
>> - - compatible: should be "wd,mbl-gpio"
>> - - reg-names: must contain
>> - "dat" - data register
>> - - reg: address + size pairs describing the GPIO register sets;
>> - order must correspond with the order of entries in reg-names
>> - - #gpio-cells: must be set to 2. The first cell is the pin number and
>> - the second cell is used to specify the gpio polarity:
>> - 0 = active high
>> - 1 = active low
>> - - gpio-controller: Marks the device node as a gpio controller.
>> -
>> -Optional properties:
>> - - no-output: GPIOs are read-only.
>> -
>> -Examples:
>> - gpio0: gpio0@...00000 {
>> - compatible = "wd,mbl-gpio";
>> - reg-names = "dat";
>> - reg = <0xe0000000 0x1>;
>> - #gpio-cells = <2>;
>> - gpio-controller;
>> - };
>> -
>> - gpio1: gpio1@...00000 {
>> - compatible = "wd,mbl-gpio";
>> - reg-names = "dat";
>> - reg = <0xe0100000 0x1>;
>> - #gpio-cells = <2>;
>> - gpio-controller;
>> - no-output;
>> - };
>> diff --git a/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.yaml b/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.yaml
>> new file mode 100644
>> index 000000000000..d1c72a42c5bc
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.yaml
>> @@ -0,0 +1,78 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: https://urldefense.com/v3/__http://devicetree.org/schemas/gpio/wd,mbl-gpio.yaml*__;Iw!!In4Qlw!qBa7N0ccPrNQmjMeU9Hi-Ao3KxY4Fj2fXQutcNmpGxlXF7GLy7qjRk7k2ABk3yN7oi7tio6PoZyY-lsOTTreIaCxp-hI$
>> +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!In4Qlw!qBa7N0ccPrNQmjMeU9Hi-Ao3KxY4Fj2fXQutcNmpGxlXF7GLy7qjRk7k2ABk3yN7oi7tio6PoZyY-lsOTTreIbT2TzBT$
>> +
>> +title: Western Digital's MyBook Live memory-mapped GPIO controllers.
>
> Drop full stop.
>
>> +
>> +maintainers:
>> + - Niall Leonard <nl250060@....com>
>> +
>> +description: |+
>> + Bindings for the Western Digital's MyBook Live memory-mapped GPIO controllers.
>
> Drop "Bindings for". Actually drop entire line - it's redundant, you
> repeat the title.
>
>> +
>> + The Western Digital MyBook Live has two memory-mapped GPIO controllers.
>> + Both GPIO controller only have a single 8-bit data register, where GPIO
>> + state can be read and/or written.
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - wd,mbl-gpio
>> +
>> + reg-names:
>> + items:
>> + - const: dat
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + "#gpio-cells":
>> + const: 2
>> +
>> + gpio-controller: true
>> +
>> + no-output:
>> + description: GPIOs are read-only.
>> +
>> + no-input:
>> + description: GPIOs are write-only.
>
>
> Split adding new property into separate patch. Each patch should do one
> logical change. New feature is another logical change. Conversion is
> that logical change (with any fixes needed for successful conversion,
> but not with new features).
>
>
>> +
>> +required:
>> + - compatible
>> + - reg-names
>> + - reg
>> + - '#gpio-cells'
>
> Use consistent quotes - either ' or "
>
>> + - gpio-controller
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + gpio0: gpio0@...00000 {
>
> gpio@
>
> (0 is not correct name of node)
>
>> + compatible = "wd,mbl-gpio";
>> + reg-names = "dat";
>> + reg = <0xe0000000 0x1>;
>> + #gpio-cells = <2>;
>> + gpio-controller;
>> + };
>> +
>> + gpio1: gpio1@...00000 {
>
> gpio@
>
>> + compatible = "wd,mbl-gpio";
>> + reg-names = "dat";
>> + reg = <0xe0100000 0x1>;
>> + #gpio-cells = <2>;
>> + gpio-controller;
>> + no-output;
>> + };
>> +
>
> Drop all examples below, they are not needed. Actually even these two
> above could be combined as they differ with only one property.
>
> Best regards,
> Krzysztof
>
Thanks for reviewing. I'll make the suggested changes.
You might note that I added myself as maintainer of this file (as it
wouldn't pass the dt checks otherwise) but that can't really be the case
going forwards. I have only converted it from txt to yaml. Who should be
the maintainer ? There was no-one listed as maintainer previously ?
Regards,
Niall.
Powered by blists - more mailing lists