diff mbox series

[V3,3/3] pinctrl: imx: Support building i.MX pinctrl core driver as module

Message ID 1599549126-17413-3-git-send-email-Anson.Huang@nxp.com
State New
Headers show
Series [V3,1/3] pinctrl: imx: Use function callbacks for SCU related functions | expand

Commit Message

Anson Huang Sept. 8, 2020, 7:12 a.m. UTC 
Change PINCTRL_IMX to tristate to support loadable module build.

And 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 different
architectures without CONFIG_OF.

Also add module author, description and license.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
---
no change.
---
 drivers/pinctrl/freescale/Kconfig       | 3 ++-
 drivers/pinctrl/freescale/pinctrl-imx.c | 5 +++++
 2 files changed, 7 insertions(+), 1 deletion(-)

Comments

Arnd Bergmann Sept. 8, 2020, 7:31 a.m. UTC | #1 
On Tue, Sep 8, 2020 at 9:20 AM Anson Huang <Anson.Huang@nxp.com> wrote:

>  # SPDX-License-Identifier: GPL-2.0-only
>  config PINCTRL_IMX
> -       bool
> +       tristate "IMX pinctrl core driver"
> +       depends on OF
>         select GENERIC_PINCTRL_GROUPS
>         select GENERIC_PINMUX_FUNCTIONS
>         select GENERIC_PINCONF

I don't see why you need to make this option user-visible when it is already
selected by the drivers that need it. Wouldn't it be enough to change
the 'bool' to 'tristate' without adding a prompt?

       Arnd
Anson Huang Sept. 8, 2020, 7:34 a.m. UTC | #2 
> Subject: Re: [PATCH V3 3/3] pinctrl: imx: Support building i.MX pinctrl core
> driver as module
> 
> On Tue, Sep 8, 2020 at 9:20 AM Anson Huang <Anson.Huang@nxp.com>
> wrote:
> 
> >  # SPDX-License-Identifier: GPL-2.0-only  config PINCTRL_IMX
> > -       bool
> > +       tristate "IMX pinctrl core driver"
> > +       depends on OF
> >         select GENERIC_PINCTRL_GROUPS
> >         select GENERIC_PINMUX_FUNCTIONS
> >         select GENERIC_PINCONF
> 
> I don't see why you need to make this option user-visible when it is already
> selected by the drivers that need it. Wouldn't it be enough to change the 'bool'
> to 'tristate' without adding a prompt?

Make sense, so it is same for PINCTRL_IMX_SCU, right?

Anson
Arnd Bergmann Sept. 8, 2020, 7:45 a.m. UTC | #3 
On Tue, Sep 8, 2020 at 9:34 AM Anson Huang <anson.huang@nxp.com> wrote:
> > Subject: Re: [PATCH V3 3/3] pinctrl: imx: Support building i.MX pinctrl core
> > driver as module
> >
> > On Tue, Sep 8, 2020 at 9:20 AM Anson Huang <Anson.Huang@nxp.com>
> > wrote:
> >
> > >  # SPDX-License-Identifier: GPL-2.0-only  config PINCTRL_IMX
> > > -       bool
> > > +       tristate "IMX pinctrl core driver"
> > > +       depends on OF
> > >         select GENERIC_PINCTRL_GROUPS
> > >         select GENERIC_PINMUX_FUNCTIONS
> > >         select GENERIC_PINCONF
> >
> > I don't see why you need to make this option user-visible when it is already
> > selected by the drivers that need it. Wouldn't it be enough to change the 'bool'
> > to 'tristate' without adding a prompt?
>
> Make sense, so it is same for PINCTRL_IMX_SCU, right?

Yes, correct.

I wasn't on Cc on the other two patches, so I missed that.

     Arnd
Anson Huang Sept. 8, 2020, 7:53 a.m. UTC | #4 
Hi, Arnd

> Subject: Re: [PATCH V3 3/3] pinctrl: imx: Support building i.MX pinctrl core
> driver as module
> 
> On Tue, Sep 8, 2020 at 9:34 AM Anson Huang <anson.huang@nxp.com>
> wrote:
> > > Subject: Re: [PATCH V3 3/3] pinctrl: imx: Support building i.MX
> > > pinctrl core driver as module
> > >
> > > On Tue, Sep 8, 2020 at 9:20 AM Anson Huang <Anson.Huang@nxp.com>
> > > wrote:
> > >
> > > >  # SPDX-License-Identifier: GPL-2.0-only  config PINCTRL_IMX
> > > > -       bool
> > > > +       tristate "IMX pinctrl core driver"
> > > > +       depends on OF
> > > >         select GENERIC_PINCTRL_GROUPS
> > > >         select GENERIC_PINMUX_FUNCTIONS
> > > >         select GENERIC_PINCONF
> > >
> > > I don't see why you need to make this option user-visible when it is
> > > already selected by the drivers that need it. Wouldn't it be enough to
> change the 'bool'
> > > to 'tristate' without adding a prompt?
> >
> > Make sense, so it is same for PINCTRL_IMX_SCU, right?
> 
> Yes, correct.
> 
> I wasn't on Cc on the other two patches, so I missed that.

Sorry, I missed to add you to the list as I just use the list from get_maintainer script, will
add you in V4. Since most of the major comments are addressed, I will send V4 soon.

Thanks,
Anson
diff mbox series

Patch

diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index 452c499..0058d3a 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 core 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 b80c450..daf28bc 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>
@@ -898,3 +899,7 @@  const struct dev_pm_ops imx_pinctrl_pm_ops = {
 					imx_pinctrl_resume)
 };
 EXPORT_SYMBOL_GPL(imx_pinctrl_pm_ops);
+
+MODULE_AUTHOR("Dong Aisheng <aisheng.dong@nxp.com>");
+MODULE_DESCRIPTION("NXP i.MX common pinctrl driver");
+MODULE_LICENSE("GPL v2");