[PATCH V5 0/9] Support i.MX8 SoCs pinctrl drivers built as module

[PATCH V5 0/9] Support i.MX8 SoCs pinctrl drivers built as module

[Anson Huang]

There are more and mroe requirements that SoC specific modules should be built
as module in order to support generic kernel image, such as Android GKI concept.

This patch series supports i.MX8 SoCs pinctrl drivers to be built as module,
including i.MX8MQ/MM/MN/MP/QXP/QM/DXL SoCs, and it also supports building
i.MX common pinctrl driver and i.MX SCU common pinctrl driver as module.

Compared to V4, the changes are as below:
	- remove unnecessary changes of replacing arch_initcall() with
	  module_platform_driver() in each SoC pinctrl driver, to make
	  sure no probe sequence change for built-in config.
	- add module author and description to each module.

Anson Huang (9):
  pinctrl: imx: Support building SCU pinctrl driver as module
  pinctrl: imx: Support building i.MX pinctrl driver as module
  pinctrl: imx8mm: Support building as module
  pinctrl: imx8mn: Support building as module
  pinctrl: imx8mq: Support building as module
  pinctrl: imx8mp: Support building as module
  pinctrl: imx8qxp: Support building as module
  pinctrl: imx8qm: Support building as module
  pinctrl: imx8dxl: Support building as module

 drivers/pinctrl/freescale/Kconfig           | 19 +++++-----
 drivers/pinctrl/freescale/pinctrl-imx.c     | 25 ++++++++-----
 drivers/pinctrl/freescale/pinctrl-imx.h     | 57 ++++++++++++-----------------
 drivers/pinctrl/freescale/pinctrl-imx8dxl.c |  8 ++++
 drivers/pinctrl/freescale/pinctrl-imx8mm.c  |  6 +++
 drivers/pinctrl/freescale/pinctrl-imx8mn.c  |  6 +++
 drivers/pinctrl/freescale/pinctrl-imx8mp.c  |  6 +++
 drivers/pinctrl/freescale/pinctrl-imx8mq.c  |  6 +++
 drivers/pinctrl/freescale/pinctrl-imx8qm.c  |  8 ++++
 drivers/pinctrl/freescale/pinctrl-imx8qxp.c |  8 ++++
 drivers/pinctrl/freescale/pinctrl-scu.c     |  9 +++++
 11 files changed, 106 insertions(+), 52 deletions(-)

-- 
2.7.4

[PATCH V5 1/9] pinctrl: imx: Support building SCU pinctrl driver as module

[Anson Huang]

To support building i.MX SCU pinctrl driver as module, below things
need to be changed:

    - Export SCU related functions and use "IS_ENABLED" instead of
      "ifdef" to support SCU pinctrl driver user and itself to be
      built as module;
    - Use function callbacks for SCU related functions in pinctrl-imx.c
      in order to support the scenario of PINCTRL_IMX is built in
      while PINCTRL_IMX_SCU is built as module;
    - All drivers using SCU pinctrl driver need to initialize the
      SCU related function callback;
    - Change PINCTR_IMX_SCU to tristate;
    - Add module author, description and license.

With above changes, i.MX SCU pinctrl driver can be built as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V4:
	- add module author and description.
---
 drivers/pinctrl/freescale/Kconfig           |  2 +-
 drivers/pinctrl/freescale/pinctrl-imx.c     | 18 ++++-----
 drivers/pinctrl/freescale/pinctrl-imx.h     | 57 ++++++++++++-----------------
 drivers/pinctrl/freescale/pinctrl-imx8dxl.c |  3 ++
 drivers/pinctrl/freescale/pinctrl-imx8qm.c  |  3 ++
 drivers/pinctrl/freescale/pinctrl-imx8qxp.c |  3 ++
 drivers/pinctrl/freescale/pinctrl-scu.c     |  9 +++++
 7 files changed, 51 insertions(+), 44 deletions(-)

diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index 4ca44dd..a3a30f1d 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -7,7 +7,7 @@ config PINCTRL_IMX
 	select REGMAP
 
 config PINCTRL_IMX_SCU
-	bool
+	tristate "IMX SCU pinctrl driver"
 	depends on IMX_SCU
 	select PINCTRL_IMX
 
diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index cb7e0f0..c1faae1 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -372,8 +372,8 @@ static int imx_pinconf_get(struct pinctrl_dev *pctldev,
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
 	const struct imx_pinctrl_soc_info *info = ipctl->info;
 
-	if (info->flags & IMX_USE_SCU)
-		return imx_pinconf_get_scu(pctldev, pin_id, config);
+	if ((info->flags & IMX_USE_SCU) && info->imx_pinconf_get)
+		return info->imx_pinconf_get(pctldev, pin_id, config);
 	else
 		return imx_pinconf_get_mmio(pctldev, pin_id, config);
 }
