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@xxxxxxxxxxxxxxxxxxxxxxxx/T/
Cheers,
David
