| Message ID | 1591775865-26872-5-git-send-email-Anson.Huang@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support i.MX8 SoCs pinctrl drivers built as module | expand |
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");
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
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.
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");
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(-)