Skip to contentSkip to conversation

staging: iio: ad9832: Use devm_iio_device_register

PATCH In review Himangi Saraogi 3 participants 4 messages Aug 3, 2014

Subsystems (4)
Staging IIO Analog Devices IIO drivers Staging - industrial io
lore
Share & tools
lore

Himangi Saraogi

author
#0 lore raw

PATCH 1/1 staging: iio: ad9832: Use devm_iio_device_register

This patch introduces the use of devm_iio_device_register and does away with the unregister in the remove function.

Signed-off-by Himangi Saraogi <[email protected]>
Acked-by Julia Lawall <[email protected]>
---
 drivers/staging/iio/frequency/ad9832.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/staging/iio/frequency/ad9832.c +1 -2
index cf68159..8a72c78 100644
@@ -304,7 +304,7 @@ static int ad9832_probe(struct spi_device *spi)
 	if (ret)
 		goto error_disable_reg;
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(&spi->dev, indio_dev);
 	if (ret)
 		goto error_disable_reg;
 

@@ -322,7 +322,6 @@ static int ad9832_remove(struct spi_device *spi)
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 	struct ad9832_state *st = iio_priv(indio_dev);
 
-	iio_device_unregister(indio_dev);
 	if (!IS_ERR(st->reg))
 		regulator_disable(st->reg);
Signature · 1 line
1.9.1
maintainer
#1 lore raw
Quoted 3 lines · Himangi Saraogi

This patch introduces the use of devm_iio_device_register and does away with the unregister in the remove function.

And changes the resulting ordering so the regulator disable occurs before the user
space interface has been removed by the unregister.

Please be very wary of using devm_iio_register. It is only correct if the remove function does nothing else.

Jonathan

Quoted 28 lines
Signed-off-by Himangi Saraogi <[email protected]>
Acked-by Julia Lawall <[email protected]>

---
drivers/staging/iio/frequency/ad9832.c | 3 +--

1 file changed, 1 insertion(+), 2 deletions(-)
drivers/staging/iio/frequency/ad9832.c +1 -2
index cf68159..8a72c78 100644
@@ -304,7 +304,7 @@ static int ad9832_probe(struct spi_device *spi)
 	if (ret)
 		goto error_disable_reg;
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(&spi->dev, indio_dev);
 	if (ret)
 		goto error_disable_reg;
 

@@ -322,7 +322,6 @@ static int ad9832_remove(struct spi_device *spi)
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 	struct ad9832_state *st = iio_priv(indio_dev);
 
-	iio_device_unregister(indio_dev);
 	if (!IS_ERR(st->reg))
 		regulator_disable(st->reg);
Signature · 1 line
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
#2 lore raw
Quoted 10 lines · Jonathan Cameron
Quoted 3 lines · Himangi Saraogi

This patch introduces the use of devm_iio_device_register and does away with the unregister in the remove function.

And changes the resulting ordering so the regulator disable occurs before the user
space interface has been removed by the unregister.

Please be very wary of using devm_iio_register. It is only correct if the remove function does nothing else.

I wonder if there could be a way to document these constraints?

julia

Quoted 33 lines

Jonathan

Quoted 28 lines
Signed-off-by Himangi Saraogi <[email protected]>
Acked-by Julia Lawall <[email protected]>

---
drivers/staging/iio/frequency/ad9832.c | 3 +--

1 file changed, 1 insertion(+), 2 deletions(-)
drivers/staging/iio/frequency/ad9832.c +1 -2
index cf68159..8a72c78 100644
@@ -304,7 +304,7 @@ static int ad9832_probe(struct spi_device *spi)
 	if (ret)
 		goto error_disable_reg;
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(&spi->dev, indio_dev);
 	if (ret)
 		goto error_disable_reg;
 

@@ -322,7 +322,6 @@ static int ad9832_remove(struct spi_device *spi)
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 	struct ad9832_state *st = iio_priv(indio_dev);
 
-	iio_device_unregister(indio_dev);
 	if (!IS_ERR(st->reg))
 		regulator_disable(st->reg);
Signature · 1 line
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
maintainer
#3 lore raw
Quoted 12 lines · Julia Lawall
Quoted 3 lines · Jonathan Cameron

On August 3, 2014 10:56:58 AM GMT+01:00, Himangi Saraogi

Quoted 1 line · <[email protected]> wrote:
Quoted 1 line

This patch introduces the use of devm_iio_device_register and does

away

Quoted 2 lines
Quoted 2 lines

with the unregister in the remove function.

Quoted 8 lines
Quoted 1 line

And changes the resulting ordering so the regulator disable occurs

before the user

Quoted 3 lines

space interface has been removed by the unregister.

Please be very wary of using devm_iio_register. It is only correct if

the remove function does nothing else.

I wonder if there could be a way to document these constraints?

Perhaps though the simple rule of not doing anything that results in the remove sequence
not being the mirror image of the probe covers most cases. Whilst there are safe ways to break this rule they make the code less obviously correct and so arguably should not be used anyway.

Quoted 36 lines

julia

Quoted 33 lines

Jonathan

Quoted 28 lines
Signed-off-by Himangi Saraogi <[email protected]>
Acked-by Julia Lawall <[email protected]>

---
drivers/staging/iio/frequency/ad9832.c | 3 +--

1 file changed, 1 insertion(+), 2 deletions(-)
drivers/staging/iio/frequency/ad9832.c +1 -2
index cf68159..8a72c78 100644
@@ -304,7 +304,7 @@ static int ad9832_probe(struct spi_device *spi)
 	if (ret)
 		goto error_disable_reg;
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(&spi->dev, indio_dev);
 	if (ret)
 		goto error_disable_reg;
 

@@ -322,7 +322,6 @@ static int ad9832_remove(struct spi_device *spi)
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 	struct ad9832_state *st = iio_priv(indio_dev);
 
-	iio_device_unregister(indio_dev);
 	if (!IS_ERR(st->reg))
 		regulator_disable(st->reg);
Signature · 1 line
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
Signature · 1 line
Sent from my Android phone with K-9 Mail. Please excuse my brevity.