Linux-csky Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/7] Fix potential resource leaks and do some code cleanups about irqchip
@ 2020-06-23  8:51 Tiezhu Yang
  2020-06-23  8:51 ` [PATCH 1/7] irqchip: Fix potential resource leaks Tiezhu Yang
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Tiezhu Yang @ 2020-06-23  8:51 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring
  Cc: Guo Ren, Baruch Siach, Huacai Chen, Jiaxun Yang, Tony Lindgren,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Kukjin Kim,
	Krzysztof Kozlowski, Michal Simek, linux-kernel, devicetree,
	linux-csky, linux-arm-kernel, linux-mips, linux-omap,
	linux-riscv, linux-samsung-soc, Xuefeng Li

When I test the irqchip code of Loongson, I read the related code of other
chips in drivers/irqchip and I find some potential resource leaks in the
error path, I think it is better to fix them. Additionally, do some code
cleanups about Loongson to make it more clean and readable.

Tiezhu Yang (7):
  irqchip: Fix potential resource leaks
  irqchip/loongson-htpic: Remove redundant kfree operation
  irqchip/loongson-htvec: Check return value of
    irq_domain_translate_onecell()
  irqchip/loongson-pch-pic: Check return value of
    irq_domain_translate_twocell()
  irqchip/loongson-pch-msi: Remove unneeded variable
  irqchip/loongson-htpic: Remove unneeded select of I8259
  dt-bindings: interrupt-controller: Fix typos in loongson,liointc.yaml

 .../interrupt-controller/loongson,liointc.yaml     |  4 ++--
 drivers/irqchip/Kconfig                            |  1 -
 drivers/irqchip/irq-ath79-misc.c                   |  3 +++
 drivers/irqchip/irq-csky-apb-intc.c                |  3 +++
 drivers/irqchip/irq-csky-mpintc.c                  | 26 +++++++++++++++++-----
 drivers/irqchip/irq-davinci-aintc.c                | 17 ++++++++++----
 drivers/irqchip/irq-davinci-cp-intc.c              | 17 +++++++++++---
 drivers/irqchip/irq-digicolor.c                    |  4 ++++
 drivers/irqchip/irq-dw-apb-ictl.c                  | 11 ++++++---
 drivers/irqchip/irq-loongson-htpic.c               |  6 ++---
 drivers/irqchip/irq-loongson-htvec.c               | 10 +++++++--
 drivers/irqchip/irq-loongson-pch-msi.c             |  7 +-----
 drivers/irqchip/irq-loongson-pch-pic.c             | 15 ++++++++-----
 drivers/irqchip/irq-ls1x.c                         |  4 +++-
 drivers/irqchip/irq-mscc-ocelot.c                  |  6 +++--
 drivers/irqchip/irq-nvic.c                         |  2 ++
 drivers/irqchip/irq-omap-intc.c                    |  4 +++-
 drivers/irqchip/irq-riscv-intc.c                   |  1 +
 drivers/irqchip/irq-s3c24xx.c                      | 20 ++++++++++++-----
 drivers/irqchip/irq-xilinx-intc.c                  |  1 +
 20 files changed, 116 insertions(+), 46 deletions(-)

-- 
2.1.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/7] irqchip: Fix potential resource leaks
  2020-06-23  8:51 [PATCH 0/7] Fix potential resource leaks and do some code cleanups about irqchip Tiezhu Yang
@ 2020-06-23  8:51 ` Tiezhu Yang
  2020-06-24  9:15   ` Krzysztof Kozlowski
  2020-06-23  8:51 ` [PATCH 2/7] irqchip/loongson-htpic: Remove redundant kfree operation Tiezhu Yang
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Tiezhu Yang @ 2020-06-23  8:51 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring
  Cc: Guo Ren, Baruch Siach, Huacai Chen, Jiaxun Yang, Tony Lindgren,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Kukjin Kim,
	Krzysztof Kozlowski, Michal Simek, linux-kernel, devicetree,
	linux-csky, linux-arm-kernel, linux-mips, linux-omap,
	linux-riscv, linux-samsung-soc, Xuefeng Li

