Message-ID: <6002097.iIbC2pHGDl@steina-w>
Date: Thu, 08 May 2025 15:18:04 +0200
From: Alexander Stein <alexander.stein@...tq-group.com>
To: Esben Haabendal <esben@...nix.com>
Cc: Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
 Stefan Wahren <wahrenst@....net>, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, Shawn Guo <shawnguo@...nel.org>
Subject: Re: [PATCH 1/1] arm64: Kconfig: Enable PINCTRL on i.MX platforms

Am Donnerstag, 8. Mai 2025, 14:43:09 CEST schrieb Esben Haabendal:
> "Alexander Stein" <alexander.stein@...tq-group.com> writes:
> 
> > Hi Esben,
> >
> > Am Donnerstag, 8. Mai 2025, 10:18:35 CEST schrieb Esben Haabendal:
> >> "Alexander Stein" <alexander.stein@...tq-group.com> writes:
> >>
> >> > Hi Esben,
> >> >
> >> > Am Donnerstag, 8. Mai 2025, 08:44:22 CEST schrieb Esben Haabendal:
> >> >> "Alexander Stein" <alexander.stein@...tq-group.com> writes:
> >> >>
> >> >> > Hi Stefan,
> >> >> >
> >> >> > Am Mittwoch, 7. Mai 2025, 16:30:33 CEST schrieb Stefan Wahren:
> >> >> >> Hi Alexander,
> >> >> >>
> >> >> >> [add Shawn and Esben]
> >> >> >>
> >> >> >> Am 07.05.25 um 14:44 schrieb Alexander Stein:
> >> >> >> > Select PINCTRL for NXP i.MX SoCs.
> >> >> >> could you please explain the motivation behind your change?
> >> >> >>
> >> >> >> Is it related to this commit 17d21001891402 ("ARM: imx: Allow user to
> >> >> >> disable pinctrl")?
> >> >> >
> >> >> > Ah, thanks for the pointer. It might be the case.
> >> >>
> >> >> The goal of the patch mentioned above was to be able to build a kernel
> >> >> for LS1021A without pinctrl framework enabled, as LS1021A does not have
> >> >> a pinctrl driver.
> >> >>
> >> >> With your patch, that would not be possible anymore.
> >> >
> >> > Why? LS1021A is arm, not arm64 which this patch is touching only.
> >>
> >> Good point :)  Sorry about that.
> >>
> >> > BTW: Commit b77bd3ba762f3 ("ARM: imx: Re-introduce the PINCTRL selection")
> >> > is actually doing the same for arm as there is some fallout from
> >> > 17d21001891402.
> >> >
> >> >> > I noticed that, when using arch/arm64/defconfig and disabling all
> >> >> > platforms despite ARCH_MXC before running make olddefconfig,
> >> >> > CONFIG_PINCTRL gets disabled as well. No platform is enabling it. I
> >> >> > noticed this when building in yocto and non-IMX platforms are disabled
> >> >> > for build time reasons.
> >> >>
> >> >> But is that something that needs to be fixed?
> >> >>
> >> >> It sounds like quite a special use-case, and why not simply enable
> >> >> CONFIG_PINCTRL in that case then?
> >> >
> >> > PINCTRL is crucial for any SoC to even boot, so this is an option which has
> >> > to be set if that platform is enabled.
> >>
> >> Yes, but PINCTRL (framework) does not by itself do anything meaningful.
> >> You need the correct pinctrl driver.
> >>
> >> Making the various SOC's select the corresponding pinctrl drivers makes
> >> sense if it is required for booting under all circumstances. And this
> >> should then indirectly enable/select PINCTRL and anything else needed
> >> for that driver.
> >
> > If you prefer I don't mind enabling PINCTRL and the SoC-specific driver
> > (e.g. PINCTRL_IMX8MP) depending on each SoC-support, e.g. SOC_IMX35 or
> > SOC_IMX8M.
> 
> For SOC_IMX35, it should be selected by default.
> 
>     config PINCTRL_IMX35
>             bool "IMX35 pinctrl driver"
>             depends on OF
>             depends on SOC_IMX35 || COMPILE_TEST
>             default SOC_IMX35
> 
> For the IMX8M* SoC's, that is not done, as there is only a common
> SOC_IMX8M config entry, which corresponds to multiple pinctrl drivers,
> which we probably don't want to select all of by default.

Well, is the SoC support is enabled, it makes totally sense to enable a
crucial driver like pinctrl by default. It's still deselectable after all.

> >> Having ARCH_MXC select PINCTRL as such is mostly pointless IMHO.
> >> Enabling a driver framework without enabling any drivers for it, when
> >> building a kernel where no SOC's requiring any pinctrl drivers is kind
> >> of weird. If you want to do that, why not simply enable both ARCH_MXC
> >> and PINCTRL in your yocto recipe?
> >
> > PINCTRL is currently only enabled because other SoCs happen to enable it,
> > just this feels just plain wrong. If these platforms are disabled or
> > removed for whatever reason, the other platforms should still work.
> 
> As it is now, to build for let's say i.MX 8M Plus, you have to enable
>     SOC_IMX8M
>     PINCTRL_IMX8MP
> to get a kernel that is likely to boot.

Both SOC_IMX8M and SOC_IMX9 are enabled by default if ARCH_MXC is enabled
too, even though you might not want to use both SoC families.

> If you enable
>     SOC_IMX8M
>     PINCTRL
> but not
>     PINCTRL_IMX8MP
> you won't have pinctrl support, and the kernel will probably not work as
> expected.
> 
> What am I missing?

PINCTRL is not enabled bydefault if only ARCH_MXC is enabled. It just happens
that defconfig enables other platforms which in turn enable PINCTRL.
If you prefer to explicitely enable CONFIG_PINCTRL=y in defconfig
I'm fine with that as well.

Best regards
Alexander
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Powered by blists - more mailing lists