2022-10-17 03:25:26

by Huacai Chen

[permalink] [raw]
Subject: [PATCH] irqchip/loongson-htvec: Add ACPI init support

HTVECINTC stands for "HyperTransport Interrupts" that described in
Section 14.3 of "Loongson 3A5000 Processor Reference Manual". For more
information please refer Documentation/loongarch/irq-chip-model.rst.

Though the extended model is the recommended one, there are still some
legacy model machines. So we add ACPI init support for HTVECINTC.

Co-developed-by: Jianmin Lv <[email protected]>
Signed-off-by: Jianmin Lv <[email protected]>
Signed-off-by: Huacai Chen <[email protected]>
---
arch/loongarch/include/asm/irq.h | 2 +-
drivers/irqchip/Kconfig | 1 +
drivers/irqchip/irq-loongson-htvec.c | 145 +++++++++++++++++++------
drivers/irqchip/irq-loongson-liointc.c | 21 +++-
4 files changed, 133 insertions(+), 36 deletions(-)

diff --git a/arch/loongarch/include/asm/irq.h b/arch/loongarch/include/asm/irq.h
index d06d4542b634..9d3d36e41afe 100644
--- a/arch/loongarch/include/asm/irq.h
+++ b/arch/loongarch/include/asm/irq.h
@@ -93,7 +93,7 @@ int liointc_acpi_init(struct irq_domain *parent,
int eiointc_acpi_init(struct irq_domain *parent,
struct acpi_madt_eio_pic *acpi_eiointc);

-struct irq_domain *htvec_acpi_init(struct irq_domain *parent,
+int htvec_acpi_init(struct irq_domain *parent,
struct acpi_madt_ht_pic *acpi_htvec);
int pch_lpc_acpi_init(struct irq_domain *parent,
struct acpi_madt_lpc_pic *acpi_pchlpc);
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 7ef9f5e696d3..17396e6e42fc 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -576,6 +576,7 @@ config IRQ_LOONGARCH_CPU
select GENERIC_IRQ_CHIP
select IRQ_DOMAIN
select GENERIC_IRQ_EFFECTIVE_AFF_MASK
+ select LOONGSON_HTVEC
select LOONGSON_LIOINTC
select LOONGSON_EIOINTC
select LOONGSON_PCH_PIC
diff --git a/drivers/irqchip/irq-loongson-htvec.c b/drivers/irqchip/irq-loongson-htvec.c
index 60a335d7e64e..1bb414ec6e78 100644
--- a/drivers/irqchip/irq-loongson-htvec.c
+++ b/drivers/irqchip/irq-loongson-htvec.c
@@ -20,7 +20,6 @@
/* Registers */
#define HTVEC_EN_OFF 0x20
#define HTVEC_MAX_PARENT_IRQ 8
-
#define VEC_COUNT_PER_REG 32
#define VEC_REG_IDX(irq_id) ((irq_id) / VEC_COUNT_PER_REG)
#define VEC_REG_BIT(irq_id) ((irq_id) % VEC_COUNT_PER_REG)
@@ -32,6 +31,8 @@ struct htvec {
raw_spinlock_t htvec_lock;
};

+static struct htvec *htvec_priv;
+
static void htvec_irq_dispatch(struct irq_desc *desc)
{
int i;
@@ -155,64 +156,140 @@ static void htvec_reset(struct htvec *priv)
}
}

-static int htvec_of_init(struct device_node *node,
- struct device_node *parent)
+static int htvec_init(phys_addr_t addr, unsigned long size,
+ int num_parents, int parent_irq[], struct fwnode_handle *domain_handle)
{
+ int i;
struct htvec *priv;
- int err, parent_irq[8], i;

priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;

+ priv->num_parents = num_parents;
+ priv->base = ioremap(addr, size);
raw_spin_lock_init(&priv->htvec_lock);
- priv->base = of_iomap(node, 0);
- if (!priv->base) {
- err = -ENOMEM;
- goto free_priv;
- }
-
- /* Interrupt may come from any of the 8 interrupt lines */
- for (i = 0; i < HTVEC_MAX_PARENT_IRQ; i++) {
- parent_irq[i] = irq_of_parse_and_map(node, i);
- if (parent_irq[i] <= 0)
- break;
-
- priv->num_parents++;
- }
-
- if (!priv->num_parents) {
- pr_err("Failed to get parent irqs\n");
- err = -ENODEV;
- goto iounmap_base;
- }

- priv->htvec_domain = irq_domain_create_linear(of_node_to_fwnode(node),
+ /* Setup IRQ domain */
+ priv->htvec_domain = irq_domain_create_linear(domain_handle,
(VEC_COUNT_PER_REG * priv->num_parents),
&htvec_domain_ops, priv);
if (!priv->htvec_domain) {
- pr_err("Failed to create IRQ domain\n");
- err = -ENOMEM;
- goto irq_dispose;
+ pr_err("loongson-htvec: cannot add IRQ domain\n");
+ goto iounmap_base;
}

htvec_reset(priv);

- for (i = 0; i < priv->num_parents; i++)
+ for (i = 0; i < priv->num_parents; i++) {
irq_set_chained_handler_and_data(parent_irq[i],
htvec_irq_dispatch, priv);
+ }
+
+ htvec_priv = priv;

return 0;

-irq_dispose:
- for (; i > 0; i--)
- irq_dispose_mapping(parent_irq[i - 1]);
iounmap_base:
iounmap(priv->base);
-free_priv:
kfree(priv);

- return err;
+ return -EINVAL;
+}
+
+#ifdef CONFIG_OF
+
+static int htvec_of_init(struct device_node *node,
+ struct device_node *parent)
+{
+ int i, err;
+ int parent_irq[8];
+ int num_parents = 0;
+ struct resource res;
+
+ if (of_address_to_resource(node, 0, &res))
+ return -EINVAL;
+
+ /* Interrupt may come from any of the 8 interrupt lines */
+ for (i = 0; i < HTVEC_MAX_PARENT_IRQ; i++) {
+ parent_irq[i] = irq_of_parse_and_map(node, i);
+ if (parent_irq[i] <= 0)
+ break;
+
+ num_parents++;
+ }
+
+ err = htvec_init(res.start, resource_size(&res),
+ num_parents, parent_irq, of_node_to_fwnode(node));
+ if (err < 0)
+ return err;
+
+ return 0;
}

IRQCHIP_DECLARE(htvec, "loongson,htvec-1.0", htvec_of_init);
+
+#endif
+
+#ifdef CONFIG_ACPI
+static int __init
+pch_pic_parse_madt(union acpi_subtable_headers *header,
+ const unsigned long end)
+{
+ struct acpi_madt_bio_pic *pchpic_entry = (struct acpi_madt_bio_pic *)header;
+
+ return pch_pic_acpi_init(htvec_priv->htvec_domain, pchpic_entry);
+}
+
+static int __init
+pch_msi_parse_madt(union acpi_subtable_headers *header,
+ const unsigned long end)
+{
+ struct acpi_madt_msi_pic *pchmsi_entry = (struct acpi_madt_msi_pic *)header;
+
+ return pch_msi_acpi_init(htvec_priv->htvec_domain, pchmsi_entry);
+}
+
+static int __init acpi_cascade_irqdomain_init(void)
+{
+ acpi_table_parse_madt(ACPI_MADT_TYPE_BIO_PIC,
+ pch_pic_parse_madt, 0);
+ acpi_table_parse_madt(ACPI_MADT_TYPE_MSI_PIC,
+ pch_msi_parse_madt, 0);
+ return 0;
+}
+
+int __init htvec_acpi_init(struct irq_domain *parent,
+ struct acpi_madt_ht_pic *acpi_htvec)
+{
+ int i, ret;
+ int num_parents, parent_irq[8];
+ struct fwnode_handle *domain_handle;
+
+ if (!acpi_htvec)
+ return -EINVAL;
+
+ num_parents = HTVEC_MAX_PARENT_IRQ;
+
+ domain_handle = irq_domain_alloc_fwnode((phys_addr_t *)acpi_htvec);
+ if (!domain_handle) {
+ pr_err("Unable to allocate domain handle\n");
+ return -ENOMEM;
+ }
+
+ /* Interrupt may come from any of the 8 interrupt lines */
+ for (i = 0; i < HTVEC_MAX_PARENT_IRQ; i++)
+ parent_irq[i] = irq_create_mapping(parent, acpi_htvec->cascade[i]);
+
+ ret = htvec_init(acpi_htvec->address, acpi_htvec->size,
+ num_parents, parent_irq, domain_handle);
+
+ if (ret == 0)
+ acpi_cascade_irqdomain_init();
+ else
+ irq_domain_free_fwnode(domain_handle);
+
+ return ret;
+}
+
+#endif
diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c
index 0da8716f8f24..ddf23b1114cb 100644
--- a/drivers/irqchip/irq-loongson-liointc.c
+++ b/drivers/irqchip/irq-loongson-liointc.c
@@ -349,6 +349,22 @@ IRQCHIP_DECLARE(loongson_liointc_2_0, "loongson,liointc-2.0", liointc_of_init);
#endif

#ifdef CONFIG_ACPI
+static int __init htintc_parse_madt(union acpi_subtable_headers *header,
+ const unsigned long end)
+{
+ struct acpi_madt_ht_pic *htintc_entry = (struct acpi_madt_ht_pic *)header;
+ struct irq_domain *parent = irq_find_matching_fwnode(liointc_handle, DOMAIN_BUS_ANY);
+
+ return htvec_acpi_init(parent, htintc_entry);
+}
+
+static int __init acpi_cascade_irqdomain_init(void)
+{
+ acpi_table_parse_madt(ACPI_MADT_TYPE_HT_PIC,
+ htintc_parse_madt, 0);
+ return 0;
+}
+
int __init liointc_acpi_init(struct irq_domain *parent, struct acpi_madt_lio_pic *acpi_liointc)
{
int ret;
@@ -365,9 +381,12 @@ int __init liointc_acpi_init(struct irq_domain *parent, struct acpi_madt_lio_pic
pr_err("Unable to allocate domain handle\n");
return -ENOMEM;
}
+
ret = liointc_init(acpi_liointc->address, acpi_liointc->size,
1, domain_handle, NULL);
- if (ret)
+ if (ret == 0)
+ acpi_cascade_irqdomain_init();
+ else
irq_domain_free_fwnode(domain_handle);

return ret;
--
2.31.1


2022-10-19 08:12:51

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH] irqchip/loongson-htvec: Add ACPI init support

On Mon, 17 Oct 2022 04:18:47 +0100,
Huacai Chen <[email protected]> wrote:
>
> HTVECINTC stands for "HyperTransport Interrupts" that described in
> Section 14.3 of "Loongson 3A5000 Processor Reference Manual". For more
> information please refer Documentation/loongarch/irq-chip-model.rst.
>
> Though the extended model is the recommended one, there are still some
> legacy model machines. So we add ACPI init support for HTVECINTC.
>
> Co-developed-by: Jianmin Lv <[email protected]>
> Signed-off-by: Jianmin Lv <[email protected]>
> Signed-off-by: Huacai Chen <[email protected]>
> ---
> arch/loongarch/include/asm/irq.h | 2 +-
> drivers/irqchip/Kconfig | 1 +
> drivers/irqchip/irq-loongson-htvec.c | 145 +++++++++++++++++++------
> drivers/irqchip/irq-loongson-liointc.c | 21 +++-
> 4 files changed, 133 insertions(+), 36 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/irq.h b/arch/loongarch/include/asm/irq.h
> index d06d4542b634..9d3d36e41afe 100644
> --- a/arch/loongarch/include/asm/irq.h
> +++ b/arch/loongarch/include/asm/irq.h
> @@ -93,7 +93,7 @@ int liointc_acpi_init(struct irq_domain *parent,
> int eiointc_acpi_init(struct irq_domain *parent,
> struct acpi_madt_eio_pic *acpi_eiointc);
>
> -struct irq_domain *htvec_acpi_init(struct irq_domain *parent,
> +int htvec_acpi_init(struct irq_domain *parent,
> struct acpi_madt_ht_pic *acpi_htvec);
> int pch_lpc_acpi_init(struct irq_domain *parent,
> struct acpi_madt_lpc_pic *acpi_pchlpc);
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 7ef9f5e696d3..17396e6e42fc 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -576,6 +576,7 @@ config IRQ_LOONGARCH_CPU
> select GENERIC_IRQ_CHIP
> select IRQ_DOMAIN
> select GENERIC_IRQ_EFFECTIVE_AFF_MASK
> + select LOONGSON_HTVEC
> select LOONGSON_LIOINTC
> select LOONGSON_EIOINTC
> select LOONGSON_PCH_PIC
> diff --git a/drivers/irqchip/irq-loongson-htvec.c b/drivers/irqchip/irq-loongson-htvec.c
> index 60a335d7e64e..1bb414ec6e78 100644
> --- a/drivers/irqchip/irq-loongson-htvec.c
> +++ b/drivers/irqchip/irq-loongson-htvec.c
> @@ -20,7 +20,6 @@
> /* Registers */
> #define HTVEC_EN_OFF 0x20
> #define HTVEC_MAX_PARENT_IRQ 8
> -
> #define VEC_COUNT_PER_REG 32
> #define VEC_REG_IDX(irq_id) ((irq_id) / VEC_COUNT_PER_REG)
> #define VEC_REG_BIT(irq_id) ((irq_id) % VEC_COUNT_PER_REG)
> @@ -32,6 +31,8 @@ struct htvec {
> raw_spinlock_t htvec_lock;
> };
>
> +static struct htvec *htvec_priv;
> +
> static void htvec_irq_dispatch(struct irq_desc *desc)
> {
> int i;
> @@ -155,64 +156,140 @@ static void htvec_reset(struct htvec *priv)
> }
> }
>
> -static int htvec_of_init(struct device_node *node,
> - struct device_node *parent)
> +static int htvec_init(phys_addr_t addr, unsigned long size,
> + int num_parents, int parent_irq[], struct fwnode_handle *domain_handle)
> {
> + int i;
> struct htvec *priv;
> - int err, parent_irq[8], i;
>
> priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> if (!priv)
> return -ENOMEM;
>
> + priv->num_parents = num_parents;
> + priv->base = ioremap(addr, size);
> raw_spin_lock_init(&priv->htvec_lock);
> - priv->base = of_iomap(node, 0);
> - if (!priv->base) {
> - err = -ENOMEM;
> - goto free_priv;
> - }
> -
> - /* Interrupt may come from any of the 8 interrupt lines */
> - for (i = 0; i < HTVEC_MAX_PARENT_IRQ; i++) {
> - parent_irq[i] = irq_of_parse_and_map(node, i);
> - if (parent_irq[i] <= 0)
> - break;
> -
> - priv->num_parents++;
> - }
> -
> - if (!priv->num_parents) {
> - pr_err("Failed to get parent irqs\n");
> - err = -ENODEV;
> - goto iounmap_base;
> - }
>
> - priv->htvec_domain = irq_domain_create_linear(of_node_to_fwnode(node),
> + /* Setup IRQ domain */
> + priv->htvec_domain = irq_domain_create_linear(domain_handle,
> (VEC_COUNT_PER_REG * priv->num_parents),
> &htvec_domain_ops, priv);
> if (!priv->htvec_domain) {
> - pr_err("Failed to create IRQ domain\n");
> - err = -ENOMEM;
> - goto irq_dispose;
> + pr_err("loongson-htvec: cannot add IRQ domain\n");
> + goto iounmap_base;
> }
>
> htvec_reset(priv);
>
> - for (i = 0; i < priv->num_parents; i++)
> + for (i = 0; i < priv->num_parents; i++) {
> irq_set_chained_handler_and_data(parent_irq[i],
> htvec_irq_dispatch, priv);
> + }
> +
> + htvec_priv = priv;
>
> return 0;
>
> -irq_dispose:
> - for (; i > 0; i--)
> - irq_dispose_mapping(parent_irq[i - 1]);
> iounmap_base:
> iounmap(priv->base);
> -free_priv:
> kfree(priv);
>
> - return err;
> + return -EINVAL;
> +}
> +
> +#ifdef CONFIG_OF
> +
> +static int htvec_of_init(struct device_node *node,
> + struct device_node *parent)
> +{
> + int i, err;
> + int parent_irq[8];
> + int num_parents = 0;
> + struct resource res;
> +
> + if (of_address_to_resource(node, 0, &res))
> + return -EINVAL;
> +
> + /* Interrupt may come from any of the 8 interrupt lines */
> + for (i = 0; i < HTVEC_MAX_PARENT_IRQ; i++) {
> + parent_irq[i] = irq_of_parse_and_map(node, i);
> + if (parent_irq[i] <= 0)
> + break;
> +
> + num_parents++;
> + }
> +
> + err = htvec_init(res.start, resource_size(&res),
> + num_parents, parent_irq, of_node_to_fwnode(node));
> + if (err < 0)
> + return err;
> +
> + return 0;
> }
>
> IRQCHIP_DECLARE(htvec, "loongson,htvec-1.0", htvec_of_init);
> +
> +#endif
> +
> +#ifdef CONFIG_ACPI
> +static int __init
> +pch_pic_parse_madt(union acpi_subtable_headers *header,
> + const unsigned long end)

Please write the function name and the return type on the same
line. Long lines are just fine.

> +{
> + struct acpi_madt_bio_pic *pchpic_entry = (struct acpi_madt_bio_pic *)header;
> +
> + return pch_pic_acpi_init(htvec_priv->htvec_domain, pchpic_entry);
> +}
> +
> +static int __init
> +pch_msi_parse_madt(union acpi_subtable_headers *header,
> + const unsigned long end)
> +{
> + struct acpi_madt_msi_pic *pchmsi_entry = (struct acpi_madt_msi_pic *)header;
> +
> + return pch_msi_acpi_init(htvec_priv->htvec_domain, pchmsi_entry);
> +}
> +
> +static int __init acpi_cascade_irqdomain_init(void)
> +{
> + acpi_table_parse_madt(ACPI_MADT_TYPE_BIO_PIC,
> + pch_pic_parse_madt, 0);
> + acpi_table_parse_madt(ACPI_MADT_TYPE_MSI_PIC,
> + pch_msi_parse_madt, 0);

What if any of these fail? They have a return value for a reason.

> + return 0;

There is only a single possible return value, which is never checked.

> +}
> +
> +int __init htvec_acpi_init(struct irq_domain *parent,
> + struct acpi_madt_ht_pic *acpi_htvec)
> +{
> + int i, ret;
> + int num_parents, parent_irq[8];
> + struct fwnode_handle *domain_handle;
> +
> + if (!acpi_htvec)
> + return -EINVAL;
> +
> + num_parents = HTVEC_MAX_PARENT_IRQ;
> +
> + domain_handle = irq_domain_alloc_fwnode((phys_addr_t *)acpi_htvec);

NAK. Enough. I already mopped the floor for you during the previous
cycle, I'm not going to do it again. Please see 7e4fd7a1a6fd.

> + if (!domain_handle) {
> + pr_err("Unable to allocate domain handle\n");
> + return -ENOMEM;
> + }
> +
> + /* Interrupt may come from any of the 8 interrupt lines */
> + for (i = 0; i < HTVEC_MAX_PARENT_IRQ; i++)
> + parent_irq[i] = irq_create_mapping(parent, acpi_htvec->cascade[i]);
> +
> + ret = htvec_init(acpi_htvec->address, acpi_htvec->size,
> + num_parents, parent_irq, domain_handle);
> +
> + if (ret == 0)
> + acpi_cascade_irqdomain_init();
> + else
> + irq_domain_free_fwnode(domain_handle);
> +
> + return ret;
> +}
> +
> +#endif
> diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c
> index 0da8716f8f24..ddf23b1114cb 100644
> --- a/drivers/irqchip/irq-loongson-liointc.c
> +++ b/drivers/irqchip/irq-loongson-liointc.c
> @@ -349,6 +349,22 @@ IRQCHIP_DECLARE(loongson_liointc_2_0, "loongson,liointc-2.0", liointc_of_init);
> #endif
>
> #ifdef CONFIG_ACPI
> +static int __init htintc_parse_madt(union acpi_subtable_headers *header,
> + const unsigned long end)
> +{
> + struct acpi_madt_ht_pic *htintc_entry = (struct acpi_madt_ht_pic *)header;
> + struct irq_domain *parent = irq_find_matching_fwnode(liointc_handle, DOMAIN_BUS_ANY);
> +
> + return htvec_acpi_init(parent, htintc_entry);
> +}
> +
> +static int __init acpi_cascade_irqdomain_init(void)
> +{
> + acpi_table_parse_madt(ACPI_MADT_TYPE_HT_PIC,
> + htintc_parse_madt, 0);
> + return 0;

Same comments as above.

> +}
> +
> int __init liointc_acpi_init(struct irq_domain *parent, struct acpi_madt_lio_pic *acpi_liointc)
> {
> int ret;
> @@ -365,9 +381,12 @@ int __init liointc_acpi_init(struct irq_domain *parent, struct acpi_madt_lio_pic
> pr_err("Unable to allocate domain handle\n");
> return -ENOMEM;
> }
> +
> ret = liointc_init(acpi_liointc->address, acpi_liointc->size,
> 1, domain_handle, NULL);
> - if (ret)
> + if (ret == 0)
> + acpi_cascade_irqdomain_init();
> + else
> irq_domain_free_fwnode(domain_handle);
>
> return ret;

M.

--
Without deviation from the norm, progress is not possible.

2022-10-19 10:36:54

by Huacai Chen

[permalink] [raw]
Subject: Re: [PATCH] irqchip/loongson-htvec: Add ACPI init support

Hi, Marc,

On Wed, Oct 19, 2022 at 3:47 PM Marc Zyngier <[email protected]> wrote:
>
> On Mon, 17 Oct 2022 04:18:47 +0100,
> Huacai Chen <[email protected]> wrote:
> >
> > HTVECINTC stands for "HyperTransport Interrupts" that described in
> > Section 14.3 of "Loongson 3A5000 Processor Reference Manual". For more
> > information please refer Documentation/loongarch/irq-chip-model.rst.
> >
> > Though the extended model is the recommended one, there are still some
> > legacy model machines. So we add ACPI init support for HTVECINTC.
> >
> > Co-developed-by: Jianmin Lv <[email protected]>
> > Signed-off-by: Jianmin Lv <[email protected]>
> > Signed-off-by: Huacai Chen <[email protected]>
> > ---
> > arch/loongarch/include/asm/irq.h | 2 +-
> > drivers/irqchip/Kconfig | 1 +
> > drivers/irqchip/irq-loongson-htvec.c | 145 +++++++++++++++++++------
> > drivers/irqchip/irq-loongson-liointc.c | 21 +++-
> > 4 files changed, 133 insertions(+), 36 deletions(-)
> >
> > diff --git a/arch/loongarch/include/asm/irq.h b/arch/loongarch/include/asm/irq.h
> > index d06d4542b634..9d3d36e41afe 100644
> > --- a/arch/loongarch/include/asm/irq.h
> > +++ b/arch/loongarch/include/asm/irq.h
> > @@ -93,7 +93,7 @@ int liointc_acpi_init(struct irq_domain *parent,
> > int eiointc_acpi_init(struct irq_domain *parent,
> > struct acpi_madt_eio_pic *acpi_eiointc);
> >
> > -struct irq_domain *htvec_acpi_init(struct irq_domain *parent,
> > +int htvec_acpi_init(struct irq_domain *parent,
> > struct acpi_madt_ht_pic *acpi_htvec);
> > int pch_lpc_acpi_init(struct irq_domain *parent,
> > struct acpi_madt_lpc_pic *acpi_pchlpc);
> > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> > index 7ef9f5e696d3..17396e6e42fc 100644
> > --- a/drivers/irqchip/Kconfig
> > +++ b/drivers/irqchip/Kconfig
> > @@ -576,6 +576,7 @@ config IRQ_LOONGARCH_CPU
> > select GENERIC_IRQ_CHIP
> > select IRQ_DOMAIN
> > select GENERIC_IRQ_EFFECTIVE_AFF_MASK
> > + select LOONGSON_HTVEC
> > select LOONGSON_LIOINTC
> > select LOONGSON_EIOINTC
> > select LOONGSON_PCH_PIC
> > diff --git a/drivers/irqchip/irq-loongson-htvec.c b/drivers/irqchip/irq-loongson-htvec.c
> > index 60a335d7e64e..1bb414ec6e78 100644
> > --- a/drivers/irqchip/irq-loongson-htvec.c
> > +++ b/drivers/irqchip/irq-loongson-htvec.c
> > @@ -20,7 +20,6 @@
> > /* Registers */
> > #define HTVEC_EN_OFF 0x20
> > #define HTVEC_MAX_PARENT_IRQ 8
> > -
> > #define VEC_COUNT_PER_REG 32
> > #define VEC_REG_IDX(irq_id) ((irq_id) / VEC_COUNT_PER_REG)
> > #define VEC_REG_BIT(irq_id) ((irq_id) % VEC_COUNT_PER_REG)
> > @@ -32,6 +31,8 @@ struct htvec {
> > raw_spinlock_t htvec_lock;
> > };
> >
> > +static struct htvec *htvec_priv;
> > +
> > static void htvec_irq_dispatch(struct irq_desc *desc)
> > {
> > int i;
> > @@ -155,64 +156,140 @@ static void htvec_reset(struct htvec *priv)
> > }
> > }
> >
> > -static int htvec_of_init(struct device_node *node,
> > - struct device_node *parent)
> > +static int htvec_init(phys_addr_t addr, unsigned long size,
> > + int num_parents, int parent_irq[], struct fwnode_handle *domain_handle)
> > {
> > + int i;
> > struct htvec *priv;
> > - int err, parent_irq[8], i;
> >
> > priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> > if (!priv)
> > return -ENOMEM;
> >
> > + priv->num_parents = num_parents;
> > + priv->base = ioremap(addr, size);
> > raw_spin_lock_init(&priv->htvec_lock);
> > - priv->base = of_iomap(node, 0);
> > - if (!priv->base) {
> > - err = -ENOMEM;
> > - goto free_priv;
> > - }
> > -
> > - /* Interrupt may come from any of the 8 interrupt lines */
> > - for (i = 0; i < HTVEC_MAX_PARENT_IRQ; i++) {
> > - parent_irq[i] = irq_of_parse_and_map(node, i);
> > - if (parent_irq[i] <= 0)
> > - break;
> > -
> > - priv->num_parents++;
> > - }
> > -
> > - if (!priv->num_parents) {
> > - pr_err("Failed to get parent irqs\n");
> > - err = -ENODEV;
> > - goto iounmap_base;
> > - }
> >
> > - priv->htvec_domain = irq_domain_create_linear(of_node_to_fwnode(node),
> > + /* Setup IRQ domain */
> > + priv->htvec_domain = irq_domain_create_linear(domain_handle,
> > (VEC_COUNT_PER_REG * priv->num_parents),
> > &htvec_domain_ops, priv);
> > if (!priv->htvec_domain) {
> > - pr_err("Failed to create IRQ domain\n");
> > - err = -ENOMEM;
> > - goto irq_dispose;
> > + pr_err("loongson-htvec: cannot add IRQ domain\n");
> > + goto iounmap_base;
> > }
> >
> > htvec_reset(priv);
> >
> > - for (i = 0; i < priv->num_parents; i++)
> > + for (i = 0; i < priv->num_parents; i++) {
> > irq_set_chained_handler_and_data(parent_irq[i],
> > htvec_irq_dispatch, priv);
> > + }
> > +
> > + htvec_priv = priv;
> >
> > return 0;
> >
> > -irq_dispose:
> > - for (; i > 0; i--)
> > - irq_dispose_mapping(parent_irq[i - 1]);
> > iounmap_base:
> > iounmap(priv->base);
> > -free_priv:
> > kfree(priv);
> >
> > - return err;
> > + return -EINVAL;
> > +}
> > +
> > +#ifdef CONFIG_OF
> > +
> > +static int htvec_of_init(struct device_node *node,
> > + struct device_node *parent)
> > +{
> > + int i, err;
> > + int parent_irq[8];
> > + int num_parents = 0;
> > + struct resource res;
> > +
> > + if (of_address_to_resource(node, 0, &res))
> > + return -EINVAL;
> > +
> > + /* Interrupt may come from any of the 8 interrupt lines */
> > + for (i = 0; i < HTVEC_MAX_PARENT_IRQ; i++) {
> > + parent_irq[i] = irq_of_parse_and_map(node, i);
> > + if (parent_irq[i] <= 0)
> > + break;
> > +
> > + num_parents++;
> > + }
> > +
> > + err = htvec_init(res.start, resource_size(&res),
> > + num_parents, parent_irq, of_node_to_fwnode(node));
> > + if (err < 0)
> > + return err;
> > +
> > + return 0;
> > }
> >
> > IRQCHIP_DECLARE(htvec, "loongson,htvec-1.0", htvec_of_init);
> > +
> > +#endif
> > +
> > +#ifdef CONFIG_ACPI
> > +static int __init
> > +pch_pic_parse_madt(union acpi_subtable_headers *header,
> > + const unsigned long end)
>
> Please write the function name and the return type on the same
> line. Long lines are just fine.
This style keeps the same as other loongson irqchips, if I change it,
then should I send another patch to restyle other drivers?

>
> > +{
> > + struct acpi_madt_bio_pic *pchpic_entry = (struct acpi_madt_bio_pic *)header;
> > +
> > + return pch_pic_acpi_init(htvec_priv->htvec_domain, pchpic_entry);
> > +}
> > +
> > +static int __init
> > +pch_msi_parse_madt(union acpi_subtable_headers *header,
> > + const unsigned long end)
> > +{
> > + struct acpi_madt_msi_pic *pchmsi_entry = (struct acpi_madt_msi_pic *)header;
> > +
> > + return pch_msi_acpi_init(htvec_priv->htvec_domain, pchmsi_entry);
> > +}
> > +
> > +static int __init acpi_cascade_irqdomain_init(void)
> > +{
> > + acpi_table_parse_madt(ACPI_MADT_TYPE_BIO_PIC,
> > + pch_pic_parse_madt, 0);
> > + acpi_table_parse_madt(ACPI_MADT_TYPE_MSI_PIC,
> > + pch_msi_parse_madt, 0);
>
> What if any of these fail? They have a return value for a reason.
Similar to above, this keeps the same as other loongson irqchips, if I
change it, then should I send another patch to adjust other drivers?

>
> > + return 0;
>
> There is only a single possible return value, which is never checked.
>
> > +}
> > +
> > +int __init htvec_acpi_init(struct irq_domain *parent,
> > + struct acpi_madt_ht_pic *acpi_htvec)
> > +{
> > + int i, ret;
> > + int num_parents, parent_irq[8];
> > + struct fwnode_handle *domain_handle;
> > +
> > + if (!acpi_htvec)
> > + return -EINVAL;
> > +
> > + num_parents = HTVEC_MAX_PARENT_IRQ;
> > +
> > + domain_handle = irq_domain_alloc_fwnode((phys_addr_t *)acpi_htvec);
>
> NAK. Enough. I already mopped the floor for you during the previous
> cycle, I'm not going to do it again. Please see 7e4fd7a1a6fd.
Sorry, this will be fixed in the next version.

Huacai
>
> > + if (!domain_handle) {
> > + pr_err("Unable to allocate domain handle\n");
> > + return -ENOMEM;
> > + }
> > +
> > + /* Interrupt may come from any of the 8 interrupt lines */
> > + for (i = 0; i < HTVEC_MAX_PARENT_IRQ; i++)
> > + parent_irq[i] = irq_create_mapping(parent, acpi_htvec->cascade[i]);
> > +
> > + ret = htvec_init(acpi_htvec->address, acpi_htvec->size,
> > + num_parents, parent_irq, domain_handle);
> > +
> > + if (ret == 0)
> > + acpi_cascade_irqdomain_init();
> > + else
> > + irq_domain_free_fwnode(domain_handle);
> > +
> > + return ret;
> > +}
> > +
> > +#endif
> > diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c
> > index 0da8716f8f24..ddf23b1114cb 100644
> > --- a/drivers/irqchip/irq-loongson-liointc.c
> > +++ b/drivers/irqchip/irq-loongson-liointc.c
> > @@ -349,6 +349,22 @@ IRQCHIP_DECLARE(loongson_liointc_2_0, "loongson,liointc-2.0", liointc_of_init);
> > #endif
> >
> > #ifdef CONFIG_ACPI
> > +static int __init htintc_parse_madt(union acpi_subtable_headers *header,
> > + const unsigned long end)
> > +{
> > + struct acpi_madt_ht_pic *htintc_entry = (struct acpi_madt_ht_pic *)header;
> > + struct irq_domain *parent = irq_find_matching_fwnode(liointc_handle, DOMAIN_BUS_ANY);
> > +
> > + return htvec_acpi_init(parent, htintc_entry);
> > +}
> > +
> > +static int __init acpi_cascade_irqdomain_init(void)
> > +{
> > + acpi_table_parse_madt(ACPI_MADT_TYPE_HT_PIC,
> > + htintc_parse_madt, 0);
> > + return 0;
>
> Same comments as above.
>
> > +}
> > +
> > int __init liointc_acpi_init(struct irq_domain *parent, struct acpi_madt_lio_pic *acpi_liointc)
> > {
> > int ret;
> > @@ -365,9 +381,12 @@ int __init liointc_acpi_init(struct irq_domain *parent, struct acpi_madt_lio_pic
> > pr_err("Unable to allocate domain handle\n");
> > return -ENOMEM;
> > }
> > +
> > ret = liointc_init(acpi_liointc->address, acpi_liointc->size,
> > 1, domain_handle, NULL);
> > - if (ret)
> > + if (ret == 0)
> > + acpi_cascade_irqdomain_init();
> > + else
> > irq_domain_free_fwnode(domain_handle);
> >
> > return ret;
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.