[PATCH 0/8] iio: Introduce devm_iio_device_{register,unregister}

[PATCH 0/8] iio: Introduce devm_iio_device_{register,unregister}

[Sachin Kamat]

With the conversion of iio drivers to use device managed APIs, most of the
drivers now have a remove function with only the iio_device_unregister()
call. Thus introduce device managed version of iio_device_register() to
clean up lot of such code.
Have shown the usage in a few drivers to illustrate the cleanup. Will do
for the rest too.
This is only compile tested as I do not have any actual hardware to test ATM.


Sachin Kamat (8):
  iio: core: Implement devm_iio_device_{register,unregister}
  iio: accel: kxsd9: Use devm_iio_device_register
  iio: adc: mcp320x: Use devm_iio_device_register
  iio: adc: mcp3422: Use devm_iio_device_register
  iio: adc: ti-adc081c: Use devm_iio_device_register
  iio: adc: twl6030-gpadc: Use devm_iio_device_register
  iio: adc: viperboard: Use devm_iio_device_register
  iio: amplifiers: ad8366: Use devm_iio_device_register

 Documentation/driver-model/devres.txt |    2 ++
 drivers/iio/accel/kxsd9.c             |   10 +---------
 drivers/iio/adc/mcp320x.c             |    3 +--
 drivers/iio/adc/mcp3422.c             |    9 +--------
 drivers/iio/adc/ti-adc081c.c          |    3 +--
 drivers/iio/adc/twl6030-gpadc.c       |    5 +----
 drivers/iio/adc/viperboard_adc.c      |   12 +----------
 drivers/iio/amplifiers/ad8366.c       |    4 +---
 drivers/iio/industrialio-core.c       |   35 +++++++++++++++++++++++++++++++++
 include/linux/iio/iio.h               |   25 +++++++++++++++++++++++
 10 files changed, 69 insertions(+), 39 deletions(-)

-- 
1.7.9.5

[PATCH 1/8] iio: core: Implement devm_iio_device_{register,unregister}

[Sachin Kamat]

Add device managed devm_iio_device_{register,unregister}()
to automatically unregister IIO drivers thus leading to
simplified IIO driver code.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Cc: Lars-Peter Clausen <lars@metafoo.de>
---
 Documentation/driver-model/devres.txt |    2 ++
 drivers/iio/industrialio-core.c       |   35 +++++++++++++++++++++++++++++++++
 include/linux/iio/iio.h               |   25 +++++++++++++++++++++++
 3 files changed, 62 insertions(+)

diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index fcb34a5..ffeab1d 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -242,6 +242,8 @@ IIO
   devm_iio_device_free()
   devm_iio_trigger_alloc()
   devm_iio_trigger_free()
+  devm_iio_device_register()
+  devm_iio_device_unregister()
 
 IO region
   devm_request_region()
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 24db185..58f494b 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1125,6 +1125,41 @@ void iio_device_unregister(struct iio_dev *indio_dev)
 	device_del(&indio_dev->dev);
 }
 EXPORT_SYMBOL(iio_device_unregister);
