staging:iio:ad799x use devm_request_threaded_irq

PATCH Superseded Hartmut Knaack 2 participants 2 messages Dec 29, 2013 – Jan 1, 2014

Subsystems (4)

Staging IIO Analog Devices IIO drivers Staging - industrial io

lore

Share & tools

lore

Hartmut Knaack

author

2013-12-29 11:51 UTC #0 lore raw

PATCH 1/1 staging:iio:ad799x use devm_request_threaded_irq

Move to devm_request_threaded_irq to make device-removal easier, and also fix error_free_irq, which did not check if an irq was requested at all.

Signed-off-by Hartmut Knaack <[email protected]>

---

drivers/staging/iio/adc/ad799x_core.c +9 ~~-12~~

index e32a555..bc886f6 100644
@@ -573,24 +573,23 @@ static int ad799x_probe(struct i2c_client *client,
 		goto error_disable_reg;
 
 	if (client->irq > 0) {
-		ret = request_threaded_irq(client->irq,
-					   NULL,
-					   ad799x_event_handler,
-					   IRQF_TRIGGER_FALLING |
-					   IRQF_ONESHOT,
-					   client->name,
-					   indio_dev);
+		ret = devm_request_threaded_irq(&client->dev,
+						client->irq,
+						NULL,
+						ad799x_event_handler,
+						IRQF_TRIGGER_FALLING |
+						IRQF_ONESHOT,
+						client->name,
+						indio_dev);
 		if (ret)
 			goto error_cleanup_ring;
 	}
 	ret = iio_device_register(indio_dev);
 	if (ret)
-		goto error_free_irq;
+		goto error_cleanup_ring;
 
 	return 0;
 
-error_free_irq:
-	free_irq(client->irq, indio_dev);
 error_cleanup_ring:
 	ad799x_ring_cleanup(indio_dev);
 error_disable_reg:

@@ -606,8 +605,6 @@ static int ad799x_remove(struct i2c_client *client)
 	struct ad799x_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
-	if (client->irq > 0)
-		free_irq(client->irq, indio_dev);
 
 	ad799x_ring_cleanup(indio_dev);
 	if (!IS_ERR(st->reg))

Jonathan Cameron

maintainer

2014-01-01 13:03 UTC #1 lore raw

Quoted 4 lines · Hartmut Knaack

Move to devm_request_threaded_irq to make device-removal easier, and also fix error_free_irq, which did not check if an irq was requested at all.
Signed-off-by Hartmut Knaack <[email protected]>

Hi Hartmut,

Please break this into two patches - the first fixing the irq check (whilst it will go away in the second patch - we will probably want to push the fix to stable, and can't do that with the other change in here).

On the devm_request_threaded_irq usage, this is often considered dangerous as it changes the ordering of resource freeing so that it doesn't match that of allocation. Here I think you are fine, but
haven't looked into that closely yet.

Lars, do you think this is a good idea?
(any chance you are planning to get this driver out of staging any time soon? :)

Quoted 53 lines

---

drivers/staging/iio/adc/ad799x_core.c +9 ~~-13~~

index e32a555..bc886f6 100644
@@ -573,24 +573,23 @@ static int ad799x_probe(struct i2c_client *client,
  		goto error_disable_reg;
 
  	if (client->irq > 0) {
-		ret = request_threaded_irq(client->irq,
-					   NULL,
-					   ad799x_event_handler,
-					   IRQF_TRIGGER_FALLING |
-					   IRQF_ONESHOT,
-					   client->name,
-					   indio_dev);
+		ret = devm_request_threaded_irq(&client->dev,
+						client->irq,
+						NULL,
+						ad799x_event_handler,
+						IRQF_TRIGGER_FALLING |
+						IRQF_ONESHOT,
+						client->name,
+						indio_dev);
  		if (ret)
  			goto error_cleanup_ring;
  	}
  	ret = iio_device_register(indio_dev);
  	if (ret)
-		goto error_free_irq;
+		goto error_cleanup_ring;
 
  	return 0;
 
-error_free_irq:
-	free_irq(client->irq, indio_dev);
  error_cleanup_ring:
  	ad799x_ring_cleanup(indio_dev);
  error_disable_reg:

@@ -606,8 +605,6 @@ static int ad799x_remove(struct i2c_client *client)
  	struct ad799x_state *st = iio_priv(indio_dev);
 
  	iio_device_unregister(indio_dev);
-	if (client->irq > 0)
-		free_irq(client->irq, indio_dev);
 
  	ad799x_ring_cleanup(indio_dev);
  	if (!IS_ERR(st->reg))
--

To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html