Messages in this thread | | | From | Saravana Kannan <> | Date | Fri, 1 May 2020 13:23:40 -0700 | Subject | Re: [RFC PATCH v1] irqchip: Add IRQCHIP_MODULE_BEGIN/END helper macros |
| |
On Fri, May 1, 2020 at 1:48 AM Marc Zyngier <maz@kernel.org> wrote: > > 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?
fw_devlink doesn't have any config and it's a command line option. So not sure how you can enforce that.
> - 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).
I agree. If they can be platform devices, they should be. But when those platform device drivers are built in, you'll either need: 1) fw_devlink=on to enforce the topological init order Or 2) have a generic irqchip probe helper function that ensures that. My patch with some additional checks added to platform_irqchip_probe() can provide (2).
In the short term, my patch series also makes it easier to convert existing non-platform drivers into platform drivers.
So if I fix up platform_irqchip_probe() to also do -EPROBE_DEFER to enforce topology, will that make this patch acceptable?
-Saravana
| |