Messages in this thread Patch in this message | | | Subject | Re: [PATCH 1/2] genirq: Get the fwnode back for irqchips being probed via ACPI namespace | From | Marc Zyngier <> | Date | Thu, 6 Jul 2017 14:05:49 +0100 |
| |
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.guo@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.garry@huawei.com> >>> Signed-off-by: Hanjun Guo <hanjun.guo@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...
| |