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

[PATCH V4 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 V3, the changes are as below:
	- change the config dependency back to original;
	- use function callbacks for SCU related functions, and all drivers
	  using SCU pinctrl driver need to initialize the function callbacks,
	  pinctrl-imx.c will check the SCU function callback and call it when
	  it is valid, then no build issue when PINCTRL_IMX is built in and
	  PINCTRL_IMX_SCU is built as 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     | 22 ++++++-----
 drivers/pinctrl/freescale/pinctrl-imx.h     | 57 ++++++++++++-----------------
 drivers/pinctrl/freescale/pinctrl-imx8dxl.c | 12 +++---
 drivers/pinctrl/freescale/pinctrl-imx8mm.c  | 10 ++---
 drivers/pinctrl/freescale/pinctrl-imx8mn.c  | 10 ++---
 drivers/pinctrl/freescale/pinctrl-imx8mp.c  | 10 ++---
 drivers/pinctrl/freescale/pinctrl-imx8mq.c  |  9 ++---
 drivers/pinctrl/freescale/pinctrl-imx8qm.c  | 12 +++---
 drivers/pinctrl/freescale/pinctrl-imx8qxp.c | 12 +++---
 drivers/pinctrl/freescale/pinctrl-scu.c     |  6 +++
 11 files changed, 86 insertions(+), 93 deletions(-)

-- 
2.7.4

[PATCH V4 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.

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

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V3:
	- change config dependency back to original;
	- use function callbacks for SCU related functions in pinctrl-imx.c
	  to support the scenario of PINCTRL_IMX is built in while
	  PINCTRL_IMX_SCU is built as module;
	- initialize SCU related functions callback in i.MX8QM/QXP/DXL;
	- remove unused SCU related stub functions in pinctrl-imx.h.
---
 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     |  6 +++
 7 files changed, 48 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..35efd5c 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,5 @@ 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_LICENSE("GPL v2");
-- 
2.7.4

[PATCH V4 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.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change.
---
 drivers/pinctrl/freescale/Kconfig       | 3 ++-
 drivers/pinctrl/freescale/pinctrl-imx.c | 4 ++++
 2 files changed, 6 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..a68d430 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,5 @@ 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_LICENSE("GPL v2");
-- 
2.7.4

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

[Anson Huang]

Support building i.MX8MM pinctrl driver as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change.
---
 drivers/pinctrl/freescale/Kconfig          |  2 +-
 drivers/pinctrl/freescale/pinctrl-imx8mm.c | 10 ++++------
 2 files changed, 5 insertions(+), 7 deletions(-)

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..eca1424 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)
 {
@@ -340,9 +342,5 @@ static struct platform_driver imx8mm_pinctrl_driver = {
 	},
 	.probe = imx8mm_pinctrl_probe,
 };
-
-static int __init imx8mm_pinctrl_init(void)
-{
-	return platform_driver_register(&imx8mm_pinctrl_driver);
-}
-arch_initcall(imx8mm_pinctrl_init);
+module_platform_driver(imx8mm_pinctrl_driver);
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

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

[Anson Huang]

Support building i.MX8MN pinctrl driver as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change.
---
 drivers/pinctrl/freescale/Kconfig          |  2 +-
 drivers/pinctrl/freescale/pinctrl-imx8mn.c | 10 ++++------
 2 files changed, 5 insertions(+), 7 deletions(-)

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..b6db780 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)
 {
@@ -340,9 +342,5 @@ static struct platform_driver imx8mn_pinctrl_driver = {
 	},
 	.probe = imx8mn_pinctrl_probe,
 };
-
-static int __init imx8mn_pinctrl_init(void)
-{
-	return platform_driver_register(&imx8mn_pinctrl_driver);
-}
-arch_initcall(imx8mn_pinctrl_init);
+module_platform_driver(imx8mn_pinctrl_driver);
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

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

[Daniel Baluta]

Maybe this is obvious but I would really like to see an explanation

of why we are switching from arch_initcall to platform_init.

Commit message act as documentation  for the reviewers.

On 10.06.2020 10:57, Anson Huang wrote:
> Support building i.MX8MN pinctrl driver as module.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> No change.
> ---
>   drivers/pinctrl/freescale/Kconfig          |  2 +-
>   drivers/pinctrl/freescale/pinctrl-imx8mn.c | 10 ++++------
>   2 files changed, 5 insertions(+), 7 deletions(-)
>
> 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..b6db780 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)
>   {
> @@ -340,9 +342,5 @@ static struct platform_driver imx8mn_pinctrl_driver = {
>   	},
>   	.probe = imx8mn_pinctrl_probe,
>   };
> -
> -static int __init imx8mn_pinctrl_init(void)
> -{
> -	return platform_driver_register(&imx8mn_pinctrl_driver);
> -}
> -arch_initcall(imx8mn_pinctrl_init);
> +module_platform_driver(imx8mn_pinctrl_driver);
> +MODULE_LICENSE("GPL v2");

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

