linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] ARM: imx: only enable pinctrl as needed
@ 2024-05-06 10:23 Esben Haabendal
  2024-05-06 10:23 ` [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl Esben Haabendal
                   ` (3 more replies)
  0 siblings, 4 replies; 25+ messages in thread
From: Esben Haabendal @ 2024-05-06 10:23 UTC (permalink / raw)
  To: Russell King, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Dong Aisheng, Jacky Bai, Linus Walleij
  Cc: Arnd Bergmann, Rasmus Villemoes, linux-arm-kernel, imx,
	linux-kernel, linux-gpio, Esben Haabendal

As not all mach-imx platforms has support for run-time changes of pin
configurations (such as LS1021A), a more selective approach to enabling
pinctrl infrastructure makes sense, so that an e.g. an LS1021A only kernel
could be built without pinctrl support.

This is a very late follow up v1 3 years ago [1]. The situation seems to be
unchanged since then, and I have tried to incorporate the requested
changes.

[1] https://lore.kernel.org/linux-arm-kernel/be1c35eb997959b4939b304ef26664dfb9cd2275.1621941451.git.esben@geanix.com/

Changes since v1:
- Changed all the pinctrl drivers to be user-configurable, allowing disable
  even for systems with pinctrl.
- Added fixup of overly generic dependency for i.MX RT pinctrl drivers.
- Allow compile-testing of i.MX pinctrl drivers using CONFIG_COMPILE_TEST.

Signed-off-by: Esben Haabendal <esben@geanix.com>
---
Esben Haabendal (3):
      ARM: imx: Allow user to disable pinctrl
      pinctrl: freescale: Use CONFIG_SOC_IMXRT to guard i.MX RT1xxx drivers
      pinctrl: freescale: enable use with COMPILE_TEST

 arch/arm/mach-imx/Kconfig         | 16 -------
 drivers/pinctrl/freescale/Kconfig | 89 +++++++++++++++++++++++++++------------
 2 files changed, 63 insertions(+), 42 deletions(-)
---
base-commit: dd5a440a31fae6e459c0d6271dddd62825505361
change-id: 20240506-imx-pinctrl-optional-63acd3db88dc

Best regards,
-- 
Esben Haabendal <esben@geanix.com>


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl
  2024-05-06 10:23 [PATCH v2 0/3] ARM: imx: only enable pinctrl as needed Esben Haabendal
@ 2024-05-06 10:23 ` Esben Haabendal
  2024-06-03  2:50   ` Shawn Guo
                     ` (2 more replies)
  2024-05-06 10:23 ` [PATCH v2 2/3] pinctrl: freescale: Use CONFIG_SOC_IMXRT to guard i.MX RT1xxx drivers Esben Haabendal
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 25+ messages in thread
From: Esben Haabendal @ 2024-05-06 10:23 UTC (permalink / raw)
  To: Russell King, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Dong Aisheng, Jacky Bai, Linus Walleij
  Cc: Arnd Bergmann, Rasmus Villemoes, linux-arm-kernel, imx,
	linux-kernel, linux-gpio, Esben Haabendal

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(-)

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

-- 
2.45.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 2/3] pinctrl: freescale: Use CONFIG_SOC_IMXRT to guard i.MX RT1xxx drivers
  2024-05-06 10:23 [PATCH v2 0/3] ARM: imx: only enable pinctrl as needed Esben Haabendal
  2024-05-06 10:23 ` [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl Esben Haabendal
@ 2024-05-06 10:23 ` Esben Haabendal
  2024-05-06 11:29   ` Arnd Bergmann
  2024-05-06 10:23 ` [PATCH v2 3/3] pinctrl: freescale: enable use with COMPILE_TEST Esben Haabendal
  2024-05-27 11:05 ` [PATCH v2 0/3] ARM: imx: only enable pinctrl as needed Linus Walleij
  3 siblings, 1 reply; 25+ messages in thread
From: Esben Haabendal @ 2024-05-06 10:23 UTC (permalink / raw)
  To: Russell King, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Dong Aisheng, Jacky Bai, Linus Walleij
  Cc: Arnd Bergmann, Rasmus Villemoes, linux-arm-kernel, imx,
	linux-kernel, linux-gpio, Esben Haabendal

This aligns with the use of SOC_IMX* as dependencies for all the other
mach-imx pinctrl drivers. Enabling i.MX RT pinctrl drivers for a kernel
with out i.MX RT SOC support is pointless.

Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 drivers/pinctrl/freescale/Kconfig | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index ef39bb6cf9cb..2b19c40978c2 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -193,7 +193,8 @@ config PINCTRL_IMX8ULP
 
 config PINCTRL_IMXRT1050
 	bool "IMXRT1050 pinctrl driver"
-	depends on ARCH_MXC
+	depends on SOC_IMXRT
+	default SOC_IMXRT
 	select PINCTRL_IMX
 	help
 	  Say Y here to enable the imxrt1050 pinctrl driver
@@ -228,7 +229,7 @@ config PINCTRL_IMX28
 
 config PINCTRL_IMXRT1170
 	bool "IMXRT1170 pinctrl driver"
-	depends on ARCH_MXC
+	depends on SOC_IMXRT
 	select PINCTRL_IMX
 	help
 	  Say Y here to enable the imxrt1170 pinctrl driver

-- 
2.45.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 3/3] pinctrl: freescale: enable use with COMPILE_TEST
  2024-05-06 10:23 [PATCH v2 0/3] ARM: imx: only enable pinctrl as needed Esben Haabendal
  2024-05-06 10:23 ` [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl Esben Haabendal
  2024-05-06 10:23 ` [PATCH v2 2/3] pinctrl: freescale: Use CONFIG_SOC_IMXRT to guard i.MX RT1xxx drivers Esben Haabendal
@ 2024-05-06 10:23 ` Esben Haabendal
  2024-05-27 11:05 ` [PATCH v2 0/3] ARM: imx: only enable pinctrl as needed Linus Walleij
  3 siblings, 0 replies; 25+ messages in thread
From: Esben Haabendal @ 2024-05-06 10:23 UTC (permalink / raw)
  To: Russell King, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Dong Aisheng, Jacky Bai, Linus Walleij
  Cc: Arnd Bergmann, Rasmus Villemoes, linux-arm-kernel, imx,
	linux-kernel, linux-gpio, Esben Haabendal

Allow compile-testing of i.MX pinctrl drivers using CONFIG_COMPILE_TEST.

Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 drivers/pinctrl/freescale/Kconfig | 73 +++++++++++++++++++++++++--------------
 1 file changed, 47 insertions(+), 26 deletions(-)

diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index 2b19c40978c2..34a4255dfd4c 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -19,7 +19,8 @@ config PINCTRL_IMX1_CORE
 
 config PINCTRL_IMX1
 	bool "IMX1 pinctrl driver"
-	depends on SOC_IMX1
+	depends on OF
+	depends on SOC_IMX1 || COMPILE_TEST
 	default SOC_IMX1
 	select PINCTRL_IMX1_CORE
 	help
@@ -27,7 +28,8 @@ config PINCTRL_IMX1
 
 config PINCTRL_IMX27
 	bool "IMX27 pinctrl driver"
-	depends on SOC_IMX27
+	depends on OF
+	depends on SOC_IMX27 || COMPILE_TEST
 	default SOC_IMX27
 	select PINCTRL_IMX1_CORE
 	help
@@ -37,7 +39,7 @@ config PINCTRL_IMX27
 config PINCTRL_IMX25
 	bool "IMX25 pinctrl driver"
 	depends on OF
-	depends on SOC_IMX25
+	depends on SOC_IMX25 || COMPILE_TEST
 	default SOC_IMX25
 	select PINCTRL_IMX
 	help
@@ -45,7 +47,8 @@ config PINCTRL_IMX25
 
 config PINCTRL_IMX35
 	bool "IMX35 pinctrl driver"
-	depends on SOC_IMX35
+	depends on OF
+	depends on SOC_IMX35 || COMPILE_TEST
 	default SOC_IMX35
 	select PINCTRL_IMX
 	help
@@ -53,7 +56,8 @@ config PINCTRL_IMX35
 
 config PINCTRL_IMX50
 	bool "IMX50 pinctrl driver"
-	depends on SOC_IMX50
+	depends on OF
+	depends on SOC_IMX50 || COMPILE_TEST
 	default SOC_IMX50
 	select PINCTRL_IMX
 	help
@@ -61,7 +65,8 @@ config PINCTRL_IMX50
 
 config PINCTRL_IMX51
 	bool "IMX51 pinctrl driver"
-	depends on SOC_IMX51
+	depends on OF
+	depends on SOC_IMX51 || COMPILE_TEST
 	default SOC_IMX51
 	select PINCTRL_IMX
 	help
@@ -69,7 +74,8 @@ config PINCTRL_IMX51
 
 config PINCTRL_IMX53
 	bool "IMX53 pinctrl driver"
-	depends on SOC_IMX53
+	depends on OF
+	depends on SOC_IMX53 || COMPILE_TEST
 	default SOC_IMX53
 	select PINCTRL_IMX
 	help
@@ -77,7 +83,8 @@ config PINCTRL_IMX53
 
 config PINCTRL_IMX6Q
 	bool "IMX6Q/DL pinctrl driver"
-	depends on SOC_IMX6Q
+	depends on OF
+	depends on SOC_IMX6Q || COMPILE_TEST
 	default SOC_IMX6Q
 	select PINCTRL_IMX
 	help
@@ -85,7 +92,8 @@ config PINCTRL_IMX6Q
 
 config PINCTRL_IMX6SL
 	bool "IMX6SL pinctrl driver"
-	depends on SOC_IMX6SL
+	depends on OF
+	depends on SOC_IMX6SL || COMPILE_TEST
 	default SOC_IMX6SL
 	select PINCTRL_IMX
 	help
@@ -93,7 +101,8 @@ config PINCTRL_IMX6SL
 
 config PINCTRL_IMX6SLL
 	bool "IMX6SLL pinctrl driver"
-	depends on SOC_IMX6SLL
+	depends on OF
+	depends on SOC_IMX6SLL || COMPILE_TEST
 	default SOC_IMX6SLL
 	select PINCTRL_IMX
 	help
@@ -101,7 +110,8 @@ config PINCTRL_IMX6SLL
 
 config PINCTRL_IMX6SX
 	bool "IMX6SX pinctrl driver"
-	depends on SOC_IMX6SX
+	depends on OF
+	depends on SOC_IMX6SX || COMPILE_TEST
 	default SOC_IMX6SX
 	select PINCTRL_IMX
 	help
@@ -109,7 +119,8 @@ config PINCTRL_IMX6SX
 
 config PINCTRL_IMX6UL
 	bool "IMX6UL pinctrl driver"
-	depends on SOC_IMX6UL
+	depends on OF
+	depends on SOC_IMX6UL || COMPILE_TEST
 	default SOC_IMX6UL
 	select PINCTRL_IMX
 	help
@@ -117,7 +128,8 @@ config PINCTRL_IMX6UL
 
 config PINCTRL_IMX7D
 	bool "IMX7D pinctrl driver"
-	depends on SOC_IMX7D
+	depends on OF
+	depends on SOC_IMX7D || COMPILE_TEST
 	default SOC_IMX7D
 	select PINCTRL_IMX
 	help
@@ -125,7 +137,8 @@ config PINCTRL_IMX7D
 
 config PINCTRL_IMX7ULP
 	bool "IMX7ULP pinctrl driver"
-	depends on SOC_IMX7ULP
+	depends on OF
+	depends on SOC_IMX7ULP || COMPILE_TEST
 	default SOC_IMX7ULP
 	select PINCTRL_IMX
 	help
@@ -134,7 +147,7 @@ config PINCTRL_IMX7ULP
 config PINCTRL_IMX8MM
 	tristate "IMX8MM pinctrl driver"
 	depends on OF
-	depends on SOC_IMX8M
+	depends on SOC_IMX8M || COMPILE_TEST
 	select PINCTRL_IMX
 	help
 	  Say Y here to enable the imx8mm pinctrl driver
@@ -142,7 +155,7 @@ config PINCTRL_IMX8MM
 config PINCTRL_IMX8MN
 	tristate "IMX8MN pinctrl driver"
 	depends on OF
-	depends on SOC_IMX8M
+	depends on SOC_IMX8M || COMPILE_TEST
 	select PINCTRL_IMX
 	help
 	  Say Y here to enable the imx8mn pinctrl driver
@@ -150,7 +163,7 @@ config PINCTRL_IMX8MN
 config PINCTRL_IMX8MP
 	tristate "IMX8MP pinctrl driver"
 	depends on OF
-	depends on SOC_IMX8M
+	depends on SOC_IMX8M || COMPILE_TEST
 	select PINCTRL_IMX
 	help
 	  Say Y here to enable the imx8mp pinctrl driver
@@ -158,42 +171,47 @@ config PINCTRL_IMX8MP
 config PINCTRL_IMX8MQ
 	tristate "IMX8MQ pinctrl driver"
 	depends on OF
-	depends on SOC_IMX8M
+	depends on SOC_IMX8M || COMPILE_TEST
 	select PINCTRL_IMX
 	help
 	  Say Y here to enable the imx8mq pinctrl driver
 
 config PINCTRL_IMX8QM
 	tristate "IMX8QM pinctrl driver"
-	depends on IMX_SCU && ARCH_MXC && ARM64
+	depends on OF
+	depends on (IMX_SCU && ARCH_MXC && ARM64) || COMPILE_TEST
 	select PINCTRL_IMX_SCU
 	help
 	  Say Y here to enable the imx8qm pinctrl driver
 
 config PINCTRL_IMX8QXP
 	tristate "IMX8QXP pinctrl driver"
-	depends on IMX_SCU && ARCH_MXC && ARM64
+	depends on OF
+	depends on (IMX_SCU && ARCH_MXC && ARM64) || COMPILE_TEST
 	select PINCTRL_IMX_SCU
 	help
 	  Say Y here to enable the imx8qxp pinctrl driver
 
 config PINCTRL_IMX8DXL
 	tristate "IMX8DXL pinctrl driver"
-	depends on IMX_SCU && ARCH_MXC && ARM64
+	depends on OF
+	depends on (IMX_SCU && ARCH_MXC && ARM64) || COMPILE_TEST
 	select PINCTRL_IMX_SCU
 	help
 	  Say Y here to enable the imx8dxl pinctrl driver
 
 config PINCTRL_IMX8ULP
 	tristate "IMX8ULP pinctrl driver"
-	depends on ARCH_MXC
+	depends on OF
+	depends on ARCH_MXC || COMPILE_TEST
 	select PINCTRL_IMX
 	help
 	  Say Y here to enable the imx8ulp pinctrl driver
 
 config PINCTRL_IMXRT1050
 	bool "IMXRT1050 pinctrl driver"
-	depends on SOC_IMXRT
+	depends on OF
+	depends on SOC_IMXRT || COMPILE_TEST
 	default SOC_IMXRT
 	select PINCTRL_IMX
 	help
@@ -201,14 +219,16 @@ config PINCTRL_IMXRT1050
 
 config PINCTRL_IMX93
 	tristate "IMX93 pinctrl driver"
-	depends on ARCH_MXC
+	depends on OF
+	depends on ARCH_MXC || COMPILE_TEST
 	select PINCTRL_IMX
 	help
 	  Say Y here to enable the imx93 pinctrl driver
 
 config PINCTRL_VF610
 	bool "Freescale Vybrid VF610 pinctrl driver"
-	depends on SOC_VF610
+	depends on OF
+	depends on SOC_VF610 || COMPILE_TEST
 	default SOC_VF610
 	select PINCTRL_IMX
 	help
@@ -229,7 +249,8 @@ config PINCTRL_IMX28
 
 config PINCTRL_IMXRT1170
 	bool "IMXRT1170 pinctrl driver"
-	depends on SOC_IMXRT
+	depends on OF
+	depends on SOC_IMXRT || COMPILE_TEST
 	select PINCTRL_IMX
 	help
 	  Say Y here to enable the imxrt1170 pinctrl driver

-- 
2.45.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 2/3] pinctrl: freescale: Use CONFIG_SOC_IMXRT to guard i.MX RT1xxx drivers
  2024-05-06 10:23 ` [PATCH v2 2/3] pinctrl: freescale: Use CONFIG_SOC_IMXRT to guard i.MX RT1xxx drivers Esben Haabendal
@ 2024-05-06 11:29   ` Arnd Bergmann
  2024-05-07  7:37     ` Esben Haabendal
  0 siblings, 1 reply; 25+ messages in thread
From: Arnd Bergmann @ 2024-05-06 11:29 UTC (permalink / raw)
  To: Esben Haabendal, Russell King, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Dong Aisheng, Jacky Bai,
	Linus Walleij
  Cc: Rasmus Villemoes, linux-arm-kernel, imx, linux-kernel,
	open list:GPIO SUBSYSTEM

On Mon, May 6, 2024, at 12:23, Esben Haabendal wrote:
> 
>  config PINCTRL_IMXRT1050
>  	bool "IMXRT1050 pinctrl driver"
> -	depends on ARCH_MXC
> +	depends on SOC_IMXRT
> +	default SOC_IMXRT
>  	select PINCTRL_IMX
>  	help
>  	  Say Y here to enable the imxrt1050 pinctrl driver

Maybe make this

       depends on SOC_IMXRT || COMPILE_TEST

I see that all the i.MX pinctrl drivers are currently missing
this, but a lot of other platforms have the ||COMPILE_TEST 
bit so it gets included in x86 allmodconfig tests that
often gets run before sending or merging changes.

    Arnd

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 2/3] pinctrl: freescale: Use CONFIG_SOC_IMXRT to guard i.MX RT1xxx drivers
  2024-05-06 11:29   ` Arnd Bergmann
@ 2024-05-07  7:37     ` Esben Haabendal
  2024-05-07  7:43       ` Arnd Bergmann
  0 siblings, 1 reply; 25+ messages in thread
From: Esben Haabendal @ 2024-05-07  7:37 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Russell King, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Dong Aisheng, Jacky Bai, Linus Walleij,
	Rasmus Villemoes, linux-arm-kernel, imx, linux-kernel,
	open list:GPIO SUBSYSTEM

"Arnd Bergmann" <arnd@arndb.de> writes:

> On Mon, May 6, 2024, at 12:23, Esben Haabendal wrote:
>>
>>  config PINCTRL_IMXRT1050
>>  	bool "IMXRT1050 pinctrl driver"
>> -	depends on ARCH_MXC
>> +	depends on SOC_IMXRT
>> +	default SOC_IMXRT
>>  	select PINCTRL_IMX
>>  	help
>>  	  Say Y here to enable the imxrt1050 pinctrl driver
>
> Maybe make this
>
>        depends on SOC_IMXRT || COMPILE_TEST

That is done in patch 3/3.

> I see that all the i.MX pinctrl drivers are currently missing
> this, but a lot of other platforms have the ||COMPILE_TEST
> bit so it gets included in x86 allmodconfig tests that
> often gets run before sending or merging changes.

Take a look at patch 3/3 in this series. It does a wholesale addition of
||COMPILE_TEST to these drivers.

/Esben

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 2/3] pinctrl: freescale: Use CONFIG_SOC_IMXRT to guard i.MX RT1xxx drivers
  2024-05-07  7:37     ` Esben Haabendal