There exists some potential resource leaks in the error path, fix them.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 drivers/irqchip/irq-ath79-misc.c      |  3 +++
 drivers/irqchip/irq-csky-apb-intc.c   |  3 +++
 drivers/irqchip/irq-csky-mpintc.c     | 26 ++++++++++++++++++++------
 drivers/irqchip/irq-davinci-aintc.c   | 17 +++++++++++++----
 drivers/irqchip/irq-davinci-cp-intc.c | 17 ++++++++++++++---
 drivers/irqchip/irq-digicolor.c       |  4 ++++
 drivers/irqchip/irq-dw-apb-ictl.c     | 11 ++++++++---
 drivers/irqchip/irq-loongson-htvec.c  |  5 ++++-
 drivers/irqchip/irq-ls1x.c            |  4 +++-
 drivers/irqchip/irq-mscc-ocelot.c     |  6 ++++--
 drivers/irqchip/irq-nvic.c            |  2 ++
 drivers/irqchip/irq-omap-intc.c       |  4 +++-
 drivers/irqchip/irq-riscv-intc.c      |  1 +
 drivers/irqchip/irq-s3c24xx.c         | 20 +++++++++++++++-----
 drivers/irqchip/irq-xilinx-intc.c     |  1 +
 15 files changed, 98 insertions(+), 26 deletions(-)

diff --git a/drivers/irqchip/irq-ath79-misc.c b/drivers/irqchip/irq-ath79-misc.c
index 3d641bb..4a86a9e 100644
--- a/drivers/irqchip/irq-ath79-misc.c
+++ b/drivers/irqchip/irq-ath79-misc.c
@@ -144,6 +144,7 @@ static int __init ath79_misc_intc_of_init(
 	base = of_iomap(node, 0);
 	if (!base) {
 		pr_err("Failed to get MISC IRQ registers\n");
+		irq_dispose_mapping(irq);
 		return -ENOMEM;
 	}
 
@@ -151,6 +152,8 @@ static int __init ath79_misc_intc_of_init(
 				&misc_irq_domain_ops, base);
 	if (!domain) {
 		pr_err("Failed to add MISC irqdomain\n");
+		iounmap(base);
+		irq_dispose_mapping(irq);
 		return -EINVAL;
 	}
 
diff --git a/drivers/irqchip/irq-csky-apb-intc.c b/drivers/irqchip/irq-csky-apb-intc.c
index 5a2ec43..aaa4ac7 100644
--- a/drivers/irqchip/irq-csky-apb-intc.c
+++ b/drivers/irqchip/irq-csky-apb-intc.c
@@ -118,6 +118,7 @@ ck_intc_init_comm(struct device_node *node, struct device_node *parent)
 					    &irq_generic_chip_ops, NULL);
 	if (!root_domain) {
 		pr_err("C-SKY Intc irq_domain_add failed.\n");
+		iounmap(reg_base);
 		return -ENOMEM;
 	}
 
@@ -126,6 +127,8 @@ ck_intc_init_comm(struct device_node *node, struct device_node *parent)
 			IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN, 0, 0);
 	if (ret) {
 		pr_err("C-SKY Intc irq_alloc_gc failed.\n");
+		irq_domain_remove(root_domain);
+		iounmap(reg_base);
 		return -ENOMEM;
 	}
 
diff --git a/drivers/irqchip/irq-csky-mpintc.c b/drivers/irqchip/irq-csky-mpintc.c
index a1534ed..c195e24 100644
--- a/drivers/irqchip/irq-csky-mpintc.c
+++ b/drivers/irqchip/irq-csky-mpintc.c
@@ -247,8 +247,10 @@ csky_mpintc_init(struct device_node *node, struct device_node *parent)
 	if (INTCG_base == NULL) {
 		INTCG_base = ioremap(mfcr("cr<31, 14>"),
 				     INTCL_SIZE*nr_cpu_ids + INTCG_SIZE);
-		if (INTCG_base == NULL)
-			return -EIO;
+		if (INTCG_base == NULL) {
+			ret = -EIO;
+			goto err_free;
+		}
 
 		INTCL_base = INTCG_base + INTCG_SIZE;
 
@@ -257,8 +259,10 @@ csky_mpintc_init(struct device_node *node, struct device_node *parent)
 
 	root_domain = irq_domain_add_linear(node, nr_irq, &csky_irqdomain_ops,
 					    NULL);
-	if (!root_domain)
-		return -ENXIO;
+	if (!root_domain) {
+		ret = -ENXIO;
+		goto err_iounmap;
+	}
 
 	/* for every cpu */
 	for_each_present_cpu(cpu) {
@@ -270,12 +274,22 @@ csky_mpintc_init(struct device_node *node, struct device_node *parent)
 
 #ifdef CONFIG_SMP
 	ipi_irq = irq_create_mapping(root_domain, IPI_IRQ);
-	if (!ipi_irq)
-		return -EIO;
+	if (!ipi_irq) {
+		ret = -EIO;
+		goto err_domain_remove;
+	}
 
 	set_send_ipi(&csky_mpintc_send_ipi, ipi_irq);
 #endif
 
 	return 0;
+
+err_domain_remove:
+	irq_domain_remove(root_domain);
+err_iounmap:
+	iounmap(INTCG_base);
+err_free:
+	kfree(__trigger);
+	return ret;
 }
 IRQCHIP_DECLARE(csky_mpintc, "csky,mpintc", csky_mpintc_init);
diff --git a/drivers/irqchip/irq-davinci-aintc.c b/drivers/irqchip/irq-davinci-aintc.c
index 810ccc4..12db502 100644
--- a/drivers/irqchip/irq-davinci-aintc.c
+++ b/drivers/irqchip/irq-davinci-aintc.c
@@ -96,7 +96,7 @@ void __init davinci_aintc_init(const struct davinci_aintc_config *config)
 				     resource_size(&config->reg));
 	if (!davinci_aintc_base) {
 		pr_err("%s: unable to ioremap register range\n", __func__);
-		return;
+		goto err_release;
 	}
 
 	/* Clear all interrupt requests */
