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

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

[Fabio Estevam]

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."

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
-- 
2.34.1

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

[Arnd Bergmann]

On Wed, Nov 27, 2024, at 20:06, Fabio Estevam wrote:
> 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."
>
> 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>

Acked-by: Arnd Bergmann <arnd@arndb.de>

     Arnd

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

[Esben Haabendal]

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

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

[Linus Walleij]

On Wed, Nov 27, 2024 at 8:06 PM Fabio Estevam <festevam@gmail.com> wrote:

> 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."
>
> 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>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

[Guenter Roeck]

On Wed, Nov 27, 2024 at 04:06:05PM -0300, Fabio Estevam wrote:
> 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."
> 
> 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>

Tested-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 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
> -- 
> 2.34.1
> 

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

[Linus Walleij]

On Wed, Nov 27, 2024 at 8:06 PM Fabio Estevam <festevam@gmail.com> wrote:

> 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.

Fabio are you pushing this in through the SoC tree as it's ARM-only
or am I expected to push it through pin control?

I expected SoC but asking because I don't want it to end up in limbo.

Yours,
Linus Walleij

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

[Fabio Estevam]

Hi Linus,

On Fri, Nov 29, 2024 at 7:15 AM Linus Walleij <linus.walleij@linaro.org> wrote:

> Fabio are you pushing this in through the SoC tree as it's ARM-only
> or am I expected to push it through pin control?

17d210018914 ("ARM: imx: Allow user to disable pinctrl") comes from
your tree and Arnd sent an Acked-by,
so I interpret it as it should go through your tree.

Could you please apply it via the pinctrl tree so that it can land in
6.13-rc1 to avoid the regression?

Thanks

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

[Linus Walleij]

On Wed, Nov 27, 2024 at 8:06 PM Fabio Estevam <festevam@gmail.com> wrote:

> 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."
>
> 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>

Patch applied for fixes!

Yours,
Linus Walleij