linux-kernel - Re: [PATCH 3/6] dt-bindings: pinctrl: Add fsl,ls1012a-pinctrl yaml file

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <f682476b-f7af-0d66-7105-1d064f5f1739@digi.com>
Date: Wed, 28 Aug 2024 02:51:19 +1000 (AEST)
From: David Leonard <David.Leonard@...i.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
cc: linux-arm-kernel@...ts.infradead.org, Dong Aisheng <aisheng.dong@....com>, 
    Fabio Estevam <festevam@...il.com>, Shawn Guo <shawnguo@...nel.org>, 
    Jacky Bai <ping.bai@....com>, 
    Pengutronix Kernel Team <kernel@...gutronix.de>, 
    Linus Walleij <linus.walleij@...aro.org>, Rob Herring <robh@...nel.org>, 
    Krzysztof Kozlowski <krzk+dt@...nel.org>, 
    Conor Dooley <conor+dt@...nel.org>, linux-gpio@...r.kernel.org, 
    devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/6] dt-bindings: pinctrl: Add fsl,ls1012a-pinctrl yaml
 file

On Tue, 27 Aug 2024, Krzysztof Kozlowski wrote:

> On Tue, Aug 27, 2024 at 12:10:44PM +1000, David Leonard wrote:
>>
>> Add a binding schema and examples for the LS1012A's pinctrl function.
>>
>> Signed-off-by: David Leonard <David.Leonard@...i.com>
>> ---
>
> It does not look like you tested the bindings, at least after quick
> look. Please run  (see
> Documentation/devicetree/bindings/writing-schema.rst for instructions).
> Maybe you need to update your dtschema and yamllint.

I've since updated tools (dt-valdate 2024.5 and yamllint 1.33.0) and
rebased onto v6.11-rc1.

>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pinctrl/fsl,ls1012a-pinctrl.yaml
>> @@ -0,0 +1,83 @@

>> +description: >
>
> Drop >
>
>> +  Bindings for LS1012A pinmux control.
>
> Drop "Bindings for" and explain the hardware.

Changed to

  description:
    The LS1012A pinmux controller can override the RCW and
    alter some pin groups' functions in a limited way.

>> +
>> +properties:
>> +  compatible:
>> +    const: fsl,ls1012a-pinctrl
>> +
>> +  reg:
>> +    description: Specifies the base address of the PMUXCR0 register.
>> +    maxItems: 2
>
> Instead list and describe the items.

Changed to

    reg:
      items:
        - description: Physical base address of the PMUXCR0 register.
        - description: Size of the PMUXCR0 register (4).

Is this what you meant?

>> +
>> +  big-endian:
>> +    description: If present, the PMUXCR0 register is implemented in big-endian.
>
> Why is this here? Either it is or it is not?

You're right. Changed to

    big-endian: true

(This also lead to some code simplification)

>> +    type: boolean
>> +
>> +  dcfg-regmap:
>
> Missing vendor prefix.

Will use "fsl,"

>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +    description:
>> +      The phandle of the syscon node for the DCFG registers.
>
> Instead explain what it is needed it for, how is it used.

Changed to

    fsl,dcfg-regmap:
      $ref: /schemas/types.yaml#/definitions/phandle
      description:
        The phandle of the syscon node for the DCFG registers
        providing the RCW's pin configuration that is in effect when
        the pinmux controller is disabled.

>> +
>> +patternProperties:
>> +  '^pinctrl-':
>
> Rather -pins$ or ^pins-

Changed to ^pins-  See example at the end.

>> +allOf:
>> +  - $ref: pinctrl.yaml#
>
> Thies goes after required.
>
>> +
>> +required:
>> +  - compatible
>> +  - reg

Relocated

>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    pinctrl: pinctrl@...0430 {
>> +        compatible = "fsl,ls1012a-pinctrl";
>> +        reg = <0x0 0x1570430 0x0 0x4>;
>> +        big-endian;
>> +        dcfg-regmap = <&dcfg>;
>> +        pinctrl_qspi_1: pinctrl-qspi-1 {
>> +            groups = "qspi_1_grp";
>> +            function = "spi";
>> +        };
>> +        pinctrl_qspi_2: pinctrl-qspi-2 {
>> +            groups = "qspi_1_grp", "qspi_2_grp";
>> +            function = "spi";
>> +        };
>> +        pinctrl_qspi_4: pinctrl-qspi-4 {
>> +            groups = "qspi_1_grp", "qspi_2_grp", "qspi_3_grp";
>> +            function = "spi";
>> +        };
>> +    };
>> +  - |
>> +    qspi: quadspi@...0000 {
>
> Drop, useless and not related.

Dropped the qspi example. Examples are now

   examples:
     - |
       pinctrl: pinctrl@...0430 {
 	  compatible = "fsl,ls1012a-pinctrl";
 	  reg = <0x0 0x1570430 0x0 0x4>;
 	  big-endian;
 	  fsl,dcfg-regmap = <&dcfg>;
 	  pinctrl_qspi_1: pins-qspi-1 { /* buswidth 1 */
 	      groups = "qspi_1_grp";
 	      function = "spi";
 	  };
 	  pinctrl_qspi_2: pins-qspi-2 { /* buswidth 2 */
 	      groups = "qspi_1_grp", "qspi_2_grp";
 	      function = "spi";
 	  };
 	  pinctrl_qspi_4: pins-qspi-4 { /* buswidth 4 */
 	      groups = "qspi_1_grp", "qspi_2_grp", "qspi_3_grp";
 	      function = "spi";
 	  };
       };


Thanks,

David