[001/001] i2c-mpc: fix WARN on module unload

Message ID 1710637.ybObhrSzVC@wirbelwind
State Changes Requested
Headers show
Series
  • [001/001] i2c-mpc: fix WARN on module unload
Related show

Commit Message

Steven Seeger April 24, 2018, 2:38 p.m.
From eaa136c301bce589b45f72249ad072c485973cc6 Mon Sep 17 00:00:00 2001
From: Steven Seeger <steven.seeger@nasa.gov>
Date: Tue, 24 Apr 2018 10:30:34 -0400
Subject: [PATCH] i2c-mpc: Remove use of of_parse_and_map_irq and
 irq_dispose_mapping since irq handler is shared. This prevents a WARN from
 procfs upon module removal for removing non-empty directory when more than
 one instance of i2c-mpc device is in the system.

Signed-off-by: Steven Seeger <steven.seeger@nasa.gov>
---
 drivers/i2c/busses/i2c-mpc.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

Comments

Wolfram Sang May 30, 2018, 8:35 p.m. | #1
Hi,

On Tue, Apr 24, 2018 at 10:38:56AM -0400, Steven Seeger wrote:
> From eaa136c301bce589b45f72249ad072c485973cc6 Mon Sep 17 00:00:00 2001
> From: Steven Seeger <steven.seeger@nasa.gov>
> Date: Tue, 24 Apr 2018 10:30:34 -0400
> Subject: [PATCH] i2c-mpc: Remove use of of_parse_and_map_irq and

Please make sure you have a one-line subject and then a multi-line
commit message.

>  irq_dispose_mapping since irq handler is shared. This prevents a WARN from
>  procfs upon module removal for removing non-empty directory when more than
>  one instance of i2c-mpc device is in the system.

I think you should limit the patch to just this and skip the
devm_conversion.

> -		result = request_irq(i2c->irq, mpc_i2c_isr,
> +		result = devm_request_irq(&op->dev, i2c->irq, mpc_i2c_isr,
>  				     IRQF_SHARED, "i2c-mpc", i2c);

Because if it is the only devm_ user...

> -	if (i2c->irq)
> -		free_irq(i2c->irq, i2c);
> -
> -	irq_dispose_mapping(i2c->irq);
>  	iounmap(i2c->base);
>  	kfree(i2c);
>  	return 0;

... then it will be freed after this. If an interrupt arrives after the
kfree(), you will likely have an OOPS because everything but the
interrupt is gone by now.

Thanks & regards,

   Wolfram

Patch

diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index d94f05c8b8b7..ec536a34f5c6 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -680,9 +680,9 @@  static int fsl_i2c_probe(struct platform_device *op)
 		goto fail_map;
 	}
 
-	i2c->irq = irq_of_parse_and_map(op->dev.of_node, 0);
+	i2c->irq = platform_get_irq(op, 0);
 	if (i2c->irq) { /* no i2c->irq implies polling */
-		result = request_irq(i2c->irq, mpc_i2c_isr,
+		result = devm_request_irq(&op->dev, i2c->irq, mpc_i2c_isr,
 				     IRQF_SHARED, "i2c-mpc", i2c);
 		if (result < 0) {
 			dev_err(i2c->dev, "failed to attach interrupt\n");
@@ -750,9 +750,7 @@  static int fsl_i2c_probe(struct platform_device *op)
  fail_add:
 	if (i2c->clk_per)
 		clk_disable_unprepare(i2c->clk_per);
-	free_irq(i2c->irq, i2c);
  fail_request:
-	irq_dispose_mapping(i2c->irq);
 	iounmap(i2c->base);
  fail_map:
 	kfree(i2c);
@@ -768,10 +766,6 @@  static int fsl_i2c_remove(struct platform_device *op)
 	if (i2c->clk_per)
 		clk_disable_unprepare(i2c->clk_per);
 
-	if (i2c->irq)
-		free_irq(i2c->irq, i2c);
-
-	irq_dispose_mapping(i2c->irq);
 	iounmap(i2c->base);
 	kfree(i2c);
 	return 0;