[PATCH 0/5] iio: device-managed conversions with devm_iio_map_array_register()

[PATCH 0/5] iio: device-managed conversions with devm_iio_map_array_register()

[Alexandru Ardelean]

This change introduces a devm_iio_map_array_register() variant for the
iio_map_array_register() function.

And converts 4 drivers to full device-managed.
These 4 drivers only call iio_map_array_unregister() and
iio_device_unregister() in their remove hooks.

These 4 drivers should make a reasonably good case for introducing this
devm_iio_map_array_register() function.

There are 7 more drivers that would use the devm_iio_map_array_register()
function, but they require a bit more handling in the remove/unwinding
part.
So, those 7 are left for later.

Alexandru Ardelean (5):
  iio: inkern: introduce devm_iio_map_array_register() short-hand
    function
  iio: adc: intel_mrfld_adc: convert probe to full device-managed
  iio: adc: axp288_adc: convert probe to full device-managed
  iio: adc: lp8788_adc: convert probe to full-device managed
  iio: adc: da9150-gpadc: convert probe to full-device managed

 .../driver-api/driver-model/devres.rst        |  1 +
 drivers/iio/adc/axp288_adc.c                  | 28 +++--------------
 drivers/iio/adc/da9150-gpadc.c                | 27 ++--------------
 drivers/iio/adc/intel_mrfld_adc.c             | 24 ++------------
 drivers/iio/adc/lp8788_adc.c                  | 31 +++----------------
 drivers/iio/inkern.c                          | 17 ++++++++++
 include/linux/iio/driver.h                    | 14 +++++++++
 7 files changed, 45 insertions(+), 97 deletions(-)

-- 
2.31.1

[PATCH 1/5] iio: inkern: introduce devm_iio_map_array_register() short-hand function

[Alexandru Ardelean]

This change introduces a device-managed variant to the
iio_map_array_register() function. It's a simple implementation of calling
iio_map_array_register() and registering a callback to
iio_map_array_unregister() with the devm_add_action_or_reset().

The function uses an explicit 'dev' parameter to bind the unwinding to. It
could have been implemented to implicitly use the parent of the IIO device,
however it shouldn't be too expensive to callers to just specify to which
device object to bind this unwind call.
It would make the API a bit more flexible.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 .../driver-api/driver-model/devres.rst          |  1 +
 drivers/iio/inkern.c                            | 17 +++++++++++++++++
 include/linux/iio/driver.h                      | 14 ++++++++++++++
 3 files changed, 32 insertions(+)

diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst
index 650096523f4f..148e19381b79 100644
--- a/Documentation/driver-api/driver-model/devres.rst
+++ b/Documentation/driver-api/driver-model/devres.rst
@@ -287,6 +287,7 @@ IIO
   devm_iio_device_register()
   devm_iio_dmaengine_buffer_setup()
   devm_iio_kfifo_buffer_setup()
+  devm_iio_map_array_register()
   devm_iio_triggered_buffer_setup()
   devm_iio_trigger_alloc()
   devm_iio_trigger_register()
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 391a3380a1d1..0222885b334c 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -85,6 +85,23 @@ int iio_map_array_unregister(struct iio_dev *indio_dev)
 }
 EXPORT_SYMBOL_GPL(iio_map_array_unregister);
 
