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

[Krzysztof Kozlowski <krzk@xxxxxxxxxx>]

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@xxxxxxxx>
> ---

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@xxxxxxxx
> +
> +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@1570430 {
> +        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@1550000 {

Drop, useless and not related.

Best regards,
Krzysztof