Re: [PATCH 0/2] irq-meson-gpio: make it possible to build as a module

Marc Zyngier [off-list ref] writes:

On 2020-10-26 16:18, Kevin Hilman wrote:

quoted

Marc Zyngier [off-list ref] writes:

quoted

On Tue, 20 Oct 2020 08:25:30 +0100,
Neil Armstrong [off-list ref] wrote:

quoted

In order to reduce the kernel Image size on multi-platform 
distributions,
make it possible to build the Amlogic GPIO IRQ controller as a module
by switching it to a platform driver.

The second patch removes MESON_IRQ_GPIO selection from ARCH_MESON to 
allow
building the driver as module.

Neil Armstrong (2):
  irqchip: irq-meson-gpio: make it possible to build as a module
  arm64: meson: remove MESON_IRQ_GPIO selection

 arch/arm64/Kconfig.platforms     |  1 -
 drivers/irqchip/Kconfig          |  5 +-
 drivers/irqchip/irq-meson-gpio.c | 89 
++++++++++++++++++++------------
 3 files changed, 59 insertions(+), 36 deletions(-)

I've tried this series on my vim3l with the this driver compiled as a
module, and lost the Ethernet interface in the process, as the phy
wasn't able to resolve its interrupt and things fail later on:

[   72.238291] meson8b-dwmac ff3f0000.ethernet eth1: no phy at addr -1
[   72.238917] meson8b-dwmac ff3f0000.ethernet eth1: stmmac_open: 
Cannot attach to PHY (error: -19)

This is a generic problem with making DT-based interrupt controllers
modular when not *all* the drivers can deal with probing deferral.

Yes, but this series still keeps the default as built-in.

If you build as a module, and you add `fw_devlink=on` to the kernel
command-line, device-links will be created based on DT dependencies
which will ensure the right module load order.

It doesn't work here. I get the exact same error (well, with eth0 
instead
of eth1). In my experience, fw_devlink isn't reliable yet. Config on 
request.

quoted

I've tested this series with `fw_devlink=on` on several Amlogic
platforms and it works just fine, but since it requires the extra
cmdline option, I think the default should remain built-in.

So, I'd still like to see this series merged so that at least it's an
option to enable this as a module.

I have taken similar patches in 5.9 for other SoC families (qcomm, mtk),
and ended up reverting them in -rc2, because there is simply too much
breakage. Even keeping it as built in changes the init order, which
tons of drivers depend on. I proposed a middle-of-the-road approach
(modules can break, built-in stays the same) which Rob pushed back on.

So either we fix fw_devlink to work for everything and be on by default,
or we keep the current setup.

quoted

Also, another reason to make it optional is that not all platforms need
this feature at all, but right now we select it for all Amlogic SoCs.

I understand that, but I don't want another episode of widespread
breakages, and this series definitely breaks things.

Yeah, I understand there's potential breakage things if you enable build
as a module, but that's not the default with this series so one would
have to do that on purpose.

If you don't manually enable as a module and leave the default as
built-in (e.g. CONFIG_MESON_IRQ_GPIO=y) does it work for you?  Using
your .config, I used the default to built-in and this series booted fine
for me on top of v5.10-rc1 on my VIM3L.

Using your .config, I do see issues with this driver when explicity
enabling as a module, and I'll debug those, but again, leaving it as
built-in, it's working fine for me with your .config.

I understand that even building as a built-in might change link order,
but I'd be pretty surprised if just changing from OF_DECLARE to a
built-in module made things break.

If so, I can commit to helping find/fix any issues there.

For that matter, I'm actively working on making all amlogic drivers
modular (including clocks, pinctrl, reset, etc.) and I have it all
working locally several SoCs, so I will also commit to actively help
find/fix any issues this uncovers with ordering.

My goal is to enable the *option* of building everything as modules (but
leave them as built-in by default) so I can help debug any remaining
fw_devlink issues.

Kevin


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