@@ -133,7 +133,7 @@ void __init davinci_aintc_init(const struct davinci_aintc_config *config)
 	if (irq_base < 0) {
 		pr_err("%s: unable to allocate interrupt descriptors: %d\n",
 		       __func__, irq_base);
-		return;
+		goto err_iounmap;
 	}
 
 	davinci_aintc_irq_domain = irq_domain_add_legacy(NULL,
@@ -141,7 +141,7 @@ void __init davinci_aintc_init(const struct davinci_aintc_config *config)
 						&irq_domain_simple_ops, NULL);
 	if (!davinci_aintc_irq_domain) {
 		pr_err("%s: unable to create interrupt domain\n", __func__);
-		return;
+		goto err_free_descs;
 	}
 
 	ret = irq_alloc_domain_generic_chips(davinci_aintc_irq_domain, 32, 1,
@@ -150,7 +150,7 @@ void __init davinci_aintc_init(const struct davinci_aintc_config *config)
 	if (ret) {
 		pr_err("%s: unable to allocate generic irq chips for domain\n",
 		       __func__);
-		return;
+		goto err_domain_remove;
 	}
 
 	for (irq_off = 0, reg_off = 0;
@@ -160,4 +160,13 @@ void __init davinci_aintc_init(const struct davinci_aintc_config *config)
 				       irq_base + irq_off, 32);
 
 	set_handle_irq(davinci_aintc_handle_irq);
+
+err_domain_remove:
+	irq_domain_remove(davinci_aintc_irq_domain);
+err_free_descs:
+	irq_free_descs(irq_base, config->num_irqs);
+err_iounmap:
+	iounmap(davinci_aintc_base);
+err_release:
+	release_mem_region(config->reg.start, resource_size(&config->reg));
 }
diff --git a/drivers/irqchip/irq-davinci-cp-intc.c b/drivers/irqchip/irq-davinci-cp-intc.c
index 276da277..991339f 100644
--- a/drivers/irqchip/irq-davinci-cp-intc.c
+++ b/drivers/irqchip/irq-davinci-cp-intc.c
@@ -175,7 +175,8 @@ davinci_cp_intc_do_init(const struct davinci_cp_intc_config *config,
 				       resource_size(&config->reg));
 	if (!davinci_cp_intc_base) {
 		pr_err("%s: unable to ioremap register range\n", __func__);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_release;
 	}
 
 	davinci_cp_intc_write(0, DAVINCI_CP_INTC_GLOBAL_ENABLE);
