Re: [PATCH v4 1/2] pinctrl: make MediaTek pinctrl v2 driver ready for buidling loadable module

[Sean Wang]

Hi, Light

On Tue, Mar 17, 2020 at 2:28 AM Light Hsieh <light.hsieh@xxxxxxxxxxxx> wrote:
>
> On Sun, 2020-03-15 at 14:35 -0700, Sean Wang wrote:
> > Hi, Light
> >
> > On Tue, Mar 10, 2020 at 12:39 AM <light.hsieh@xxxxxxxxxxxx> wrote:
> > >
> > > From: Light Hsieh <light.hsieh@xxxxxxxxxxxx>
> > >
> >
> > Correct the prefix of the subject with "pinctrl: mediatek".
> >
> > > In the future we want to be able to build the MediaTek pinctrl driver,
> > > based on paris, as kernel module. This patch allows pinctrl-paris.c, the
> > > external interrupt controller mtk-eint.c, and pinctrl-mtk-common-v2.c to
> > > be loadable as module.
> > >
> > > Signed-off-by: Light Hsieh <light.hsieh@xxxxxxxxxxxx>
> > > Reviewed-by: Matthias Brugger <matthias.bgg@xxxxxxxxx>
> > > ---
> > >  drivers/pinctrl/mediatek/Kconfig                 | 13 +++++++++++--
> > >  drivers/pinctrl/mediatek/Makefile                |  5 +++--
> > >  drivers/pinctrl/mediatek/mtk-eint.c              |  9 +++++++++
> > >  drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c | 24 ++++++++++++++++++++++++
> > >  drivers/pinctrl/mediatek/pinctrl-paris.c         |  6 ++++++
> > >  drivers/pinctrl/pinconf-generic.c                |  1 +
> >
> > Separate the patches for the device driver and for the core respectively.
>
> Since the patch for pinconf-generic.c had been done in  b88d145191ad
> ("pinctrl: Export some needed symbols at module load time").
> I will remove patch for pinconf-generic.c.
>
> > >  6 files changed, 54 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/pinctrl/mediatek/Kconfig b/drivers/pinctrl/mediatek/Kconfig
> > > index 701f9af..4cd1109 100644
> > > --- a/drivers/pinctrl/mediatek/Kconfig
> > > +++ b/drivers/pinctrl/mediatek/Kconfig
> > > @@ -3,10 +3,12 @@ menu "MediaTek pinctrl drivers"
> > >         depends on ARCH_MEDIATEK || COMPILE_TEST
> > >
> > >  config EINT_MTK
> > > -       bool "MediaTek External Interrupt Support"
> > > +       tristate "MediaTek External Interrupt Support"
> > >         depends on PINCTRL_MTK || PINCTRL_MTK_MOORE || PINCTRL_MTK_PARIS || COMPILE_TEST
> > >         select GPIOLIB
> > >         select IRQ_DOMAIN
> > > +       default y if PINCTRL_MTK || PINCTRL_MTK_MOORE
> > > +       default PINCTRL_MTK_PARIS
> > >
> > >  config PINCTRL_MTK
> > >         bool
> > > @@ -17,23 +19,30 @@ config PINCTRL_MTK
> > >         select EINT_MTK
> > >         select OF_GPIO
> > >
> > > +config PINCTRL_MTK_V2
> > > +       tristate
> > > +       depends on PINCTRL_MTK_MOORE || PINCTRL_MTK_PARIS
> >
> > PINCTRL_MTK_V2 doesn't have to depend on PINCTRL_MTK_MOORE or
> > PINCTRL_MTK_PARIS to work and it can build on it own so that lets us
> > remove the dependency.
> >
> The usage of PINCTRL_MTK had been describe in cover letter:
>
> 1.4 PINCTRL_MTK_V2: add this tristate config which depends on
>         PINCTRL_MTK_PARIS and PINCTRL_MTK_MOORE. No need to specify in
>         defconfig file.
>       * PINCTRL_MTK_V2 will be y if either PINCTRL_MTK_PARIS or
>         PINCTRL_MTK_MOORE is y.
>       * PINCTRL_MTK_V2 will be n if both PINCTRL_MTK_PARIS and
>         PINCTRL_MTK_MOORE are n.
>       * PINCTRL_MTK_V2 will be m if PINCTRL_MTK_PARIS is m and
>         PINCTRL_MTK_MOORE is n.
>
>
> Without PINCTRL_MTK_V2, we cannot specify whether
> pinctrl-mtk-common-v2.c is built-in or loadable kernel module.
>

What I was actually meaning is

config PINCTRL_MTK_V2
          tristate

with that should work for your case.

> > > +
> > >  config PINCTRL_MTK_MOORE
> > >         bool
> > >         depends on OF
> > >         select GENERIC_PINCONF
> > >         select GENERIC_PINCTRL_GROUPS
> > >         select GENERIC_PINMUX_FUNCTIONS
> > > +       select EINT_MTK
> >
> > The original design of PINCTRL_MTK_MOORE doesn't have to work with
> > EINT_MTK coupled tightly so that let us remove the reverse dependency.
> >
>
> Actually, PINCTRL_MTK_MOORE use EINT_MTK in the following call flow:
> mtk_moore_pinctrl_probe() -> mtk_build_eint() -> mtk_eint_do_init().
> mtk_eint_do_init() is defined in mtk-eint.c.

CONFIG_EINT_MTK is actually an option to Moore pinctrl since Moore
pinctrl has already carefully considered the condition with
CONFIG_EINT_MTK=n in mtk_build_eint.

>
> > >         select GPIOLIB
> > >         select OF_GPIO
> > > +       select PINCTRL_MTK_V2