[<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
Powered by blists - more mailing lists