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

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

[Huacai Chen]

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 <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       |   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

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

[Marc Zyngier]

On Mon, 17 Oct 2022 04:18:47 +0100,
Huacai Chen <chenhuacai@loongson.cn> 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 <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       |   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.

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

[Huacai Chen]

Hi, Marc,

On Wed, Oct 19, 2022 at 3:47 PM Marc Zyngier <maz@kernel.org> wrote:
>
> On Mon, 17 Oct 2022 04:18:47 +0100,
> Huacai Chen <chenhuacai@loongson.cn> 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 <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       |   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.