@ 2024-05-07  7:43       ` Arnd Bergmann
  0 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2024-05-07  7:43 UTC (permalink / raw)
  To: Esben Haabendal
  Cc: Russell King, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Dong Aisheng, Jacky Bai, Linus Walleij,
	Rasmus Villemoes, linux-arm-kernel, imx, linux-kernel,
	open list:GPIO SUBSYSTEM

On Tue, May 7, 2024, at 09:37, Esben Haabendal wrote:
> "Arnd Bergmann" <arnd@arndb.de> writes:
>
>> On Mon, May 6, 2024, at 12:23, Esben Haabendal wrote:
>>>
>>>  config PINCTRL_IMXRT1050
>>>  	bool "IMXRT1050 pinctrl driver"
>>> -	depends on ARCH_MXC
>>> +	depends on SOC_IMXRT
>>> +	default SOC_IMXRT
>>>  	select PINCTRL_IMX
>>>  	help
>>>  	  Say Y here to enable the imxrt1050 pinctrl driver
>>
>> Maybe make this
>>
>>        depends on SOC_IMXRT || COMPILE_TEST
>
> That is done in patch 3/3.
>
>> I see that all the i.MX pinctrl drivers are currently missing
>> this, but a lot of other platforms have the ||COMPILE_TEST
>> bit so it gets included in x86 allmodconfig tests that
>> often gets run before sending or merging changes.
>
> Take a look at patch 3/3 in this series. It does a wholesale addition of
> ||COMPILE_TEST to these drivers.

Ok, great! Sorry I missed that bit. Whole series

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 0/3] ARM: imx: only enable pinctrl as needed
  2024-05-06 10:23 [PATCH v2 0/3] ARM: imx: only enable pinctrl as needed Esben Haabendal
                   ` (2 preceding siblings ...)
  2024-05-06 10:23 ` [PATCH v2 3/3] pinctrl: freescale: enable use with COMPILE_TEST Esben Haabendal
@ 2024-05-27 11:05 ` Linus Walleij
  2024-10-01 11:55   ` Linus Walleij
  3 siblings, 1 reply; 25+ messages in thread
From: Linus Walleij @ 2024-05-27 11:05 UTC (permalink / raw)
  To: Esben Haabendal
  Cc: Russell King, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Dong Aisheng, Jacky Bai, Arnd Bergmann,
	Rasmus Villemoes, linux-arm-kernel, imx, linux-kernel, linux-gpio

On Mon, May 6, 2024 at 12:24 PM Esben Haabendal <esben@geanix.com> wrote:

> As not all mach-imx platforms has support for run-time changes of pin
> configurations (such as LS1021A), a more selective approach to enabling
> pinctrl infrastructure makes sense, so that an e.g. an LS1021A only kernel
> could be built without pinctrl support.
>
> This is a very late follow up v1 3 years ago [1]. The situation seems to be
> unchanged since then, and I have tried to incorporate the requested
> changes.

What is the verdict from the i.MX pin control maintainers on this?

I can merge this into the pin control tree for v6.11 but it'd be nice
to get an ACK from the maintainers first.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl
  2024-05-06 10:23 ` [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl Esben Haabendal
@ 2024-06-03  2:50   ` Shawn Guo
  2024-09-13  7:21     ` Esben Haabendal
  2024-09-24  7:45   ` Linus Walleij
  2024-11-26 16:17   ` Guenter Roeck
  2 siblings, 1 reply; 25+ messages in thread
From: Shawn Guo @ 2024-06-03  2:50 UTC (permalink / raw)
  To: Esben Haabendal
  Cc: Russell King, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Dong Aisheng, Jacky Bai, Linus Walleij,
	Arnd Bergmann, Rasmus Villemoes, linux-arm-kernel, imx,
	linux-kernel, linux-gpio

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>


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl
  2024-06-03  2:50   ` Shawn Guo
@ 2024-09-13  7:21     ` Esben Haabendal
  0 siblings, 0 replies; 25+ messages in thread
From: Esben Haabendal @ 2024-09-13  7:21 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Russell King, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Dong Aisheng, Jacky Bai, Linus Walleij,
	Arnd Bergmann, Rasmus Villemoes, linux-arm-kernel, imx,
	linux-kernel, linux-gpio

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl
  2024-05-06 10:23 ` [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl Esben Haabendal
  2024-06-03  2:50   ` Shawn Guo
@ 2024-09-24  7:45   ` Linus Walleij
  2024-09-26  7:28     ` Esben Haabendal
  2024-09-26  8:40     ` Shawn Guo
  2024-11-26 16:17   ` Guenter Roeck
  2 siblings, 2 replies; 25+ messages in thread
From: Linus Walleij @ 2024-09-24  7:45 UTC (permalink / raw)
  To: Esben Haabendal
  Cc: Russell King, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Dong Aisheng, Jacky Bai, Arnd Bergmann,
	Rasmus Villemoes, linux-arm-kernel, imx, linux-kernel, linux-gpio

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl
  2024-09-24  7:45   ` Linus Walleij
@ 2024-09-26  7:28     ` Esben Haabendal
  2024-09-26  8:40     ` Shawn Guo
  1 sibling, 0 replies; 25+ messages in thread
From: Esben Haabendal @ 2024-09-26  7:28 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Russell King, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Dong Aisheng, Jacky Bai, Arnd Bergmann,
	Rasmus Villemoes, linux-arm-kernel, imx, linux-kernel, linux-gpio

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl
  2024-09-24  7:45   ` Linus Walleij
  2024-09-26  7:28     ` Esben Haabendal
@ 2024-09-26  8:40     ` Shawn Guo
  1 sibling, 0 replies; 25+ messages in thread
From: Shawn Guo @ 2024-09-26  8:40 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Esben Haabendal, Russell King, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Dong Aisheng, Jacky Bai,
	Arnd Bergmann, Rasmus Villemoes, linux-arm-kernel, imx,
	linux-kernel, linux-gpio

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/


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 0/3] ARM: imx: only enable pinctrl as needed
  2024-05-27 11:05 ` [PATCH v2 0/3] ARM: imx: only enable pinctrl as needed Linus Walleij
@ 2024-10-01 11:55   ` Linus Walleij
  0 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2024-10-01 11:55 UTC (permalink / raw)
  To: Esben Haabendal
  Cc: Russell King, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Dong Aisheng, Jacky Bai, Arnd Bergmann,
	Rasmus Villemoes, linux-arm-kernel, imx, linux-kernel, linux-gpio

