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

[Arnd Bergmann <arnd@arndb.de>]

On Mon, Jul 27, 2020 at 2:23 PM Anson Huang <anson.huang@nxp.com> wrote:
> > Subject: Re: [PATCH V2 1/4] gpio: mxc: Support module build
> >
> > > 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.

Do you have an example that is actually broken? I checked how the gpio
chip is actually used and found that "regulator-fixed", "virtual,mdio-gpio",
"regulator-gpio", "gpio-leds", "marvell,mv88e6085", "microchip,usb2513b",
"fsl,imx7d-usdhc", "fsl,imx6sx-fec", "mmc-pwrseq-simple", "brcm,bcm43438-bt",
 "rohm,bd71837", "nxp,pca9546",  "rtc-m41t80", should all work fine here.

I'm not sure about "fsl,mma8451", maybe test that one manually or look
at the driver in more detail.

"fsl,imx8mq-pcie" looks broken but easily fixed, and this is something we
have already discussed.

imx8mq-nitrogen.c has a "vsel-gpios" property in its "fcs,fan53555"
device node that is neither part of the binding nor handled by the
driver, so this is broken regardless of the gpio driver.

> 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?

I think you should be reasonably sure that making the driver a loadable module
does not break other drivers that might rely on the probe order and
that are known
to be used with an i.MX chip. With the list above, that seems to actually be
the case for the most part, but testing is always better.

If there are boards that use other drivers which do not support deferred probing
but don't have those listed in the dts files in the kernel, then that
is not something
you have to worry about I think.

I'll let Linus Walleij comment on whether he thinks the initcall should stay
at subsys_initcall() to avoid breaking users with buggy drivers, or whether
this should be changed to module_init() or builtin_platform_driver() to
have a better chance of finding and fixing those broken drivers.

     Arnd