[Anson Huang]

Hi, Daniel

> Subject: Re: [PATCH V4 4/9] pinctrl: imx8mn: Support building as module
> 
> Maybe this is obvious but I would really like to see an explanation
> 
> of why we are switching from arch_initcall to platform_init.
> 
> Commit message act as documentation  for the reviewers.

Yes, I noticed this, and it looks like unnecessary change, and I just replied in
mail that I will change it back to arch_initcall() in order to make sure nothing
changed for built-in config. Below is what I replied in cover-letter mail:

" I will keep the arch_initcall() there in next version patch series, it can make sure
the change does NOT impact built-in config. For module build, the arch_initcall()
will be same as module_init(), user needs to insmod the .ko with correct sequence."

Thanks,
Anson

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

[Daniel Baluta]

On 11.06.2020 11:48, Anson Huang wrote:
> Hi, Daniel
>
>> Subject: Re: [PATCH V4 4/9] pinctrl: imx8mn: Support building as module
>>
>> Maybe this is obvious but I would really like to see an explanation
>>
>> of why we are switching from arch_initcall to platform_init.
>>
>> Commit message act as documentation  for the reviewers.
> Yes, I noticed this, and it looks like unnecessary change, and I just replied in
> mail that I will change it back to arch_initcall() in order to make sure nothing
> changed for built-in config. Below is what I replied in cover-letter mail:
>
> " I will keep the arch_initcall() there in next version patch series, it can make sure
> the change does NOT impact built-in config. For module build, the arch_initcall()
> will be same as module_init(), user needs to insmod the .ko with correct sequence."

Ok, that's great. Lets try to keep in mind that the commit message should

answer to a simple question: "Why the change is needed" :).


Then, the details about the change should be added.

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

[Anson Huang]

Support building i.MX8MQ pinctrl driver as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change.
---
 drivers/pinctrl/freescale/Kconfig          | 2 +-
 drivers/pinctrl/freescale/pinctrl-imx8mq.c | 9 ++++-----
 2 files changed, 5 insertions(+), 6 deletions(-)

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..db5b41a 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)
 {
@@ -345,8 +347,5 @@ static struct platform_driver imx8mq_pinctrl_driver = {
 	.probe = imx8mq_pinctrl_probe,
 };
 
-static int __init imx8mq_pinctrl_init(void)
-{
-	return platform_driver_register(&imx8mq_pinctrl_driver);
-}
-arch_initcall(imx8mq_pinctrl_init);
+module_platform_driver(imx8mq_pinctrl_driver);
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

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

[Anson Huang]

Support building i.MX8MP pinctrl driver as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change.
---
 drivers/pinctrl/freescale/Kconfig          |  2 +-
 drivers/pinctrl/freescale/pinctrl-imx8mp.c | 10 ++++------
 2 files changed, 5 insertions(+), 7 deletions(-)

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..f3f3bdd 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)
 {
@@ -337,9 +339,5 @@ static struct platform_driver imx8mp_pinctrl_driver = {
 	},
 	.probe = imx8mp_pinctrl_probe,
 };
-
-static int __init imx8mp_pinctrl_init(void)
-{
-	return platform_driver_register(&imx8mp_pinctrl_driver);
-}
-arch_initcall(imx8mp_pinctrl_init);
+module_platform_driver(imx8mp_pinctrl_driver);
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

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

[Anson Huang]

Support building i.MX8QXP pinctrl driver as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change.
---
 drivers/pinctrl/freescale/Kconfig           | 2 +-
 drivers/pinctrl/freescale/pinctrl-imx8qxp.c | 9 +++------
 2 files changed, 4 insertions(+), 7 deletions(-)

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..4544822 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)
 {
@@ -227,9 +228,5 @@ static struct platform_driver imx8qxp_pinctrl_driver = {
 	},
 	.probe = imx8qxp_pinctrl_probe,
 };
-
-static int __init imx8qxp_pinctrl_init(void)
-{
-	return platform_driver_register(&imx8qxp_pinctrl_driver);
-}
-arch_initcall(imx8qxp_pinctrl_init);
+module_platform_driver(imx8qxp_pinctrl_driver);
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

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

[Anson Huang]

Support building i.MX8QM pinctrl driver as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change.
---
 drivers/pinctrl/freescale/Kconfig          | 2 +-
 drivers/pinctrl/freescale/pinctrl-imx8qm.c | 9 +++------
 2 files changed, 4 insertions(+), 7 deletions(-)

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..a30f074 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)
 {
@@ -321,9 +322,5 @@ static struct platform_driver imx8qm_pinctrl_driver = {
 	},
 	.probe = imx8qm_pinctrl_probe,
 };
-
-static int __init imx8qm_pinctrl_init(void)
-{
-	return platform_driver_register(&imx8qm_pinctrl_driver);
-}
-arch_initcall(imx8qm_pinctrl_init);
+module_platform_driver(imx8qm_pinctrl_driver);
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

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

