Re: [RFC PATCH v1] irqchip: Add IRQCHIP_MODULE_BEGIN/END helper macros

[John Stultz]

On Sat, Apr 11, 2020 at 2:14 AM Marc Zyngier <maz@xxxxxxxxxx> wrote:
> On Sat, 11 Apr 2020 05:59:18 +0100,
> Saravana Kannan <saravanak@xxxxxxxxxx> wrote:
> >
> > Add helper macros IRQCHIP_MODULE_BEGIN and IRQCHIP_MODULE_END that add
> > the boilerplate code to be able to compile an irqchip driver as a
> > module.
> >
> > The driver developer just needs to do add IRQCHIP_MODULE_BEGIN and
> > IRQCHIP_MODULE_END(driver_name) around the IRQCHIP_DECLARE macros, like
> > so:
> >
> > IRQCHIP_MODULE_BEGIN
> > IRQCHIP_DECLARE(foo, "acme,foo", acme_foo_init)
> > IRQCHIP_DECLARE(bar, "acme,bar", acme_bar_init)
> > IRQCHIP_MODULE_END(acme_irq)
> >
> > Cc: John Stultz <john.stultz@xxxxxxxxxx>
> > Signed-off-by: Saravana Kannan <saravanak@xxxxxxxxxx>
> > ---
> > I don't expect this patch to be perfect or the final version. But I'd
> > like to introduce macros like this that don't need the driver developer
> > to copy/paste or repeat the same thing (compat string, function name,
> > etc) in multiple places for the driver to work as a module. If the exact
> > style of my macros aren't appealing, I'm open to other suggestions.
> >
> > There are some checkpatch warning about the > 80 columns that my patch
> > doesn't add. There are also checkpatch warnings about the trailing ; in
> > a macro, but I need those for IRQCHIP_DECLARE to work when the driver is
> > builtin.
>
> I think you are looking at the problem from the wrong end, and adding
> syntactic sugar should be the least of your worries. The reason for
> not allowing irqchip drivers to be modular is that there is no
> refcounting in place to prevent drivers from being removed whilst the
> IRQ stack still has irq_desc, irq_data and various other objects
> indirectly referencing the driver.
>
> I'm all for addressing these issues, though it begs the question of
> *why* you want to do this. We have been perfectly happy with built-in
> irqchips so far (they are pretty small, and there aren't millions of
> them), so what changed?
>

I can't speak for Saravana, but my sense is that moving functionality
to loadable modules is becoming more important for Android devices due
to their efforts to utilize a single kernel image across various
vendor devices[1].  Obviously with mobile device constraints
minimizing the unused vendor code in the kernel image is important,
and modules help there. While the unloading issues you raised are
important, one can still benefit from having a permanent module
(modules that don't have a .remove handler), as they can be only
loaded on the devices that use them. You're also right that irqchip
drivers are fairly small, but one issue is that any code they depend
on also has to be built in if they are not able to be configured as a
module, so by allowing the irqchip driver to be a module, you can also
modularize whatever platform calls are made from that driver.

thanks
-john

[1]: See:
  https://arstechnica.com/gadgets/2019/11/google-outlines-plans-for-mainline-linux-kernel-support-in-android/
  https://www.linuxplumbersconf.org/event/2/contributions/61/attachments/69/80/Android_and_Linux_Kernel__Herding_billions_of_penguins_one_version_at_a_time.pdf
  https://linuxplumbersconf.org/event/4/contributions/401/attachments/318/542/GKI_Progress.pdf