+static void iio_map_array_unregister_cb(void *indio_dev)
+{
+	iio_map_array_unregister(indio_dev);
+}
+
+int devm_iio_map_array_register(struct device *dev, struct iio_dev *indio_dev, struct iio_map *maps)
+{
+	int ret;
+
+	ret = iio_map_array_register(indio_dev, maps);
+	if (ret)
+		return ret;
+
+	return devm_add_action_or_reset(dev, iio_map_array_unregister_cb, indio_dev);
+}
+EXPORT_SYMBOL_GPL(devm_iio_map_array_register);
+
 static const struct iio_chan_spec
 *iio_chan_spec_from_name(const struct iio_dev *indio_dev, const char *name)
 {
diff --git a/include/linux/iio/driver.h b/include/linux/iio/driver.h
index 36de60a5da7a..7a157ed218f6 100644
--- a/include/linux/iio/driver.h
+++ b/include/linux/iio/driver.h
@@ -8,6 +8,7 @@
 #ifndef _IIO_INKERN_H_
 #define _IIO_INKERN_H_
 
+struct device;
 struct iio_dev;
 struct iio_map;
 
@@ -26,4 +27,17 @@ int iio_map_array_register(struct iio_dev *indio_dev,
  */
 int iio_map_array_unregister(struct iio_dev *indio_dev);
 
+/**
+ * devm_iio_map_array_register - device-managed version of iio_map_array_register
+ * @dev:	Device object to which to bind the unwinding of this registration
+ * @indio_dev:	Pointer to the iio_dev structure
+ * @maps:	Pointer to an IIO map object which is to be registered to this IIO device
+ *
+ * This function will call iio_map_array_register() to register an IIO map object
+ * and will also hook a callback to the iio_map_array_unregister() function to
+ * handle de-registration of the IIO map object when the device's refcount goes to
+ * zero.
+ */
+int devm_iio_map_array_register(struct device *dev, struct iio_dev *indio_dev, struct iio_map *maps);
+
 #endif
-- 
2.31.1

[PATCH 2/5] iio: adc: intel_mrfld_adc: convert probe to full device-managed

[Alexandru Ardelean]

The only call in the remove hook is the iio_map_array_unregister() call.
Since we have a devm_iio_map_array_register() function now, we can use that
and remove the remove hook entirely.
The IIO device was registered with the devm_iio_device_register() prior to
this change.

Also, the platform_set_drvdata() can be removed now, since it was used only
in the remove hook.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/iio/adc/intel_mrfld_adc.c | 24 ++----------------------
 1 file changed, 2 insertions(+), 22 deletions(-)

diff --git a/drivers/iio/adc/intel_mrfld_adc.c b/drivers/iio/adc/intel_mrfld_adc.c
index 75394350eb4c..616de0c3a049 100644
--- a/drivers/iio/adc/intel_mrfld_adc.c
+++ b/drivers/iio/adc/intel_mrfld_adc.c
@@ -205,8 +205,6 @@ static int mrfld_adc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	platform_set_drvdata(pdev, indio_dev);
-
 	indio_dev->name = pdev->name;
 
 	indio_dev->channels = mrfld_adc_channels;
@@ -214,28 +212,11 @@ static int mrfld_adc_probe(struct platform_device *pdev)
 	indio_dev->info = &mrfld_adc_iio_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
-	ret = iio_map_array_register(indio_dev, iio_maps);
+	ret = devm_iio_map_array_register(dev, indio_dev, iio_maps);
 	if (ret)
 		return ret;
 
-	ret = devm_iio_device_register(dev, indio_dev);
-	if (ret < 0)
-		goto err_array_unregister;
-
-	return 0;
-
-err_array_unregister:
-	iio_map_array_unregister(indio_dev);
-	return ret;
-}
-
-static int mrfld_adc_remove(struct platform_device *pdev)
-{
-	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
-
-	iio_map_array_unregister(indio_dev);
-
-	return 0;
+	return devm_iio_device_register(dev, indio_dev);
 }
 
 static const struct platform_device_id mrfld_adc_id_table[] = {
@@ -249,7 +230,6 @@ static struct platform_driver mrfld_adc_driver = {
 		.name = "mrfld_bcove_adc",
 	},
 	.probe = mrfld_adc_probe,
-	.remove = mrfld_adc_remove,
 	.id_table = mrfld_adc_id_table,
 };
 module_platform_driver(mrfld_adc_driver);
-- 
2.31.1

[PATCH 3/5] iio: adc: axp288_adc: convert probe to full device-managed

[Alexandru Ardelean]

This change converts the probe of this driver to use device-managed
functions only, which means that the remove hook can be removed.
The remove hook has only 2 calls to iio_device_unregister() and
iio_map_array_unregister(). Both these can now be done via devm register
functions, now that there's also a devm_iio_map_array_register() function.

The platform_set_drvdata() can also be removed now.

This change also removes the error print for when the iio_device_register()
call fails. This isn't required now.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/iio/adc/axp288_adc.c | 28 ++++------------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/drivers/iio/adc/axp288_adc.c b/drivers/iio/adc/axp288_adc.c
index 5f5e8b39e4d2..a4b8be5b8f88 100644
--- a/drivers/iio/adc/axp288_adc.c
+++ b/drivers/iio/adc/axp288_adc.c
@@ -259,7 +259,7 @@ static int axp288_adc_probe(struct platform_device *pdev)
 	info->irq = platform_get_irq(pdev, 0);
 	if (info->irq < 0)
 		return info->irq;
-	platform_set_drvdata(pdev, indio_dev);
+
 	info->regmap = axp20x->regmap;
 	/*
 	 * Set ADC to enabled state at all time, including system suspend.
@@ -276,31 +276,12 @@ static int axp288_adc_probe(struct platform_device *pdev)
 	indio_dev->num_channels = ARRAY_SIZE(axp288_adc_channels);
 	indio_dev->info = &axp288_adc_iio_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
-	ret = iio_map_array_register(indio_dev, axp288_adc_default_maps);
+
+	ret = devm_iio_map_array_register(&pdev->dev, indio_dev, axp288_adc_default_maps);
 	if (ret < 0)
 		return ret;
 
-	ret = iio_device_register(indio_dev);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "unable to register iio device\n");
-		goto err_array_unregister;
-	}
-	return 0;
-
-err_array_unregister:
-	iio_map_array_unregister(indio_dev);
-
-	return ret;
-}
-
-static int axp288_adc_remove(struct platform_device *pdev)
-{
-	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
-
-	iio_device_unregister(indio_dev);
-	iio_map_array_unregister(indio_dev);
-
-	return 0;
+	return devm_iio_device_register(&pdev->dev, indio_dev);
 }
 
 static const struct platform_device_id axp288_adc_id_table[] = {
@@ -310,7 +291,6 @@ static const struct platform_device_id axp288_adc_id_table[] = {
 
 static struct platform_driver axp288_adc_driver = {
 	.probe = axp288_adc_probe,
-	.remove = axp288_adc_remove,
 	.id_table = axp288_adc_id_table,
 	.driver = {
 		.name = "axp288_adc",
-- 
2.31.1

[PATCH 4/5] iio: adc: lp8788_adc: convert probe to full-device managed

[Alexandru Ardelean]

This change converts the probe of this driver to use device-managed
functions only, which means that the remove hook can be removed.
The remove hook has only 2 calls to iio_device_unregister() and
iio_map_array_unregister(). Both these can now be done via devm register
functions, now that there's also a devm_iio_map_array_register() function.

The platform_set_drvdata() can also be removed now.

This change also removes the error print for when the iio_device_register()
call fails. This isn't required now.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/iio/adc/lp8788_adc.c | 31 +++++--------------------------
 1 file changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/iio/adc/lp8788_adc.c b/drivers/iio/adc/lp8788_adc.c
index 8fb57e375529..6d9b354bc705 100644
--- a/drivers/iio/adc/lp8788_adc.c
+++ b/drivers/iio/adc/lp8788_adc.c
@@ -163,7 +163,8 @@ static struct iio_map lp8788_default_iio_maps[] = {
 	{ }
 };
 
-static int lp8788_iio_map_register(struct iio_dev *indio_dev,
+static int lp8788_iio_map_register(struct device *dev,
+				struct iio_dev *indio_dev,
 				struct lp8788_platform_data *pdata,
 				struct lp8788_adc *adc)
 {
@@ -173,7 +174,7 @@ static int lp8788_iio_map_register(struct iio_dev *indio_dev,
 	map = (!pdata || !pdata->adc_pdata) ?
 		lp8788_default_iio_maps : pdata->adc_pdata;
 
-	ret = iio_map_array_register(indio_dev, map);
+	ret = devm_iio_map_array_register(dev, indio_dev, map);
 	if (ret) {
 		dev_err(&indio_dev->dev, "iio map err: %d\n", ret);
 		return ret;
@@ -196,9 +197,8 @@ static int lp8788_adc_probe(struct platform_device *pdev)
 
 	adc = iio_priv(indio_dev);
 	adc->lp = lp;
-	platform_set_drvdata(pdev, indio_dev);
 
-	ret = lp8788_iio_map_register(indio_dev, lp->pdata, adc);
+	ret = lp8788_iio_map_register(&pdev->dev, indio_dev, lp->pdata, adc);
 	if (ret)
 		return ret;
 
@@ -210,32 +210,11 @@ static int lp8788_adc_probe(struct platform_device *pdev)
 	indio_dev->channels = lp8788_adc_channels;
 	indio_dev->num_channels = ARRAY_SIZE(lp8788_adc_channels);
 
-	ret = iio_device_register(indio_dev);
-	if (ret) {
-		dev_err(&pdev->dev, "iio dev register err: %d\n", ret);
-		goto err_iio_device;
-	}
-
-	return 0;
-
-err_iio_device:
-	iio_map_array_unregister(indio_dev);
-	return ret;
-}
-
-static int lp8788_adc_remove(struct platform_device *pdev)
-{
-	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
-
-	iio_device_unregister(indio_dev);
-	iio_map_array_unregister(indio_dev);
-
-	return 0;
+	return devm_iio_device_register(&pdev->dev, indio_dev);
 }
 
 static struct platform_driver lp8788_adc_driver = {
 	.probe = lp8788_adc_probe,
-	.remove = lp8788_adc_remove,
 	.driver = {
 		.name = LP8788_DEV_ADC,
 	},
-- 
2.31.1

[PATCH 5/5] iio: adc: da9150-gpadc: convert probe to full-device managed

[Alexandru Ardelean]

This change converts the probe of this driver to use device-managed
functions only, which means that the remove hook can be removed.
The remove hook has only 2 calls to iio_device_unregister() and
iio_map_array_unregister(). Both these can now be done via devm register
functions, now that there's also a devm_iio_map_array_register() function.

The platform_set_drvdata() can also be removed now.

This change also removes the error print for when the iio_device_register()
call fails. This isn't required now.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/iio/adc/da9150-gpadc.c | 27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

diff --git a/drivers/iio/adc/da9150-gpadc.c b/drivers/iio/adc/da9150-gpadc.c
index 7a7a54a7ed76..8f0d3fb63b67 100644
--- a/drivers/iio/adc/da9150-gpadc.c
+++ b/drivers/iio/adc/da9150-gpadc.c
@@ -330,7 +330,6 @@ static int da9150_gpadc_probe(struct platform_device *pdev)
 	}
 	gpadc = iio_priv(indio_dev);
 
-	platform_set_drvdata(pdev, indio_dev);
 	gpadc->da9150 = da9150;
 	gpadc->dev = dev;
 	mutex_init(&gpadc->lock);
@@ -347,7 +346,7 @@ static int da9150_gpadc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = iio_map_array_register(indio_dev, da9150_gpadc_default_maps);
+	ret = devm_iio_map_array_register(&pdev->dev, indio_dev, da9150_gpadc_default_maps);
 	if (ret) {
 		dev_err(dev, "Failed to register IIO maps: %d\n", ret);
 		return ret;
@@ -359,28 +358,7 @@ static int da9150_gpadc_probe(struct platform_device *pdev)
 	indio_dev->channels = da9150_gpadc_channels;
 	indio_dev->num_channels = ARRAY_SIZE(da9150_gpadc_channels);
 
-	ret = iio_device_register(indio_dev);
-	if (ret) {
-		dev_err(dev, "Failed to register IIO device: %d\n", ret);
-		goto iio_map_unreg;
-	}
-
-	return 0;
-
-iio_map_unreg:
-	iio_map_array_unregister(indio_dev);
-
-	return ret;
-}
-
-static int da9150_gpadc_remove(struct platform_device *pdev)
-{
-	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
-
-	iio_device_unregister(indio_dev);
-	iio_map_array_unregister(indio_dev);
-
-	return 0;
+	return devm_iio_device_register(&pdev->dev, indio_dev);
 }
 
 static struct platform_driver da9150_gpadc_driver = {
@@ -388,7 +366,6 @@ static struct platform_driver da9150_gpadc_driver = {
 		.name = "da9150-gpadc",
 	},
 	.probe = da9150_gpadc_probe,
-	.remove = da9150_gpadc_remove,
 };
 
 module_platform_driver(da9150_gpadc_driver);
-- 
2.31.1

Re: [PATCH 3/5] iio: adc: axp288_adc: convert probe to full device-managed

[Hans de Goede]

Hi,

On 9/3/21 9:29 AM, Alexandru Ardelean wrote:
> This change converts the probe of this driver to use device-managed
> functions only, which means that the remove hook can be removed.
> The remove hook has only 2 calls to iio_device_unregister() and
> iio_map_array_unregister(). Both these can now be done via devm register
> functions, now that there's also a devm_iio_map_array_register() function.
> 
> The platform_set_drvdata() can also be removed now.
> 
> This change also removes the error print for when the iio_device_register()
> call fails. This isn't required now.
> > Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans



> ---
>  drivers/iio/adc/axp288_adc.c | 28 ++++------------------------
>  1 file changed, 4 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/iio/adc/axp288_adc.c b/drivers/iio/adc/axp288_adc.c
> index 5f5e8b39e4d2..a4b8be5b8f88 100644
> --- a/drivers/iio/adc/axp288_adc.c
> +++ b/drivers/iio/adc/axp288_adc.c
> @@ -259,7 +259,7 @@ static int axp288_adc_probe(struct platform_device *pdev)
>  	info->irq = platform_get_irq(pdev, 0);
>  	if (info->irq < 0)
>  		return info->irq;
> -	platform_set_drvdata(pdev, indio_dev);
> +
>  	info->regmap = axp20x->regmap;
>  	/*
>  	 * Set ADC to enabled state at all time, including system suspend.
> @@ -276,31 +276,12 @@ static int axp288_adc_probe(struct platform_device *pdev)
>  	indio_dev->num_channels = ARRAY_SIZE(axp288_adc_channels);
>  	indio_dev->info = &axp288_adc_iio_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> -	ret = iio_map_array_register(indio_dev, axp288_adc_default_maps);
> +
> +	ret = devm_iio_map_array_register(&pdev->dev, indio_dev, axp288_adc_default_maps);
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = iio_device_register(indio_dev);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev, "unable to register iio device\n");
> -		goto err_array_unregister;
> -	}
> -	return 0;
> -
> -err_array_unregister:
> -	iio_map_array_unregister(indio_dev);
> -
> -	return ret;
> -}
> -
> -static int axp288_adc_remove(struct platform_device *pdev)
> -{
> -	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> -
> -	iio_device_unregister(indio_dev);
> -	iio_map_array_unregister(indio_dev);
> -
> -	return 0;
> +	return devm_iio_device_register(&pdev->dev, indio_dev);
>  }
>  
>  static const struct platform_device_id axp288_adc_id_table[] = {
> @@ -310,7 +291,6 @@ static const struct platform_device_id axp288_adc_id_table[] = {
>  
>  static struct platform_driver axp288_adc_driver = {
>  	.probe = axp288_adc_probe,
> -	.remove = axp288_adc_remove,
>  	.id_table = axp288_adc_id_table,
>  	.driver = {
>  		.name = "axp288_adc",
> 

Re: [PATCH 2/5] iio: adc: intel_mrfld_adc: convert probe to full device-managed

[Andy Shevchenko]

On Fri, Sep 03, 2021 at 10:29:14AM +0300, Alexandru Ardelean wrote:
> The only call in the remove hook is the iio_map_array_unregister() call.
> Since we have a devm_iio_map_array_register() function now, we can use that
> and remove the remove hook entirely.
> The IIO device was registered with the devm_iio_device_register() prior to
> this change.
> 
> Also, the platform_set_drvdata() can be removed now, since it was used only
> in the remove hook.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
> ---
>  drivers/iio/adc/intel_mrfld_adc.c | 24 ++----------------------
>  1 file changed, 2 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/iio/adc/intel_mrfld_adc.c b/drivers/iio/adc/intel_mrfld_adc.c
> index 75394350eb4c..616de0c3a049 100644
> --- a/drivers/iio/adc/intel_mrfld_adc.c
> +++ b/drivers/iio/adc/intel_mrfld_adc.c
> @@ -205,8 +205,6 @@ static int mrfld_adc_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	platform_set_drvdata(pdev, indio_dev);
> -
>  	indio_dev->name = pdev->name;
>  
>  	indio_dev->channels = mrfld_adc_channels;
> @@ -214,28 +212,11 @@ static int mrfld_adc_probe(struct platform_device *pdev)
>  	indio_dev->info = &mrfld_adc_iio_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  
> -	ret = iio_map_array_register(indio_dev, iio_maps);
> +	ret = devm_iio_map_array_register(dev, indio_dev, iio_maps);
>  	if (ret)
>  		return ret;
>  
> -	ret = devm_iio_device_register(dev, indio_dev);
> -	if (ret < 0)
> -		goto err_array_unregister;
> -
> -	return 0;
> -
> -err_array_unregister:
> -	iio_map_array_unregister(indio_dev);
> -	return ret;
> -}
> -
> -static int mrfld_adc_remove(struct platform_device *pdev)
> -{
> -	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> -
> -	iio_map_array_unregister(indio_dev);
> -
> -	return 0;
> +	return devm_iio_device_register(dev, indio_dev);
>  }
>  
>  static const struct platform_device_id mrfld_adc_id_table[] = {
> @@ -249,7 +230,6 @@ static struct platform_driver mrfld_adc_driver = {
>  		.name = "mrfld_bcove_adc",
>  	},
>  	.probe = mrfld_adc_probe,
> -	.remove = mrfld_adc_remove,
>  	.id_table = mrfld_adc_id_table,
>  };
>  module_platform_driver(mrfld_adc_driver);
> -- 
> 2.31.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/5] iio: inkern: introduce devm_iio_map_array_register() short-hand function

[Andy Shevchenko]

On Fri, Sep 03, 2021 at 10:29:13AM +0300, Alexandru Ardelean wrote:
> This change introduces a device-managed variant to the
> iio_map_array_register() function. It's a simple implementation of calling
> iio_map_array_register() and registering a callback to
> iio_map_array_unregister() with the devm_add_action_or_reset().
> 
> The function uses an explicit 'dev' parameter to bind the unwinding to. It
> could have been implemented to implicitly use the parent of the IIO device,
> however it shouldn't be too expensive to callers to just specify to which
> device object to bind this unwind call.
> It would make the API a bit more flexible.

AFAIU this dev pointer is kinda discussable thing. What scenario do you expect
(have in mind) when it shouldn't use parent?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/5] iio: inkern: introduce devm_iio_map_array_register() short-hand function

[Alexandru Ardelean]

On Fri, 3 Sept 2021 at 16:40, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Sep 03, 2021 at 10:29:13AM +0300, Alexandru Ardelean wrote:
> > This change introduces a device-managed variant to the
> > iio_map_array_register() function. It's a simple implementation of calling
> > iio_map_array_register() and registering a callback to
> > iio_map_array_unregister() with the devm_add_action_or_reset().
> >
> > The function uses an explicit 'dev' parameter to bind the unwinding to. It
> > could have been implemented to implicitly use the parent of the IIO device,
> > however it shouldn't be too expensive to callers to just specify to which
> > device object to bind this unwind call.
> > It would make the API a bit more flexible.
>
> AFAIU this dev pointer is kinda discussable thing. What scenario do you expect
> (have in mind) when it shouldn't use parent?

So, this brings me back to an older discussion about devm_ when I
thought about making some devm_ function that implicitly takes uses
the parent of the IIO device.

Jonathan mentioned that if we go that route, maybe we should prefix it
with iiom_ .
But we weren't sure at the time if that makes sense.
The idea was to bind the management of the unwinding to either the
parent of the IIO device, or the IIO device itself (indio_dev->dev).

We kind of concluded that it may probably not be a good to hide
anything and make standard a devm_ function with an explicit 'dev'
object parameter.

I found a recent mention here (while searching for iiom_  on linux-iio):
https://lore.kernel.org/linux-iio/20210313192150.74c0a91b@archlinux/


>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH 1/5] iio: inkern: introduce devm_iio_map_array_register() short-hand function

[Jonathan Cameron]

On Fri, 3 Sep 2021 16:56:43 +0300
Alexandru Ardelean <aardelean@deviqon.com> wrote:

> On Fri, 3 Sept 2021 at 16:40, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Fri, Sep 03, 2021 at 10:29:13AM +0300, Alexandru Ardelean wrote:  
> > > This change introduces a device-managed variant to the
> > > iio_map_array_register() function. It's a simple implementation of calling
> > > iio_map_array_register() and registering a callback to
> > > iio_map_array_unregister() with the devm_add_action_or_reset().
> > >
> > > The function uses an explicit 'dev' parameter to bind the unwinding to. It
> > > could have been implemented to implicitly use the parent of the IIO device,
> > > however it shouldn't be too expensive to callers to just specify to which
> > > device object to bind this unwind call.
> > > It would make the API a bit more flexible.  
> >
> > AFAIU this dev pointer is kinda discussable thing. What scenario do you expect
> > (have in mind) when it shouldn't use parent?  
> 
> So, this brings me back to an older discussion about devm_ when I
> thought about making some devm_ function that implicitly takes uses
> the parent of the IIO device.
> 
> Jonathan mentioned that if we go that route, maybe we should prefix it
> with iiom_ .
> But we weren't sure at the time if that makes sense.
> The idea was to bind the management of the unwinding to either the
> parent of the IIO device, or the IIO device itself (indio_dev->dev).

I'm not keen on playing the same games that say pcim does where it
effectively combines all cleanup into one devres call because that
can end up doing things in an order that isn't strict reversal if
the setup path isn't carefully organised.  It's 'clever' but to my mind
doing something similar in IIO would need to subtle bugs.

So we would simply be using iiom_ as a shortcut to say bind to the parent
device.  Might be worth considering long term but I'm not keen to do
it for a random little used function first!

Series looks good to me, but I'll let it sit a little longer before applying.

Jonathan


> 
> We kind of concluded that it may probably not be a good to hide
> anything and make standard a devm_ function with an explicit 'dev'
> object parameter.
> 
> I found a recent mention here (while searching for iiom_  on linux-iio):
> https://lore.kernel.org/linux-iio/20210313192150.74c0a91b@archlinux/
> 
> 
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
> >
> >  

Re: [PATCH 0/5] iio: device-managed conversions with devm_iio_map_array_register()

[Jonathan Cameron]

On Fri,  3 Sep 2021 10:29:12 +0300
Alexandru Ardelean <aardelean@deviqon.com> wrote:

> This change introduces a devm_iio_map_array_register() variant for the
> iio_map_array_register() function.
> 
> And converts 4 drivers to full device-managed.
> These 4 drivers only call iio_map_array_unregister() and
> iio_device_unregister() in their remove hooks.
> 
> These 4 drivers should make a reasonably good case for introducing this
> devm_iio_map_array_register() function.
> 
> There are 7 more drivers that would use the devm_iio_map_array_register()
> function, but they require a bit more handling in the remove/unwinding
> part.
> So, those 7 are left for later.

Series applied to the togreg branch of iio.git and pushed out as testing
so 0-day can work it's magic.

Thanks,

Jonathan

> 
> Alexandru Ardelean (5):
>   iio: inkern: introduce devm_iio_map_array_register() short-hand
>     function
>   iio: adc: intel_mrfld_adc: convert probe to full device-managed
>   iio: adc: axp288_adc: convert probe to full device-managed
>   iio: adc: lp8788_adc: convert probe to full-device managed
>   iio: adc: da9150-gpadc: convert probe to full-device managed
> 
>  .../driver-api/driver-model/devres.rst        |  1 +
>  drivers/iio/adc/axp288_adc.c                  | 28 +++--------------
>  drivers/iio/adc/da9150-gpadc.c                | 27 ++--------------
>  drivers/iio/adc/intel_mrfld_adc.c             | 24 ++------------
>  drivers/iio/adc/lp8788_adc.c                  | 31 +++----------------
>  drivers/iio/inkern.c                          | 17 ++++++++++
>  include/linux/iio/driver.h                    | 14 +++++++++
>  7 files changed, 45 insertions(+), 97 deletions(-)
>