| Message ID | 20240506-imx-pinctrl-optional-v2-1-bdff75085156@geanix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: imx: only enable pinctrl as needed | expand |
On Mon, May 06, 2024 at 12:23:53PM +0200, Esben Haabendal wrote: > Making pinctrl drivers and subsequently the pinctrl framework > user-controllable, allows building a kernel without this. > While in many (most) cases, this could make the system unbootable, it > does allow building smaller kernels for those situations where picntrl > is not needed. > > One such situation is when building a kernel for NXP LS1021A systems, > which does not have run-time controllable pinctrl, so pinctrl framework > and drivers are 100% dead-weight. > > > Signed-off-by: Esben Haabendal <esben@geanix.com> Acked-by: Shawn Guo <shawnguo@kernel.org>
Shawn Guo <shawnguo2@yeah.net> writes: > On Mon, May 06, 2024 at 12:23:53PM +0200, Esben Haabendal wrote: >> Making pinctrl drivers and subsequently the pinctrl framework >> user-controllable, allows building a kernel without this. >> While in many (most) cases, this could make the system unbootable, it >> does allow building smaller kernels for those situations where picntrl >> is not needed. >> >> One such situation is when building a kernel for NXP LS1021A systems, >> which does not have run-time controllable pinctrl, so pinctrl framework >> and drivers are 100% dead-weight. >> >> >> Signed-off-by: Esben Haabendal <esben@geanix.com> > > Acked-by: Shawn Guo <shawnguo@kernel.org> Was this only for this patch 1/3, or the whole series? /Esben
On Mon, May 6, 2024 at 12:24 PM Esben Haabendal <esben@geanix.com> wrote: > Making pinctrl drivers and subsequently the pinctrl framework > user-controllable, allows building a kernel without this. > While in many (most) cases, this could make the system unbootable, it > does allow building smaller kernels for those situations where picntrl > is not needed. > > One such situation is when building a kernel for NXP LS1021A systems, > which does not have run-time controllable pinctrl, so pinctrl framework > and drivers are 100% dead-weight. > > > Signed-off-by: Esben Haabendal <esben@geanix.com> Acked-by: Linus Walleij <linus.walleij@linaro.org> I guess this needs to be merged through the SoC tree. Yours, Linus Walleij
Linus Walleij <linus.walleij@linaro.org> writes: > On Mon, May 6, 2024 at 12:24 PM Esben Haabendal <esben@geanix.com> wrote: > >> Making pinctrl drivers and subsequently the pinctrl framework >> user-controllable, allows building a kernel without this. >> While in many (most) cases, this could make the system unbootable, it >> does allow building smaller kernels for those situations where picntrl >> is not needed. >> >> One such situation is when building a kernel for NXP LS1021A systems, >> which does not have run-time controllable pinctrl, so pinctrl framework >> and drivers are 100% dead-weight. >> >> >> Signed-off-by: Esben Haabendal <esben@geanix.com> > > Acked-by: Linus Walleij <linus.walleij@linaro.org> > > I guess this needs to be merged through the SoC tree. Anything more I need to make that happen? /Esben
On Tue, Sep 24, 2024 at 09:45:32AM +0200, Linus Walleij wrote: > On Mon, May 6, 2024 at 12:24 PM Esben Haabendal <esben@geanix.com> wrote: > > > Making pinctrl drivers and subsequently the pinctrl framework > > user-controllable, allows building a kernel without this. > > While in many (most) cases, this could make the system unbootable, it > > does allow building smaller kernels for those situations where picntrl > > is not needed. > > > > One such situation is when building a kernel for NXP LS1021A systems, > > which does not have run-time controllable pinctrl, so pinctrl framework > > and drivers are 100% dead-weight. > > > > > > Signed-off-by: Esben Haabendal <esben@geanix.com> > > Acked-by: Linus Walleij <linus.walleij@linaro.org> > > I guess this needs to be merged through the SoC tree. Hi Linus, Reading your comment[1], I was thinking that you will merge the series through pinctrl tree, no? Shawn [1] https://lore.kernel.org/linux-arm-kernel/CACRpkdYbOTXmap-vJy4JNZSaZnE=yzC35EPD2F=bD8gWdD8-GQ@mail.gmail.com/
On Mon, May 06, 2024 at 12:23:53PM +0200, Esben Haabendal wrote: > Making pinctrl drivers and subsequently the pinctrl framework > user-controllable, allows building a kernel without this. > While in many (most) cases, this could make the system unbootable, it > does allow building smaller kernels for those situations where picntrl > is not needed. > > One such situation is when building a kernel for NXP LS1021A systems, > which does not have run-time controllable pinctrl, so pinctrl framework > and drivers are 100% dead-weight. > > > Signed-off-by: Esben Haabendal <esben@geanix.com> This patch didn't update default configurations, meaning PINCTRL is now disabled by affected configurations such as imx_v4_v5_defconfig or imx_v6_v7_defconfig, making pretty much all imx platforms non-bootable unless the default configuration is changed manually. Guenter
On Tue, Nov 26, 2024 at 5:17 PM Guenter Roeck <linux@roeck-us.net> wrote: > On Mon, May 06, 2024 at 12:23:53PM +0200, Esben Haabendal wrote: > > Making pinctrl drivers and subsequently the pinctrl framework > > user-controllable, allows building a kernel without this. > > While in many (most) cases, this could make the system unbootable, it > > does allow building smaller kernels for those situations where picntrl > > is not needed. > > > > One such situation is when building a kernel for NXP LS1021A systems, > > which does not have run-time controllable pinctrl, so pinctrl framework > > and drivers are 100% dead-weight. > > > > > > Signed-off-by: Esben Haabendal <esben@geanix.com> > > This patch didn't update default configurations, meaning PINCTRL is now > disabled by affected configurations such as imx_v4_v5_defconfig or > imx_v6_v7_defconfig, making pretty much all imx platforms non-bootable > unless the default configuration is changed manually. Since the patch tries to add default selects for all drivers I suspect this oneliner is the culprit: @@ -6,7 +6,6 @@ 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 Should we just add that one line back? Yours, Linus Walleij
On 11/26/24 13:24, Linus Walleij wrote: > On Tue, Nov 26, 2024 at 5:17 PM Guenter Roeck <linux@roeck-us.net> wrote: >> On Mon, May 06, 2024 at 12:23:53PM +0200, Esben Haabendal wrote: > >>> Making pinctrl drivers and subsequently the pinctrl framework >>> user-controllable, allows building a kernel without this. >>> While in many (most) cases, this could make the system unbootable, it >>> does allow building smaller kernels for those situations where picntrl >>> is not needed. >>> >>> One such situation is when building a kernel for NXP LS1021A systems, >>> which does not have run-time controllable pinctrl, so pinctrl framework >>> and drivers are 100% dead-weight. >>> >>> >>> Signed-off-by: Esben Haabendal <esben@geanix.com> >> >> This patch didn't update default configurations, meaning PINCTRL is now >> disabled by affected configurations such as imx_v4_v5_defconfig or >> imx_v6_v7_defconfig, making pretty much all imx platforms non-bootable >> unless the default configuration is changed manually. > > Since the patch tries to add default selects for all drivers I suspect this > oneliner is the culprit: > > @@ -6,7 +6,6 @@ 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 > > Should we just add that one line back? > My understanding (which may be wrong) is that being able to disable PINCTRL was the whole point of the patch. Fabio submitted a patch enabling PINCTRL for imx_v4_v5_defconfig and imx_v6_v7_defconfig explicitly [1]. I don't know if that fixes the problem for good - I see CONFIG_ARCH_MXC in other configurations as well. Guenter --- [1] https://lore.kernel.org/imx/d718ddd2-d473-4455-b21a-15024e46787c@roeck-us.net/T/#mc71dc21d99e0b013c5ce46c0d90940fd8806ae9a
On Tue, Nov 26, 2024 at 8:53 PM Guenter Roeck <linux@roeck-us.net> wrote: > My understanding (which may be wrong) is that being able to disable > PINCTRL was the whole point of the patch. Exactly. Adding back the "select PINCTRL" line defeats the purpose of the patch in Subject. > Fabio submitted a patch enabling PINCTRL for imx_v4_v5_defconfig and > imx_v6_v7_defconfig explicitly [1]. I don't know if that fixes the > problem for good - I see CONFIG_ARCH_MXC in other configurations as > well. Good point. I can send a v2 adding CONFIG_PINCTRL=y to the other defconfigs. However, after thinking more about it, I wonder if the patch in Subject is worth it. It can help reduce the kernel size for LS1021A that does not need pinctrl, but on the other hand, it will cause pain to lots of people who have i.MX products running custom defconfigs. When they update their kernel to 6.13-rc1 their i.MX will not boot and that will be a very unpleasant experience. What do you think? Should we go with the approach of selecting CONFIG_PINCTRL=y or should we revert this patch? IMHO, the benefit of this patch does not justify the bad impact on users.
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig index ab767f059929..e4fe059cd861 100644 --- a/arch/arm/mach-imx/Kconfig +++ b/arch/arm/mach-imx/Kconfig @@ -6,7 +6,6 @@ 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 @@ -49,7 +48,6 @@ config SOC_IMX31 config SOC_IMX35 bool "i.MX35 support" select MXC_AVIC - select PINCTRL_IMX35 help This enables support for Freescale i.MX35 processor @@ -61,7 +59,6 @@ config SOC_IMX1 bool "i.MX1 support" select CPU_ARM920T select MXC_AVIC - select PINCTRL_IMX1 help This enables support for Freescale i.MX1 processor @@ -73,7 +70,6 @@ config SOC_IMX25 bool "i.MX25 support" select CPU_ARM926T select MXC_AVIC - select PINCTRL_IMX25 help This enables support for Freescale i.MX25 processor @@ -81,7 +77,6 @@ config SOC_IMX27 bool "i.MX27 support" select CPU_ARM926T select MXC_AVIC - select PINCTRL_IMX27 help This enables support for Freescale i.MX27 processor @@ -98,7 +93,6 @@ config SOC_IMX5 config SOC_IMX50 bool "i.MX50 support" - select PINCTRL_IMX50 select SOC_IMX5 help @@ -106,14 +100,12 @@ config SOC_IMX50 config SOC_IMX51 bool "i.MX51 support" - select PINCTRL_IMX51 select SOC_IMX5 help This enables support for Freescale i.MX51 processor config SOC_IMX53 bool "i.MX53 support" - select PINCTRL_IMX53 select SOC_IMX5 help @@ -137,7 +129,6 @@ config SOC_IMX6Q select ARM_ERRATA_775420 select HAVE_ARM_SCU if SMP select HAVE_ARM_TWD - select PINCTRL_IMX6Q select SOC_IMX6 help @@ -147,7 +138,6 @@ config SOC_IMX6SL bool "i.MX6 SoloLite support" select ARM_ERRATA_754322 select ARM_ERRATA_775420 - select PINCTRL_IMX6SL select SOC_IMX6 help @@ -157,7 +147,6 @@ config SOC_IMX6SLL bool "i.MX6 SoloLiteLite support" select ARM_ERRATA_754322 select ARM_ERRATA_775420 - select PINCTRL_IMX6SLL select SOC_IMX6 help @@ -167,7 +156,6 @@ config SOC_IMX6SX bool "i.MX6 SoloX support" select ARM_ERRATA_754322 select ARM_ERRATA_775420 - select PINCTRL_IMX6SX select SOC_IMX6 help @@ -175,7 +163,6 @@ config SOC_IMX6SX config SOC_IMX6UL bool "i.MX6 UltraLite support" - select PINCTRL_IMX6UL select SOC_IMX6 select ARM_ERRATA_814220 @@ -211,7 +198,6 @@ config SOC_IMX7D_CM4 config SOC_IMX7D bool "i.MX7 Dual support" - select PINCTRL_IMX7D select SOC_IMX7D_CA7 if ARCH_MULTI_V7 select SOC_IMX7D_CM4 if ARM_SINGLE_ARMV7M select ARM_ERRATA_814220 if ARCH_MULTI_V7 @@ -221,7 +207,6 @@ config SOC_IMX7D config SOC_IMX7ULP bool "i.MX7ULP support" select CLKSRC_IMX_TPM - select PINCTRL_IMX7ULP select SOC_IMX7D_CA7 if ARCH_MULTI_V7 select SOC_IMX7D_CM4 if ARM_SINGLE_ARMV7M help @@ -237,7 +222,6 @@ config SOC_IMXRT config SOC_VF610 bool "Vybrid Family VF610 support" select ARM_GIC if ARCH_MULTI_V7 - select PINCTRL_VF610 help This enables support for Freescale Vybrid VF610 processor. diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig index 27bdc548f3a7..ef39bb6cf9cb 100644 --- a/drivers/pinctrl/freescale/Kconfig +++ b/drivers/pinctrl/freescale/Kconfig @@ -20,6 +20,7 @@ config PINCTRL_IMX1_CORE config PINCTRL_IMX1 bool "IMX1 pinctrl driver" depends on SOC_IMX1 + default SOC_IMX1 select PINCTRL_IMX1_CORE help Say Y here to enable the imx1 pinctrl driver @@ -27,6 +28,7 @@ config PINCTRL_IMX1 config PINCTRL_IMX27 bool "IMX27 pinctrl driver" depends on SOC_IMX27 + default SOC_IMX27 select PINCTRL_IMX1_CORE help Say Y here to enable the imx27 pinctrl driver @@ -36,6 +38,7 @@ config PINCTRL_IMX25 bool "IMX25 pinctrl driver" depends on OF depends on SOC_IMX25 + default SOC_IMX25 select PINCTRL_IMX help Say Y here to enable the imx25 pinctrl driver @@ -43,6 +46,7 @@ config PINCTRL_IMX25 config PINCTRL_IMX35 bool "IMX35 pinctrl driver" depends on SOC_IMX35 + default SOC_IMX35 select PINCTRL_IMX help Say Y here to enable the imx35 pinctrl driver @@ -50,6 +54,7 @@ config PINCTRL_IMX35 config PINCTRL_IMX50 bool "IMX50 pinctrl driver" depends on SOC_IMX50 + default SOC_IMX50 select PINCTRL_IMX help Say Y here to enable the imx50 pinctrl driver @@ -57,6 +62,7 @@ config PINCTRL_IMX50 config PINCTRL_IMX51 bool "IMX51 pinctrl driver" depends on SOC_IMX51 + default SOC_IMX51 select PINCTRL_IMX help Say Y here to enable the imx51 pinctrl driver @@ -64,6 +70,7 @@ config PINCTRL_IMX51 config PINCTRL_IMX53 bool "IMX53 pinctrl driver" depends on SOC_IMX53 + default SOC_IMX53 select PINCTRL_IMX help Say Y here to enable the imx53 pinctrl driver @@ -71,6 +78,7 @@ config PINCTRL_IMX53 config PINCTRL_IMX6Q bool "IMX6Q/DL pinctrl driver" depends on SOC_IMX6Q + default SOC_IMX6Q select PINCTRL_IMX help Say Y here to enable the imx6q/dl pinctrl driver @@ -78,6 +86,7 @@ config PINCTRL_IMX6Q config PINCTRL_IMX6SL bool "IMX6SL pinctrl driver" depends on SOC_IMX6SL + default SOC_IMX6SL select PINCTRL_IMX help Say Y here to enable the imx6sl pinctrl driver @@ -85,6 +94,7 @@ config PINCTRL_IMX6SL config PINCTRL_IMX6SLL bool "IMX6SLL pinctrl driver" depends on SOC_IMX6SLL + default SOC_IMX6SLL select PINCTRL_IMX help Say Y here to enable the imx6sll pinctrl driver @@ -92,6 +102,7 @@ config PINCTRL_IMX6SLL config PINCTRL_IMX6SX bool "IMX6SX pinctrl driver" depends on SOC_IMX6SX + default SOC_IMX6SX select PINCTRL_IMX help Say Y here to enable the imx6sx pinctrl driver @@ -99,6 +110,7 @@ config PINCTRL_IMX6SX config PINCTRL_IMX6UL bool "IMX6UL pinctrl driver" depends on SOC_IMX6UL + default SOC_IMX6UL select PINCTRL_IMX help Say Y here to enable the imx6ul pinctrl driver @@ -106,6 +118,7 @@ config PINCTRL_IMX6UL config PINCTRL_IMX7D bool "IMX7D pinctrl driver" depends on SOC_IMX7D + default SOC_IMX7D select PINCTRL_IMX help Say Y here to enable the imx7d pinctrl driver @@ -113,6 +126,7 @@ config PINCTRL_IMX7D config PINCTRL_IMX7ULP bool "IMX7ULP pinctrl driver" depends on SOC_IMX7ULP + default SOC_IMX7ULP select PINCTRL_IMX help Say Y here to enable the imx7ulp pinctrl driver @@ -194,6 +208,7 @@ config PINCTRL_IMX93 config PINCTRL_VF610 bool "Freescale Vybrid VF610 pinctrl driver" depends on SOC_VF610 + default SOC_VF610 select PINCTRL_IMX help Say Y here to enable the Freescale Vybrid VF610 pinctrl driver
Making pinctrl drivers and subsequently the pinctrl framework user-controllable, allows building a kernel without this. While in many (most) cases, this could make the system unbootable, it does allow building smaller kernels for those situations where picntrl is not needed. One such situation is when building a kernel for NXP LS1021A systems, which does not have run-time controllable pinctrl, so pinctrl framework and drivers are 100% dead-weight. Signed-off-by: Esben Haabendal <esben@geanix.com> --- arch/arm/mach-imx/Kconfig | 16 ---------------- drivers/pinctrl/freescale/Kconfig | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 16 deletions(-)