On Mon, May 27, 2024 at 1:05 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, May 6, 2024 at 12:24 PM Esben Haabendal <esben@geanix.com> wrote:
>
> > As not all mach-imx platforms has support for run-time changes of pin
> > configurations (such as LS1021A), a more selective approach to enabling
> > pinctrl infrastructure makes sense, so that an e.g. an LS1021A only kernel
> > could be built without pinctrl support.
> >
> > This is a very late follow up v1 3 years ago [1]. The situation seems to be
> > unchanged since then, and I have tried to incorporate the requested
> > changes.
>
> What is the verdict from the i.MX pin control maintainers on this?
>
> I can merge this into the pin control tree for v6.11 but it'd be nice
> to get an ACK from the maintainers first.

I ran out of patience and Shawn ACKed patch 1/3 so patches
applied.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl
  2024-05-06 10:23 ` [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl Esben Haabendal
  2024-06-03  2:50   ` Shawn Guo
  2024-09-24  7:45   ` Linus Walleij
@ 2024-11-26 16:17   ` Guenter Roeck
  2024-11-26 21:24     ` Linus Walleij
  2 siblings, 1 reply; 25+ messages in thread
From: Guenter Roeck @ 2024-11-26 16:17 UTC (permalink / raw)
  To: Esben Haabendal
  Cc: Russell King, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Dong Aisheng, Jacky Bai, Linus Walleij,
	Arnd Bergmann, Rasmus Villemoes, linux-arm-kernel, imx,
	linux-kernel, linux-gpio

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl
  2024-11-26 16:17   ` Guenter Roeck