@@ -422,8 +422,8 @@ static int imx_pinconf_set(struct pinctrl_dev *pctldev,
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
 	const struct imx_pinctrl_soc_info *info = ipctl->info;
 
-	if (info->flags & IMX_USE_SCU)
-		return imx_pinconf_set_scu(pctldev, pin_id,
+	if ((info->flags & IMX_USE_SCU) && info->imx_pinconf_set)
+		return info->imx_pinconf_set(pctldev, pin_id,
 					   configs, num_configs);
 	else
 		return imx_pinconf_set_mmio(pctldev, pin_id,
@@ -439,8 +439,8 @@ static void imx_pinconf_dbg_show(struct pinctrl_dev *pctldev,
 	unsigned long config;
 	int ret;
 
-	if (info->flags & IMX_USE_SCU) {
-		ret = imx_pinconf_get_scu(pctldev, pin_id, &config);
+	if ((info->flags & IMX_USE_SCU) && info->imx_pinconf_get) {
+		ret = info->imx_pinconf_get(pctldev, pin_id, &config);
 		if (ret) {
 			dev_err(ipctl->dev, "failed to get %s pinconf\n",
 				pin_get_name(pctldev, pin_id));
@@ -628,9 +628,9 @@ static int imx_pinctrl_parse_groups(struct device_node *np,
 
 	for (i = 0; i < grp->num_pins; i++) {
 		pin = &((struct imx_pin *)(grp->data))[i];
-		if (info->flags & IMX_USE_SCU)
-			imx_pinctrl_parse_pin_scu(ipctl, &grp->pins[i],
-						  pin, &list);
+		if ((info->flags & IMX_USE_SCU) && info->imx_pinctrl_parse_pin)
+			info->imx_pinctrl_parse_pin(ipctl, &grp->pins[i],
+						    pin, &list);
 		else
 			imx_pinctrl_parse_pin_mmio(ipctl, &grp->pins[i],
 						   pin, &list, np);
diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h b/drivers/pinctrl/freescale/pinctrl-imx.h
index 333d32b..bdb86c2 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.h
+++ b/drivers/pinctrl/freescale/pinctrl-imx.h
@@ -75,6 +75,21 @@ struct imx_cfg_params_decode {
 	bool invert;
 };
 
+/**
+ * @dev: a pointer back to containing device
+ * @base: the offset to the controller in virtual memory
+ */
+struct imx_pinctrl {
+	struct device *dev;
+	struct pinctrl_dev *pctl;
+	void __iomem *base;
+	void __iomem *input_sel_base;
+	const struct imx_pinctrl_soc_info *info;
+	struct imx_pin_reg *pin_regs;
+	unsigned int group_index;
+	struct mutex mutex;
+};
+
 struct imx_pinctrl_soc_info {
 	const struct pinctrl_pin_desc *pins;
 	unsigned int npins;
@@ -98,21 +113,13 @@ struct imx_pinctrl_soc_info {
 				  struct pinctrl_gpio_range *range,
 				  unsigned offset,
 				  bool input);
-};
-
-/**
- * @dev: a pointer back to containing device
- * @base: the offset to the controller in virtual memory
- */
-struct imx_pinctrl {
-	struct device *dev;
-	struct pinctrl_dev *pctl;
-	void __iomem *base;
-	void __iomem *input_sel_base;
-	const struct imx_pinctrl_soc_info *info;
-	struct imx_pin_reg *pin_regs;
-	unsigned int group_index;
-	struct mutex mutex;
+	int (*imx_pinconf_get)(struct pinctrl_dev *pctldev, unsigned int pin_id,
+			       unsigned long *config);
+	int (*imx_pinconf_set)(struct pinctrl_dev *pctldev, unsigned int pin_id,
+			       unsigned long *configs, unsigned int num_configs);
+	void (*imx_pinctrl_parse_pin)(struct imx_pinctrl *ipctl,
+				      unsigned int *pin_id, struct imx_pin *pin,
+				      const __be32 **list_p);
 };
 
 #define IMX_CFG_PARAMS_DECODE(p, m, o) \
@@ -137,7 +144,7 @@ struct imx_pinctrl {
 int imx_pinctrl_probe(struct platform_device *pdev,
 			const struct imx_pinctrl_soc_info *info);
 
-#ifdef CONFIG_PINCTRL_IMX_SCU
+#if IS_ENABLED(CONFIG_PINCTRL_IMX_SCU)
 #define BM_PAD_CTL_GP_ENABLE		BIT(30)
 #define BM_PAD_CTL_IFMUX_ENABLE		BIT(31)
 #define BP_PAD_CTL_IFMUX		27
@@ -150,23 +157,5 @@ int imx_pinconf_set_scu(struct pinctrl_dev *pctldev, unsigned pin_id,
 void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl,
 			       unsigned int *pin_id, struct imx_pin *pin,
 			       const __be32 **list_p);
-#else
-static inline int imx_pinconf_get_scu(struct pinctrl_dev *pctldev,
-				      unsigned pin_id, unsigned long *config)
-{
-	return -EINVAL;
-}
-static inline int imx_pinconf_set_scu(struct pinctrl_dev *pctldev,
-				      unsigned pin_id, unsigned long *configs,
-				      unsigned num_configs)
-{
-	return -EINVAL;
-}
-static inline void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl,
-					    unsigned int *pin_id,
-					    struct imx_pin *pin,
-					    const __be32 **list_p)
-{
-}
 #endif
 #endif /* __DRIVERS_PINCTRL_IMX_H */
diff --git a/drivers/pinctrl/freescale/pinctrl-imx8dxl.c b/drivers/pinctrl/freescale/pinctrl-imx8dxl.c
index 7f32e57..be3b09e 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx8dxl.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx8dxl.c
@@ -159,6 +159,9 @@ static struct imx_pinctrl_soc_info imx8dxl_pinctrl_info = {
 	.pins = imx8dxl_pinctrl_pads,
 	.npins = ARRAY_SIZE(imx8dxl_pinctrl_pads),
 	.flags = IMX_USE_SCU,
+	.imx_pinconf_get = imx_pinconf_get_scu,
+	.imx_pinconf_set = imx_pinconf_set_scu,
+	.imx_pinctrl_parse_pin = imx_pinctrl_parse_pin_scu,
 };
 
 static const struct of_device_id imx8dxl_pinctrl_of_match[] = {
diff --git a/drivers/pinctrl/freescale/pinctrl-imx8qm.c b/drivers/pinctrl/freescale/pinctrl-imx8qm.c
index 0b6029b..9ba3249 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx8qm.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx8qm.c
@@ -292,6 +292,9 @@ static const struct imx_pinctrl_soc_info imx8qm_pinctrl_info = {
 	.pins = imx8qm_pinctrl_pads,
 	.npins = ARRAY_SIZE(imx8qm_pinctrl_pads),
 	.flags = IMX_USE_SCU,
+	.imx_pinconf_get = imx_pinconf_get_scu,
+	.imx_pinconf_set = imx_pinconf_set_scu,
+	.imx_pinctrl_parse_pin = imx_pinctrl_parse_pin_scu,
 };
 
 static const struct of_device_id imx8qm_pinctrl_of_match[] = {
diff --git a/drivers/pinctrl/freescale/pinctrl-imx8qxp.c b/drivers/pinctrl/freescale/pinctrl-imx8qxp.c
index 1131dc3..05906b9 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx8qxp.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx8qxp.c
@@ -198,6 +198,9 @@ static struct imx_pinctrl_soc_info imx8qxp_pinctrl_info = {
 	.pins = imx8qxp_pinctrl_pads,
 	.npins = ARRAY_SIZE(imx8qxp_pinctrl_pads),
 	.flags = IMX_USE_SCU,
+	.imx_pinconf_get = imx_pinconf_get_scu,
+	.imx_pinconf_set = imx_pinconf_set_scu,
+	.imx_pinctrl_parse_pin = imx_pinctrl_parse_pin_scu,
 };
 
 static const struct of_device_id imx8qxp_pinctrl_of_match[] = {
diff --git a/drivers/pinctrl/freescale/pinctrl-scu.c b/drivers/pinctrl/freescale/pinctrl-scu.c
index 23cf04b..59b5f8a 100644
--- a/drivers/pinctrl/freescale/pinctrl-scu.c
+++ b/drivers/pinctrl/freescale/pinctrl-scu.c
@@ -7,6 +7,7 @@
 
 #include <linux/err.h>
 #include <linux/firmware/imx/sci.h>
+#include <linux/module.h>
 #include <linux/of_address.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/platform_device.h>
@@ -41,6 +42,7 @@ int imx_pinctrl_sc_ipc_init(struct platform_device *pdev)
 {
 	return imx_scu_get_handle(&pinctrl_ipc_handle);
 }
+EXPORT_SYMBOL_GPL(imx_pinctrl_sc_ipc_init);
 
 int imx_pinconf_get_scu(struct pinctrl_dev *pctldev, unsigned pin_id,
 			unsigned long *config)
@@ -66,6 +68,7 @@ int imx_pinconf_get_scu(struct pinctrl_dev *pctldev, unsigned pin_id,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(imx_pinconf_get_scu);
 
 int imx_pinconf_set_scu(struct pinctrl_dev *pctldev, unsigned pin_id,
 			unsigned long *configs, unsigned num_configs)
@@ -101,6 +104,7 @@ int imx_pinconf_set_scu(struct pinctrl_dev *pctldev, unsigned pin_id,
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(imx_pinconf_set_scu);
 
 void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl,
 			       unsigned int *pin_id, struct imx_pin *pin,
@@ -119,3 +123,8 @@ void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl,
 	dev_dbg(ipctl->dev, "%s: 0x%x 0x%08lx", info->pins[pin->pin].name,
 		pin_scu->mux_mode, pin_scu->config);
 }
+EXPORT_SYMBOL_GPL(imx_pinctrl_parse_pin_scu);
+
+MODULE_AUTHOR("Dong Aisheng <aisheng.dong@nxp.com>");
+MODULE_DESCRIPTION("NXP i.MX SCU common pinctrl driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH V5 2/9] pinctrl: imx: Support building i.MX pinctrl driver as module

[Anson Huang]

Export necessary functions to support building i.MX common pinctrl
driver and its user to be built as module.

i.MX common pinctrl driver should depend on CONFIG_OF to make sure
no build error when i.MX common pinctrl driver is enabled for difference
architectures without CONFIG_OF.

Also, module author, description and license should be added.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V4:
	- add module author and description.
---
 drivers/pinctrl/freescale/Kconfig       | 3 ++-
 drivers/pinctrl/freescale/pinctrl-imx.c | 7 +++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index a3a30f1d..e281c3f 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config PINCTRL_IMX
-	bool
+	tristate "IMX pinctrl driver"
+	depends on OF
 	select GENERIC_PINCTRL_GROUPS
 	select GENERIC_PINMUX_FUNCTIONS
 	select GENERIC_PINCONF
diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index c1faae1..28be1d6 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -11,6 +11,7 @@
 #include <linux/init.h>
 #include <linux/io.h>
 #include <linux/mfd/syscon.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/of_address.h>
@@ -878,6 +879,7 @@ int imx_pinctrl_probe(struct platform_device *pdev,
 
 	return pinctrl_enable(ipctl->pctl);
 }
+EXPORT_SYMBOL_GPL(imx_pinctrl_probe);
 
 static int __maybe_unused imx_pinctrl_suspend(struct device *dev)
 {
@@ -897,3 +899,8 @@ const struct dev_pm_ops imx_pinctrl_pm_ops = {
 	SET_LATE_SYSTEM_SLEEP_PM_OPS(imx_pinctrl_suspend,
 					imx_pinctrl_resume)
 };
+EXPORT_SYMBOL_GPL(imx_pinctrl_pm_ops);
+
+MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>");
+MODULE_DESCRIPTION("NXP i.MX common pinctrl driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH V5 3/9] pinctrl: imx8mm: Support building as module

[Anson Huang]

Change configuration to "tristate", add module device table,
author, description and license to support building i.MX8MM
pinctrl driver as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V4:
	- remove unnecessary change of replacing arch_initcall() with module_platform_driver();
	- add module author and description;
	- add more details to commit message.
---
 drivers/pinctrl/freescale/Kconfig          | 2 +-
 drivers/pinctrl/freescale/pinctrl-imx8mm.c | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index e281c3f..556adc3 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -125,7 +125,7 @@ config PINCTRL_IMX7ULP
 	  Say Y here to enable the imx7ulp pinctrl driver
 
 config PINCTRL_IMX8MM
-	bool "IMX8MM pinctrl driver"
+	tristate "IMX8MM pinctrl driver"
 	depends on ARCH_MXC
 	select PINCTRL_IMX
 	help
diff --git a/drivers/pinctrl/freescale/pinctrl-imx8mm.c b/drivers/pinctrl/freescale/pinctrl-imx8mm.c
index 6d1038a..31c5d88 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx8mm.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx8mm.c
@@ -5,6 +5,7 @@
 
 #include <linux/err.h>
 #include <linux/init.h>
+#include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/platform_device.h>
@@ -326,6 +327,7 @@ static const struct of_device_id imx8mm_pinctrl_of_match[] = {
 	{ .compatible = "fsl,imx8mm-iomuxc", .data = &imx8mm_pinctrl_info, },
 	{ /* sentinel */ }
 };
+MODULE_DEVICE_TABLE(of, imx8mm_pinctrl_of_match);
 
 static int imx8mm_pinctrl_probe(struct platform_device *pdev)
 {
@@ -346,3 +348,7 @@ static int __init imx8mm_pinctrl_init(void)
 	return platform_driver_register(&imx8mm_pinctrl_driver);
 }
 arch_initcall(imx8mm_pinctrl_init);
+
+MODULE_AUTHOR("Bai Ping <ping.bai@nxp.com>");
+MODULE_DESCRIPTION("NXP i.MX8MM pinctrl driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH V5 4/9] pinctrl: imx8mn: Support building as module

[Anson Huang]

Change configuration to "tristate", add module device table,
author, description and license to support building i.MX8MN
pinctrl driver as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V4:
	- remove unnecessary change of replacing arch_initcall() with module_platform_driver();
	- add module author and description;
	- add more details to commit message.
---
 drivers/pinctrl/freescale/Kconfig          | 2 +-
 drivers/pinctrl/freescale/pinctrl-imx8mn.c | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index 556adc3..fe3e49c 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -132,7 +132,7 @@ config PINCTRL_IMX8MM
 	  Say Y here to enable the imx8mm pinctrl driver
 
 config PINCTRL_IMX8MN
-	bool "IMX8MN pinctrl driver"
+	tristate "IMX8MN pinctrl driver"
 	depends on ARCH_MXC
 	select PINCTRL_IMX
 	help
diff --git a/drivers/pinctrl/freescale/pinctrl-imx8mn.c b/drivers/pinctrl/freescale/pinctrl-imx8mn.c
index 100ed8c..14c9deb 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx8mn.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx8mn.c
@@ -5,6 +5,7 @@
 
 #include <linux/err.h>
 #include <linux/init.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/platform_device.h>
@@ -326,6 +327,7 @@ static const struct of_device_id imx8mn_pinctrl_of_match[] = {
 	{ .compatible = "fsl,imx8mn-iomuxc", .data = &imx8mn_pinctrl_info, },
 	{ /* sentinel */ }
 };
+MODULE_DEVICE_TABLE(of, imx8mn_pinctrl_of_match);
 
 static int imx8mn_pinctrl_probe(struct platform_device *pdev)
 {
@@ -346,3 +348,7 @@ static int __init imx8mn_pinctrl_init(void)
 	return platform_driver_register(&imx8mn_pinctrl_driver);
 }
 arch_initcall(imx8mn_pinctrl_init);
+
+MODULE_AUTHOR("Anson Huang <Anson.Huang@nxp.com>");
+MODULE_DESCRIPTION("NXP i.MX8MN pinctrl driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH V5 5/9] pinctrl: imx8mq: Support building as module

[Anson Huang]

Change configuration to "tristate", add module device table,
author, description and license to support building i.MX8MQ
pinctrl driver as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V4:
	- remove unnecessary change of replacing arch_initcall() with module_platform_driver();
	- add module author and description;
	- add more details to commit message.
---
 drivers/pinctrl/freescale/Kconfig          | 2 +-
 drivers/pinctrl/freescale/pinctrl-imx8mq.c | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index fe3e49c..968c1e9 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -146,7 +146,7 @@ config PINCTRL_IMX8MP
 	  Say Y here to enable the imx8mp pinctrl driver
 
 config PINCTRL_IMX8MQ
-	bool "IMX8MQ pinctrl driver"
+	tristate "IMX8MQ pinctrl driver"
 	depends on ARCH_MXC
 	select PINCTRL_IMX
 	help
diff --git a/drivers/pinctrl/freescale/pinctrl-imx8mq.c b/drivers/pinctrl/freescale/pinctrl-imx8mq.c
index 50aa1c0..ae3ea5b 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx8mq.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx8mq.c
@@ -8,6 +8,7 @@
 #include <linux/err.h>
 #include <linux/init.h>
 #include <linux/io.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/pinctrl/pinctrl.h>
@@ -329,6 +330,7 @@ static const struct of_device_id imx8mq_pinctrl_of_match[] = {
 	{ .compatible = "fsl,imx8mq-iomuxc", .data = &imx8mq_pinctrl_info, },
 	{ /* sentinel */ }
 };
+MODULE_DEVICE_TABLE(of, imx8mq_pinctrl_of_match);
 
 static int imx8mq_pinctrl_probe(struct platform_device *pdev)
 {
@@ -350,3 +352,7 @@ static int __init imx8mq_pinctrl_init(void)
 	return platform_driver_register(&imx8mq_pinctrl_driver);
 }
 arch_initcall(imx8mq_pinctrl_init);
+
+MODULE_AUTHOR("Lucas Stach <l.stach@pengutronix.de>");
+MODULE_DESCRIPTION("NXP i.MX8MQ pinctrl driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH V5 6/9] pinctrl: imx8mp: Support building as module

[Anson Huang]

Change configuration to "tristate", add module device table,
author, description and license to support building i.MX8MP
pinctrl driver as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V4:
	- remove unnecessary change of replacing arch_initcall() with module_platform_driver();
	- add module author and description;
	- add more details to commit message.
---
 drivers/pinctrl/freescale/Kconfig          | 2 +-
 drivers/pinctrl/freescale/pinctrl-imx8mp.c | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index 968c1e9..af28f14 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -139,7 +139,7 @@ config PINCTRL_IMX8MN
 	  Say Y here to enable the imx8mn pinctrl driver
 
 config PINCTRL_IMX8MP
-	bool "IMX8MP pinctrl driver"
+	tristate "IMX8MP pinctrl driver"
 	depends on ARCH_MXC
 	select PINCTRL_IMX
 	help
diff --git a/drivers/pinctrl/freescale/pinctrl-imx8mp.c b/drivers/pinctrl/freescale/pinctrl-imx8mp.c
index e3f644c..bf4bbb5 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx8mp.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx8mp.c
@@ -5,6 +5,7 @@
 
 #include <linux/err.h>
 #include <linux/init.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/platform_device.h>
@@ -324,6 +325,7 @@ static const struct of_device_id imx8mp_pinctrl_of_match[] = {
 	{ .compatible = "fsl,imx8mp-iomuxc", .data = &imx8mp_pinctrl_info, },
 	{ /* sentinel */ }
 };
+MODULE_DEVICE_TABLE(of, imx8mp_pinctrl_of_match);
 
 static int imx8mp_pinctrl_probe(struct platform_device *pdev)
 {
@@ -343,3 +345,7 @@ static int __init imx8mp_pinctrl_init(void)
 	return platform_driver_register(&imx8mp_pinctrl_driver);
 }
 arch_initcall(imx8mp_pinctrl_init);
+
+MODULE_AUTHOR("Anson Huang <Anson.Huang@nxp.com>");
+MODULE_DESCRIPTION("NXP i.MX8MP pinctrl driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH V5 7/9] pinctrl: imx8qxp: Support building as module

[Anson Huang]

Change configuration to "tristate", add module device table,
author, description and license to support building i.MX8QXP
pinctrl driver as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V4:
	- remove unnecessary change of replacing arch_initcall() with module_platform_driver();
	- add module author and description;
	- add more details to commit message.
---
 drivers/pinctrl/freescale/Kconfig           | 2 +-
 drivers/pinctrl/freescale/pinctrl-imx8qxp.c | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index af28f14..412d581 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -160,7 +160,7 @@ config PINCTRL_IMX8QM
 	  Say Y here to enable the imx8qm pinctrl driver
 
 config PINCTRL_IMX8QXP
-	bool "IMX8QXP pinctrl driver"
+	tristate "IMX8QXP pinctrl driver"
 	depends on IMX_SCU && ARCH_MXC && ARM64
 	select PINCTRL_IMX_SCU
 	help
diff --git a/drivers/pinctrl/freescale/pinctrl-imx8qxp.c b/drivers/pinctrl/freescale/pinctrl-imx8qxp.c
index 05906b9..6776ad6 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx8qxp.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx8qxp.c
@@ -207,6 +207,7 @@ static const struct of_device_id imx8qxp_pinctrl_of_match[] = {
 	{ .compatible = "fsl,imx8qxp-iomuxc", },
 	{ /* sentinel */ }
 };
+MODULE_DEVICE_TABLE(of, imx8qxp_pinctrl_of_match);
 
 static int imx8qxp_pinctrl_probe(struct platform_device *pdev)
 {
@@ -233,3 +234,7 @@ static int __init imx8qxp_pinctrl_init(void)
 	return platform_driver_register(&imx8qxp_pinctrl_driver);
 }
 arch_initcall(imx8qxp_pinctrl_init);
+
+MODULE_AUTHOR("Aisheng Dong <aisheng.dong@nxp.com>");
+MODULE_DESCRIPTION("NXP i.MX8QXP pinctrl driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH V5 8/9] pinctrl: imx8qm: Support building as module

[Anson Huang]

Change configuration to "tristate", add module device table,
author, description and license to support building i.MX8QM
pinctrl driver as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V4:
	- remove unnecessary change of replacing arch_initcall() with module_platform_driver();
	- add module author and description;
	- add more details to commit message.
---
 drivers/pinctrl/freescale/Kconfig          | 2 +-
 drivers/pinctrl/freescale/pinctrl-imx8qm.c | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index 412d581..de8d571 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -153,7 +153,7 @@ config PINCTRL_IMX8MQ
 	  Say Y here to enable the imx8mq pinctrl driver
 
 config PINCTRL_IMX8QM
-	bool "IMX8QM pinctrl driver"
+	tristate "IMX8QM pinctrl driver"
 	depends on IMX_SCU && ARCH_MXC && ARM64
 	select PINCTRL_IMX_SCU
 	help
diff --git a/drivers/pinctrl/freescale/pinctrl-imx8qm.c b/drivers/pinctrl/freescale/pinctrl-imx8qm.c
index 9ba3249..8f46b940 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx8qm.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx8qm.c
@@ -301,6 +301,7 @@ static const struct of_device_id imx8qm_pinctrl_of_match[] = {
 	{ .compatible = "fsl,imx8qm-iomuxc", },
 	{ /* sentinel */ }
 };
+MODULE_DEVICE_TABLE(of, imx8qm_pinctrl_of_match);
 
 static int imx8qm_pinctrl_probe(struct platform_device *pdev)
 {
@@ -327,3 +328,7 @@ static int __init imx8qm_pinctrl_init(void)
 	return platform_driver_register(&imx8qm_pinctrl_driver);
 }
 arch_initcall(imx8qm_pinctrl_init);
+
+MODULE_AUTHOR("Aisheng Dong <aisheng.dong@nxp.com>");
+MODULE_DESCRIPTION("NXP i.MX8QM pinctrl driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH V5 9/9] pinctrl: imx8dxl: Support building as module

[Anson Huang]

Change configuration to "tristate", add module device table,
author, description and license to support building i.MX8DXL
pinctrl driver as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V4:
	- remove unnecessary change of replacing arch_initcall() with module_platform_driver();
	- add module author and description;
	- add more details to commit message.
---
 drivers/pinctrl/freescale/Kconfig           | 2 +-
 drivers/pinctrl/freescale/pinctrl-imx8dxl.c | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index de8d571..922ae4b 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -167,7 +167,7 @@ config PINCTRL_IMX8QXP
 	  Say Y here to enable the imx8qxp pinctrl driver
 
 config PINCTRL_IMX8DXL
-	bool "IMX8DXL pinctrl driver"
+	tristate "IMX8DXL pinctrl driver"
 	depends on IMX_SCU && ARCH_MXC && ARM64
 	select PINCTRL_IMX_SCU
 	help
diff --git a/drivers/pinctrl/freescale/pinctrl-imx8dxl.c b/drivers/pinctrl/freescale/pinctrl-imx8dxl.c
index be3b09e..d3020c0 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx8dxl.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx8dxl.c
@@ -168,6 +168,7 @@ static const struct of_device_id imx8dxl_pinctrl_of_match[] = {
 	{ .compatible = "fsl,imx8dxl-iomuxc", },
 	{ /* sentinel */ }
 };
+MODULE_DEVICE_TABLE(of, imx8dxl_pinctrl_of_match);
 
 static int imx8dxl_pinctrl_probe(struct platform_device *pdev)
 {
@@ -194,3 +195,7 @@ static int __init imx8dxl_pinctrl_init(void)
 	return platform_driver_register(&imx8dxl_pinctrl_driver);
 }
 arch_initcall(imx8dxl_pinctrl_init);
+
+MODULE_AUTHOR("Anson Huang <Anson.Huang@nxp.com>");
+MODULE_DESCRIPTION("NXP i.MX8DXL pinctrl driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

RE: [PATCH V5 1/9] pinctrl: imx: Support building SCU pinctrl driver as module

[Aisheng Dong]

> From: Anson Huang <Anson.Huang@nxp.com>
> Sent: Thursday, June 11, 2020 7:35 PM
> 
> To support building i.MX SCU pinctrl driver as module, below things need to be
> changed:
> 
>     - Export SCU related functions and use "IS_ENABLED" instead of
>       "ifdef" to support SCU pinctrl driver user and itself to be
>       built as module;
>     - Use function callbacks for SCU related functions in pinctrl-imx.c
>       in order to support the scenario of PINCTRL_IMX is built in
>       while PINCTRL_IMX_SCU is built as module;
>     - All drivers using SCU pinctrl driver need to initialize the
>       SCU related function callback;
>     - Change PINCTR_IMX_SCU to tristate;
>     - Add module author, description and license.
> 
> With above changes, i.MX SCU pinctrl driver can be built as module.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> Changes since V4:
> 	- add module author and description.
> ---
>  drivers/pinctrl/freescale/Kconfig           |  2 +-
>  drivers/pinctrl/freescale/pinctrl-imx.c     | 18 ++++-----
>  drivers/pinctrl/freescale/pinctrl-imx.h     | 57 ++++++++++++-----------------
>  drivers/pinctrl/freescale/pinctrl-imx8dxl.c |  3 ++
> drivers/pinctrl/freescale/pinctrl-imx8qm.c  |  3 ++
> drivers/pinctrl/freescale/pinctrl-imx8qxp.c |  3 ++
>  drivers/pinctrl/freescale/pinctrl-scu.c     |  9 +++++
>  7 files changed, 51 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
> index 4ca44dd..a3a30f1d 100644
> --- a/drivers/pinctrl/freescale/Kconfig
> +++ b/drivers/pinctrl/freescale/Kconfig
> @@ -7,7 +7,7 @@ config PINCTRL_IMX
>  	select REGMAP
> 
>  config PINCTRL_IMX_SCU
> -	bool
> +	tristate "IMX SCU pinctrl driver"
>  	depends on IMX_SCU
>  	select PINCTRL_IMX
> 
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c
> b/drivers/pinctrl/freescale/pinctrl-imx.c
> index cb7e0f0..c1faae1 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx.c
> @@ -372,8 +372,8 @@ static int imx_pinconf_get(struct pinctrl_dev *pctldev,
>  	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
>  	const struct imx_pinctrl_soc_info *info = ipctl->info;
> 
> -	if (info->flags & IMX_USE_SCU)
> -		return imx_pinconf_get_scu(pctldev, pin_id, config);
> +	if ((info->flags & IMX_USE_SCU) && info->imx_pinconf_get)
> +		return info->imx_pinconf_get(pctldev, pin_id, config);

Pointer check here seems not be necessary

> diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h
> b/drivers/pinctrl/freescale/pinctrl-imx.h
> index 333d32b..bdb86c2 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx.h
> +++ b/drivers/pinctrl/freescale/pinctrl-imx.h
> @@ -75,6 +75,21 @@ struct imx_cfg_params_decode {
>  	bool invert;
>  };
> 
> +/**
> + * @dev: a pointer back to containing device
> + * @base: the offset to the controller in virtual memory  */ struct
> +imx_pinctrl {
> +	struct device *dev;
> +	struct pinctrl_dev *pctl;
> +	void __iomem *base;
> +	void __iomem *input_sel_base;
> +	const struct imx_pinctrl_soc_info *info;
> +	struct imx_pin_reg *pin_regs;
> +	unsigned int group_index;
> +	struct mutex mutex;
> +};
> +
>  struct imx_pinctrl_soc_info {
>  	const struct pinctrl_pin_desc *pins;
>  	unsigned int npins;
> @@ -98,21 +113,13 @@ struct imx_pinctrl_soc_info {
>  				  struct pinctrl_gpio_range *range,
>  				  unsigned offset,
>  				  bool input);
> -};
> -
> -/**
> - * @dev: a pointer back to containing device
> - * @base: the offset to the controller in virtual memory
> - */
> -struct imx_pinctrl {
> -	struct device *dev;
> -	struct pinctrl_dev *pctl;
> -	void __iomem *base;
> -	void __iomem *input_sel_base;
> -	const struct imx_pinctrl_soc_info *info;
> -	struct imx_pin_reg *pin_regs;
> -	unsigned int group_index;
> -	struct mutex mutex;
> +	int (*imx_pinconf_get)(struct pinctrl_dev *pctldev, unsigned int pin_id,
> +			       unsigned long *config);
> +	int (*imx_pinconf_set)(struct pinctrl_dev *pctldev, unsigned int pin_id,
> +			       unsigned long *configs, unsigned int num_configs);
> +	void (*imx_pinctrl_parse_pin)(struct imx_pinctrl *ipctl,
> +				      unsigned int *pin_id, struct imx_pin *pin,
> +				      const __be32 **list_p);

Compared with V4, this new implementation seems a bit complicated.
I guess we don't have to support PINCTRL_IMX=y && PINCTRL_IMX_SCU=m case.
Will that make the support a bit easier?

Regards
Aisheng

>  };
> 
>  #define IMX_CFG_PARAMS_DECODE(p, m, o) \ @@ -137,7 +144,7 @@ struct
> imx_pinctrl {  int imx_pinctrl_probe(struct platform_device *pdev,
>  			const struct imx_pinctrl_soc_info *info);
> 
> -#ifdef CONFIG_PINCTRL_IMX_SCU
> +#if IS_ENABLED(CONFIG_PINCTRL_IMX_SCU)
>  #define BM_PAD_CTL_GP_ENABLE		BIT(30)
>  #define BM_PAD_CTL_IFMUX_ENABLE		BIT(31)
>  #define BP_PAD_CTL_IFMUX		27
> @@ -150,23 +157,5 @@ int imx_pinconf_set_scu(struct pinctrl_dev *pctldev,
> unsigned pin_id,  void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl,
>  			       unsigned int *pin_id, struct imx_pin *pin,
>  			       const __be32 **list_p);
> -#else
> -static inline int imx_pinconf_get_scu(struct pinctrl_dev *pctldev,
> -				      unsigned pin_id, unsigned long *config)
> -{
> -	return -EINVAL;
> -}
> -static inline int imx_pinconf_set_scu(struct pinctrl_dev *pctldev,
> -				      unsigned pin_id, unsigned long *configs,
> -				      unsigned num_configs)
> -{
> -	return -EINVAL;
> -}
> -static inline void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl,
> -					    unsigned int *pin_id,
> -					    struct imx_pin *pin,
> -					    const __be32 **list_p)
> -{
> -}
>  #endif
>  #endif /* __DRIVERS_PINCTRL_IMX_H */
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx8dxl.c
> b/drivers/pinctrl/freescale/pinctrl-imx8dxl.c
> index 7f32e57..be3b09e 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx8dxl.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx8dxl.c
> @@ -159,6 +159,9 @@ static struct imx_pinctrl_soc_info imx8dxl_pinctrl_info
> = {
>  	.pins = imx8dxl_pinctrl_pads,
>  	.npins = ARRAY_SIZE(imx8dxl_pinctrl_pads),
>  	.flags = IMX_USE_SCU,
> +	.imx_pinconf_get = imx_pinconf_get_scu,
> +	.imx_pinconf_set = imx_pinconf_set_scu,
> +	.imx_pinctrl_parse_pin = imx_pinctrl_parse_pin_scu,
>  };
> 
>  static const struct of_device_id imx8dxl_pinctrl_of_match[] = { diff --git
> a/drivers/pinctrl/freescale/pinctrl-imx8qm.c
> b/drivers/pinctrl/freescale/pinctrl-imx8qm.c
> index 0b6029b..9ba3249 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx8qm.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx8qm.c
> @@ -292,6 +292,9 @@ static const struct imx_pinctrl_soc_info
> imx8qm_pinctrl_info = {
>  	.pins = imx8qm_pinctrl_pads,
>  	.npins = ARRAY_SIZE(imx8qm_pinctrl_pads),
>  	.flags = IMX_USE_SCU,
> +	.imx_pinconf_get = imx_pinconf_get_scu,
> +	.imx_pinconf_set = imx_pinconf_set_scu,
> +	.imx_pinctrl_parse_pin = imx_pinctrl_parse_pin_scu,
>  };
> 
>  static const struct of_device_id imx8qm_pinctrl_of_match[] = { diff --git
> a/drivers/pinctrl/freescale/pinctrl-imx8qxp.c
> b/drivers/pinctrl/freescale/pinctrl-imx8qxp.c
> index 1131dc3..05906b9 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx8qxp.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx8qxp.c
> @@ -198,6 +198,9 @@ static struct imx_pinctrl_soc_info imx8qxp_pinctrl_info
> = {
>  	.pins = imx8qxp_pinctrl_pads,
>  	.npins = ARRAY_SIZE(imx8qxp_pinctrl_pads),
>  	.flags = IMX_USE_SCU,
> +	.imx_pinconf_get = imx_pinconf_get_scu,
> +	.imx_pinconf_set = imx_pinconf_set_scu,
> +	.imx_pinctrl_parse_pin = imx_pinctrl_parse_pin_scu,
>  };
> 
>  static const struct of_device_id imx8qxp_pinctrl_of_match[] = { diff --git
> a/drivers/pinctrl/freescale/pinctrl-scu.c b/drivers/pinctrl/freescale/pinctrl-scu.c
> index 23cf04b..59b5f8a 100644
> --- a/drivers/pinctrl/freescale/pinctrl-scu.c
> +++ b/drivers/pinctrl/freescale/pinctrl-scu.c
> @@ -7,6 +7,7 @@
> 
>  #include <linux/err.h>
>  #include <linux/firmware/imx/sci.h>
> +#include <linux/module.h>
>  #include <linux/of_address.h>
>  #include <linux/pinctrl/pinctrl.h>
>  #include <linux/platform_device.h>
> @@ -41,6 +42,7 @@ int imx_pinctrl_sc_ipc_init(struct platform_device *pdev)
> {
>  	return imx_scu_get_handle(&pinctrl_ipc_handle);
>  }
> +EXPORT_SYMBOL_GPL(imx_pinctrl_sc_ipc_init);
> 
>  int imx_pinconf_get_scu(struct pinctrl_dev *pctldev, unsigned pin_id,
>  			unsigned long *config)
> @@ -66,6 +68,7 @@ int imx_pinconf_get_scu(struct pinctrl_dev *pctldev,
> unsigned pin_id,
> 
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(imx_pinconf_get_scu);
> 
>  int imx_pinconf_set_scu(struct pinctrl_dev *pctldev, unsigned pin_id,
>  			unsigned long *configs, unsigned num_configs) @@ -101,6
> +104,7 @@ int imx_pinconf_set_scu(struct pinctrl_dev *pctldev, unsigned
> pin_id,
> 
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(imx_pinconf_set_scu);
> 
>  void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl,
>  			       unsigned int *pin_id, struct imx_pin *pin, @@ -119,3
> +123,8 @@ void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl,
>  	dev_dbg(ipctl->dev, "%s: 0x%x 0x%08lx", info->pins[pin->pin].name,
>  		pin_scu->mux_mode, pin_scu->config);
>  }
> +EXPORT_SYMBOL_GPL(imx_pinctrl_parse_pin_scu);
> +
> +MODULE_AUTHOR("Dong Aisheng <aisheng.dong@nxp.com>");
> +MODULE_DESCRIPTION("NXP i.MX SCU common pinctrl driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4

RE: [PATCH V5 1/9] pinctrl: imx: Support building SCU pinctrl driver as module

[Anson Huang]

> Subject: RE: [PATCH V5 1/9] pinctrl: imx: Support building SCU pinctrl driver as
> module
> 
> > From: Anson Huang <Anson.Huang@nxp.com>
> > Sent: Thursday, June 11, 2020 7:35 PM
> >
> > To support building i.MX SCU pinctrl driver as module, below things
> > need to be
> > changed:
> >
> >     - Export SCU related functions and use "IS_ENABLED" instead of
> >       "ifdef" to support SCU pinctrl driver user and itself to be
> >       built as module;
> >     - Use function callbacks for SCU related functions in pinctrl-imx.c
> >       in order to support the scenario of PINCTRL_IMX is built in
> >       while PINCTRL_IMX_SCU is built as module;
> >     - All drivers using SCU pinctrl driver need to initialize the
> >       SCU related function callback;
> >     - Change PINCTR_IMX_SCU to tristate;
> >     - Add module author, description and license.
> >
> > With above changes, i.MX SCU pinctrl driver can be built as module.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > Changes since V4:
> > 	- add module author and description.
> > ---
> >  drivers/pinctrl/freescale/Kconfig           |  2 +-
> >  drivers/pinctrl/freescale/pinctrl-imx.c     | 18 ++++-----
> >  drivers/pinctrl/freescale/pinctrl-imx.h     | 57 ++++++++++++-----------------
> >  drivers/pinctrl/freescale/pinctrl-imx8dxl.c |  3 ++
> > drivers/pinctrl/freescale/pinctrl-imx8qm.c  |  3 ++
> > drivers/pinctrl/freescale/pinctrl-imx8qxp.c |  3 ++
> >  drivers/pinctrl/freescale/pinctrl-scu.c     |  9 +++++
> >  7 files changed, 51 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/pinctrl/freescale/Kconfig
> > b/drivers/pinctrl/freescale/Kconfig
> > index 4ca44dd..a3a30f1d 100644
> > --- a/drivers/pinctrl/freescale/Kconfig
> > +++ b/drivers/pinctrl/freescale/Kconfig
> > @@ -7,7 +7,7 @@ config PINCTRL_IMX
> >  	select REGMAP
> >
> >  config PINCTRL_IMX_SCU
> > -	bool
> > +	tristate "IMX SCU pinctrl driver"
> >  	depends on IMX_SCU
> >  	select PINCTRL_IMX
> >
> > diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c
> > b/drivers/pinctrl/freescale/pinctrl-imx.c
> > index cb7e0f0..c1faae1 100644
> > --- a/drivers/pinctrl/freescale/pinctrl-imx.c
> > +++ b/drivers/pinctrl/freescale/pinctrl-imx.c
> > @@ -372,8 +372,8 @@ static int imx_pinconf_get(struct pinctrl_dev
> *pctldev,
> >  	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> >  	const struct imx_pinctrl_soc_info *info = ipctl->info;
> >
> > -	if (info->flags & IMX_USE_SCU)
> > -		return imx_pinconf_get_scu(pctldev, pin_id, config);
> > +	if ((info->flags & IMX_USE_SCU) && info->imx_pinconf_get)
> > +		return info->imx_pinconf_get(pctldev, pin_id, config);
> 
> Pointer check here seems not be necessary

I think it is NOT harmful and it is just in case the drivers using scu pinctrl
do NOT initialize these functions callback and lead to NULL pointer dump.

> 
> > diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h
> > b/drivers/pinctrl/freescale/pinctrl-imx.h
> > index 333d32b..bdb86c2 100644
> > --- a/drivers/pinctrl/freescale/pinctrl-imx.h
> > +++ b/drivers/pinctrl/freescale/pinctrl-imx.h
> > @@ -75,6 +75,21 @@ struct imx_cfg_params_decode {
> >  	bool invert;
> >  };
> >
> > +/**
> > + * @dev: a pointer back to containing device
> > + * @base: the offset to the controller in virtual memory  */ struct
> > +imx_pinctrl {
> > +	struct device *dev;
> > +	struct pinctrl_dev *pctl;
> > +	void __iomem *base;
> > +	void __iomem *input_sel_base;
> > +	const struct imx_pinctrl_soc_info *info;
> > +	struct imx_pin_reg *pin_regs;
> > +	unsigned int group_index;
> > +	struct mutex mutex;
> > +};
> > +
> >  struct imx_pinctrl_soc_info {
> >  	const struct pinctrl_pin_desc *pins;
> >  	unsigned int npins;
> > @@ -98,21 +113,13 @@ struct imx_pinctrl_soc_info {
> >  				  struct pinctrl_gpio_range *range,
> >  				  unsigned offset,
> >  				  bool input);
> > -};
> > -
> > -/**
> > - * @dev: a pointer back to containing device
> > - * @base: the offset to the controller in virtual memory
> > - */
> > -struct imx_pinctrl {
> > -	struct device *dev;
> > -	struct pinctrl_dev *pctl;
> > -	void __iomem *base;
> > -	void __iomem *input_sel_base;
> > -	const struct imx_pinctrl_soc_info *info;
> > -	struct imx_pin_reg *pin_regs;
> > -	unsigned int group_index;
> > -	struct mutex mutex;
> > +	int (*imx_pinconf_get)(struct pinctrl_dev *pctldev, unsigned int pin_id,
> > +			       unsigned long *config);
> > +	int (*imx_pinconf_set)(struct pinctrl_dev *pctldev, unsigned int pin_id,
> > +			       unsigned long *configs, unsigned int num_configs);
> > +	void (*imx_pinctrl_parse_pin)(struct imx_pinctrl *ipctl,
> > +				      unsigned int *pin_id, struct imx_pin *pin,
> > +				      const __be32 **list_p);
> 
> Compared with V4, this new implementation seems a bit complicated.
> I guess we don't have to support PINCTRL_IMX=y && PINCTRL_IMX_SCU=m
> case.
> Will that make the support a bit easier?

I am NOT sure if such scenario meets requirement, the fact is other non-i.MX SoC
also selects the PINCTRL_IMX which will make PINCTRL_IMX=y, so in that case, even
all i.MX PINCTRL drivers are set to module, it will still have PINCTRL_IMX=y and PINCTRL_IMX_SCU=m,
then build will fail. And I believe the auto build test may also cover such case and build
error will be reported, that is why this change is needed and with this change, function
is NOT impacted,

Anson.

RE: [PATCH V5 1/9] pinctrl: imx: Support building SCU pinctrl driver as module

[Aisheng Dong]

> From: Anson Huang <anson.huang@nxp.com>
> Sent: Tuesday, June 16, 2020 6:44 PM
> 
> > Subject: RE: [PATCH V5 1/9] pinctrl: imx: Support building SCU pinctrl
> > driver as module
> >
> > > From: Anson Huang <Anson.Huang@nxp.com>
> > > Sent: Thursday, June 11, 2020 7:35 PM
> > >
> > > To support building i.MX SCU pinctrl driver as module, below things
> > > need to be
> > > changed:
> > >
> > >     - Export SCU related functions and use "IS_ENABLED" instead of
> > >       "ifdef" to support SCU pinctrl driver user and itself to be
> > >       built as module;
> > >     - Use function callbacks for SCU related functions in pinctrl-imx.c
> > >       in order to support the scenario of PINCTRL_IMX is built in
> > >       while PINCTRL_IMX_SCU is built as module;
> > >     - All drivers using SCU pinctrl driver need to initialize the
> > >       SCU related function callback;
> > >     - Change PINCTR_IMX_SCU to tristate;
> > >     - Add module author, description and license.
> > >
> > > With above changes, i.MX SCU pinctrl driver can be built as module.
> > >
> > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > ---
> > > Changes since V4:
> > > 	- add module author and description.
> > > ---
> > >  drivers/pinctrl/freescale/Kconfig           |  2 +-
> > >  drivers/pinctrl/freescale/pinctrl-imx.c     | 18 ++++-----
> > >  drivers/pinctrl/freescale/pinctrl-imx.h     | 57
> ++++++++++++-----------------
> > >  drivers/pinctrl/freescale/pinctrl-imx8dxl.c |  3 ++
> > > drivers/pinctrl/freescale/pinctrl-imx8qm.c  |  3 ++
> > > drivers/pinctrl/freescale/pinctrl-imx8qxp.c |  3 ++
> > >  drivers/pinctrl/freescale/pinctrl-scu.c     |  9 +++++
> > >  7 files changed, 51 insertions(+), 44 deletions(-)
> > >
> > > diff --git a/drivers/pinctrl/freescale/Kconfig
> > > b/drivers/pinctrl/freescale/Kconfig
> > > index 4ca44dd..a3a30f1d 100644
> > > --- a/drivers/pinctrl/freescale/Kconfig
> > > +++ b/drivers/pinctrl/freescale/Kconfig
> > > @@ -7,7 +7,7 @@ config PINCTRL_IMX
> > >  	select REGMAP
> > >
> > >  config PINCTRL_IMX_SCU
> > > -	bool
> > > +	tristate "IMX SCU pinctrl driver"
> > >  	depends on IMX_SCU
> > >  	select PINCTRL_IMX
> > >
> > > diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c
> > > b/drivers/pinctrl/freescale/pinctrl-imx.c
> > > index cb7e0f0..c1faae1 100644
> > > --- a/drivers/pinctrl/freescale/pinctrl-imx.c
> > > +++ b/drivers/pinctrl/freescale/pinctrl-imx.c
> > > @@ -372,8 +372,8 @@ static int imx_pinconf_get(struct pinctrl_dev
> > *pctldev,
> > >  	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> > >  	const struct imx_pinctrl_soc_info *info = ipctl->info;
> > >
> > > -	if (info->flags & IMX_USE_SCU)
> > > -		return imx_pinconf_get_scu(pctldev, pin_id, config);
> > > +	if ((info->flags & IMX_USE_SCU) && info->imx_pinconf_get)
> > > +		return info->imx_pinconf_get(pctldev, pin_id, config);
> >
> > Pointer check here seems not be necessary
> 
> I think it is NOT harmful and it is just in case the drivers using scu pinctrl do NOT
> initialize these functions callback and lead to NULL pointer dump.
> 

It is a bit harmful to the code readability as we already use flag IMX_USE_SCU to distinguish
the difference. Not need double check the pointer again because platforms driver must have
defined it.

> >
> > > diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h
> > > b/drivers/pinctrl/freescale/pinctrl-imx.h
> > > index 333d32b..bdb86c2 100644
> > > --- a/drivers/pinctrl/freescale/pinctrl-imx.h
> > > +++ b/drivers/pinctrl/freescale/pinctrl-imx.h
> > > @@ -75,6 +75,21 @@ struct imx_cfg_params_decode {
> > >  	bool invert;
> > >  };
> > >
> > > +/**
> > > + * @dev: a pointer back to containing device
> > > + * @base: the offset to the controller in virtual memory  */ struct
> > > +imx_pinctrl {
> > > +	struct device *dev;
> > > +	struct pinctrl_dev *pctl;
> > > +	void __iomem *base;
> > > +	void __iomem *input_sel_base;
> > > +	const struct imx_pinctrl_soc_info *info;
> > > +	struct imx_pin_reg *pin_regs;
> > > +	unsigned int group_index;
> > > +	struct mutex mutex;
> > > +};
> > > +
> > >  struct imx_pinctrl_soc_info {
> > >  	const struct pinctrl_pin_desc *pins;
> > >  	unsigned int npins;
> > > @@ -98,21 +113,13 @@ struct imx_pinctrl_soc_info {
> > >  				  struct pinctrl_gpio_range *range,
> > >  				  unsigned offset,
> > >  				  bool input);
> > > -};
> > > -
> > > -/**
> > > - * @dev: a pointer back to containing device
> > > - * @base: the offset to the controller in virtual memory
> > > - */
> > > -struct imx_pinctrl {
> > > -	struct device *dev;
> > > -	struct pinctrl_dev *pctl;
> > > -	void __iomem *base;
> > > -	void __iomem *input_sel_base;
> > > -	const struct imx_pinctrl_soc_info *info;
> > > -	struct imx_pin_reg *pin_regs;
> > > -	unsigned int group_index;
> > > -	struct mutex mutex;
> > > +	int (*imx_pinconf_get)(struct pinctrl_dev *pctldev, unsigned int pin_id,
> > > +			       unsigned long *config);
> > > +	int (*imx_pinconf_set)(struct pinctrl_dev *pctldev, unsigned int pin_id,
> > > +			       unsigned long *configs, unsigned int num_configs);
> > > +	void (*imx_pinctrl_parse_pin)(struct imx_pinctrl *ipctl,
> > > +				      unsigned int *pin_id, struct imx_pin *pin,
> > > +				      const __be32 **list_p);
> >
> > Compared with V4, this new implementation seems a bit complicated.
> > I guess we don't have to support PINCTRL_IMX=y && PINCTRL_IMX_SCU=m
> > case.
> > Will that make the support a bit easier?
> 
> I am NOT sure if such scenario meets requirement, the fact is other non-i.MX
> SoC also selects the PINCTRL_IMX which will make PINCTRL_IMX=y, so in that
> case, even all i.MX PINCTRL drivers are set to module, it will still have
> PINCTRL_IMX=y and PINCTRL_IMX_SCU=m, then build will fail. And I believe the
> auto build test may also cover such case and build error will be reported, that is
> why this change is needed and with this change, function is NOT impacted,
> 

Is it possible to add some constrainst to make sure PINCTRL_IMX_SCU value is the same
as PINCTRL_IMX? Or combine them into one?
If we can do that, it may ease the implementation a lot and make the code still clean.

Regards
Aisheng

> Anson.

RE: [PATCH V5 1/9] pinctrl: imx: Support building SCU pinctrl driver as module

[Anson Huang]

> Subject: RE: [PATCH V5 1/9] pinctrl: imx: Support building SCU pinctrl driver as
> module
> 
> > From: Anson Huang <anson.huang@nxp.com>
> > Sent: Tuesday, June 16, 2020 6:44 PM
> >
> > > Subject: RE: [PATCH V5 1/9] pinctrl: imx: Support building SCU
> > > pinctrl driver as module
> > >
> > > > From: Anson Huang <Anson.Huang@nxp.com>
> > > > Sent: Thursday, June 11, 2020 7:35 PM
> > > >
> > > > To support building i.MX SCU pinctrl driver as module, below
> > > > things need to be
> > > > changed:
> > > >
> > > >     - Export SCU related functions and use "IS_ENABLED" instead of
> > > >       "ifdef" to support SCU pinctrl driver user and itself to be
> > > >       built as module;
> > > >     - Use function callbacks for SCU related functions in pinctrl-imx.c
> > > >       in order to support the scenario of PINCTRL_IMX is built in
> > > >       while PINCTRL_IMX_SCU is built as module;
> > > >     - All drivers using SCU pinctrl driver need to initialize the
> > > >       SCU related function callback;
> > > >     - Change PINCTR_IMX_SCU to tristate;
> > > >     - Add module author, description and license.
> > > >
> > > > With above changes, i.MX SCU pinctrl driver can be built as module.
> > > >
> > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > > ---
> > > > Changes since V4:
> > > > 	- add module author and description.
> > > > ---
> > > >  drivers/pinctrl/freescale/Kconfig           |  2 +-
> > > >  drivers/pinctrl/freescale/pinctrl-imx.c     | 18 ++++-----
> > > >  drivers/pinctrl/freescale/pinctrl-imx.h     | 57
> > ++++++++++++-----------------
> > > >  drivers/pinctrl/freescale/pinctrl-imx8dxl.c |  3 ++
> > > > drivers/pinctrl/freescale/pinctrl-imx8qm.c  |  3 ++
> > > > drivers/pinctrl/freescale/pinctrl-imx8qxp.c |  3 ++
> > > >  drivers/pinctrl/freescale/pinctrl-scu.c     |  9 +++++
> > > >  7 files changed, 51 insertions(+), 44 deletions(-)
> > > >
> > > > diff --git a/drivers/pinctrl/freescale/Kconfig
> > > > b/drivers/pinctrl/freescale/Kconfig
> > > > index 4ca44dd..a3a30f1d 100644
> > > > --- a/drivers/pinctrl/freescale/Kconfig
> > > > +++ b/drivers/pinctrl/freescale/Kconfig
> > > > @@ -7,7 +7,7 @@ config PINCTRL_IMX
> > > >  	select REGMAP
> > > >
> > > >  config PINCTRL_IMX_SCU
> > > > -	bool
> > > > +	tristate "IMX SCU pinctrl driver"
> > > >  	depends on IMX_SCU
> > > >  	select PINCTRL_IMX
> > > >
> > > > diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c
> > > > b/drivers/pinctrl/freescale/pinctrl-imx.c
> > > > index cb7e0f0..c1faae1 100644
> > > > --- a/drivers/pinctrl/freescale/pinctrl-imx.c
> > > > +++ b/drivers/pinctrl/freescale/pinctrl-imx.c
> > > > @@ -372,8 +372,8 @@ static int imx_pinconf_get(struct pinctrl_dev
> > > *pctldev,
> > > >  	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> > > >  	const struct imx_pinctrl_soc_info *info = ipctl->info;
> > > >
> > > > -	if (info->flags & IMX_USE_SCU)
> > > > -		return imx_pinconf_get_scu(pctldev, pin_id, config);
> > > > +	if ((info->flags & IMX_USE_SCU) && info->imx_pinconf_get)
> > > > +		return info->imx_pinconf_get(pctldev, pin_id, config);
> > >
> > > Pointer check here seems not be necessary
> >
> > I think it is NOT harmful and it is just in case the drivers using scu
> > pinctrl do NOT initialize these functions callback and lead to NULL pointer
> dump.
> >
> 
> It is a bit harmful to the code readability as we already use flag IMX_USE_SCU
> to distinguish the difference. Not need double check the pointer again because
> platforms driver must have defined it.

I am fine, it is just because checking the function callback before calling it is better.
I can remove it if you insist to NOT check it. If there is other comment, will remove
them together in next version.

> 
> > >
> > > > diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h
> > > > b/drivers/pinctrl/freescale/pinctrl-imx.h
> > > > index 333d32b..bdb86c2 100644
> > > > --- a/drivers/pinctrl/freescale/pinctrl-imx.h
> > > > +++ b/drivers/pinctrl/freescale/pinctrl-imx.h
> > > > @@ -75,6 +75,21 @@ struct imx_cfg_params_decode {
> > > >  	bool invert;
> > > >  };
> > > >
> > > > +/**
> > > > + * @dev: a pointer back to containing device
> > > > + * @base: the offset to the controller in virtual memory  */
> > > > +struct imx_pinctrl {
> > > > +	struct device *dev;
> > > > +	struct pinctrl_dev *pctl;
> > > > +	void __iomem *base;
> > > > +	void __iomem *input_sel_base;
> > > > +	const struct imx_pinctrl_soc_info *info;
> > > > +	struct imx_pin_reg *pin_regs;
> > > > +	unsigned int group_index;
> > > > +	struct mutex mutex;
> > > > +};
> > > > +
> > > >  struct imx_pinctrl_soc_info {
> > > >  	const struct pinctrl_pin_desc *pins;
> > > >  	unsigned int npins;
> > > > @@ -98,21 +113,13 @@ struct imx_pinctrl_soc_info {
> > > >  				  struct pinctrl_gpio_range *range,
> > > >  				  unsigned offset,
> > > >  				  bool input);
> > > > -};
> > > > -
> > > > -/**
> > > > - * @dev: a pointer back to containing device
> > > > - * @base: the offset to the controller in virtual memory
> > > > - */
> > > > -struct imx_pinctrl {
> > > > -	struct device *dev;
> > > > -	struct pinctrl_dev *pctl;
> > > > -	void __iomem *base;
> > > > -	void __iomem *input_sel_base;
> > > > -	const struct imx_pinctrl_soc_info *info;
> > > > -	struct imx_pin_reg *pin_regs;
> > > > -	unsigned int group_index;
> > > > -	struct mutex mutex;
> > > > +	int (*imx_pinconf_get)(struct pinctrl_dev *pctldev, unsigned int
> pin_id,
> > > > +			       unsigned long *config);
> > > > +	int (*imx_pinconf_set)(struct pinctrl_dev *pctldev, unsigned int
> pin_id,
> > > > +			       unsigned long *configs, unsigned int num_configs);
> > > > +	void (*imx_pinctrl_parse_pin)(struct imx_pinctrl *ipctl,
> > > > +				      unsigned int *pin_id, struct imx_pin *pin,
> > > > +				      const __be32 **list_p);
> > >
> > > Compared with V4, this new implementation seems a bit complicated.
> > > I guess we don't have to support PINCTRL_IMX=y && PINCTRL_IMX_SCU=m
> > > case.
> > > Will that make the support a bit easier?
> >
> > I am NOT sure if such scenario meets requirement, the fact is other
> > non-i.MX SoC also selects the PINCTRL_IMX which will make
> > PINCTRL_IMX=y, so in that case, even all i.MX PINCTRL drivers are set
> > to module, it will still have PINCTRL_IMX=y and PINCTRL_IMX_SCU=m,
> > then build will fail. And I believe the auto build test may also cover
> > such case and build error will be reported, that is why this change is
> > needed and with this change, function is NOT impacted,
> >
> 
> Is it possible to add some constrainst to make sure PINCTRL_IMX_SCU value is
> the same as PINCTRL_IMX? Or combine them into one?
> If we can do that, it may ease the implementation a lot and make the code still
> clean.

Combine PINCTRL_IMX_SCU and PINCTRL_IMX is NOT making sense, since for non-SCU
platforms, PINCTRL_IMX_SCU is NOT necessary, to make PINCTRL_IMX_SCU same value
as PINCTRL_IMX, unless make "select PINCTRL_IMX_SCU" in PINCTRL_IMX, but that is
also NOT making sense, because, PINCTRL_IMX does NOT depends on PINCTRL_IMX_SCU
at all.

The change is NOT that big IMO, and no better idea in my mind, have tried that in previous versions
of patch series.

Anson

RE: [PATCH V5 1/9] pinctrl: imx: Support building SCU pinctrl driver as module

[Aisheng Dong]

[...]

> > > > > - * @dev: a pointer back to containing device
> > > > > - * @base: the offset to the controller in virtual memory
> > > > > - */
> > > > > -struct imx_pinctrl {
> > > > > -	struct device *dev;
> > > > > -	struct pinctrl_dev *pctl;
> > > > > -	void __iomem *base;
> > > > > -	void __iomem *input_sel_base;
> > > > > -	const struct imx_pinctrl_soc_info *info;
> > > > > -	struct imx_pin_reg *pin_regs;
> > > > > -	unsigned int group_index;
> > > > > -	struct mutex mutex;
> > > > > +	int (*imx_pinconf_get)(struct pinctrl_dev *pctldev, unsigned
> > > > > +int
> > pin_id,
> > > > > +			       unsigned long *config);
> > > > > +	int (*imx_pinconf_set)(struct pinctrl_dev *pctldev, unsigned
> > > > > +int
> > pin_id,
> > > > > +			       unsigned long *configs, unsigned int num_configs);
> > > > > +	void (*imx_pinctrl_parse_pin)(struct imx_pinctrl *ipctl,
> > > > > +				      unsigned int *pin_id, struct imx_pin *pin,
> > > > > +				      const __be32 **list_p);
> > > >
> > > > Compared with V4, this new implementation seems a bit complicated.
> > > > I guess we don't have to support PINCTRL_IMX=y &&
> > > > PINCTRL_IMX_SCU=m case.
> > > > Will that make the support a bit easier?
> > >
> > > I am NOT sure if such scenario meets requirement, the fact is other
> > > non-i.MX SoC also selects the PINCTRL_IMX which will make
> > > PINCTRL_IMX=y, so in that case, even all i.MX PINCTRL drivers are
> > > set to module, it will still have PINCTRL_IMX=y and
> > > PINCTRL_IMX_SCU=m, then build will fail. And I believe the auto
> > > build test may also cover such case and build error will be
> > > reported, that is why this change is needed and with this change,
> > > function is NOT impacted,
> > >
> >
> > Is it possible to add some constrainst to make sure PINCTRL_IMX_SCU
> > value is the same as PINCTRL_IMX? Or combine them into one?
> > If we can do that, it may ease the implementation a lot and make the
> > code still clean.
> 
> Combine PINCTRL_IMX_SCU and PINCTRL_IMX is NOT making sense, since for
> non-SCU platforms, PINCTRL_IMX_SCU is NOT necessary, to make
> PINCTRL_IMX_SCU same value as PINCTRL_IMX, unless make "select
> PINCTRL_IMX_SCU" in PINCTRL_IMX, but that is also NOT making sense,
> because, PINCTRL_IMX does NOT depends on PINCTRL_IMX_SCU at all.
> 

PINCTRL_IMX_SCU could be conditionally compiled. 
Something like follows:
obj-$(CONFIG_PINCTRL_IMX) += pinctrl-imx-core.o
pinctrl-imx-core-y := pinctrl-imx.o
pinctrl-imx-core-$(CONFIG_PINCTRL_IMX_SCU) += pinctrl-scu.o

Can you try if this way could work?

Regards
Aisheng

> The change is NOT that big IMO, and no better idea in my mind, have tried that
> in previous versions of patch series.
> 
> Anson

RE: [PATCH V5 1/9] pinctrl: imx: Support building SCU pinctrl driver as module

[Anson Huang]

> Subject: RE: [PATCH V5 1/9] pinctrl: imx: Support building SCU pinctrl driver as
> module
> 
> [...]
> 
> > > > > > - * @dev: a pointer back to containing device
> > > > > > - * @base: the offset to the controller in virtual memory
> > > > > > - */
> > > > > > -struct imx_pinctrl {
> > > > > > -	struct device *dev;
> > > > > > -	struct pinctrl_dev *pctl;
> > > > > > -	void __iomem *base;
> > > > > > -	void __iomem *input_sel_base;
> > > > > > -	const struct imx_pinctrl_soc_info *info;
> > > > > > -	struct imx_pin_reg *pin_regs;
> > > > > > -	unsigned int group_index;
> > > > > > -	struct mutex mutex;
> > > > > > +	int (*imx_pinconf_get)(struct pinctrl_dev *pctldev, unsigned
> > > > > > +int
> > > pin_id,
> > > > > > +			       unsigned long *config);
> > > > > > +	int (*imx_pinconf_set)(struct pinctrl_dev *pctldev, unsigned
> > > > > > +int
> > > pin_id,
> > > > > > +			       unsigned long *configs, unsigned int
> num_configs);
> > > > > > +	void (*imx_pinctrl_parse_pin)(struct imx_pinctrl *ipctl,
> > > > > > +				      unsigned int *pin_id, struct imx_pin *pin,
> > > > > > +				      const __be32 **list_p);
> > > > >
> > > > > Compared with V4, this new implementation seems a bit complicated.
> > > > > I guess we don't have to support PINCTRL_IMX=y &&
> > > > > PINCTRL_IMX_SCU=m case.
> > > > > Will that make the support a bit easier?
> > > >
> > > > I am NOT sure if such scenario meets requirement, the fact is
> > > > other non-i.MX SoC also selects the PINCTRL_IMX which will make
> > > > PINCTRL_IMX=y, so in that case, even all i.MX PINCTRL drivers are
> > > > set to module, it will still have PINCTRL_IMX=y and
> > > > PINCTRL_IMX_SCU=m, then build will fail. And I believe the auto
> > > > build test may also cover such case and build error will be
> > > > reported, that is why this change is needed and with this change,
> > > > function is NOT impacted,
> > > >
> > >
> > > Is it possible to add some constrainst to make sure PINCTRL_IMX_SCU
> > > value is the same as PINCTRL_IMX? Or combine them into one?
> > > If we can do that, it may ease the implementation a lot and make the
> > > code still clean.
> >
> > Combine PINCTRL_IMX_SCU and PINCTRL_IMX is NOT making sense, since
> for
> > non-SCU platforms, PINCTRL_IMX_SCU is NOT necessary, to make
> > PINCTRL_IMX_SCU same value as PINCTRL_IMX, unless make "select
> > PINCTRL_IMX_SCU" in PINCTRL_IMX, but that is also NOT making sense,
> > because, PINCTRL_IMX does NOT depends on PINCTRL_IMX_SCU at all.
> >
> 
> PINCTRL_IMX_SCU could be conditionally compiled.
> Something like follows:
> obj-$(CONFIG_PINCTRL_IMX) += pinctrl-imx-core.o pinctrl-imx-core-y :=
> pinctrl-imx.o
> pinctrl-imx-core-$(CONFIG_PINCTRL_IMX_SCU) += pinctrl-scu.o
> 
> Can you try if this way could work?

It does NOW work, when PINCTRL_IMX=y and PINCTRL_IMX_SCU=m, config as below,
build will failed because some SCU functions NOT implemented, this is the exact reason
why have to use function callback. Currently, when PINCTRL_IMX=y, PINCTRL_IMX_SCU
MUST be =y, but that is NOT making enough sense for i.MX8M SoCs:

 CONFIG_PINCTRL_IMX=y
 CONFIG_PINCTRL_IMX_SCU=m
 CONFIG_PINCTRL_IMX8MM=y
 CONFIG_PINCTRL_IMX8MN=m
 CONFIG_PINCTRL_IMX8MP=m
 CONFIG_PINCTRL_IMX8MQ=m

aarch64-poky-linux-ld: drivers/pinctrl/freescale/pinctrl-imx.o: in function `imx_pinconf_dbg_show':
/home/b20788/workspace/stash/linux-next/drivers/pinctrl/freescale/pinctrl-imx.c:444: undefined reference to `imx_pinconf_get_scu'
aarch64-poky-linux-ld: drivers/pinctrl/freescale/pinctrl-imx.o: in function `imx_pinconf_get':
/home/b20788/workspace/stash/linux-next/drivers/pinctrl/freescale/pinctrl-imx.c:377: undefined reference to `imx_pinconf_get_scu'
aarch64-poky-linux-ld: drivers/pinctrl/freescale/pinctrl-imx.o: in function `imx_pinconf_set':
/home/b20788/workspace/stash/linux-next/drivers/pinctrl/freescale/pinctrl-imx.c:427: undefined reference to `imx_pinconf_set_scu'
aarch64-poky-linux-ld: drivers/pinctrl/freescale/pinctrl-imx.o: in function `imx_pinctrl_parse_groups':
/home/b20788/workspace/stash/linux-next/drivers/pinctrl/freescale/pinctrl-imx.c:633: undefined reference to `imx_pinctrl_parse_pin_scu'

Anson