Re: [PATCH v2] ARM: imx: Re-introduce the PINCTRL selection

[Esben Haabendal <esben@geanix.com>]

Fabio Estevam <festevam@gmail.com> writes:

> From: Fabio Estevam <festevam@denx.de>
>
> Since commit 17d210018914 ("ARM: imx: Allow user to disable pinctrl"),
> the CONFIG_PINCTRL option is no longer implicitly selected, causing
> several i.MX SoC pinctrl drivers no longer getting selected by default.
>
> This causes boot regressions on the ARMv4, ARMv5, ARMv6 and ARMv7
> i.MX SoCs.
>
> Fix it by selecting CONFIG_PINCTRL as before.
>
> This defeats the purpose of 7d210018914 ("ARM: imx: Allow user to disable
> pinctrl"), but it is the less invasive fix for the boot regressions.
>
> The attempt to build Layerscape without pinctrl can still be explored
> later as suggested by Arnd:
>
> "Overall, my best advice here is still to not change the way
> i.MX pinctrl works at all, but just fix Layerscape to not depend
> on i.MX. The reason for the 'select' here is clearly that the
> i.MX machines would fail to boot without pinctrl, and changing
> that because of Layerscape seems backwards."

While that is possibly true in most cases, wouldn't it be possible to
use a kernel for an i.MX platform without having pinctrl support in the
kernel. If bootloader or something else before the running kernel have
setup pin functions properly.

By reverting the change, you will also make it impossible to do this
without patching the kernel.

But if not breaking existing defconfigs is more important than allowing
flexible configurations on the short term, then I guess that is how it
should go.

> Fixes: 17d210018914 ("ARM: imx: Allow user to disable pinctrl")
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Closes: https://lore.kernel.org/linux-arm-kernel/49ff070a-ce67-42d7-84ec-8b54fd7e9742@roeck-us.net/
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> ---
> Changes since v1:
> - Reintroduce "select PINCTRL" as a minimal fix for 6.13-rc.
>
>  arch/arm/mach-imx/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index e4fe059cd861..dc47b2312127 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -6,6 +6,7 @@ menuconfig ARCH_MXC
>  	select CLKSRC_IMX_GPT
>  	select GENERIC_IRQ_CHIP
>  	select GPIOLIB
> +	select PINCTRL
>  	select PM_OPP if PM
>  	select SOC_BUS
>  	select SRAM

Instead of doing this at top level ARCH_MXC level, it could also be done
at the SOC level.  Something like

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index e4fe059cd861..26d29dea1721 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -59,6 +59,7 @@ config SOC_IMX1
        bool "i.MX1 support"
        select CPU_ARM920T
        select MXC_AVIC
+       select PINCTRL
        help
          This enables support for Freescale i.MX1 processor

That way existing defconfigs should work as before, while SOC_LS1021A
builds would be able to leave out pinctrl.

Even thouh LS1021A is clearly officially a Layerscape SoC, I think the
reasons for it being placed in mach-imx is that the effort needed to not
do that is more than it might be worth.

/Esben