Message-ID: <CALHNRZ_gy_wJxAW85d0EnpY4Qa2h+tuR=CM2AE26r0UEdYz8ag@mail.gmail.com>
Date: Tue, 10 Jun 2025 10:34:35 -0500
From: Aaron Kling <webgeek1234@...il.com>
To: Thierry Reding <thierry.reding@...il.com>
Cc: Linus Walleij <linus.walleij@...aro.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Jonathan Hunter <jonathanh@...dia.com>, Bartosz Golaszewski <brgl@...ev.pl>, linux-gpio@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-tegra@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/3] pinctrl: tegra: Add Tegra186 pinmux driver

On Tue, Jun 10, 2025 at 4:40 AM Thierry Reding <thierry.reding@...il.com> wrote:
>
> On Sun, Jun 08, 2025 at 09:13:14PM -0500, Aaron Kling via B4 Relay wrote:
> > From: Aaron Kling <webgeek1234@...il.com>
> >
> > This is based on Nvidia's downstream 5.10 driver, rewritten to match the
> > mainline Tegra194 pinmux driver.
>
> A few words upfront, to justify why I'm being pedantic. Originally the

I don't mind pedantic. Get it right the first time, save later pain.

> pinmux drivers were generated using the tegra-pinmux-scripts[0]. This
> project was later archived because Tegra210 was deemed to be the last
> chip to need a pin controller. It then turned out that Tegra194 needed
> pinmuxing for certain pins, and then more, so we ended up with a full
> pinmux driver for it. However, we also deemed Tegra194 to be an
> exception, so that's why that pinctrl driver was a one-off job.

Tegra234 also has a pinctrl driver, which makes tegra186 the
exception, when looking at the driver list without any other context.

>
> I now regret these decisions because the same formatting mistakes are
> now proliferating, which is exactly what the scripts were meant to
> avoid.
>
> One thing that's not clear from this patch set is whether we actually
> need the Tegra186 pinmux driver, or you're only adding it because it
> happens to be present in a 5.10 downstream driver. Do you actually have
> a requirement for setting pins dynamically at runtime? Do you need to be
> able to set a static configuration at boot that can't be set using some
> earlier bootloader/firmware mechanism?

I have a device that's based on p3509+p3636 with an audio codec,
originally targeting l4t r32. The odm provided instructions to use the
codec was to first run the jetson pinmux python script after boot... I
made that less bad by putting the pinmux in the kernel dt. But I see
similar recommendations all over the nvidia dev forums, even for t194
and t234. Solely for that reason, I'd think it reasonable to support
the kernel pinmux driver on all tegra platforms: to allow easier
porting of l4t devices. And a secondary argument could be made for the
devkits: they have expansion headers with mux-able pins, prototyping /
experimenting with those at runtime via sysfs is a valid use case.

Taking a look through the public cboot sources for t186, I'm not
seeing that it handles pinmux at all. With one or two exceptions like
the t194 pcie stuff. How is it expected to configure pinmux at the
bootloader level? Using the auto-generated mb1 flash config? I'm
unaware of this being publicly documented anywhere, not just for t186
but for any tegra arch. And the format of those aren't particularly
easy to read and hand modify.

>
> If we really need this pinctrl driver it may be worth resurrecting the
> tegra-pinmux-scripts so that we can add these drivers based on the
> generated files as originally intended.

Imo, this would be ideal. That will require some updates to the
scripts to handle the main/aon split. I don't think it's something
that can be done externally either as it depends on the internal only
pinmux spreadsheets as the starting point for a soc or device. But if
that's done, all the format issues will be sidestepped and this series
will be superseded.

Sincerely,
Aaron Kling

Powered by blists - more mailing lists