Messages in this thread | | | Date | Wed, 15 Jun 2022 08:14:29 +0100 | From | Marc Zyngier <> | Subject | Re: [PATCH V12 01/10] APCI: irq: Add support for multiple GSI domains |
| |
On Wed, 15 Jun 2022 07:07:21 +0100, Jianmin Lv <lvjianmin@loongson.cn> wrote: > > From: Marc Zyngier <maz@kernel.org> > > In an unfortunate departure from the ACPI spec, the LoongArch > architecture split its GSI space across multiple interrupt > controllers. > > In order to be able to reuse sthe core code and prevent > architectures from reinventing an already square wheel, offer > the arch code the ability to register a dispatcher function > that will return the domain fwnode for a given GSI. > > The ARM GIC drivers are updated to support this (with a single > domain, as intended). > > Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn>
I don't think this tag is appropriate here.
> Signed-off-by: Marc Zyngier <maz@kernel.org> > Cc: Hanjun Guo <guohanjun@huawei.com> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn> > --- > drivers/acpi/irq.c | 40 +++++++++++++++++++++++----------------- > drivers/irqchip/irq-gic-v3.c | 18 ++++++++++++------ > drivers/irqchip/irq-gic.c | 18 ++++++++++++------ > include/linux/acpi.h | 2 +- > 4 files changed, 48 insertions(+), 30 deletions(-) > > diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c > index c68e694..b7460ab 100644 > --- a/drivers/acpi/irq.c > +++ b/drivers/acpi/irq.c > @@ -12,7 +12,7 @@ > > enum acpi_irq_model_id acpi_irq_model; > > -static struct fwnode_handle *acpi_gsi_domain_id; > +static struct fwnode_handle *(*acpi_get_gsi_domain_id)(u32 gsi); > > /** > * acpi_gsi_to_irq() - Retrieve the linux irq number for a given GSI > @@ -26,10 +26,7 @@ > */ > int acpi_gsi_to_irq(u32 gsi, unsigned int *irq) > { > - struct irq_domain *d = irq_find_matching_fwnode(acpi_gsi_domain_id, > - DOMAIN_BUS_ANY); > - > - *irq = irq_find_mapping(d, gsi); > + *irq = acpi_register_gsi(NULL, gsi, -1, -1);
What is this?
- This wasn't part of my initial patch, and randomly changing patches without mentioning it isn't acceptable
- you *cannot* trigger a registration here, as this isn't what the API advertises
- what makes you think that passing random values (NULL, -1... )to acpi_register_gsi() is an acceptable thing to do?
The original patch had:
@@ -26,8 +26,10 @@ static struct fwnode_handle *acpi_gsi_domain_id; */ int acpi_gsi_to_irq(u32 gsi, unsigned int *irq) { - struct irq_domain *d = irq_find_matching_fwnode(acpi_gsi_domain_id, - DOMAIN_BUS_ANY); + struct irq_domain *d; + + d = irq_find_matching_fwnode(acpi_get_gsi_domain_id(gsi), + DOMAIN_BUS_ANY); *irq = irq_find_mapping(d, gsi); /*
and I don't think it needs anything else. If something breaks, let's discuss it, but don't abuse the API nor the fact that I usually don't review my own patches to sneak things in...
M.
-- Without deviation from the norm, progress is not possible.
| |