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

From: Marc Zyngier <maz@kernel.org>
To: Saravana Kannan <saravanak@google.com>
Cc: John Stultz <john.stultz@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jason Cooper <jason@lakedaemon.net>,
	Android Kernel Team <kernel-team@android.com>,
	lkml <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH v1] irqchip: Add IRQCHIP_MODULE_BEGIN/END helper macros
Date: Fri, 01 May 2020 09:48:39 +0100	[thread overview]
Message-ID: <5b605bd46d3ef213c7ec82d02967e4bb@kernel.org> (raw)
In-Reply-To: <CAGETcx_d0aM+MdeOFDaXDnTEs85rVY=H7zvhZf7NNB4w-t_CGg@mail.gmail.com>

On 2020-04-29 20:04, Saravana Kannan wrote:
> On Wed, Apr 29, 2020 at 2:28 AM Marc Zyngier <maz@kernel.org> wrote:

[...]

>> One thing though: this seems to be exclusively DT driven. Have you
>> looked into how that would look like for other firmware types such as
>> ACPI?
> 
> I'm not very familiar with ACPI at all. I've just started to learn
> about how it works in the past few months poking at code when I have
> some time. So I haven't tried to get this to work with ACPI nor do I
> think I'll be able to do that anytime in the near future. I hope that
> doesn't block this from being used for DT based platforms.

As long as you don't try to modularise a driver that does both DT and
ACPI, you'll be safe. I'm also actively trying to discourage people
from inventing custom irqchips on ACPI platforms (the spec almost
forbids them, but not quite).

>> Another thing is the handling of dependencies. Statically built
>> irqchips are initialized in the right order based on the topology
>> described in DT, and are initialized early enough that client devices
>> will find their irqchip This doesn't work here, obviously.
> 
> Yeah, I read that code thoroughly :)
> 
>> How do you
>> propose we handle these dependencies, both between irqchip drivers and
>> client drivers?
> 
> For client drivers, we don't need to do anything. The IRQ apis seem to
> already handle -EPROBE_DEFER correctly in this case.
> 
> For irqchip drivers, the easy answer can be: Load the IRQ modules
> early if you make them modules.

Uhuh. I'm afraid that's not a practical solution. We need to offer the
same behaviour for both and not rely on the user to understand the
topology of the SoC.

> But in my case, I've been testing this with fw_devlink=on. The TL;DR
> of "fw_devlink=on" in this context is that the IRQ devices will get
> device links created based on "interrupt-parent" property. So, with
> the magic of device links, these IRQ devices will probe in the right
> topological order without any wasted deferred probe attempts. For
> cases without fw_devlink=on, I think I can improve
> platform_irqchip_probe() in my patch to check if the parent device has
> probed and defer if it hasn't.

Seems like an interesting option. Two things then:

- Can we enforce the use of fw_devlink for modularized irqchips?
- For those irqchips that can be modularized, it is apparent that they
   should have been written as platform devices the first place. Maybe
   we should just do that (long term, though).

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...