On 06/07/17 10:01, Hanjun Guo wrote:
> Hi Marc,
>
> On 2017/7/6 15:43, Marc Zyngier wrote:
>> On 06/07/17 05:35, Hanjun Guo wrote:
>>> From: Hanjun Guo <hanjun....@linaro.org>
>>>
>>> commit d59f6617eef0 (genirq: Allow fwnode to carry name information only)
>>> forgot to do "domain->fwnode = fwnode;" for irqchips being probed via
>>> ACPI namesapce (DSDT/SSDT), that will break platforms with irqchip such
>>> as mbigen or qcom irq combiner, set the fwnode back to fix the issue.
>>>
>>> Reported-by: John Garry <john.ga...@huawei.com>
>>> Signed-off-by: Hanjun Guo <hanjun....@linaro.org>
>>> ---
>>> kernel/irq/irqdomain.c | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
>>> index 14fe862..1bc38fa 100644
>>> --- a/kernel/irq/irqdomain.c
>>> +++ b/kernel/irq/irqdomain.c
>>> @@ -151,7 +151,6 @@ struct irq_domain *__irq_domain_add(struct
>>> fwnode_handle *fwnode, int size,
>>> domain->flags |= IRQ_DOMAIN_NAME_ALLOCATED;
>>> break;
>>> default:
>>> - domain->fwnode = fwnode;
>>> domain->name = fwid->name;
>>> break;
>>> }
>>> @@ -172,7 +171,6 @@ struct irq_domain *__irq_domain_add(struct
>>> fwnode_handle *fwnode, int size,
>>> strreplace(name, '/', ':');
>>>
>>> domain->name = name;
>>> - domain->fwnode = fwnode;
>>> domain->flags |= IRQ_DOMAIN_NAME_ALLOCATED;
>>> }
>>>
>>> @@ -196,6 +194,7 @@ struct irq_domain *__irq_domain_add(struct
>>> fwnode_handle *fwnode, int size,
>>> INIT_RADIX_TREE(&domain->revmap_tree, GFP_KERNEL);
>>> domain->ops = ops;
>>> domain->host_data = host_data;
>>> + domain->fwnode = fwnode;
>>> domain->hwirq_max = hwirq_max;
>>> domain->revmap_size = size;
>>> domain->revmap_direct_max_irq = direct_max;
>>>
>> This doesn't seem right.
>>
>> Why is is_fwnode_irqchip() returning false when presented with an
>> irqchip probed via the ACPI namespace? That's what you should consider
>> fixing instead of moving that code around.
>
> irqchips probed via the ACPI namespace will have a fwnode handler
> with the fwnode type FWNODE_ACPI, similar to irqchips probed
> via DT having a fwnode handler with type FWNODE_OF, in this
> function with DT case, fwnode is set for irqdomain's fwnode,
> ACPI just reuse that code because they are similar.
>
> is_fwnode_irqchip() just checks if it's FWNODE_IRQCHIP, which is only
> available for irqchips probing via ACPI static tables such as ITS, GIC
> on ARM, or via DMAR on x86, those cases don't have a fwnode handler then
> need to create one via __irq_domain_alloc_fwnode(), which is different
> from DT/ACPI namesapce scan mechanism. So the patch above it's the best
> solution I got so far which just resume the code as before, correct me
> if you have something new :)
This violates the irqdomain API that takes two kind of fwnodes:
FWNODE_OF or FWNODE_IRQCHIP, and no others. You got away with it so
far, but it is over.
You have two choices here: either you allocate a FWNODE_IRQCHIP in your
irqchip driver, and use this as a handle for your IRQ domain, or you
make the irqdomain code able to deal with any FWNODE_ACPI fwnode.
Does the following hack work for you?
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 14fe862aa2e3..37f4aa3985b3 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -155,6 +155,8 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle
*fwnode, int size,
domain->name = fwid->name;
break;
}
+ } else if (is_acpi_device_node(fwnode)) {
+ domain->fwnode = fwnode;
} else if (of_node) {
char *name;
If it does, we can then find weird and wonderful ways to give the
domain a shiny name in debugfs...
M.
--
Jazz is not dead. It just smells funny...