@ 2024-11-26 21:24     ` Linus Walleij
  2024-11-26 23:53       ` Guenter Roeck
  0 siblings, 1 reply; 25+ messages in thread
From: Linus Walleij @ 2024-11-26 21:24 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Esben Haabendal, Russell King, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Dong Aisheng, Jacky Bai,
	Arnd Bergmann, Rasmus Villemoes, linux-arm-kernel, imx,
	linux-kernel, linux-gpio

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl
  2024-11-26 21:24     ` Linus Walleij
@ 2024-11-26 23:53       ` Guenter Roeck
  2024-11-27  0:12         ` Fabio Estevam
  0 siblings, 1 reply; 25+ messages in thread
From: Guenter Roeck @ 2024-11-26 23:53 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Esben Haabendal, Russell King, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Dong Aisheng, Jacky Bai,
	Arnd Bergmann, Rasmus Villemoes, linux-arm-kernel, imx,
	linux-kernel, linux-gpio, Fabio Estevam

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl
  2024-11-26 23:53       ` Guenter Roeck
@ 2024-11-27  0:12         ` Fabio Estevam
  2024-11-27  6:59           ` Arnd Bergmann
  2024-11-27  9:13           ` Rasmus Villemoes
  0 siblings, 2 replies; 25+ messages in thread
From: Fabio Estevam @ 2024-11-27  0:12 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Linus Walleij, Esben Haabendal, Russell King, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Dong Aisheng, Jacky Bai,
	Arnd Bergmann, Rasmus Villemoes, linux-arm-kernel, imx,
	linux-kernel, linux-gpio, Fabio Estevam

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.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl
  2024-11-27  0:12         ` Fabio Estevam
