Messages in this thread Patch in this message | | | Subject | Re: [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support | From | Jianmin Lv <> | Date | Tue, 7 Jun 2022 11:41:22 +0800 |
| |
On 2022/6/6 下午6:02, Marc Zyngier wrote: > + Lorenzo and Hanjun who maintain the ACPI irq code > > On Thu, 02 Jun 2022 04:16:30 +0100, > Jianmin Lv <lvjianmin@loongson.cn> wrote: >>>>>> + >>>>>> +int acpi_gsi_to_irq(u32 gsi, unsigned int *irqp) >>>>>> +{ >>>>>> + if (irqp != NULL) >>>>>> + *irqp = acpi_register_gsi(NULL, gsi, -1, -1); >>>>>> + return (*irqp >= 0) ? 0 : -EINVAL; >>>>>> +} >>>>>> +EXPORT_SYMBOL_GPL(acpi_gsi_to_irq); >>>>>> + >>>>>> +int acpi_isa_irq_to_gsi(unsigned int isa_irq, u32 *gsi) >>>>>> +{ >>>>>> + if (gsi) >>>>>> + *gsi = isa_irq; >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +/* >>>>>> + * success: return IRQ number (>=0) >>>>>> + * failure: return < 0 >>>>>> + */ >>>>>> +int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity) >>>>>> +{ >>>>>> + int id; >>>>>> + struct irq_fwspec fwspec; >>>>>> + >>>>>> + switch (gsi) { >>>>>> + case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ: >>>>>> + fwspec.fwnode = liointc_domain->fwnode; >>>>>> + fwspec.param[0] = gsi - GSI_MIN_CPU_IRQ; >>>>>> + fwspec.param_count = 1; >>>>>> + >>>>>> + return irq_create_fwspec_mapping(&fwspec); >>>>>> + >>>>>> + case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ: >>>>>> + if (!pch_lpc_domain) >>>>>> + return -EINVAL; >>>>>> + >>>>>> + fwspec.fwnode = pch_lpc_domain->fwnode; >>>>>> + fwspec.param[0] = gsi - GSI_MIN_LPC_IRQ; >>>>>> + fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity); >>>>>> + fwspec.param_count = 2; >>>>>> + >>>>>> + return irq_create_fwspec_mapping(&fwspec); >>>>>> + >>>>>> + case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ: >>>>>> + id = find_pch_pic(gsi); >>>>>> + if (id < 0) >>>>>> + return -EINVAL; >>>>>> + >>>>>> + fwspec.fwnode = pch_pic_domain[id]->fwnode; >>>>>> + fwspec.param[0] = gsi - acpi_pchpic[id]->gsi_base; >>>>>> + fwspec.param[1] = IRQ_TYPE_LEVEL_HIGH; >>>>>> + fwspec.param_count = 2; >>>>>> + >>>>>> + return irq_create_fwspec_mapping(&fwspec); >>>>>> + } >>>>> So all the complexity here seems to stem from the fact that you deal >>>>> with three ranges of interrupts, managed by three different pieces of >>>>> code? >>>>> >>>> Yes. >>>> >>>>> Other architectures have similar requirements, and don't require to >>>>> re-implement a private version of the ACPI API. Instead, they expose a >>>>> single irqdomain, and deal with the various ranges internally. >>>>> >>>>> Clearly, not being able to reuse drivers/acpi/irq.c *is* an issue. >>>>> >>>> Thanks, I agree, that sounds a good and reasonable suggestion, and >>>> I'll reserach it further and reuse code from drivers/acpi/irq.c as >>>> can as possible. >>>> >> Hi, Marc, according to your suggestion, I carefully looked into gic >> driver of ARM, and I found one possible gsi mapping path as following: >> >> acpi_register_gsi /* whatever the gsi is, gic domain for ARM is only >> single domain to use.*/ >> ->irq_create_fwspec_mapping >> ->irq_find_mapping /* return irq in the mapping of irqdomain with >> fwnode_handle of acpi_gsi_domain_id if configured. */ >> ->irq_domain_alloc_irqs /* if not configured and hierarchy domain */ >> ->irq_domain_alloc_irqs_hierarchy >> ->domain->ops->alloc /* call gic_irq_domain_alloc */ >> ->gic_irq_domain_map /* handle different GSI range as >> following code: */ >> >> >> switch (__get_intid_range(hw)) { >> case SGI_RANGE: >> case PPI_RANGE: >> case EPPI_RANGE: >> irq_set_percpu_devid(irq); >> irq_domain_set_info(d, irq, hw, chip, d->host_data, >> handle_percpu_devid_irq, NULL, NULL); >> break; >> >> case SPI_RANGE: >> case ESPI_RANGE: >> irq_domain_set_info(d, irq, hw, chip, d->host_data, >> handle_fasteoi_irq, NULL, NULL); >> irq_set_probe(irq); >> irqd_set_single_target(irqd); >> break; >> >> case LPI_RANGE: >> if (!gic_dist_supports_lpis()) >> return -EPERM; >> irq_domain_set_info(d, irq, hw, chip, d->host_data, >> handle_fasteoi_irq, NULL, NULL); >> break; >> >> Yes, it's well for ARM by this way, and I found that only one >> domain(specified by acpi_gsi_domain_id)is used. >> >> But for LoongArch, different GSI range have to be handled in different >> domain(e.g. GSI for LIOINTC irqchip can be only mapped in LIOINTC >> domain.). The hwirq->irq mapping of different GSI range is stored in >> related separate domain. The reason leading to this is that an >> interrupt source is hardcodingly to connected to an interrupt vector >> for these irqchip(LIOINTC,LPC-PIC and PCH-PIC), and the interrupt >> source of them need to be configured with GSI in DSDT or >> FADT(e.g. SCI). >> >> If only exposing one domain for LoongArch, when calling >> irq_find_mapping in acpi_register_gsi flow, the irq is returned only >> from the domain specfied by acpi_gsi_domain_id, so I'm afraid it's >> unsuitable to expose a single domain for acpi_register_gsi. >> >> I'm so sorry, I really don't find a way to reuse driver/acpi/irq.c >> after my humble work. > I don't think reimplementing ACPI is the solution. What could be a > reasonable approach is a way to overload the retrieval of the > acpi_gsi_domain_id fwnode with a GSI parameter. > > I hacked the following patch, which will give you an idea of what I > have in mind (only compile-tested).
Hi, Marc, thanks so much for your patch. I have verified it on my LoongArch machine and it works well.
BTW, in acpi_get_irq_source_fwhandle(), maybe acpi_get_gsi_domain_id(ctx->index) is needed to changed to acpi_get_gsi_domain_id(irq->interrupts[ctx->index])?
I have another question, for LoongArch, acpi_isa_irq_to_gsi is required to implemente, but no common version, do we need to implemente an weak version in driver/acpi/irq.c as following?
int __weak acpi_isa_irq_to_gsi(unsigned int isa_irq, u32 *gsi) { if (gsi) *gsi = isa_irq; return 0; }
I'll use the way you provided here to reuse driver/acpi/irq.c in next version. How should I do next? Should I integrate your patch into my next version or wait for you to merge it first?
> Thanks, > > M. > > From a3fdc06a53cbcc0e6b77863aae9a7b01a0848fd0 Mon Sep 17 00:00:00 2001 > From: Marc Zyngier <maz@kernel.org> > Date: Mon, 6 Jun 2022 10:49:14 +0100 > Subject: [PATCH] APCI: irq: Add support for multiple GSI domains > > 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 > rachitectures 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). > > Signed-off-by: Marc Zyngier <maz@kernel.org> > Cc: Hanjun Guo <guohanjun@huawei.com> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > --- > drivers/acpi/irq.c | 41 +++++++++++++++++++++++------------- > drivers/irqchip/irq-gic-v3.c | 18 ++++++++++------ > drivers/irqchip/irq-gic.c | 18 ++++++++++------ > include/linux/acpi.h | 2 +- > 4 files changed, 51 insertions(+), 28 deletions(-) > > diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c > index c68e694fca26..6e1633ac1756 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,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); > /* > @@ -53,12 +55,12 @@ int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, > { > struct irq_fwspec fwspec; > > - if (WARN_ON(!acpi_gsi_domain_id)) { > + fwspec.fwnode = acpi_get_gsi_domain_id(gsi); > + if (WARN_ON(!fwspec.fwnode)) { > pr_warn("GSI: No registered irqchip, giving up\n"); > return -EINVAL; > } > > - fwspec.fwnode = acpi_gsi_domain_id; > fwspec.param[0] = gsi; > fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity); > fwspec.param_count = 2; > @@ -73,13 +75,14 @@ EXPORT_SYMBOL_GPL(acpi_register_gsi); > */ > void acpi_unregister_gsi(u32 gsi) > { > - struct irq_domain *d = irq_find_matching_fwnode(acpi_gsi_domain_id, > - DOMAIN_BUS_ANY); > + struct irq_domain *d; > int irq; > > if (WARN_ON(acpi_irq_model == ACPI_IRQ_MODEL_GIC && gsi < 16)) > return; > > + d = irq_find_matching_fwnode(acpi_get_gsi_domain_id(gsi), > + DOMAIN_BUS_ANY); > irq = irq_find_mapping(d, gsi); > irq_dispose_mapping(irq); > } > @@ -97,7 +100,8 @@ EXPORT_SYMBOL_GPL(acpi_unregister_gsi); > * The referenced device fwhandle or NULL on failure > */ > static struct fwnode_handle * > -acpi_get_irq_source_fwhandle(const struct acpi_resource_source *source) > +acpi_get_irq_source_fwhandle(const struct acpi_resource_source *source, > + u32 gsi) > { > struct fwnode_handle *result; > struct acpi_device *device; > @@ -105,7 +109,7 @@ acpi_get_irq_source_fwhandle(const struct acpi_resource_source *source) > acpi_status status; > > if (!source->string_length) > - return acpi_gsi_domain_id; > + return acpi_get_gsi_domain_id(gsi); > > status = acpi_get_handle(NULL, source->string_ptr, &handle); > if (WARN_ON(ACPI_FAILURE(status))) > @@ -194,7 +198,7 @@ static acpi_status acpi_irq_parse_one_cb(struct acpi_resource *ares, > ctx->index -= irq->interrupt_count; > return AE_OK; > } > - fwnode = acpi_gsi_domain_id; > + fwnode = acpi_get_gsi_domain_id(ctx->index); > acpi_irq_parse_one_match(fwnode, irq->interrupts[ctx->index], > irq->triggering, irq->polarity, > irq->shareable, ctx); > @@ -207,7 +211,8 @@ static acpi_status acpi_irq_parse_one_cb(struct acpi_resource *ares, > ctx->index -= eirq->interrupt_count; > return AE_OK; > } > - fwnode = acpi_get_irq_source_fwhandle(&eirq->resource_source); > + fwnode = acpi_get_irq_source_fwhandle(&eirq->resource_source, > + eirq->interrupts[ctx->index]); > acpi_irq_parse_one_match(fwnode, eirq->interrupts[ctx->index], > eirq->triggering, eirq->polarity, > eirq->shareable, ctx); > @@ -291,10 +296,10 @@ EXPORT_SYMBOL_GPL(acpi_irq_get); > * GSI interrupts > */ > void __init acpi_set_irq_model(enum acpi_irq_model_id model, > - struct fwnode_handle *fwnode) > + struct fwnode_handle *(*fn)(u32)) > { > acpi_irq_model = model; > - acpi_gsi_domain_id = fwnode; > + acpi_get_gsi_domain_id = fn; > } > > /** > @@ -312,8 +317,14 @@ struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags, > const struct irq_domain_ops *ops, > void *host_data) > { > - struct irq_domain *d = irq_find_matching_fwnode(acpi_gsi_domain_id, > - DOMAIN_BUS_ANY); > + struct irq_domain *d; > + > + /* This only works for the GIC model... */ > + if (acpi_irq_model != ACPI_IRQ_MODEL_GIC) > + return NULL; > + > + d = irq_find_matching_fwnode(acpi_get_gsi_domain_id(0), > + DOMAIN_BUS_ANY); > > if (!d) > return NULL; > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c > index 2be8dea6b6b0..87b1f53a65ec 100644 > --- a/drivers/irqchip/irq-gic-v3.c > +++ b/drivers/irqchip/irq-gic-v3.c > @@ -2357,11 +2357,17 @@ static void __init gic_acpi_setup_kvm_info(void) > vgic_set_kvm_info(&gic_v3_kvm_info); > } > > +static struct fwnode_handle *gsi_domain_handle; > + > +static struct fwnode_handle *gic_v3_get_gsi_domain_id(u32 gsi) > +{ > + return gsi_domain_handle; > +} > + > static int __init > gic_acpi_init(union acpi_subtable_headers *header, const unsigned long end) > { > struct acpi_madt_generic_distributor *dist; > - struct fwnode_handle *domain_handle; > size_t size; > int i, err; > > @@ -2393,18 +2399,18 @@ gic_acpi_init(union acpi_subtable_headers *header, const unsigned long end) > if (err) > goto out_redist_unmap; > > - domain_handle = irq_domain_alloc_fwnode(&dist->base_address); > - if (!domain_handle) { > + gsi_domain_handle = irq_domain_alloc_fwnode(&dist->base_address); > + if (!gsi_domain_handle) { > err = -ENOMEM; > goto out_redist_unmap; > } > > err = gic_init_bases(acpi_data.dist_base, acpi_data.redist_regs, > - acpi_data.nr_redist_regions, 0, domain_handle); > + acpi_data.nr_redist_regions, 0, gsi_domain_handle); > if (err) > goto out_fwhandle_free; > > - acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, domain_handle); > + acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, gic_v3_get_gsi_domain_id); > > if (static_branch_likely(&supports_deactivate_key)) > gic_acpi_setup_kvm_info(); > @@ -2412,7 +2418,7 @@ gic_acpi_init(union acpi_subtable_headers *header, const unsigned long end) > return 0; > > out_fwhandle_free: > - irq_domain_free_fwnode(domain_handle); > + irq_domain_free_fwnode(gsi_domain_handle); > out_redist_unmap: > for (i = 0; i < acpi_data.nr_redist_regions; i++) > if (acpi_data.redist_regs[i].redist_base) > diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c > index 820404cb56bc..4c7bae0ec8f9 100644 > --- a/drivers/irqchip/irq-gic.c > +++ b/drivers/irqchip/irq-gic.c > @@ -1682,11 +1682,17 @@ static void __init gic_acpi_setup_kvm_info(void) > vgic_set_kvm_info(&gic_v2_kvm_info); > } > > +static struct fwnode_handle *gsi_domain_handle; > + > +static struct fwnode_handle *gic_v2_get_gsi_domain_id(u32 gsi) > +{ > + return gsi_domain_handle; > +} > + > static int __init gic_v2_acpi_init(union acpi_subtable_headers *header, > const unsigned long end) > { > struct acpi_madt_generic_distributor *dist; > - struct fwnode_handle *domain_handle; > struct gic_chip_data *gic = &gic_data[0]; > int count, ret; > > @@ -1724,22 +1730,22 @@ static int __init gic_v2_acpi_init(union acpi_subtable_headers *header, > /* > * Initialize GIC instance zero (no multi-GIC support). > */ > - domain_handle = irq_domain_alloc_fwnode(&dist->base_address); > - if (!domain_handle) { > + gsi_domain_handle = irq_domain_alloc_fwnode(&dist->base_address); > + if (!gsi_domain_handle) { > pr_err("Unable to allocate domain handle\n"); > gic_teardown(gic); > return -ENOMEM; > } > > - ret = __gic_init_bases(gic, domain_handle); > + ret = __gic_init_bases(gic, gsi_domain_handle); > if (ret) { > pr_err("Failed to initialise GIC\n"); > - irq_domain_free_fwnode(domain_handle); > + irq_domain_free_fwnode(gsi_domain_handle); > gic_teardown(gic); > return ret; > } > > - acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, domain_handle); > + acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, gic_v2_get_gsi_domain_id); > > if (IS_ENABLED(CONFIG_ARM_GIC_V2M)) > gicv2m_init(NULL, gic_data[0].domain); > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index 4f82a5bc6d98..957e23f727ea 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -356,7 +356,7 @@ int acpi_gsi_to_irq (u32 gsi, unsigned int *irq); > int acpi_isa_irq_to_gsi (unsigned isa_irq, u32 *gsi); > > void acpi_set_irq_model(enum acpi_irq_model_id model, > - struct fwnode_handle *fwnode); > + struct fwnode_handle *(*)(u32)); > > struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags, > unsigned int size,
| |