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

Krzysztof Kozlowski
2023-02-08T12:01:33Z

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@nvidia.com>
>>> ---
>>> 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