[PATCH 3/6] dt-bindings: pinctrl: Add fsl,ls1012a-pinctrl yaml file
Krzysztof Kozlowski
krzk at kernel.org
Tue Aug 27 22:43:42 PDT 2024
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
More information about the linux-arm-kernel
mailing list