From: marc.zyngier@arm•com (Marc Zyngier)
To: linux-arm-kernel@lists•infradead.org
Subject: [Patch Part2 v3 01/24] irqdomain: Introduce new interfaces to support hierarchy irqdomains
Date: Wed, 29 Oct 2014 09:27:37 +0000 [thread overview]
Message-ID: <5450B309.4040607@arm.com> (raw)
In-Reply-To: <alpine.DEB.2.11.1410282116220.5308@nanos>
On 28/10/14 20:23, Thomas Gleixner wrote:
> On Tue, 28 Oct 2014, Marc Zyngier wrote:
>> On 28/10/14 19:37, Thomas Gleixner wrote:
>>> So while we are at it:
>>>
>>>> + if (irq_domain_is_hierarchy(domain)) {
>>>> + if (domain->ops->xlate) {
>>>> + /*
>>>> + * If we've already configured this interrupt,
>>>> + * don't do it again, or hell will break loose.
>>>> + */
>>>> + virq = irq_find_mapping(domain, hwirq);
>>>> + if (virq)
>>>> + return virq;
>>>
>>> I can understand that it is an issue if the mapping exists already,
>>> but I have to ask WHY is it correct behaviour to call into that code
>>> for an existing mapping.
>>
>> As I have originally looked at this, I'll answer the question:
>>
>> The generic DT code parses the whole tree, and generates platform
>> devices as it goes. As part of the platform device creation, it
>> populates the IRQ resources, which translates into calling into
>> irq_create_of_mapping(). You could argue that this behaviour is crazy,
>> and I wouldn't disagree.
>
> Mooo.
Quite.
>> See http://www.spinics.net/lists/devicetree/msg53164.html for more gory
>> details.
>>
>>> And why would this check only apply if domain->ops->xlate is set?
>>> irq_create_mapping() does it unconditionally.
>>
>> My original code used the xlate callback to parse the opaque irq_data,
>> computing hwirq, and I suspect this is a leftover of it. The above code
>> seems to pull hwirq out of thin air, which is probably not the intended
>> behaviour. Joe?
>
> No. Here is the full patch from Joe:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/296543.html
>
> hwirq gets either set from hwirq = irq_data->args[0] or from the xlate
> call.
Ah, that makes a lot more sense.
> But my question still stands:
>
> Why would this check only apply if domain->ops->xlate is set?
> irq_create_mapping() does it unconditionally.
I don't think we should consider xlate at all. We already resolved hwirq
(either directly or through a xlate call), and the check should always
be performed (otherwise we're likely to fall into the same trap again).
Looks like a bug to me.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
next prev parent reply other threads:[~2014-10-29 9:27 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-28 8:26 [Patch Part2 v3 00/24] Enable hierarchy irqdomian on x86 platforms Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 01/24] irqdomain: Introduce new interfaces to support hierarchy irqdomains Jiang Liu
2014-10-28 9:48 ` Yingjoe Chen
2014-10-28 19:37 ` Thomas Gleixner
2014-10-28 20:13 ` Marc Zyngier
2014-10-28 20:23 ` Thomas Gleixner
2014-10-29 9:27 ` Marc Zyngier [this message]
2014-10-29 10:10 ` Yingjoe Chen
2014-10-28 8:26 ` [Patch Part2 v3 02/24] genirq: Introduce helper functions to support stacked irq_chip Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 03/24] x86, irq: Save destination CPU ID in irq_cfg Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 04/24] x86, irq: Use hierarchy irqdomain to manage CPU interrupt vectors Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 05/24] x86, hpet: Use new irqdomain interfaces to allocate/free IRQ Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 06/24] x86, MSI: " Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 07/24] x86, uv: " Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 08/24] x86, htirq: " Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 09/24] x86, dmar: " Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 10/24] x86: irq_remapping: Introduce new interfaces to support hierarchy irqdomain Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 11/24] iommu/vt-d: Change prototypes to prepare for enabling " Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 12/24] iommu/vt-d: Enhance Intel IR driver to suppport " Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 13/24] iommu/amd: Enhance AMD " Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 14/24] x86, hpet: Enhance HPET IRQ to support " Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 15/24] x86, MSI: Use hierarchy irqdomain to manage MSI interrupts Jiang Liu
2014-10-28 21:37 ` Thomas Gleixner
2014-10-29 8:48 ` Jiang Liu
2014-10-29 9:19 ` Thomas Gleixner
2014-10-30 4:50 ` Jiang Liu
2014-10-30 10:39 ` Thomas Gleixner
2014-10-31 12:04 ` Jiang Liu
2014-10-31 14:00 ` Thomas Gleixner
2014-10-28 8:26 ` [Patch Part2 v3 16/24] x86, irq: Directly call native_compose_msi_msg() for DMAR IRQ Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 17/24] iommu/vt-d: Clean up unused MSI related code Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 18/24] iommu/amd: " Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 19/24] x86: irq_remapping: " Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 20/24] x86, irq: Clean up unused MSI related code and interfaces Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 21/24] iommu/vt-d: Refine the interfaces to create IRQ for DMAR unit Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 22/24] x86, irq: Use hierarchy irqdomain to manage DMAR interrupts Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 23/24] x86, htirq: Use hierarchy irqdomain to manage Hypertransport interrupts Jiang Liu
2014-10-28 8:26 ` [Patch Part2 v3 24/24] x86, uv: Use hierarchy irqdomain to manage UV interrupts Jiang Liu
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=5450B309.4040607@arm.com \
--to=marc.zyngier@arm$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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