+
+static void devm_iio_device_unreg(struct device *dev, void *res)
+{
+	iio_device_unregister(*(struct iio_dev **)res);
+}
+
+int devm_iio_device_register(struct device *dev, struct iio_dev *indio_dev)
+{
+	void *ptr;
+	int iio_dev;
+
+	ptr = devres_alloc(devm_iio_device_unreg, 0, GFP_KERNEL);
+	if (!ptr)
+		return -ENOMEM;
+
+	iio_dev = iio_device_register(indio_dev);
+	if (!iio_dev)
+		devres_add(dev, ptr);
+	else
+		devres_free(ptr);
+
+	return iio_dev;
+}
+EXPORT_SYMBOL_GPL(devm_iio_device_register);
+
+void devm_iio_device_unregister(struct device *dev, struct iio_dev *iio_dev)
+{
+	int rc;
+
+	rc = devres_release(dev, devm_iio_device_unreg,
+			    devm_iio_device_match, iio_dev);
+	WARN_ON(rc);
+}
+EXPORT_SYMBOL_GPL(devm_iio_device_unregister);
+
 subsys_initcall(iio_init);
 module_exit(iio_exit);
 
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index ac1cb8f..be16dd6 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -453,6 +453,31 @@ int iio_device_register(struct iio_dev *indio_dev);
 void iio_device_unregister(struct iio_dev *indio_dev);
 
 /**
+ * devm_iio_device_register - Resource-managed iio_device_register()
+ * @dev:	Device to allocate iio_dev for
+ * @indio_dev:	Device structure filled by the device driver
+ *
+ * Managed iio_device_register.  iio_dev registered with this function is
+ * automatically unregistered on driver detach.
+ *
+ * If an iio_dev registered with this function needs to be unregistered
+ * separately, devm_iio_device_unregister() must be used.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int devm_iio_device_register(struct device *dev, struct iio_dev *indio_dev);
+
+/**
+ * devm_iio_device_unregister - Resource-managed iio_device_unregister()
+ * @dev:	Device this iio_dev belongs to
+ * @indio_dev:	the iio_dev associated with the device
+ *
+ * Unregister iio_dev registered with devm_iio_device_register().
+ */
+void devm_iio_device_unregister(struct device *dev, struct iio_dev *indio_dev);
+
+/**
  * iio_push_event() - try to add event to the list for userspace reading
  * @indio_dev:		IIO device structure
  * @ev_code:		What event
-- 
1.7.9.5

[PATCH 2/8] iio: accel: kxsd9: Use devm_iio_device_register

[Sachin Kamat]

devm_iio_device_register simplifies the code.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/iio/accel/kxsd9.c |   10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index 709c132..3048686 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -244,20 +244,13 @@ static int kxsd9_probe(struct spi_device *spi)
 	spi_setup(spi);
 	kxsd9_power_up(st);
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(&spi->dev, indio_dev);
 	if (ret)
 		return ret;
 
 	return 0;
 }
 
-static int kxsd9_remove(struct spi_device *spi)
-{
-	iio_device_unregister(spi_get_drvdata(spi));
-
-	return 0;
-}
-
 static const struct spi_device_id kxsd9_id[] = {
 	{"kxsd9", 0},
 	{ },
@@ -270,7 +263,6 @@ static struct spi_driver kxsd9_driver = {
 		.owner = THIS_MODULE,
 	},
 	.probe = kxsd9_probe,
-	.remove = kxsd9_remove,
 	.id_table = kxsd9_id,
 };
 module_spi_driver(kxsd9_driver);
-- 
1.7.9.5

[PATCH 3/8] iio: adc: mcp320x: Use devm_iio_device_register

[Sachin Kamat]

devm_iio_device_register simplifies the code.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/iio/adc/mcp320x.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
index 28a086e..5333558 100644
--- a/drivers/iio/adc/mcp320x.c
+++ b/drivers/iio/adc/mcp320x.c
@@ -203,7 +203,7 @@ static int mcp320x_probe(struct spi_device *spi)
 
 	mutex_init(&adc->lock);
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(&spi->dev, indio_dev);
 	if (ret < 0)
 		goto reg_disable;
 
@@ -220,7 +220,6 @@ static int mcp320x_remove(struct spi_device *spi)
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 	struct mcp320x *adc = iio_priv(indio_dev);
 
-	iio_device_unregister(indio_dev);
 	regulator_disable(adc->reg);
 
 	return 0;
-- 
1.7.9.5

[PATCH 4/8] iio: adc: mcp3422: Use devm_iio_device_register

[Sachin Kamat]

devm_iio_device_register simplifies the code.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/iio/adc/mcp3422.c |    9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/iio/adc/mcp3422.c b/drivers/iio/adc/mcp3422.c
index bc93f53..f172c29 100644
--- a/drivers/iio/adc/mcp3422.c
+++ b/drivers/iio/adc/mcp3422.c
@@ -361,7 +361,7 @@ static int mcp3422_probe(struct i2c_client *client,
 		| MCP3422_SAMPLE_RATE_VALUE(MCP3422_SRATE_240));
 	mcp3422_update_config(adc, config);
 
-	err = iio_device_register(indio_dev);
+	err = devm_iio_device_register(&client->dev, indio_dev);
 	if (err < 0)
 		return err;
 
@@ -370,12 +370,6 @@ static int mcp3422_probe(struct i2c_client *client,
 	return 0;
 }
 
-static int mcp3422_remove(struct i2c_client *client)
-{
-	iio_device_unregister(i2c_get_clientdata(client));
-	return 0;
-}
-
 static const struct i2c_device_id mcp3422_id[] = {
 	{ "mcp3422", 2 },
 	{ "mcp3423", 3 },
@@ -399,7 +393,6 @@ static struct i2c_driver mcp3422_driver = {
 		.of_match_table = of_match_ptr(mcp3422_of_match),
 	},
 	.probe = mcp3422_probe,
-	.remove = mcp3422_remove,
 	.id_table = mcp3422_id,
 };
 module_i2c_driver(mcp3422_driver);
-- 
1.7.9.5

[PATCH 5/8] iio: adc: ti-adc081c: Use devm_iio_device_register

[Sachin Kamat]

devm_iio_device_register simplifies the code.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/iio/adc/ti-adc081c.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ti-adc081c.c b/drivers/iio/adc/ti-adc081c.c
index ee5f72b..f0b7e73 100644
--- a/drivers/iio/adc/ti-adc081c.c
+++ b/drivers/iio/adc/ti-adc081c.c
@@ -97,7 +97,7 @@ static int adc081c_probe(struct i2c_client *client,
 	iio->channels = &adc081c_channel;
 	iio->num_channels = 1;
 
-	err = iio_device_register(iio);
+	err = devm_iio_device_register(&client->dev, iio);
 	if (err < 0)
 		goto regulator_disable;
 
@@ -116,7 +116,6 @@ static int adc081c_remove(struct i2c_client *client)
 	struct iio_dev *iio = i2c_get_clientdata(client);
 	struct adc081c *adc = iio_priv(iio);
 
-	iio_device_unregister(iio);
 	regulator_disable(adc->ref);
 
 	return 0;
-- 
1.7.9.5

[PATCH 6/8] iio: adc: twl6030-gpadc: Use devm_iio_device_register

[Sachin Kamat]

devm_iio_device_register simplifies the code.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/iio/adc/twl6030-gpadc.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/iio/adc/twl6030-gpadc.c b/drivers/iio/adc/twl6030-gpadc.c
index 0ea96c0..cd16077 100644
--- a/drivers/iio/adc/twl6030-gpadc.c
+++ b/drivers/iio/adc/twl6030-gpadc.c
@@ -948,17 +948,14 @@ static int twl6030_gpadc_probe(struct platform_device *pdev)
 	indio_dev->channels = pdata->iio_channels;
 	indio_dev->num_channels = pdata->nchannels;
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(&pdev->dev, indio_dev);
 
 	return ret;
 }
 
 static int twl6030_gpadc_remove(struct platform_device *pdev)
 {
-	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
-
 	twl6030_gpadc_disable_irq(TWL6030_GPADC_RT_SW1_EOC_MASK);
-	iio_device_unregister(indio_dev);
 
 	return 0;
 }
-- 
1.7.9.5

[PATCH 7/8] iio: adc: viperboard: Use devm_iio_device_register

[Sachin Kamat]

devm_iio_device_register simplifies the code.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/iio/adc/viperboard_adc.c |   12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/iio/adc/viperboard_adc.c b/drivers/iio/adc/viperboard_adc.c
index 09727a7..674b333 100644
--- a/drivers/iio/adc/viperboard_adc.c
+++ b/drivers/iio/adc/viperboard_adc.c
@@ -139,7 +139,7 @@ static int vprbrd_adc_probe(struct platform_device *pdev)
 	indio_dev->channels = vprbrd_adc_iio_channels;
 	indio_dev->num_channels = ARRAY_SIZE(vprbrd_adc_iio_channels);
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(&pdev->dev, indio_dev);
 	if (ret) {
 		dev_err(&pdev->dev, "could not register iio (adc)");
 		return ret;
@@ -150,22 +150,12 @@ static int vprbrd_adc_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int vprbrd_adc_remove(struct platform_device *pdev)
-{
-	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
-
-	iio_device_unregister(indio_dev);
-
-	return 0;
-}
-
 static struct platform_driver vprbrd_adc_driver = {
 	.driver = {
 		.name	= "viperboard-adc",
 		.owner	= THIS_MODULE,
 	},
 	.probe		= vprbrd_adc_probe,
-	.remove		= vprbrd_adc_remove,
 };
 
 module_platform_driver(vprbrd_adc_driver);
-- 
1.7.9.5

[PATCH 8/8] iio: amplifiers: ad8366: Use devm_iio_device_register

[Sachin Kamat]

devm_iio_device_register simplifies the code.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/iio/amplifiers/ad8366.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iio/amplifiers/ad8366.c b/drivers/iio/amplifiers/ad8366.c
index ba6f6a9..6af459c 100644
--- a/drivers/iio/amplifiers/ad8366.c
+++ b/drivers/iio/amplifiers/ad8366.c
@@ -162,7 +162,7 @@ static int ad8366_probe(struct spi_device *spi)
 	indio_dev->channels = ad8366_channels;
 	indio_dev->num_channels = ARRAY_SIZE(ad8366_channels);
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(&spi->dev, indio_dev);
 	if (ret)
 		goto error_disable_reg;
 
@@ -183,8 +183,6 @@ static int ad8366_remove(struct spi_device *spi)
 	struct ad8366_state *st = iio_priv(indio_dev);
 	struct regulator *reg = st->reg;
 
-	iio_device_unregister(indio_dev);
-
 	if (!IS_ERR(reg))
 		regulator_disable(reg);
 
-- 
1.7.9.5

Re: [PATCH 8/8] iio: amplifiers: ad8366: Use devm_iio_device_register

[Jonathan Cameron]

Sachin Kamat <sachin.kamat@linaro.org> wrote:
>devm_iio_device_register simplifies the code.
>
>Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Is there not a race here?
>---
> drivers/iio/amplifiers/ad8366.c |    4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
>diff --git a/drivers/iio/amplifiers/ad8366.c
>b/drivers/iio/amplifiers/ad8366.c
>index ba6f6a9..6af459c 100644
>--- a/drivers/iio/amplifiers/ad8366.c
>+++ b/drivers/iio/amplifiers/ad8366.c
>@@ -162,7 +162,7 @@ static int ad8366_probe(struct spi_device *spi)
> 	indio_dev->channels = ad8366_channels;
> 	indio_dev->num_channels = ARRAY_SIZE(ad8366_channels);
> 
>-	ret = iio_device_register(indio_dev);
>+	ret = devm_iio_device_register(&spi->dev, indio_dev);
> 	if (ret)
> 		goto error_disable_reg;
> 
>@@ -183,8 +183,6 @@ static int ad8366_remove(struct spi_device *spi)
> 	struct ad8366_state *st = iio_priv(indio_dev);
> 	struct regulator *reg = st->reg;
> 
>-	iio_device_unregister(indio_dev);
>-
> 	if (!IS_ERR(reg))
> 		regulator_disable(reg);

So now the reg will be disabled before the user space interfaces have gone away?

I wondered about a similar patch when the alloc patches turned up and concluded that there were too many cases like this to make it a good idea...

> 

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

Re: [PATCH 0/8] iio: Introduce devm_iio_device_{register,unregister}

[Jonathan Cameron]

Sachin Kamat <sachin.kamat@linaro.org> wrote:
>With the conversion of iio drivers to use device managed APIs, most of
>the
>drivers now have a remove function with only the
>iio_device_unregister()
>call.
I am not sure this is a good idea if there is anything else in those functions. The unregister is responsible for marking the device as going away so until it has happened all interfaces should still work.
In at least two of this small set that is not the case...

 Thus introduce device managed version of iio_device_register() to
>clean up lot of such code.
>Have shown the usage in a few drivers to illustrate the cleanup. Will
>do
>for the rest too.
>This is only compile tested as I do not have any actual hardware to
>test ATM.
>
>
>Sachin Kamat (8):
>  iio: core: Implement devm_iio_device_{register,unregister}
>  iio: accel: kxsd9: Use devm_iio_device_register
>  iio: adc: mcp320x: Use devm_iio_device_register
>  iio: adc: mcp3422: Use devm_iio_device_register
>  iio: adc: ti-adc081c: Use devm_iio_device_register
>  iio: adc: twl6030-gpadc: Use devm_iio_device_register
>  iio: adc: viperboard: Use devm_iio_device_register
>  iio: amplifiers: ad8366: Use devm_iio_device_register
>
> Documentation/driver-model/devres.txt |    2 ++
> drivers/iio/accel/kxsd9.c             |   10 +---------
> drivers/iio/adc/mcp320x.c             |    3 +--
> drivers/iio/adc/mcp3422.c             |    9 +--------
> drivers/iio/adc/ti-adc081c.c          |    3 +--
> drivers/iio/adc/twl6030-gpadc.c       |    5 +----
> drivers/iio/adc/viperboard_adc.c      |   12 +----------
> drivers/iio/amplifiers/ad8366.c       |    4 +---
>drivers/iio/industrialio-core.c       |   35
>+++++++++++++++++++++++++++++++++
> include/linux/iio/iio.h               |   25 +++++++++++++++++++++++
> 10 files changed, 69 insertions(+), 39 deletions(-)

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

Re: [PATCH 0/8] iio: Introduce devm_iio_device_{register,unregister}

[Sachin Kamat]

On 24 September 2013 18:43, Jonathan Cameron <jic23@kernel.org> wrote:
>
>
> Sachin Kamat <sachin.kamat@linaro.org> wrote:
>>With the conversion of iio drivers to use device managed APIs, most of
>>the
>>drivers now have a remove function with only the
>>iio_device_unregister()
>>call.
> I am not sure this is a good idea if there is anything else in those functions. The unregister is responsible for marking the device as going away so until it has happened all interfaces should still work.

This will be done in only those drivers whose remove function contains
nothing other than the unregister call thereby removing it altogether
or where unregister happens to be the last operation in remove.

> In at least two of this small set that is not the case...

I had added these 2 purposefully to get your feedback as I wasn't
fully sure if disabling regulator would cause any ordering related
issues.

-- 
With warm regards,
Sachin