Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <1595382353-17486-1-git-send-email-Anson.Huang@nxp.com>
 <CAK8P3a13gcF_+dkfxZW0u_YuJ92hY1JukWfzM+e30iM=YUhraQ@mail.gmail.com>
 <DB3PR0402MB3916F080E4912B27B18BEADEF5720@DB3PR0402MB3916.eurprd04.prod.outlook.com>
 <CAK8P3a2CBYV2xEkedQYmzL4XgHPeu02=vmLffq+RWwvEvuUGKQ@mail.gmail.com> <DB3PR0402MB391674F67A1B9F2732883C0BF5720@DB3PR0402MB3916.eurprd04.prod.outlook.com>
In-Reply-To: <DB3PR0402MB391674F67A1B9F2732883C0BF5720@DB3PR0402MB3916.eurprd04.prod.outlook.com>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Mon, 27 Jul 2020 13:56:50 +0200
Message-ID: <CAK8P3a0XpKnbz79dH4i7HofGgpAodtmgdBmVBVQOKfCiJMkpPw@mail.gmail.com>
Subject: Re: [PATCH V2 1/4] gpio: mxc: Support module build
To:     Anson Huang <anson.huang@nxp.com>
Cc:     Russell King - ARM Linux <linux@armlinux.org.uk>,
        Shawn Guo <shawnguo@kernel.org>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        Sascha Hauer <kernel@pengutronix.de>,
        Fabio Estevam <festevam@gmail.com>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>,
        Linus Walleij <linus.walleij@linaro.org>,
        Bartosz Golaszewski <bgolaszewski@baylibre.com>,
        Peter Chen <peter.chen@nxp.com>,
        "oleksandr.suvorov@toradex.com" <oleksandr.suvorov@toradex.com>,
        Andreas Kemnade <andreas@kemnade.info>,
        Peng Fan <peng.fan@nxp.com>,
        Hans Verkuil <hverkuil-cisco@xs4all.nl>,
        Olof Johansson <olof@lixom.net>,
        Krzysztof Kozlowski <krzk@kernel.org>,
        Alexandre Torgue <alexandre.torgue@st.com>,
        Patrice Chotard <patrice.chotard@st.com>,
        Marek Szyprowski <m.szyprowski@samsung.com>,
        Joel Stanley <joel@jms.id.au>, Lubomir Rintel <lkundrak@v3.sk>,
        Christian Gmeiner <christian.gmeiner@gmail.com>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Leo Li <leoyang.li@nxp.com>,
        Geert Uytterhoeven <geert+renesas@glider.be>,
        "michael@walle.cc" <michael@walle.cc>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
        dl-linux-imx <linux-imx@nxp.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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

      Arnd