@ 2024-11-27  6:59           ` Arnd Bergmann
  2024-11-27  9:13           ` Rasmus Villemoes
  1 sibling, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2024-11-27  6:59 UTC (permalink / raw)
  To: Fabio Estevam, Guenter Roeck
  Cc: Linus Walleij, Esben Haabendal, Russell King, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Dong Aisheng, Jacky Bai,
	Rasmus Villemoes, linux-arm-kernel, imx, linux-kernel,
	open list:GPIO SUBSYSTEM, Fabio Estevam

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

How about moving CONFIG_SOC_LS1021A out of arch/arm/mach-imx/Kconfig
entirely? As far as I can tell, the only file that is even
compiled in there is arch/arm/mach-imx/mach-ls1021a.c, and that
one is not actually required.

I think we can just have an ARCH_LAYERSCAPE entry in
arch/arm/Kconfig.platforms instead, or maybe keep it in
mach-imx/Kconfig but drop the CONFIG_IMX dependency.

The only thing that imx and layerscape seem to share is
the platsmp.c file, which has code for both, but nothing
in there seems shared. Maybe layerscape can just use
enable-method="psci" anyway, if that is implemented by
the firmware?

       Arnd

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl
  2024-11-27  0:12         ` Fabio Estevam
  2024-11-27  6:59           ` Arnd Bergmann
@ 2024-11-27  9:13           ` Rasmus Villemoes
  2024-11-27  9:18             ` Arnd Bergmann
  1 sibling, 1 reply; 25+ messages in thread
From: Rasmus Villemoes @ 2024-11-27  9:13 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Guenter Roeck, Linus Walleij, Esben Haabendal, Russell King,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Dong Aisheng,
	Jacky Bai, Arnd Bergmann, linux-arm-kernel, imx, linux-kernel,
	linux-gpio, Fabio Estevam

On Tue, Nov 26 2024, Fabio Estevam <festevam@gmail.com> wrote:

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

Yup.

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

Instead of doing that, isn't this exactly what the 'imply' keyword is
for?

- weak reverse dependencies: "imply" <symbol> ["if" <expr>]

  This is similar to "select" as it enforces a lower limit on another
  symbol except that the "implied" symbol's value may still be set to n
  from a direct dependency or with a visible prompt.


So how about adding 'imply PINCTRL' in lieu of the previous 'select
PINCTRL'? And that would also better match the intention of the patch in
question (namely that the user needs to take explicit action to disable
PINCTRL).

