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: <pywfy4ypttq7y2llfkdgkwgpjfvnzk3lcgd67efp2v6qu6f2it@fdgiw5pac7uz>
Date: Tue, 27 Aug 2024 08:17:22 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: David Leonard <David.Leonard@...i.com>
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, 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.

>  .../bindings/pinctrl/fsl,ls1012a-pinctrl.yaml | 83 +++++++++++++++++++
>  1 file changed, 83 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/fsl,ls1012a-pinctrl.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/fsl,ls1012a-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/fsl,ls1012a-pinctrl.yaml
> new file mode 100644
> index 000000000000..599df49b44d4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/fsl,ls1012a-pinctrl.yaml
> @@ -0,0 +1,83 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/fsl,ls1012a-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP QorIQ LS1012A pin multiplexing
> +
> +maintainers:
> +  - David.Leonard@...i.com
> +
> +description: >

Drop >

> +  Bindings for LS1012A pinmux control.

Drop "Bindings for" and explain the hardware.

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

Instead list and describe the items.

> +
> +  big-endian:
> +    description: If present, the PMUXCR0 register is implemented in big-endian.

Why is this here? Either it is or it is not?

> +    type: boolean
> +
> +  dcfg-regmap:

Missing vendor prefix.

> +    $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.

> +
> +patternProperties:
> +  '^pinctrl-':

Rather -pins$ or ^pins-

> +    type: object
> +    $ref: pinmux-node.yaml#
> +    unevaluatedProperties: false
> +
> +    properties:
> +      function:
> +        enum: [ i2c, spi, gpio, gpio_reset ]
> +
> +      groups:
> +        items:
> +          enum: [ qspi_1_grp, qspi_2_grp, qspi_3_grp ]
> +
> +allOf:
> +  - $ref: pinctrl.yaml#

Thies goes after required.

> +
> +required:
> +  - compatible
> +  - reg
> +
> +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.

Best regards,
Krzysztof