Re: [PATCH 3/3] arm64: tegra: Add Tegra234 pinmux device

[Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>]

On 08/02/2023 12:00, Thierry Reding wrote:
> On Tue, Feb 07, 2023 at 04:33:42PM +0100, Krzysztof Kozlowski wrote:
>> On 07/02/2023 12:56, Prathamesh Shete wrote:
>>> This change adds pinmux node for Tegra234.
>>>
>>> Signed-off-by: Prathamesh Shete <pshete@xxxxxxxxxx>
>>> ---
>>>  arch/arm64/boot/dts/nvidia/tegra234.dtsi | 14 ++++++++++++++
>>>  1 file changed, 14 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
>>> index eaf05ee9acd1..c91b88bc56d1 100644
>>> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
>>> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
>>> @@ -701,6 +701,13 @@
>>>  			interrupt-controller;
>>>  			#gpio-cells = <2>;
>>>  			gpio-controller;
>>> +			gpio-ranges = <&pinmux 0 0 164>;
>>> +		};
>>> +
>>> +		pinmux: pinmux@2430000 {
>>> +			compatible = "nvidia,tegra234-pinmux";
>>> +			reg = <0x2430000 0x19100>;
>>> +			status = "okay";
>>
>> Why? Anything disabled it?
>>
>>>  		};
>>>  
>>>  		mc: memory-controller@2c00000 {
>>> @@ -1664,6 +1671,13 @@
>>>  			interrupt-controller;
>>>  			#gpio-cells = <2>;
>>>  			gpio-controller;
>>> +			gpio-range = <&pinmux_aon 0 0 32>;
>>> +		};
>>> +
>>> +		pinmux_aon: pinmux@c300000 {
>>> +			compatible = "nvidia,tegra234-pinmux-aon";
>>> +			reg = <0xc300000 0x4000>;
>>> +			status = "okay";
>>
>> Also why?
> 
> These are probably copy-pasted from Tegra194 where these snuck in. I can
> drop those when applying. I'll also prepare a patch to drop these from
> the tegra194.dtsi.
> 
> I wonder if there's a good way to detect these. We'd have to run checks
> on the DT source files, so that's a bit difficult. I do have an
> experimental script that tries to capture some common pitfalls on
> sources but it's quite ugly and slow, but I guess I could add something
> like this. But perhaps there are better ways?

One way to easy spot them is to override always by label, thus every
node defined like above is a new node. However I think we talked about
this and you do not follow this practice, thus there is no way to tell -
is the status reasonable or not.

Automated tools could help here as well - run fdtdump on DTB and look
for status=okay.

Best regards,
Krzysztof