Thread (8 messages) 8 messages, 3 authors, 2024-08-28

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

From: Krzysztof Kozlowski <krzk@kernel.org>
Date: 2024-08-28 05:43:49
Also in: linux-devicetree, linux-gpio, lkml

On 28/08/2024 02:43, David Leonard wrote:
On Tue, 27 Aug 2024, Krzysztof Kozlowski wrote:
quoted
On 27/08/2024 18:51, David Leonard wrote:
quoted
quoted
quoted
+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.
quoted
quoted
quoted
quoted
+  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

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help