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: Wed, 03 Jun 2020 11:12:49 +0100 [thread overview]
Message-ID: <d17908a4313ed0f5ccfa8265611738b2@kernel.org> (raw)
In-Reply-To: <CAGETcx9kYKOEAmLbJzmOucR2Z4qy9PCY2=UCYdYTJWTL=BeZNQ@mail.gmail.com>
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...
next prev parent reply other threads:[~2020-06-03 10:12 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-11 4:59 [RFC PATCH v1] irqchip: Add IRQCHIP_MODULE_BEGIN/END helper macros Saravana Kannan
2020-04-11 9:14 ` Marc Zyngier
2020-04-13 22:13 ` John Stultz
2020-04-13 22:43 ` Saravana Kannan
2020-04-29 9:28 ` Marc Zyngier
2020-04-29 19:04 ` Saravana Kannan
2020-05-01 8:48 ` Marc Zyngier
2020-05-01 20:23 ` Saravana Kannan
2020-06-03 1:59 ` Saravana Kannan
2020-06-03 10:12 ` Marc Zyngier [this message]
2020-07-17 2:55 ` Saravana Kannan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d17908a4313ed0f5ccfa8265611738b2@kernel.org \
--to=maz@kernel.org \
--cc=jason@lakedaemon.net \
--cc=john.stultz@linaro.org \
--cc=kernel-team@android.com \
--cc=linux-kernel@vger.kernel.org \
--cc=saravanak@google.com \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).