[Anson Huang]

Support building i.MX8DXL pinctrl driver as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change.
---
 drivers/pinctrl/freescale/Kconfig           | 2 +-
 drivers/pinctrl/freescale/pinctrl-imx8dxl.c | 9 +++------
 2 files changed, 4 insertions(+), 7 deletions(-)

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..9af09e3 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)
 {
@@ -188,9 +189,5 @@ static struct platform_driver imx8dxl_pinctrl_driver = {
 	},
 	.probe = imx8dxl_pinctrl_probe,
 };
-
-static int __init imx8dxl_pinctrl_init(void)
-{
-	return platform_driver_register(&imx8dxl_pinctrl_driver);
-}
-arch_initcall(imx8dxl_pinctrl_init);
+module_platform_driver(imx8dxl_pinctrl_driver);
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

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

[Daniel Baluta]

Hi Anson,

Patch series mostly looks good to me. I have a comment about adding

the MODULE_LICENSE. This is a pretty important change.


Can you please add this change in a separate patch with a proper explanation

of why it is needed.


Most likely it is because it was forgotten in the previous patches.


thanks,

daniel.

On 10.06.2020 10:57, Anson Huang wrote:
> 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 V3, the changes are as below:
> 	- change the config dependency back to original;
> 	- use function callbacks for SCU related functions, and all drivers
> 	  using SCU pinctrl driver need to initialize the function callbacks,
> 	  pinctrl-imx.c will check the SCU function callback and call it when
> 	  it is valid, then no build issue when PINCTRL_IMX is built in and
> 	  PINCTRL_IMX_SCU is built as 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     | 22 ++++++-----
>   drivers/pinctrl/freescale/pinctrl-imx.h     | 57 ++++++++++++-----------------
>   drivers/pinctrl/freescale/pinctrl-imx8dxl.c | 12 +++---
>   drivers/pinctrl/freescale/pinctrl-imx8mm.c  | 10 ++---
>   drivers/pinctrl/freescale/pinctrl-imx8mn.c  | 10 ++---
>   drivers/pinctrl/freescale/pinctrl-imx8mp.c  | 10 ++---
>   drivers/pinctrl/freescale/pinctrl-imx8mq.c  |  9 ++---
>   drivers/pinctrl/freescale/pinctrl-imx8qm.c  | 12 +++---
>   drivers/pinctrl/freescale/pinctrl-imx8qxp.c | 12 +++---
>   drivers/pinctrl/freescale/pinctrl-scu.c     |  6 +++
>   11 files changed, 86 insertions(+), 93 deletions(-)
>

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

[Anson Huang]

Hi, Daniel

> Subject: Re: [PATCH V4 0/9] Support i.MX8 SoCs pinctrl drivers built as module
> 
> Hi Anson,
> 
> Patch series mostly looks good to me. I have a comment about adding
> 
> the MODULE_LICENSE. This is a pretty important change.
> 
> 
> Can you please add this change in a separate patch with a proper explanation
> 
> of why it is needed.
> 
> 
> Most likely it is because it was forgotten in the previous patches.

Yes, it is obviously missed in the previous patches, as previously these pinctrl drivers
do NOT support module build at all.

And MODULE_LICENSE is a MUST when drivers supporting module build, build will report failure
if module license missed, so I think it is also part of the module build support patch,
do you mean it is better to add a separate patch to add the MODULE_LICENSE to all pinctrl drivers missing it?
Maybe we can get more opinion from maintainer, I am NOT very sure whether it is better to separate
the module license as a single patch....

thanks,
Anson

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

[Daniel Baluta]

On 11.06.2020 11:44, Anson Huang wrote:
> Hi, Daniel
>
>> Subject: Re: [PATCH V4 0/9] Support i.MX8 SoCs pinctrl drivers built as module
>>
>> Hi Anson,
>>
>> Patch series mostly looks good to me. I have a comment about adding
>>
>> the MODULE_LICENSE. This is a pretty important change.
>>
>>
>> Can you please add this change in a separate patch with a proper explanation
>>
>> of why it is needed.
>>
>>
>> Most likely it is because it was forgotten in the previous patches.
> Yes, it is obviously missed in the previous patches, as previously these pinctrl drivers
> do NOT support module build at all.
>
> And MODULE_LICENSE is a MUST when drivers supporting module build, build will report failure
> if module license missed, so I think it is also part of the module build support patch,
> do you mean it is better to add a separate patch to add the MODULE_LICENSE to all pinctrl drivers missing it?
> Maybe we can get more opinion from maintainer, I am NOT very sure whether it is better to separate
> the module license as a single patch....


Hi Anson,

This is my feeling. That the first patch in the series should add the 
MODULE_LICENSE to all

of files.

This keeps things simple, explains our intention with next patches.

Daniel.