lkml.org 
[lkml]   [2020]   [Jun]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC PATCH v1] irqchip: Add IRQCHIP_MODULE_BEGIN/END helper macros
    Hi Saravana,

    On 2020-05-01 21:23, Saravana Kannan wrote:
    > 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.

    By having a config option that forces it on if that option is selected
    by modular irqchips? More importantly, what is the drawback of having
    fw_devlink on at all times? It definitely looks like the best thing
    since sliced bread (with cheese), so what is the catch?

    >
    >> - 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

    That would have my preference, provided that there is no drawbacks.

    > 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?

    That'd be a lot better. We also need some guards for things that
    cannot be a driver (primary interrupt controllers don't have a struct
    device).

    Thanks,

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

    \
     
     \ /
      Last update: 2020-06-03 12:14    [W:5.314 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site