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

[Anson Huang <anson.huang@xxxxxxx>]

Hi, Arnd


> Subject: Re: [PATCH V2 1/4] gpio: mxc: Support module build
> 
> On Mon, Jul 27, 2020 at 1:21 PM Anson Huang <anson.huang@xxxxxxx>
> wrote:
> > > Subject: Re: [PATCH V2 1/4] gpio: mxc: Support module build On Mon,
> > > Jul 27, 2020 at 10:18 AM Anson Huang <anson.huang@xxxxxxx> wrote:
> > > > > Subject: Re: [PATCH V2 1/4] gpio: mxc: Support module build
> > > >
> > > > commit e188cbf7564fba80e8339b9406e8740f3e495c63
> > > > Author: Vladimir Zapolskiy <vladimir_zapolskiy@xxxxxxxxxx>
> > > > Date:   Thu Sep 8 04:48:15 2016 +0300
> > > >
> > > >     gpio: mxc: shift gpio_mxc_init() to subsys_initcall level
> > >
> > > That commit made the initialization later not earlier, as it
> > > originally was a postcore_initcall(). In the loadable module case,
> > > you make it even later than that, possibly as the last module loaded
> > > when booting up the system (followed by a storm of deferred probes).
> > >
> >
> > Yes, loadable module will make it even later, the assumption is
> > userspace can load it before any users depend on GPIO driver. Given
> > that we have to support loadable module for all SoC specific module,
> > do you have any other suggestion of how to proceed this requirement for
> SoC GPIO driver?
> 
> I think in general, drivers should be prepared for -EPROBE_DEFER error codes
> returned from interfaces such as devm_gpiod_get().
> 
> > > I don't think we can rely on user space to coordinate module load order.
> > > The modules are generally loaded in an arbitrary order during the
> > > coldplug phase of the boot when user space looks at the available
> > > devices and loads a module for each one of them in the order it finds them
> in sysfs.
> > >
> > > This means all drivers that rely on gpio, pinctrl or irqchip
> > > interfaces exported from this driver have to be able to deal with
> > > them not being there. This can also happen when the pinctrl driver
> > > is the only one that is a loadable module, while everything else is
> > > built-in. While that is not a configuration that users would likely choose
> intentionally, I don't see a reason why it shouldn't work.
> > >
> > > Using module_init() or builtin_platform_driver() here would make
> > > give similar behavior for the built-in and modular cases and be
> > > somewhat more consistent, so you don't run into bugs only when the
> > > driver is a loadable module but make them obvious even to existing users
> with a builtin driver.
> > >
> >
> > My original idea of adding loadable module support for SoC specific
> > module is, try to keep it exactly same when the driver is built-in,
> > but for GKI support, first, we need to support GPIO driver built as
> > module, and we definitely need to think about the module load sequence
> > to handle these dependency, but thinking about the common module
> > widely used by devices, such as pinctrl, clock and GPIO, maybe other
> modules need some patches to handle the dependency, but that will be done
> later when we are working for those modules.
> 
> Overall, my feeling is that making sure all drivers that depend on the pinctrl
> driver can deal with deferred probing is a prerequisite before this can be made
> a loadable module itself (same for clk, irqchip, etc drivers that others may rely
> on).
> 
> I understand that your primary motivation is to fit into Google's GKI
> framework, but I think that doing the conversion only partially would neither
> serve to improve the kernel nor actually meet the GKI requirements.
> 
> Most pinctrl drivers are currently always built-in to work around the load order
> dependencies. This of course is a bit of a hack and we'd be better off if all
> drivers managed to avoid the dependencies, but this can also require a lot of
> work.
> 
> > So, could you please help advise how to proceed it for this GPIO
> > driver to  support loadable module?
> 
> I would start by getting a reference board to work with a kernel in which all
> drivers are built-in except for the pinctrl driver, to see what exactly breaks
> when you do that, and what other drivers may have the same problems.
> Maybe it's not that bad after all and you only need a few modifications.
> 

I agreed, but the situation is i.MX SoC contains more than 20 modules, and most of
them are NOT owned by me, so I am NOT sure when the module owner will start
working on the support. And if with minimum devices enabled, such as tiny kernel
with ramfs, it is working even with pinctrl/clock etc. built as loadable module.

Meanwhile, as you said, most of the users are still using built-in model, so adding the
support for GPIO can be in parallel with other modules' work, in other words, with this
GPIO loadable module support patch, if other modules can NOT work due to lack of
defer probe implementation, then the patch should be done in other module, adding
that the default configuration of GPIO is still built-in, do you think it can be an independent
patch and get into linux-next first?

Thanks,
Anson