Rasmus

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl
  2024-11-27  9:13           ` Rasmus Villemoes
@ 2024-11-27  9:18             ` Arnd Bergmann
  2024-11-27 10:13               ` Rasmus Villemoes
  0 siblings, 1 reply; 25+ messages in thread
From: Arnd Bergmann @ 2024-11-27  9:18 UTC (permalink / raw)
  To: Rasmus Villemoes, Fabio Estevam
  Cc: Guenter Roeck, Linus Walleij, Esben Haabendal, Russell King,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Dong Aisheng,
	Jacky Bai, linux-arm-kernel, imx, linux-kernel,
	open list:GPIO SUBSYSTEM, Fabio Estevam

On Wed, Nov 27, 2024, at 10:13, Rasmus Villemoes wrote:
> On Tue, Nov 26 2024, Fabio Estevam <festevam@gmail.com> wrote:
>>> 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.
>>
>
> Instead of doing that, isn't this exactly what the 'imply' keyword is
> for?
>
> - weak reverse dependencies: "imply" <symbol> ["if" <expr>]
>
>   This is similar to "select" as it enforces a lower limit on another
>   symbol except that the "implied" symbol's value may still be set to n
>   from a direct dependency or with a visible prompt.
>
>
> So how about adding 'imply PINCTRL' in lieu of the previous 'select
> PINCTRL'? And that would also better match the intention of the patch in
> question (namely that the user needs to take explicit action to disable
> PINCTRL).

Please never use imply. Even if you think it's the right
thing in a particular case, it will come back to bite you
later.

See also https://en.wikipedia.org/wiki/COMEFROM ;-)

I would prefer we completely kill off that keyword from the Kconfig
language and replace it with the reverse 'default'. In this
particular case, having 'default ARCH_IMX' in 'PINCTRL'
would of course not be a great idea, but for the exact same
reason, the 'imply' is wrong here.

       Arnd

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl
  2024-11-27  9:18             ` Arnd Bergmann
@ 2024-11-27 10:13               ` Rasmus Villemoes
  2024-11-27 11:29                 ` Arnd Bergmann
  0 siblings, 1 reply; 25+ messages in thread
From: Rasmus Villemoes @ 2024-11-27 10:13 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Fabio Estevam, Guenter Roeck, Linus Walleij, Esben Haabendal,
	Russell King, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Dong Aisheng, Jacky Bai, linux-arm-kernel, imx, linux-kernel,
	open list:GPIO SUBSYSTEM, Fabio Estevam

On Wed, Nov 27 2024, "Arnd Bergmann" <arnd@arndb.de> wrote:

> On Wed, Nov 27, 2024, at 10:13, Rasmus Villemoes wrote:
>> On Tue, Nov 26 2024, Fabio Estevam <festevam@gmail.com> wrote:
>>>> 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.
>>>
>>
>> Instead of doing that, isn't this exactly what the 'imply' keyword is
>> for?
>>
>> - weak reverse dependencies: "imply" <symbol> ["if" <expr>]
>>
>>   This is similar to "select" as it enforces a lower limit on another
>>   symbol except that the "implied" symbol's value may still be set to n
>>   from a direct dependency or with a visible prompt.
>>
>>
>> So how about adding 'imply PINCTRL' in lieu of the previous 'select
>> PINCTRL'? And that would also better match the intention of the patch in
>> question (namely that the user needs to take explicit action to disable
>> PINCTRL).
>
> Please never use imply. Even if you think it's the right
> thing in a particular case, it will come back to bite you
> later.

Could you elaborate?

> See also https://en.wikipedia.org/wiki/COMEFROM ;-)

Yes yes, we've probably all seen that at some point and chuckled, but I
fail to see why imply would be worse than select.

> I would prefer we completely kill off that keyword from the Kconfig
> language and replace it with the reverse 'default'. In this
> particular case, having 'default ARCH_IMX' in 'PINCTRL'
> would of course not be a great idea,

Just to be clear, it would be 'default y if ARCH_MXC', not 'default
ARCH_IMX', right? Yes, given that the PINCTRL entry doesn't currently
have a list of "default y if ...", it would seem inappropriate.

> but for the exact same reason, the 'imply' is wrong here.

I don't follow, sorry.

Rasmus

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl
  2024-11-27 10:13               ` Rasmus Villemoes
@ 2024-11-27 11:29                 ` Arnd Bergmann
  2024-11-27 14:36                   ` Fabio Estevam
  0 siblings, 1 reply; 25+ messages in thread
From: Arnd Bergmann @ 2024-11-27 11:29 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Fabio Estevam, Guenter Roeck, Linus Walleij, Esben Haabendal,
	Russell King, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Dong Aisheng, Jacky Bai, linux-arm-kernel, imx, linux-kernel,
	open list:GPIO SUBSYSTEM, Fabio Estevam

On Wed, Nov 27, 2024, at 11:13, Rasmus Villemoes wrote:
> On Wed, Nov 27 2024, "Arnd Bergmann" <arnd@arndb.de> wrote:
>> On Wed, Nov 27, 2024, at 10:13, Rasmus Villemoes wrote:
>>> On Tue, Nov 26 2024, Fabio Estevam <festevam@gmail.com> wrote:
>>
>> Please never use imply. Even if you think it's the right
>> thing in a particular case, it will come back to bite you
>> later.
>
> Could you elaborate?
>
>> See also https://en.wikipedia.org/wiki/COMEFROM ;-)
>
> Yes yes, we've probably all seen that at some point and chuckled, but I
> fail to see why imply would be worse than select.

There are multiple problems here that I conflated, let
me try to explain better:

- The patch here replaces a hard 'select' with a softer 'default'
  for the i.MX drivers. Both variants are used in multiple
  pinctrl drivers, and there are sensible arguments to go
  one way or another. However, mixing 'select' and 'default'
  on a given platform would be wrong, and my point here is
  that mixing 'imply' and 'default' on a single platform is
  just as wrong, specifically because of the COMEFROM issue:
  even if it all works as intended, a reader will have a hard
  time figuring out why exactly it works like this, and this
  likely leads to bugs in the future.

- In the more common cases I've seen, the use of 'imply' is
  itself a bug, usually developers pick it because there is a
  hard dependency between two drivers, but using 'select'
  causes build issues, either from broken Kconfig constraints
  or from link failures. I tend to find out about them when
  a randconfig build still runs into the link failure and
  the 'imply' just made it less likely to happen.

>> I would prefer we completely kill off that keyword from the Kconfig
>> language and replace it with the reverse 'default'. In this
>> particular case, having 'default ARCH_IMX' in 'PINCTRL'
>> would of course not be a great idea,
>
> Just to be clear, it would be 'default y if ARCH_MXC', not 'default
> ARCH_IMX', right?

Either one, the two statements have the same effect in this case
since both ARCH_IMX and PINCTRL are 'bool' symbols.

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.

On the other hand, removing all 'select PINCTRL' and instead
using 'default' consistently may be a good idea for the
long run, especially if we want to do the same for clk, irqchip,
timer, regulator etc. At the moment, we have an arbitrary
cutoff where some subsystems are always considered essential,
while others are always considered optional and some are in
the middle even if turning them off still makes the system
unusable.

       Arnd

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl
  2024-11-27 11:29                 ` Arnd Bergmann
@ 2024-11-27 14:36                   ` Fabio Estevam
  2024-11-28  7:32                     ` Esben Haabendal
  0 siblings, 1 reply; 25+ messages in thread
From: Fabio Estevam @ 2024-11-27 14:36 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Rasmus Villemoes, Guenter Roeck, Linus Walleij, Esben Haabendal,
	Russell King, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Dong Aisheng, Jacky Bai, linux-arm-kernel, imx, linux-kernel,
	open list:GPIO SUBSYSTEM, Fabio Estevam

On Wed, Nov 27, 2024 at 8:30 AM Arnd Bergmann <arnd@arndb.de> wrote:

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

The suggestion to make Layerscape independent of i.MX makes sense, but
I don't know if it can be safely applied in 6.13-rc.

This proposed change also has the risk of causing regressions.

What if we revert the patch in Subject for now and then someone (maybe
Esben) tries again for a proper fix targeting 6.14?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl
  2024-11-27 14:36                   ` Fabio Estevam
@ 2024-11-28  7:32                     ` Esben Haabendal
  0 siblings, 0 replies; 25+ messages in thread
From: Esben Haabendal @ 2024-11-28  7:32 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Arnd Bergmann, Rasmus Villemoes, Guenter Roeck, Linus Walleij,
	Russell King, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Dong Aisheng, Jacky Bai, linux-arm-kernel, imx, linux-kernel,
	open list:GPIO SUBSYSTEM, Fabio Estevam

Fabio Estevam <festevam@gmail.com> writes:

> On Wed, Nov 27, 2024 at 8:30 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
>> 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.
>
> The suggestion to make Layerscape independent of i.MX makes sense, but
> I don't know if it can be safely applied in 6.13-rc.
>
> This proposed change also has the risk of causing regressions.
>
> What if we revert the patch in Subject for now and then someone (maybe
> Esben) tries again for a proper fix targeting 6.14?

What about your fix to the in-tree imx defconfigs [1]? Was it dropped/rejected?

[1]
https://lore.kernel.org/imx/d718ddd2-d473-4455-b21a-15024e46787c@roeck-us.net/T/#mc71dc21d99e0b013c5ce46c0d90940fd8806ae9a

/Esben

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2024-11-28  7:33 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-06 10:23 [PATCH v2 0/3] ARM: imx: only enable pinctrl as needed Esben Haabendal
2024-05-06 10:23 ` [PATCH v2 1/3] ARM: imx: Allow user to disable pinctrl Esben Haabendal
2024-06-03  2:50   ` Shawn Guo
2024-09-13  7:21     ` Esben Haabendal
2024-09-24  7:45   ` Linus Walleij
2024-09-26  7:28     ` Esben Haabendal
2024-09-26  8:40     ` Shawn Guo
2024-11-26 16:17   ` Guenter Roeck
2024-11-26 21:24     ` Linus Walleij
2024-11-26 23:53       ` Guenter Roeck
2024-11-27  0:12         ` Fabio Estevam
2024-11-27  6:59           ` Arnd Bergmann
2024-11-27  9:13           ` Rasmus Villemoes
2024-11-27  9:18             ` Arnd Bergmann
2024-11-27 10:13               ` Rasmus Villemoes
2024-11-27 11:29                 ` Arnd Bergmann
2024-11-27 14:36                   ` Fabio Estevam
2024-11-28  7:32                     ` Esben Haabendal
2024-05-06 10:23 ` [PATCH v2 2/3] pinctrl: freescale: Use CONFIG_SOC_IMXRT to guard i.MX RT1xxx drivers Esben Haabendal
2024-05-06 11:29   ` Arnd Bergmann
2024-05-07  7:37     ` Esben Haabendal
2024-05-07  7:43       ` Arnd Bergmann
2024-05-06 10:23 ` [PATCH v2 3/3] pinctrl: freescale: enable use with COMPILE_TEST Esben Haabendal
2024-05-27 11:05 ` [PATCH v2 0/3] ARM: imx: only enable pinctrl as needed Linus Walleij
2024-10-01 11:55   ` Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).