@@ -210,7 +211,8 @@ davinci_cp_intc_do_init(const struct davinci_cp_intc_config *config,
 	if (irq_base < 0) {
 		pr_err("%s: unable to allocate interrupt descriptors: %d\n",
 		       __func__, irq_base);
-		return irq_base;
+		ret = irq_base;
+		goto err_iounmap;
 	}
 
 	davinci_cp_intc_irq_domain = irq_domain_add_legacy(
@@ -219,7 +221,8 @@ davinci_cp_intc_do_init(const struct davinci_cp_intc_config *config,
 
 	if (!davinci_cp_intc_irq_domain) {
 		pr_err("%s: unable to create an interrupt domain\n", __func__);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_free_descs;
 	}
 
 	set_handle_irq(davinci_cp_intc_handle_irq);
@@ -228,6 +231,14 @@ davinci_cp_intc_do_init(const struct davinci_cp_intc_config *config,
 	davinci_cp_intc_write(1, DAVINCI_CP_INTC_GLOBAL_ENABLE);
 
 	return 0;
+
+err_free_descs:
+	irq_free_descs(irq_base, config->num_irqs);
+err_iounmap:
+	iounmap(davinci_cp_intc_base);
+err_release:
+	release_mem_region(config->reg.start, resource_size(&config->reg));
+	return ret;
 }
 
 int __init davinci_cp_intc_init(const struct davinci_cp_intc_config *config)
diff --git a/drivers/irqchip/irq-digicolor.c b/drivers/irqchip/irq-digicolor.c
index fc38d2d..a3c73c5 100644
--- a/drivers/irqchip/irq-digicolor.c
+++ b/drivers/irqchip/irq-digicolor.c
@@ -89,6 +89,7 @@ static int __init digicolor_of_init(struct device_node *node,
 	ucregs = syscon_regmap_lookup_by_phandle(node, "syscon");
 	if (IS_ERR(ucregs)) {
 		pr_err("%pOF: unable to map UC registers\n", node);
+		iounmap(reg_base);
 		return PTR_ERR(ucregs);
 	}
 	/* channel 1, regular IRQs */
@@ -98,6 +99,7 @@ static int __init digicolor_of_init(struct device_node *node,
 		irq_domain_add_linear(node, 64, &irq_generic_chip_ops, NULL);
 	if (!digicolor_irq_domain) {
 		pr_err("%pOF: unable to create IRQ domain\n", node);
+		iounmap(reg_base);
 		return -ENOMEM;
 	}
 
@@ -106,6 +108,8 @@ static int __init digicolor_of_init(struct device_node *node,
 					     clr, 0, 0);
 	if (ret) {
 		pr_err("%pOF: unable to allocate IRQ gc\n", node);
+		irq_domain_remove(digicolor_irq_domain);
+		iounmap(reg_base);
 		return ret;
 	}
 
diff --git a/drivers/irqchip/irq-dw-apb-ictl.c b/drivers/irqchip/irq-dw-apb-ictl.c
index e4550e9..bc9b750 100644
--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c
@@ -86,12 +86,13 @@ static int __init dw_apb_ictl_init(struct device_node *np,
 	ret = of_address_to_resource(np, 0, &r);
 	if (ret) {
 		pr_err("%pOF: unable to get resource\n", np);
-		return ret;
+		goto err_irq_dispose;
 	}
 
 	if (!request_mem_region(r.start, resource_size(&r), np->full_name)) {
 		pr_err("%pOF: unable to request mem region\n", np);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto err_irq_dispose;
 	}
 
 	iobase = ioremap(r.start, resource_size(&r));
@@ -133,7 +134,7 @@ static int __init dw_apb_ictl_init(struct device_node *np,
 					     IRQ_GC_INIT_MASK_CACHE);
 	if (ret) {
 		pr_err("%pOF: unable to alloc irq domain gc\n", np);
-		goto err_unmap;
+		goto err_domain_remove;
 	}
 
 	for (i = 0; i < DIV_ROUND_UP(nrirqs, 32); i++) {
@@ -150,10 +151,14 @@ static int __init dw_apb_ictl_init(struct device_node *np,
 
 	return 0;
 
+err_domain_remove:
+	irq_domain_remove(domain);
 err_unmap:
 	iounmap(iobase);
 err_release:
 	release_mem_region(r.start, resource_size(&r));
+err_irq_dispose:
+	irq_dispose_mapping(irq);
 	return ret;
 }
 IRQCHIP_DECLARE(dw_apb_ictl,
diff --git a/drivers/irqchip/irq-loongson-htvec.c b/drivers/irqchip/irq-loongson-htvec.c
index 1ece933..b36d403 100644
--- a/drivers/irqchip/irq-loongson-htvec.c
+++ b/drivers/irqchip/irq-loongson-htvec.c
@@ -192,7 +192,7 @@ static int htvec_of_init(struct device_node *node,
 	if (!priv->htvec_domain) {
 		pr_err("Failed to create IRQ domain\n");
 		err = -ENOMEM;
-		goto iounmap_base;
+		goto irq_dispose;
 	}
 
 	htvec_reset(priv);
@@ -203,6 +203,9 @@ static int htvec_of_init(struct device_node *node,
 
 	return 0;
 
+irq_dispose:
+	for (; i > 0; i--)
+		irq_dispose_mapping(parent_irq[i - 1]);
 iounmap_base:
 	iounmap(priv->base);
 free_priv:
diff --git a/drivers/irqchip/irq-ls1x.c b/drivers/irqchip/irq-ls1x.c
index 353111a..409001b 100644
--- a/drivers/irqchip/irq-ls1x.c
+++ b/drivers/irqchip/irq-ls1x.c
@@ -131,7 +131,7 @@ static int __init ls1x_intc_of_init(struct device_node *node,
 	if (!priv->domain) {
 		pr_err("ls1x-irq: cannot add IRQ domain\n");
 		err = -ENOMEM;
-		goto out_iounmap;
+		goto out_dispose_irq;
 	}
 
 	err = irq_alloc_domain_generic_chips(priv->domain, 32, 2,
@@ -182,6 +182,8 @@ static int __init ls1x_intc_of_init(struct device_node *node,
 
 out_free_domain:
 	irq_domain_remove(priv->domain);
+out_dispose_irq:
+	irq_dispose_mapping(parent_irq);
 out_iounmap:
 	iounmap(priv->intc_base);
 out_free_priv:
diff --git a/drivers/irqchip/irq-mscc-ocelot.c b/drivers/irqchip/irq-mscc-ocelot.c
index 88143c0..e676ae2 100644
--- a/drivers/irqchip/irq-mscc-ocelot.c
+++ b/drivers/irqchip/irq-mscc-ocelot.c
@@ -73,7 +73,8 @@ static int __init ocelot_irq_init(struct device_node *node,
 				       &irq_generic_chip_ops, NULL);
 	if (!domain) {
 		pr_err("%pOFn: unable to add irq domain\n", node);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto err_irq_dispose;
 	}
 
 	ret = irq_alloc_domain_generic_chips(domain, OCELOT_NR_IRQ, 1,
@@ -109,9 +110,10 @@ static int __init ocelot_irq_init(struct device_node *node,
 
 err_gc_free:
 	irq_free_generic_chip(gc);
-
 err_domain_remove:
 	irq_domain_remove(domain);
+err_irq_dispose:
+	irq_dispose_mapping(parent_irq);
 
 	return ret;
 }
diff --git a/drivers/irqchip/irq-nvic.c b/drivers/irqchip/irq-nvic.c
index f747e22..2395e8e 100644
--- a/drivers/irqchip/irq-nvic.c
+++ b/drivers/irqchip/irq-nvic.c
@@ -94,6 +94,7 @@ static int __init nvic_of_init(struct device_node *node,
 
 	if (!nvic_irq_domain) {
 		pr_warn("Failed to allocate irq domain\n");
+		iounmap(nvic_base);
 		return -ENOMEM;
 	}
 
@@ -103,6 +104,7 @@ static int __init nvic_of_init(struct device_node *node,
 	if (ret) {
 		pr_warn("Failed to allocate irq chips\n");
 		irq_domain_remove(nvic_irq_domain);
+		iounmap(nvic_base);
 		return ret;
 	}
 
diff --git a/drivers/irqchip/irq-omap-intc.c b/drivers/irqchip/irq-omap-intc.c
index d360a6e..e711530 100644
--- a/drivers/irqchip/irq-omap-intc.c
+++ b/drivers/irqchip/irq-omap-intc.c
@@ -254,8 +254,10 @@ static int __init omap_init_irq_of(struct device_node *node)
 	omap_irq_soft_reset();
 
 	ret = omap_alloc_gc_of(domain, omap_irq_base);
-	if (ret < 0)
+	if (ret < 0) {
 		irq_domain_remove(domain);
+		iounmap(omap_irq_base);
+	}
 
 	return ret;
 }
diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
index a6f97fa..8d6286c 100644
--- a/drivers/irqchip/irq-riscv-intc.c
+++ b/drivers/irqchip/irq-riscv-intc.c
@@ -122,6 +122,7 @@ static int __init riscv_intc_init(struct device_node *node,
 	rc = set_handle_irq(&riscv_intc_irq);
 	if (rc) {
 		pr_err("failed to set irq handler\n");
+		irq_domain_remove(intc_domain);
 		return rc;
 	}
 
diff --git a/drivers/irqchip/irq-s3c24xx.c b/drivers/irqchip/irq-s3c24xx.c
index d2031fe..ef5d645 100644
--- a/drivers/irqchip/irq-s3c24xx.c
+++ b/drivers/irqchip/irq-s3c24xx.c
@@ -1239,7 +1239,8 @@ static int __init s3c_init_intc_of(struct device_node *np,
 						     &s3c24xx_irq_ops_of, NULL);
 	if (!domain) {
 		pr_err("irq: could not create irq-domain\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_iounmap;
 	}
 
 	for (i = 0; i < num_ctrl; i++) {
@@ -1248,15 +1249,17 @@ static int __init s3c_init_intc_of(struct device_node *np,
 		pr_debug("irq: found controller %s\n", ctrl->name);
 
 		intc = kzalloc(sizeof(struct s3c_irq_intc), GFP_KERNEL);
-		if (!intc)
-			return -ENOMEM;
+		if (!intc) {
+			ret = -ENOMEM;
+			goto out_domain_remove;
+		}
 
 		intc->domain = domain;
 		intc->irqs = kcalloc(32, sizeof(struct s3c_irq_data),
 				     GFP_KERNEL);
 		if (!intc->irqs) {
-			kfree(intc);
-			return -ENOMEM;
+			ret = -ENOMEM;
+			goto out_free;
 		}
 
 		if (ctrl->parent) {
@@ -1285,6 +1288,13 @@ static int __init s3c_init_intc_of(struct device_node *np,
 	set_handle_irq(s3c24xx_handle_irq);
 
 	return 0;
+
+out_free:
+	kfree(intc);
+out_domain_remove:
+	irq_domain_remove(domain);
+out_iounmap:
+	iounmap(reg_base);
 }
 
 static struct s3c24xx_irq_of_ctrl s3c2410_ctrl[] = {
diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index 1d3d273..be8b9a6 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -241,6 +241,7 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
 		} else {
 			pr_err("irq-xilinx: interrupts property not in DT\n");
 			ret = -EINVAL;
+			irq_domain_remove(irqc->root_domain);
 			goto error;
 		}
 	} else {
-- 
2.1.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/7] irqchip/loongson-htpic: Remove redundant kfree operation
  2020-06-23  8:51 [PATCH 0/7] Fix potential resource leaks and do some code cleanups about irqchip Tiezhu Yang
  2020-06-23  8:51 ` [PATCH 1/7] irqchip: Fix potential resource leaks Tiezhu Yang
@ 2020-06-23  8:51 ` Tiezhu Yang
  2020-06-23  8:51 ` [PATCH 3/7] irqchip/loongson-htvec: Check return value of irq_domain_translate_onecell() Tiezhu Yang
  2020-06-23  8:51 ` [PATCH 4/7] irqchip/loongson-pch-pic: Check return value of irq_domain_translate_twocell() Tiezhu Yang
  3 siblings, 0 replies; 7+ messages in thread
From: Tiezhu Yang @ 2020-06-23  8:51 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring
  Cc: Guo Ren, Baruch Siach, Huacai Chen, Jiaxun Yang, Tony Lindgren,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Kukjin Kim,
	Krzysztof Kozlowski, Michal Simek, linux-kernel, devicetree,
	linux-csky, linux-arm-kernel, linux-mips, linux-omap,
	linux-riscv, linux-samsung-soc, Xuefeng Li

In the function htpic_of_init(), when kzalloc htpic fails, it should
return -ENOMEM directly, no need to execute "goto" to kfree.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 drivers/irqchip/irq-loongson-htpic.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-loongson-htpic.c b/drivers/irqchip/irq-loongson-htpic.c
index dd018c2..63f7280 100644
--- a/drivers/irqchip/irq-loongson-htpic.c
+++ b/drivers/irqchip/irq-loongson-htpic.c
@@ -93,10 +93,8 @@ int __init htpic_of_init(struct device_node *node, struct device_node *parent)
 	}
 
 	htpic = kzalloc(sizeof(*htpic), GFP_KERNEL);
-	if (!htpic) {
-		err = -ENOMEM;
-		goto out_free;
-	}
+	if (!htpic)
+		return -ENOMEM;
 
 	htpic->base = of_iomap(node, 0);
 	if (!htpic->base) {
-- 
2.1.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/7] irqchip/loongson-htvec: Check return value of irq_domain_translate_onecell()
  2020-06-23  8:51 [PATCH 0/7] Fix potential resource leaks and do some code cleanups about irqchip Tiezhu Yang
  2020-06-23  8:51 ` [PATCH 1/7] irqchip: Fix potential resource leaks Tiezhu Yang
  2020-06-23  8:51 ` [PATCH 2/7] irqchip/loongson-htpic: Remove redundant kfree operation Tiezhu Yang
@ 2020-06-23  8:51 ` Tiezhu Yang
  2020-06-23  8:51 ` [PATCH 4/7] irqchip/loongson-pch-pic: Check return value of irq_domain_translate_twocell() Tiezhu Yang
  3 siblings, 0 replies; 7+ messages in thread
From: Tiezhu Yang @ 2020-06-23  8:51 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring
  Cc: Guo Ren, Baruch Siach, Huacai Chen, Jiaxun Yang, Tony Lindgren,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Kukjin Kim,
	Krzysztof Kozlowski, Michal Simek, linux-kernel, devicetree,
	linux-csky, linux-arm-kernel, linux-mips, linux-omap,
	linux-riscv, linux-samsung-soc, Xuefeng Li

Check the return value of irq_domain_translate_onecell() due to
it may returns -EINVAL if failed.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 drivers/irqchip/irq-loongson-htvec.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-loongson-htvec.c b/drivers/irqchip/irq-loongson-htvec.c
index b36d403..720cf96 100644
--- a/drivers/irqchip/irq-loongson-htvec.c
+++ b/drivers/irqchip/irq-loongson-htvec.c
@@ -109,11 +109,14 @@ static struct irq_chip htvec_irq_chip = {
 static int htvec_domain_alloc(struct irq_domain *domain, unsigned int virq,
 			      unsigned int nr_irqs, void *arg)
 {
+	int ret;
 	unsigned long hwirq;
 	unsigned int type, i;
 	struct htvec *priv = domain->host_data;
 
-	irq_domain_translate_onecell(domain, arg, &hwirq, &type);
+	ret = irq_domain_translate_onecell(domain, arg, &hwirq, &type);
+	if (ret)
+		return ret;
 
 	for (i = 0; i < nr_irqs; i++) {
 		irq_domain_set_info(domain, virq + i, hwirq + i, &htvec_irq_chip,
-- 
2.1.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 4/7] irqchip/loongson-pch-pic: Check return value of irq_domain_translate_twocell()
  2020-06-23  8:51 [PATCH 0/7] Fix potential resource leaks and do some code cleanups about irqchip Tiezhu Yang
                   ` (2 preceding siblings ...)
  2020-06-23  8:51 ` [PATCH 3/7] irqchip/loongson-htvec: Check return value of irq_domain_translate_onecell() Tiezhu Yang
@ 2020-06-23  8:51 ` Tiezhu Yang
  3 siblings, 0 replies; 7+ messages in thread
From: Tiezhu Yang @ 2020-06-23  8:51 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring
  Cc: Guo Ren, Baruch Siach, Huacai Chen, Jiaxun Yang, Tony Lindgren,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Kukjin Kim,
	Krzysztof Kozlowski, Michal Simek, linux-kernel, devicetree,
	linux-csky, linux-arm-kernel, linux-mips, linux-omap,
	linux-riscv, linux-samsung-soc, Xuefeng Li

Check the return value of irq_domain_translate_twocell() due to
it may returns -EINVAL if failed and use variable fwspec for it,
and then use a new variable parent_fwspec which is proper for
irq_domain_alloc_irqs_parent().

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 drivers/irqchip/irq-loongson-pch-pic.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/irqchip/irq-loongson-pch-pic.c b/drivers/irqchip/irq-loongson-pch-pic.c
index 2a05b93..016f32c 100644
--- a/drivers/irqchip/irq-loongson-pch-pic.c
+++ b/drivers/irqchip/irq-loongson-pch-pic.c
@@ -135,16 +135,19 @@ static int pch_pic_alloc(struct irq_domain *domain, unsigned int virq,
 	int err;
 	unsigned int type;
 	unsigned long hwirq;
-	struct irq_fwspec fwspec;
+	struct irq_fwspec *fwspec = arg;
+	struct irq_fwspec parent_fwspec;
 	struct pch_pic *priv = domain->host_data;
 
-	irq_domain_translate_twocell(domain, arg, &hwirq, &type);
+	err = irq_domain_translate_twocell(domain, fwspec, &hwirq, &type);
+	if (err)
+		return err;
 
-	fwspec.fwnode = domain->parent->fwnode;
-	fwspec.param_count = 1;
-	fwspec.param[0] = hwirq + priv->ht_vec_base;
+	parent_fwspec.fwnode = domain->parent->fwnode;
+	parent_fwspec.param_count = 1;
+	parent_fwspec.param[0] = hwirq + priv->ht_vec_base;
 
-	err = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
+	err = irq_domain_alloc_irqs_parent(domain, virq, 1, &parent_fwspec);
 	if (err)
 		return err;
 
-- 
2.1.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/7] irqchip: Fix potential resource leaks
  2020-06-23  8:51 ` [PATCH 1/7] irqchip: Fix potential resource leaks Tiezhu Yang
@ 2020-06-24  9:15   ` Krzysztof Kozlowski
  2020-06-24  9:24     ` Tiezhu Yang
  0 siblings, 1 reply; 7+ messages in thread
From: Krzysztof Kozlowski @ 2020-06-24  9:15 UTC (permalink / raw)
  To: Tiezhu Yang
  Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring,
	Guo Ren, Baruch Siach, Huacai Chen, Jiaxun Yang, Tony Lindgren,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Kukjin Kim,
	Michal Simek, linux-kernel, devicetree, linux-csky,
	linux-arm-kernel, linux-mips, linux-omap, linux-riscv,
	linux-samsung-soc, Xuefeng Li

On Tue, 23 Jun 2020 at 10:51, Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> There exists some potential resource leaks in the error path, fix them.

This should be split per driver and per bug (although mostly in driver
it's just one bug). Otherwise it is difficult to review, backport and
revert.

Best regards,
Krzysztof


> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>  drivers/irqchip/irq-ath79-misc.c      |  3 +++
>  drivers/irqchip/irq-csky-apb-intc.c   |  3 +++
>  drivers/irqchip/irq-csky-mpintc.c     | 26 ++++++++++++++++++++------
>  drivers/irqchip/irq-davinci-aintc.c   | 17 +++++++++++++----
>  drivers/irqchip/irq-davinci-cp-intc.c | 17 ++++++++++++++---
>  drivers/irqchip/irq-digicolor.c       |  4 ++++
>  drivers/irqchip/irq-dw-apb-ictl.c     | 11 ++++++++---
>  drivers/irqchip/irq-loongson-htvec.c  |  5 ++++-
>  drivers/irqchip/irq-ls1x.c            |  4 +++-
>  drivers/irqchip/irq-mscc-ocelot.c     |  6 ++++--
>  drivers/irqchip/irq-nvic.c            |  2 ++
>  drivers/irqchip/irq-omap-intc.c       |  4 +++-
>  drivers/irqchip/irq-riscv-intc.c      |  1 +
>  drivers/irqchip/irq-s3c24xx.c         | 20 +++++++++++++++-----
>  drivers/irqchip/irq-xilinx-intc.c     |  1 +
>  15 files changed, 98 insertions(+), 26 deletions(-)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/7] irqchip: Fix potential resource leaks
  2020-06-24  9:15   ` Krzysztof Kozlowski
@ 2020-06-24  9:24     ` Tiezhu Yang
  0 siblings, 0 replies; 7+ messages in thread
From: Tiezhu Yang @ 2020-06-24  9:24 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring,
	Guo Ren, Baruch Siach, Huacai Chen, Jiaxun Yang, Tony Lindgren,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Kukjin Kim,
	Michal Simek, linux-kernel, devicetree, linux-csky,
	linux-arm-kernel, linux-mips, linux-omap, linux-riscv,
	linux-samsung-soc, Xuefeng Li

On 06/24/2020 05:15 PM, Krzysztof Kozlowski wrote:
> On Tue, 23 Jun 2020 at 10:51, Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>> There exists some potential resource leaks in the error path, fix them.
> This should be split per driver and per bug (although mostly in driver
> it's just one bug). Otherwise it is difficult to review, backport and
> revert.

Thanks for your suggestion, I have split it into a patch series [1],
I will resend it some days later due to git send-email always failed.

[1] https://lore.kernel.org/patchwork/cover/1263192/

>
> Best regards,
> Krzysztof
>
>
>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>> ---
>>   drivers/irqchip/irq-ath79-misc.c      |  3 +++
>>   drivers/irqchip/irq-csky-apb-intc.c   |  3 +++
>>   drivers/irqchip/irq-csky-mpintc.c     | 26 ++++++++++++++++++++------
>>   drivers/irqchip/irq-davinci-aintc.c   | 17 +++++++++++++----
>>   drivers/irqchip/irq-davinci-cp-intc.c | 17 ++++++++++++++---
>>   drivers/irqchip/irq-digicolor.c       |  4 ++++
>>   drivers/irqchip/irq-dw-apb-ictl.c     | 11 ++++++++---
>>   drivers/irqchip/irq-loongson-htvec.c  |  5 ++++-
>>   drivers/irqchip/irq-ls1x.c            |  4 +++-
>>   drivers/irqchip/irq-mscc-ocelot.c     |  6 ++++--
>>   drivers/irqchip/irq-nvic.c            |  2 ++
>>   drivers/irqchip/irq-omap-intc.c       |  4 +++-
>>   drivers/irqchip/irq-riscv-intc.c      |  1 +
>>   drivers/irqchip/irq-s3c24xx.c         | 20 +++++++++++++++-----
>>   drivers/irqchip/irq-xilinx-intc.c     |  1 +
>>   15 files changed, 98 insertions(+), 26 deletions(-)


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-23  8:51 [PATCH 0/7] Fix potential resource leaks and do some code cleanups about irqchip Tiezhu Yang
2020-06-23  8:51 ` [PATCH 1/7] irqchip: Fix potential resource leaks Tiezhu Yang
2020-06-24  9:15   ` Krzysztof Kozlowski
2020-06-24  9:24     ` Tiezhu Yang
2020-06-23  8:51 ` [PATCH 2/7] irqchip/loongson-htpic: Remove redundant kfree operation Tiezhu Yang
2020-06-23  8:51 ` [PATCH 3/7] irqchip/loongson-htvec: Check return value of irq_domain_translate_onecell() Tiezhu Yang
2020-06-23  8:51 ` [PATCH 4/7] irqchip/loongson-pch-pic: Check return value of irq_domain_translate_twocell() Tiezhu Yang

Linux-csky Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-csky/0 linux-csky/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-csky linux-csky/ https://lore.kernel.org/linux-csky \
		linux-csky@vger.kernel.org
	public-inbox-index linux-csky

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-csky


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git