Messages in this thread |  | | Subject | Re: [PATCH V14 10/15] irqchip/loongson-liointc: Add ACPI init support | From | Jianmin Lv <> | Date | Fri, 8 Jul 2022 14:13:02 +0800 |
| |
On 2022/7/7 下午6:34, Marc Zyngier wrote: > On Sun, 03 Jul 2022 09:45:27 +0100, > Jianmin Lv <lvjianmin@loongson.cn> wrote: >> >> From: Huacai Chen <chenhuacai@loongson.cn> >> >> LIOINTC stands for "Legacy I/O Interrupts" that described in Section >> 11.1 of "Loongson 3A5000 Processor Reference Manual". For more >> information please refer Documentation/loongarch/irq-chip-model.rst. >> >> Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn> >> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn> >> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> >> --- >> arch/loongarch/include/asm/irq.h | 4 +- >> arch/loongarch/kernel/irq.c | 1 - >> drivers/irqchip/irq-loongson-liointc.c | 227 ++++++++++++++++++++++----------- >> 3 files changed, 154 insertions(+), 78 deletions(-) >> >> diff --git a/arch/loongarch/include/asm/irq.h b/arch/loongarch/include/asm/irq.h >> index 8775dc6..b4c7956 100644 >> --- a/arch/loongarch/include/asm/irq.h >> +++ b/arch/loongarch/include/asm/irq.h >> @@ -97,7 +97,7 @@ static inline void eiointc_enable(void) >> >> struct irq_domain *loongarch_cpu_irq_init(void); >> >> -struct irq_domain *liointc_acpi_init(struct irq_domain *parent, >> +int liointc_acpi_init(struct irq_domain *parent, >> struct acpi_madt_lio_pic *acpi_liointc); >> struct irq_domain *eiointc_acpi_init(struct irq_domain *parent, >> struct acpi_madt_eio_pic *acpi_eiointc); >> @@ -122,7 +122,7 @@ int pch_pic_acpi_init(struct irq_domain *parent, >> extern struct acpi_madt_bio_pic *acpi_pchpic[MAX_IO_PICS]; >> >> extern struct irq_domain *cpu_domain; >> -extern struct irq_domain *liointc_domain; >> +extern struct fwnode_handle *liointc_handle; >> extern struct fwnode_handle *pch_lpc_handle; >> extern struct fwnode_handle *pch_pic_handle[MAX_IO_PICS]; >> >> diff --git a/arch/loongarch/kernel/irq.c b/arch/loongarch/kernel/irq.c >> index ce21281..b04201c 100644 >> --- a/arch/loongarch/kernel/irq.c >> +++ b/arch/loongarch/kernel/irq.c >> @@ -26,7 +26,6 @@ >> EXPORT_PER_CPU_SYMBOL(irq_stat); >> >> struct irq_domain *cpu_domain; >> -struct irq_domain *liointc_domain; >> >> /* >> * 'what should we do if we get a hw irq event on an illegal vector'. >> diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c >> index 8d05d8b..526ade4 100644 >> --- a/drivers/irqchip/irq-loongson-liointc.c >> +++ b/drivers/irqchip/irq-loongson-liointc.c >> @@ -23,7 +23,7 @@ >> #endif >> >> #define LIOINTC_CHIP_IRQ 32 >> -#define LIOINTC_NUM_PARENT 4 >> +#define LIOINTC_NUM_PARENT 4 >> #define LIOINTC_NUM_CORES 4 >> >> #define LIOINTC_INTC_CHIP_START 0x20 >> @@ -58,6 +58,8 @@ struct liointc_priv { >> bool has_lpc_irq_errata; >> }; >> >> +struct fwnode_handle *liointc_handle; >> + >> static void liointc_chained_handle_irq(struct irq_desc *desc) >> { >> struct liointc_handler_data *handler = irq_desc_get_handler_data(desc); >> @@ -153,97 +155,82 @@ static void liointc_resume(struct irq_chip_generic *gc) >> irq_gc_unlock_irqrestore(gc, flags); >> } >> >> -static const char * const parent_names[] = {"int0", "int1", "int2", "int3"}; >> -static const char * const core_reg_names[] = {"isr0", "isr1", "isr2", "isr3"}; >> +static int parent_irq[LIOINTC_NUM_PARENT]; >> +static u32 parent_int_map[LIOINTC_NUM_PARENT]; >> +static const char *const parent_names[] = {"int0", "int1", "int2", "int3"}; >> +static const char *const core_reg_names[] = {"isr0", "isr1", "isr2", "isr3"}; >> >> -static void __iomem *liointc_get_reg_byname(struct device_node *node, >> - const char *name) >> +#ifdef CONFIG_ACPI >> +static int liointc_domain_xlate(struct irq_domain *d, struct device_node *ctrlr, >> + const u32 *intspec, unsigned int intsize, >> + unsigned long *out_hwirq, unsigned int *out_type) >> { >> - int index = of_property_match_string(node, "reg-names", name); >> - >> - if (index < 0) >> - return NULL; >> - >> - return of_iomap(node, index); >> + if (WARN_ON(intsize < 1)) >> + return -EINVAL; >> + *out_hwirq = intspec[0] - GSI_MIN_CPU_IRQ; >> + *out_type = IRQ_TYPE_NONE; >> + return 0; >> } >> >> -static int __init liointc_of_init(struct device_node *node, >> - struct device_node *parent) >> +const struct irq_domain_ops acpi_irq_gc_ops = { > > This should be static. >
Ok, thanks, I'll change it.
>> + .map = irq_map_generic_chip, >> + .unmap = irq_unmap_generic_chip, >> + .xlate = liointc_domain_xlate, >> +}; >> +#endif >> + >> +static int liointc_init(phys_addr_t addr, unsigned long size, int revision, >> + struct fwnode_handle *domain_handle, struct device_node *node) >> { >> + int i, err; >> + void __iomem *base; >> + struct irq_chip_type *ct; >> struct irq_chip_generic *gc; >> struct irq_domain *domain; >> - struct irq_chip_type *ct; >> struct liointc_priv *priv; >> - void __iomem *base; >> - u32 of_parent_int_map[LIOINTC_NUM_PARENT]; >> - int parent_irq[LIOINTC_NUM_PARENT]; >> - bool have_parent = FALSE; >> - int sz, i, err = 0; >> >> priv = kzalloc(sizeof(*priv), GFP_KERNEL); >> if (!priv) >> return -ENOMEM; >> >> - if (of_device_is_compatible(node, "loongson,liointc-2.0")) { >> - base = liointc_get_reg_byname(node, "main"); >> - if (!base) { >> - err = -ENODEV; >> - goto out_free_priv; >> - } >> + base = ioremap(addr, size); >> + if (!base) >> + goto out_free_priv; >> >> - for (i = 0; i < LIOINTC_NUM_CORES; i++) >> - priv->core_isr[i] = liointc_get_reg_byname(node, core_reg_names[i]); >> - if (!priv->core_isr[0]) { >> - err = -ENODEV; >> - goto out_iounmap_base; >> - } >> - } else { >> - base = of_iomap(node, 0); >> - if (!base) { >> - err = -ENODEV; >> - goto out_free_priv; >> - } >> + for (i = 0; i < LIOINTC_NUM_CORES; i++) >> + priv->core_isr[i] = base + LIOINTC_REG_INTC_STATUS; >> >> - for (i = 0; i < LIOINTC_NUM_CORES; i++) >> - priv->core_isr[i] = base + LIOINTC_REG_INTC_STATUS; >> - } >> + for (i = 0; i < LIOINTC_NUM_PARENT; i++) >> + priv->handler[i].parent_int_map = parent_int_map[i]; >> >> - for (i = 0; i < LIOINTC_NUM_PARENT; i++) { >> - parent_irq[i] = of_irq_get_byname(node, parent_names[i]); >> - if (parent_irq[i] > 0) >> - have_parent = TRUE; >> - } >> - if (!have_parent) { >> - err = -ENODEV; >> - goto out_iounmap_isr; >> - } >> + if (revision > 1) { >> + for (i = 0; i < LIOINTC_NUM_CORES; i++) { >> + int index = of_property_match_string(node, >> + "reg-names", core_reg_names[i]); >> >> - sz = of_property_read_variable_u32_array(node, >> - "loongson,parent_int_map", >> - &of_parent_int_map[0], >> - LIOINTC_NUM_PARENT, >> - LIOINTC_NUM_PARENT); >> - if (sz < 4) { >> - pr_err("loongson-liointc: No parent_int_map\n"); >> - err = -ENODEV; >> - goto out_iounmap_isr; >> - } >> + if (index < 0) >> + return -EINVAL; >> >> - for (i = 0; i < LIOINTC_NUM_PARENT; i++) >> - priv->handler[i].parent_int_map = of_parent_int_map[i]; >> + priv->core_isr[i] = of_iomap(node, index); >> + } >> + } >> >> /* Setup IRQ domain */ >> - domain = irq_domain_add_linear(node, 32, >> +#ifdef CONFIG_ACPI >> + domain = irq_domain_create_linear(domain_handle, LIOINTC_CHIP_IRQ, >> + &acpi_irq_gc_ops, priv); >> +#else >> + domain = irq_domain_create_linear(domain_handle, LIOINTC_CHIP_IRQ, >> &irq_generic_chip_ops, priv); >> +#endif > > This feels wrong. The ACPI vs OF decision should be a runtime > one. Maybe you don't have that issue yet because MIPS is only OF and > LoongArch only ACPI, but it remains that this should be done > dynamically. > > Please try to rewrite it as: > > if (!acpi_disabled) > [... ACPI stuff ...] > else > [... OF stuff ...] > > This should apply to all other cases where you have similar distinctions. >
Ok, thanks, I'll change all the patch series according to your suggestion here.
> Thanks, > > M. >
|  |