RE: [PATCH 1/3] gpio: mxc: Support module build

[Anson Huang <anson.huang@nxp.com>]

Hi, Linus

> Subject: Re: [PATCH 1/3] gpio: mxc: Support module build
> 
> On Wed, Jul 15, 2020 at 4:44 AM Anson Huang <anson.huang@nxp.com>
> wrote:
> 
> > > Subject: RE: [PATCH 1/3] gpio: mxc: Support module build
> > >
> > > Hi, Linus
> > >
> > > > Subject: Re: [PATCH 1/3] gpio: mxc: Support module build
> > > >
> > > > On Wed, Jul 8, 2020 at 1:28 AM Anson Huang <Anson.Huang@nxp.com>
> > > > wrote:
> > > >
> > > > >  subsys_initcall(gpio_mxc_init);
> > > > > +
> > > > > +MODULE_AUTHOR("Shawn Guo <shawn.guo@linaro.org>");
> > > > > +MODULE_DESCRIPTION("i.MX GPIO Driver");
> MODULE_LICENSE("GPL");
> > > >
> > > > You are making this modualrizable but keeping the
> > > > subsys_initcall(), which doesn't make very much sense. It is
> > > > obviously not necessary to do this probe at subsys_initcall() time, right?
> > > >
> > >
> > > If building it as module, the subsys_initcall() will be equal to
> > > module_init(), I keep it unchanged is because I try to make it
> > > identical when built-in, since most of the config will still have it built-in,
> except the Android GKI support.
> > > Does it make sense?
> > >
> > > > Take this opportunity to convert the driver to use
> > > > module_platform_driver() as well.
> > >
> > > If you think it has to be or it is better to use
> > > module_platform_driver(), I will do it in V2.
> >
> > I tried to replace the subsys_initcall() with
> > module_platform_driver(), but met issue about "
> > register_syscore_ops(&mxc_gpio_syscore_ops);" which is called in
> > gpio_mxc_init() function, this function should be called ONLY once,
> > moving it to .probe function is NOT working, so we may need to keep the
> gpio_mxc_init(), that is another reason that we may need to keep
> subsys_initcall()?
> 
> This looks a bit dangerous to keep like this while allowing this code to be used
> from a module.
> 
> What happens if you insmod and rmmod this a few times, really?
> How is this tested?
> 
> This is not really modularized if that isn't working, just that modprobing once
> works isn't real modularization IMO, it seems more like a quick and dirty way
> to get Androids GKI somewhat working with the module while not properly
> making the module a module.
> 
> You need input from the driver maintainers on how to handle this.

As far as I know, some general/critical modules are NOT supporting rmmod, like
clk, pinctrl, gpio etc., and I am NOT sure whether Android GKI need to support
rmmod for these system-wide-used module, I will ask them for more detail about
this.

The requirement I received is to support loadable module, but so far no hard requirement
to support module remove for gpio driver, so, is it OK to add it step by step, and this patch
series ONLY to support module build and one time modprobe?  

Thanks,
Anson
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel