* [PATCH 3/6] dt-bindings: pinctrl: Add fsl,ls1012a-pinctrl yaml file
@ 2024-08-27 2:10 David Leonard
2024-08-27 3:22 ` Rob Herring (Arm)
2024-08-27 6:17 ` Krzysztof Kozlowski
0 siblings, 2 replies; 8+ messages in thread
From: David Leonard @ 2024-08-27 2:10 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Dong Aisheng, Fabio Estevam, Shawn Guo, Jacky Bai,
Pengutronix Kernel Team, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, David.Leonard, linux-gpio,
devicetree, linux-kernel
Add a binding schema and examples for the LS1012A's pinctrl function.
Signed-off-by: David Leonard <David.Leonard@digi.com>
---
.../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@digi.com
+
+description: >
+ Bindings for LS1012A pinmux control.
+
+properties:
+ compatible:
+ const: fsl,ls1012a-pinctrl
+
+ reg:
+ description: Specifies the base address of the PMUXCR0 register.
+ maxItems: 2
+
+ big-endian:
+ description: If present, the PMUXCR0 register is implemented in big-endian.
+ type: boolean
+
+ dcfg-regmap:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ The phandle of the syscon node for the DCFG registers.
+
+patternProperties:
+ '^pinctrl-':
+ 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#
+
+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 {
+ compatible = "fsl,ls1021a-qspi";
+ reg = <0 0x1550000 0 0x10000>;
+ /* QSPI pins for buswidth 2 */
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_qspi_2>;
+ status = "okay";
+ };
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/6] dt-bindings: pinctrl: Add fsl,ls1012a-pinctrl yaml file
2024-08-27 2:10 [PATCH 3/6] dt-bindings: pinctrl: Add fsl,ls1012a-pinctrl yaml file David Leonard
@ 2024-08-27 3:22 ` Rob Herring (Arm)
2024-08-27 6:17 ` Krzysztof Kozlowski
1 sibling, 0 replies; 8+ messages in thread
From: Rob Herring (Arm) @ 2024-08-27 3:22 UTC (permalink / raw)
To: David Leonard
Cc: Pengutronix Kernel Team, devicetree, Jacky Bai, linux-kernel,
Conor Dooley, Shawn Guo, linux-gpio, Krzysztof Kozlowski,
Dong Aisheng, Linus Walleij, linux-arm-kernel, Fabio Estevam
On Tue, 27 Aug 2024 12:10:44 +1000, David Leonard wrote:
>
> Add a binding schema and examples for the LS1012A's pinctrl function.
>
> Signed-off-by: David Leonard <David.Leonard@digi.com>
> ---
> .../bindings/pinctrl/fsl,ls1012a-pinctrl.yaml | 83 +++++++++++++++++++
> 1 file changed, 83 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/fsl,ls1012a-pinctrl.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/fsl,ls1012a-pinctrl.example.dtb: quadspi@1550000: $nodename:0: 'quadspi@1550000' does not match '^spi(@.*|-([0-9]|[1-9][0-9]+))?$'
from schema $id: http://devicetree.org/schemas/spi/fsl,spi-fsl-qspi.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/fsl,ls1012a-pinctrl.example.dtb: quadspi@1550000: 'reg-names' is a required property
from schema $id: http://devicetree.org/schemas/spi/fsl,spi-fsl-qspi.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/fsl,ls1012a-pinctrl.example.dtb: quadspi@1550000: 'clocks' is a required property
from schema $id: http://devicetree.org/schemas/spi/fsl,spi-fsl-qspi.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/fsl,ls1012a-pinctrl.example.dtb: quadspi@1550000: 'clock-names' is a required property
from schema $id: http://devicetree.org/schemas/spi/fsl,spi-fsl-qspi.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/fsl,ls1012a-pinctrl.example.dtb: quadspi@1550000: 'oneOf' conditional failed, one must be fixed:
'interrupts' is a required property
'interrupts-extended' is a required property
from schema $id: http://devicetree.org/schemas/spi/fsl,spi-fsl-qspi.yaml#
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/a5c1eef7-372d-082b-066e-ecd5e001d1cf@digi.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/6] dt-bindings: pinctrl: Add fsl,ls1012a-pinctrl yaml file
2024-08-27 2:10 [PATCH 3/6] dt-bindings: pinctrl: Add fsl,ls1012a-pinctrl yaml file David Leonard
2024-08-27 3:22 ` Rob Herring (Arm)
@ 2024-08-27 6:17 ` Krzysztof Kozlowski
2024-08-27 16:51 ` David Leonard
1 sibling, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2024-08-27 6:17 UTC (permalink / raw)
To: David Leonard
Cc: linux-arm-kernel, Dong Aisheng, Fabio Estevam, Shawn Guo,
Jacky Bai, Pengutronix Kernel Team, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-gpio, devicetree,
linux-kernel
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@digi.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@digi.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@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/6] dt-bindings: pinctrl: Add fsl,ls1012a-pinctrl yaml file
2024-08-27 6:17 ` Krzysztof Kozlowski
@ 2024-08-27 16:51 ` David Leonard
2024-08-27 17:28 ` Krzysztof Kozlowski
0 siblings, 1 reply; 8+ messages in thread
From: David Leonard @ 2024-08-27 16:51 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-arm-kernel, Dong Aisheng, Fabio Estevam, Shawn Guo,
Jacky Bai, Pengutronix Kernel Team, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-gpio, devicetree,
linux-kernel
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@digi.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@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.
Dropped the qspi example. Examples are now
examples:
- |
pinctrl: pinctrl@1570430 {
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/6] dt-bindings: pinctrl: Add fsl,ls1012a-pinctrl yaml file
2024-08-27 16:51 ` David Leonard
@ 2024-08-27 17:28 ` Krzysztof Kozlowski
2024-08-28 0:43 ` David Leonard
0 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2024-08-27 17:28 UTC (permalink / raw)
To: David Leonard
Cc: linux-arm-kernel, Dong Aisheng, Fabio Estevam, Shawn Guo,
Jacky Bai, Pengutronix Kernel Team, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-gpio, devicetree,
linux-kernel
On 27/08/2024 18:51, David Leonard wrote:
>>> +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?
Almost, second reg is not a size. You claim there are two IO address
spaces. Each address space contains base address and size. Look at other
bindings how they do it.
>
>>> +
>>> + 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)
OK, but I still wonder why is it here. Without it the hardware will work
in little-endian?
>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/6] dt-bindings: pinctrl: Add fsl,ls1012a-pinctrl yaml file
2024-08-27 17:28 ` Krzysztof Kozlowski
@ 2024-08-28 0:43 ` David Leonard
2024-08-28 2:51 ` David Leonard
2024-08-28 5:43 ` Krzysztof Kozlowski
0 siblings, 2 replies; 8+ messages in thread
From: David Leonard @ 2024-08-28 0:43 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-arm-kernel, Dong Aisheng, Fabio Estevam, Shawn Guo,
Jacky Bai, Pengutronix Kernel Team, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-gpio, devicetree,
linux-kernel
On Tue, 27 Aug 2024, Krzysztof Kozlowski wrote:
> On 27/08/2024 18:51, David Leonard wrote:
>>>> +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?
>
> Almost, second reg is not a size. You claim there are two IO address
> spaces. Each address space contains base address and size. Look at other
> bindings how they do it.
Previously, dt_binding_check was giving me a warning that 'maxItems' needed
to be supplied. Which confused me because I thought of reg as an opaque subspace
descriptor, but I was just trying to placate the checks. After tool upgrades,
that warning doesn't appear any more.
So the neatest documentation would be:
reg:
description: Specifies the address of the PMUXCR0 register.
>>>> + 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)
>
> OK, but I still wonder why is it here. Without it the hardware will work
> in little-endian?
Well, it's here firstly because I was trying to follow a perceived convention in
dts/freescale/fsl,ls1012a.dtsi. That DT uses big-endian in child
nodes of /soc that match up with memory map tables from the datasheet.
(Not only do different and adjacent parts of the register map have
opposing endianess, some register layouts also seem to be reversed
bitwise, others bytewise.)
The second reason is practical/dodgy. The pinctrl driver should logically
be a child of the scfg node, but the syscon driver doesn't populate its
child nodes. To get the pinctrl driver to work meant making it a sibling
node with an unsatisfactory overlap with the scfg's address region
0x1570000+0x10000. (No driver binds to "fsl,ls1012a-scfg".)
soc: soc {
compatible = "simple-bus";
#address-cells = <2>;
#size-cells = <2>;
...
scfg: scfg@1570000 {
compatible = "fsl,ls1012a-scfg", "syscon";
reg = <0x0 0x1570000 0x0 0x10000>;
big-endian;
};
pinmux: pinmux@157040c {
compatible = "fsl,ls1046a-pinctrl";
reg = <0 0x157040c 0 4>;
big-endian;
...
};
};
The better device tree would be:
soc: soc {
compatible = "simple-bus";
...
scfg: scfg@1570000 {
compatible = "fsl,ls1046a-scfg", "syscon";
big-endian;
#address-cells = <1>;
#size-cells = <1>;
...
pinmux: pinmux@40c {
compatible = "fsl,ls1046a-pinctrl";
reg = <0x40c 4>;
...
};
};
};
And this would resolve the big-endian property issue.
There was a discussion of syscon populating its child nodes at
https://lore.kernel.org/lkml/1403513950.4136.34.camel@paszta.hi.pengutronix.de/T/
Cheers,
David
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/6] dt-bindings: pinctrl: Add fsl,ls1012a-pinctrl yaml file
2024-08-28 0:43 ` David Leonard
@ 2024-08-28 2:51 ` David Leonard
2024-08-28 5:43 ` Krzysztof Kozlowski
1 sibling, 0 replies; 8+ messages in thread
From: David Leonard @ 2024-08-28 2:51 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-arm-kernel, Dong Aisheng, Fabio Estevam, Shawn Guo,
Jacky Bai, Pengutronix Kernel Team, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-gpio, devicetree,
linux-kernel
On Wed, 28 Aug 2024, David Leonard wrote:
>> OK, but I still wonder why is it here. Without it the hardware will work
>> in little-endian?
>
> Well, it's here firstly because I was trying to follow a perceived convention
> in
> dts/freescale/fsl,ls1012a.dtsi. That DT uses big-endian in child
> nodes of /soc that match up with memory map tables from the datasheet.
> (Not only do different and adjacent parts of the register map have
> opposing endianess, some register layouts also seem to be reversed
> bitwise, others bytewise.)
>
> The second reason is practical/dodgy. The pinctrl driver should logically
> be a child of the scfg node, but the syscon driver doesn't populate its
> child nodes. To get the pinctrl driver to work meant making it a sibling
> node with an unsatisfactory overlap with the scfg's address region
> 0x1570000+0x10000. (No driver binds to "fsl,ls1012a-scfg".)
Sorry, my examples had mixed up some ls1046a with ls1012a, which
must be confusing. Corrections follow, should the meaning have been lost:
soc {
compatible = "simple-bus";
#address-cells = <2>;
#size-cells = <2>;
...
scfg: scfg@1570000 {
compatible = "fsl,ls1012a-scfg", "syscon";
reg = <0x0 0x1570000 0x0 0x10000>;
big-endian;
};
pinctrl: pinctrl@1570430 {
compatible = "fsl,ls1012a-pinctrl";
reg = <0x0 0x1570430 0x0 0x4>; /* SCFG PMUXCR0 */
big-endian;
fsl,dcfg-regmap = <&dcfg>;
...
};
};
> The better device tree would be:
soc {
compatible = "simple-bus";
#address-cells = <2>;
#size-cells = <2>;
...
scfg: scfg@1570000 {
compatible = "fsl,ls1012a-scfg", "syscon";
reg = <0x0 0x1570000 0x0 0x10000>;
big-endian;
#address-cells = <1>;
#size-cells = <1>;
...
pinctrl: pinctrl@430 {
compatible = "fsl,ls1012a-pinctrl";
reg = <0x430 0x4>; /* SCFG PMUXCR0 */
fsl,dcfg-regmap = <&dcfg (416/8)>;
...
};
};
};
> And this would resolve the big-endian property issue.
>
> There was a discussion of syscon populating its child nodes at
> https://lore.kernel.org/lkml/1403513950.4136.34.camel@paszta.hi.pengutronix.de/T/
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/6] dt-bindings: pinctrl: Add fsl,ls1012a-pinctrl yaml file
2024-08-28 0:43 ` David Leonard
2024-08-28 2:51 ` David Leonard
@ 2024-08-28 5:43 ` Krzysztof Kozlowski
1 sibling, 0 replies; 8+ messages in thread
From: Krzysztof Kozlowski @ 2024-08-28 5:43 UTC (permalink / raw)
To: David Leonard
Cc: linux-arm-kernel, Dong Aisheng, Fabio Estevam, Shawn Guo,
Jacky Bai, Pengutronix Kernel Team, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-gpio, devicetree,
linux-kernel
On 28/08/2024 02:43, David Leonard wrote:
> On Tue, 27 Aug 2024, Krzysztof Kozlowski wrote:
>
>> On 27/08/2024 18:51, David Leonard wrote:
>>>>> +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?
>>
>> Almost, second reg is not a size. You claim there are two IO address
>> spaces. Each address space contains base address and size. Look at other
>> bindings how they do it.
>
> Previously, dt_binding_check was giving me a warning that 'maxItems' needed
> to be supplied. Which confused me because I thought of reg as an opaque subspace
No.
> descriptor, but I was just trying to placate the checks. After tool upgrades,
> that warning doesn't appear any more.
>
> So the neatest documentation would be:
>
> reg:
> description: Specifies the address of the PMUXCR0 register.
No. Please go through example-schema. Or any other binding... If you
find any binding with above syntax, let me know.
Instead: maxItems: 1.
>
>>>>> + 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)
>>
>> OK, but I still wonder why is it here. Without it the hardware will work
>> in little-endian?
>
> Well, it's here firstly because I was trying to follow a perceived convention in
> dts/freescale/fsl,ls1012a.dtsi. That DT uses big-endian in child
I am confused. Are you adding new device or converting existing bindings?
> nodes of /soc that match up with memory map tables from the datasheet.
> (Not only do different and adjacent parts of the register map have
> opposing endianess, some register layouts also seem to be reversed
> bitwise, others bytewise.)
>
> The second reason is practical/dodgy. The pinctrl driver should logically
> be a child of the scfg node, but the syscon driver doesn't populate its
> child nodes. To get the pinctrl driver to work meant making it a sibling
So fix driver...
> node with an unsatisfactory overlap with the scfg's address region
> 0x1570000+0x10000. (No driver binds to "fsl,ls1012a-scfg".)
>
Several big-endian properties were added unnecessarily and are now being
removed. You cannot provide me answer whether this hardware is
little/big endian, so it also feels unnecessary.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-08-28 5:43 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-27 2:10 [PATCH 3/6] dt-bindings: pinctrl: Add fsl,ls1012a-pinctrl yaml file David Leonard
2024-08-27 3:22 ` Rob Herring (Arm)
2024-08-27 6:17 ` Krzysztof Kozlowski
2024-08-27 16:51 ` David Leonard
2024-08-27 17:28 ` Krzysztof Kozlowski
2024-08-28 0:43 ` David Leonard
2024-08-28 2:51 ` David Leonard
2024-08-28 5:43 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).