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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b5c70885-a307-40c0-8d5c-3ee5260b7a81@kernel.org>
Date: Wed, 28 Aug 2024 07:43:42 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: David Leonard <David.Leonard@...i.com>
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 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