[PATCH 0/5] iio: Using devm_* APIs in some sensor drivers

[PATCH 0/5] iio: Using devm_* APIs in some sensor drivers

[Beomho Seo]

This patch use devm_* APIs for some sensor drivers.
Using them make sensor driver simpler.

Beomho Seo (5):
  iio: adc: exynos_adc: Use devm_* APIs
  iio: light: cm32181: Use devm_* APIs
  iio: light: cm36651: Use devm_* APIs
  iio: light: gp2ap020a00f: Use devm_* APIs
  iio: magnetometer: ak8975: Use devm_* APIs

 drivers/iio/adc/exynos_adc.c      |   11 +++------
 drivers/iio/light/cm32181.c       |   11 +--------
 drivers/iio/light/cm36651.c       |   11 ++++-----
 drivers/iio/light/gp2ap020a00f.c  |   10 +++-----
 drivers/iio/magnetometer/ak8975.c |   48 ++++++++-----------------------------
 5 files changed, 21 insertions(+), 70 deletions(-)

-- 
1.7.9.5

[PATCH 1/5] iio: adc: exynos_adc: Use devm_* APIs

[Beomho Seo]

This patch changes APIs from request_irq() and iio_device_register()
to devm_* APIs. Using them, make code simpler.

Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
---
 drivers/iio/adc/exynos_adc.c |   11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index d325aea..37b6eb5 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -337,7 +337,7 @@ static int exynos_adc_probe(struct platform_device *pdev)
 	else
 		indio_dev->num_channels = MAX_ADC_V2_CHANNELS;
 
-	ret = request_irq(info->irq, exynos_adc_isr,
+	ret = devm_request_irq(&pdev->dev, info->irq, exynos_adc_isr,
 					0, dev_name(&pdev->dev), info);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed requesting irq, irq = %d\n",
@@ -345,9 +345,9 @@ static int exynos_adc_probe(struct platform_device *pdev)
 		goto err_disable_clk;
 	}
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(&pdev->dev, indio_dev);
 	if (ret)
-		goto err_irq;
+		goto err_disable_clk;
 
 	exynos_adc_hw_init(info);
 
@@ -362,9 +362,6 @@ static int exynos_adc_probe(struct platform_device *pdev)
 err_of_populate:
 	device_for_each_child(&pdev->dev, NULL,
 				exynos_adc_remove_devices);
-	iio_device_unregister(indio_dev);
-err_irq:
-	free_irq(info->irq, info);
 err_disable_clk:
 	writel(0, info->enable_reg);
 	clk_disable_unprepare(info->clk);
@@ -380,8 +377,6 @@ static int exynos_adc_remove(struct platform_device *pdev)
 
 	device_for_each_child(&pdev->dev, NULL,
 				exynos_adc_remove_devices);
-	iio_device_unregister(indio_dev);
-	free_irq(info->irq, info);
 	writel(0, info->enable_reg);
 	clk_disable_unprepare(info->clk);
 	regulator_disable(info->vdd);
-- 
1.7.9.5

[PATCH 2/5] iio: light: cm32181: Use devm_* APIs

[Beomho Seo]

This patch changes API from iio_device_register() to devm_* API.
Using API make code simpler.

Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
---
 drivers/iio/light/cm32181.c |   11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
index d976e6c..ad36b29 100644
--- a/drivers/iio/light/cm32181.c
+++ b/drivers/iio/light/cm32181.c
@@ -331,7 +331,7 @@ static int cm32181_probe(struct i2c_client *client,
 		return ret;
 	}
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(&client->dev, indio_dev);
 	if (ret) {
 		dev_err(&client->dev,
 			"%s: regist device failed\n",
@@ -342,14 +342,6 @@ static int cm32181_probe(struct i2c_client *client,
 	return 0;
 }
 
-static int cm32181_remove(struct i2c_client *client)
-{
-	struct iio_dev *indio_dev = i2c_get_clientdata(client);
-
-	iio_device_unregister(indio_dev);
-	return 0;
-}
-
 static const struct i2c_device_id cm32181_id[] = {
 	{ "cm32181", 0 },
 	{ }
@@ -370,7 +362,6 @@ static struct i2c_driver cm32181_driver = {
 	},
 	.id_table       = cm32181_id,
 	.probe		= cm32181_probe,
-	.remove		= cm32181_remove,
 };
 
 module_i2c_driver(cm32181_driver);
-- 
1.7.9.5

[PATCH 3/5] iio: light: cm36651: Use devm_* APIs

[Beomho Seo]

This patch changes APIs from request_threaded_irq()
and iio_device_register() to devm_* APIS.
Using them make code simpler.

Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
---
 drivers/iio/light/cm36651.c |   11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/light/cm36651.c b/drivers/iio/light/cm36651.c
index 39fc67e..2fd9579 100644
--- a/drivers/iio/light/cm36651.c
+++ b/drivers/iio/light/cm36651.c
@@ -679,7 +679,8 @@ static int cm36651_probe(struct i2c_client *client,
 		goto error_i2c_unregister_ara;
 	}
 
-	ret = request_threaded_irq(client->irq, NULL, cm36651_irq_handler,
+	ret = devm_request_threaded_irq(&client->dev, client->irq,
+					NULL, cm36651_irq_handler,
 					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
 							"cm36651", indio_dev);
 	if (ret) {
@@ -687,16 +688,14 @@ static int cm36651_probe(struct i2c_client *client,
 		goto error_i2c_unregister_ara;
 	}
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(&client->dev, indio_dev);
 	if (ret) {
 		dev_err(&client->dev, "%s: regist device failed\n", __func__);
-		goto error_free_irq;
+		goto error_i2c_unregister_ara;
 	}
 
 	return 0;
 
-error_free_irq:
-	free_irq(client->irq, indio_dev);
 error_i2c_unregister_ara:
 	i2c_unregister_device(cm36651->ara_client);
 error_i2c_unregister_ps:
@@ -711,9 +710,7 @@ static int cm36651_remove(struct i2c_client *client)
 	struct iio_dev *indio_dev = i2c_get_clientdata(client);
 	struct cm36651_data *cm36651 = iio_priv(indio_dev);
 
-	iio_device_unregister(indio_dev);
 	regulator_disable(cm36651->vled_reg);
-	free_irq(client->irq, indio_dev);
 	i2c_unregister_device(cm36651->ps_client);
 	i2c_unregister_device(cm36651->ara_client);
 
-- 
1.7.9.5

[PATCH 4/5] iio: light: gp2ap020a00f: Use devm_* APIs

[Beomho Seo]

This patch changes APIs from request_threaded_irq()
and iio_device_register() to devm_* APIs. Using them make simpler.
Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
---
 drivers/iio/light/gp2ap020a00f.c |   10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index 221ed16..1745c6a 100644
--- a/drivers/iio/light/gp2ap020a00f.c
+++ b/drivers/iio/light/gp2ap020a00f.c
@@ -1562,7 +1562,7 @@ static int gp2ap020a00f_probe(struct i2c_client *client,
 	}
 
 	/* This needs to be requested here for read_raw calls to work. */
-	err = request_threaded_irq(client->irq, NULL,
+	err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
 				   &gp2ap020a00f_thresh_event_handler,
 				   IRQF_TRIGGER_FALLING |
 				   IRQF_ONESHOT,
@@ -1581,10 +1581,10 @@ static int gp2ap020a00f_probe(struct i2c_client *client,
 	err = iio_trigger_register(data->trig);
 	if (err < 0) {
 		dev_err(&client->dev, "Failed to register iio trigger.\n");
-		goto error_free_irq;
+		goto iio_uninit_buffer;
 	}
 
-	err = iio_device_register(indio_dev);
+	err = devm_iio_device_register(&client->dev, indio_dev);
 	if (err < 0)
 		goto error_trigger_unregister;
 
@@ -1592,8 +1592,6 @@ static int gp2ap020a00f_probe(struct i2c_client *client,
 
 error_trigger_unregister:
 	iio_trigger_unregister(data->trig);
-error_free_irq:
-	free_irq(client->irq, indio_dev);
 error_uninit_buffer:
 	iio_triggered_buffer_cleanup(indio_dev);
 error_regulator_disable:
@@ -1613,9 +1611,7 @@ static int gp2ap020a00f_remove(struct i2c_client *client)
 	if (err < 0)
 		dev_err(&indio_dev->dev, "Failed to power off the device.\n");
 
-	iio_device_unregister(indio_dev);
 	iio_trigger_unregister(data->trig);
-	free_irq(client->irq, indio_dev);
 	iio_triggered_buffer_cleanup(indio_dev);
 	regulator_disable(data->vled_reg);
 
-- 
1.7.9.5

[PATCH 5/5] iio: magnetometer: ak8975: Use devm_* APIs

[Beomho Seo]

This patch use devm_* APIs and make driver simpler.

Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
---
 drivers/iio/magnetometer/ak8975.c |   48 ++++++++-----------------------------
 1 file changed, 10 insertions(+), 38 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 09ea5c4..ad71160 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -165,7 +165,7 @@ static int ak8975_setup_irq(struct ak8975_data *data)
 	else
 		irq = gpio_to_irq(data->eoc_gpio);
 
-	rc = request_irq(irq, ak8975_irq_handler,
+	rc = devm_request_irq(&client->dev, irq, ak8975_irq_handler,
 			 IRQF_TRIGGER_RISING | IRQF_ONESHOT,
 			 dev_name(&client->dev), data);
 	if (rc < 0) {
@@ -520,20 +520,21 @@ static int ak8975_probe(struct i2c_client *client,
 	/* We may not have a GPIO based IRQ to scan, that is fine, we will
 	   poll if so */
 	if (gpio_is_valid(eoc_gpio)) {
-		err = gpio_request_one(eoc_gpio, GPIOF_IN, "ak_8975");
+		err = devm_gpio_request_one(&client->dev, eoc_gpio,
+							GPIOF_IN, "ak_8975");
 		if (err < 0) {
 			dev_err(&client->dev,
 				"failed to request GPIO %d, error %d\n",
 							eoc_gpio, err);
-			goto exit;
+			return err;
 		}
 	}
 
 	/* Register with IIO */
-	indio_dev = iio_device_alloc(sizeof(*data));
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
 	if (indio_dev == NULL) {
 		err = -ENOMEM;
-		goto exit_gpio;
+		return err;
 	}
 	data = iio_priv(indio_dev);
 	i2c_set_clientdata(client, indio_dev);
@@ -551,7 +552,7 @@ static int ak8975_probe(struct i2c_client *client,
 		name = ak8975_match_acpi_device(&client->dev, &data->chipset);
 	else {
 		err = -ENOSYS;
-		goto exit_free_iio;
+		return err;
 	}
 	dev_dbg(&client->dev, "Asahi compass chip %s\n", name);
 
@@ -559,7 +560,7 @@ static int ak8975_probe(struct i2c_client *client,
 	err = ak8975_setup(client);
 	if (err < 0) {
 		dev_err(&client->dev, "AK8975 initialization fails\n");
-		goto exit_free_iio;
+		return err;
 	}
 
 	data->client = client;
@@ -571,37 +572,9 @@ static int ak8975_probe(struct i2c_client *client,
 	indio_dev->info = &ak8975_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->name = name;
-	err = iio_device_register(indio_dev);
+	err = devm_iio_device_register(&client->dev, indio_dev);
 	if (err < 0)
-		goto exit_free_iio;
-
-	return 0;
-
-exit_free_iio:
-	iio_device_free(indio_dev);
-	if (data->eoc_irq)
-		free_irq(data->eoc_irq, data);
-exit_gpio:
-	if (gpio_is_valid(eoc_gpio))
-		gpio_free(eoc_gpio);
-exit:
-	return err;
-}
-
-static int ak8975_remove(struct i2c_client *client)
-{
-	struct iio_dev *indio_dev = i2c_get_clientdata(client);
-	struct ak8975_data *data = iio_priv(indio_dev);
-
-	iio_device_unregister(indio_dev);
-
-	if (data->eoc_irq)
-		free_irq(data->eoc_irq, data);
-
-	if (gpio_is_valid(data->eoc_gpio))
-		gpio_free(data->eoc_gpio);
-
-	iio_device_free(indio_dev);
+		return err;
 
 	return 0;
 }
@@ -628,7 +601,6 @@ static struct i2c_driver ak8975_driver = {
 		.acpi_match_table = ACPI_PTR(ak_acpi_match),
 	},
 	.probe		= ak8975_probe,
-	.remove		= ak8975_remove,
 	.id_table	= ak8975_id,
 };
 module_i2c_driver(ak8975_driver);
-- 
1.7.9.5

Re: [PATCH 1/5] iio: adc: exynos_adc: Use devm_* APIs

[Sachin Kamat]

Hi Beomho,

On Wed, Jun 18, 2014 at 11:51 AM, Beomho Seo <beomho.seo@samsung.com> wrote:
> This patch changes APIs from request_irq() and iio_device_register()
> to devm_* APIs. Using them, make code simpler.
>
> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
> ---
>  drivers/iio/adc/exynos_adc.c |   11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> index d325aea..37b6eb5 100644
> --- a/drivers/iio/adc/exynos_adc.c
> +++ b/drivers/iio/adc/exynos_adc.c
> @@ -337,7 +337,7 @@ static int exynos_adc_probe(struct platform_device *pdev)
>         else
>                 indio_dev->num_channels = MAX_ADC_V2_CHANNELS;
>
> -       ret = request_irq(info->irq, exynos_adc_isr,
> +       ret = devm_request_irq(&pdev->dev, info->irq, exynos_adc_isr,
>                                         0, dev_name(&pdev->dev), info);
>         if (ret < 0) {
>                 dev_err(&pdev->dev, "failed requesting irq, irq = %d\n",
> @@ -345,9 +345,9 @@ static int exynos_adc_probe(struct platform_device *pdev)
>                 goto err_disable_clk;
>         }
>
> -       ret = iio_device_register(indio_dev);
> +       ret = devm_iio_device_register(&pdev->dev, indio_dev);
>         if (ret)
> -               goto err_irq;
> +               goto err_disable_clk;
>
>         exynos_adc_hw_init(info);
>
> @@ -362,9 +362,6 @@ static int exynos_adc_probe(struct platform_device *pdev)
>  err_of_populate:
>         device_for_each_child(&pdev->dev, NULL,
>                                 exynos_adc_remove_devices);
> -       iio_device_unregister(indio_dev);
> -err_irq:
> -       free_irq(info->irq, info);
>  err_disable_clk:
>         writel(0, info->enable_reg);
>         clk_disable_unprepare(info->clk);
> @@ -380,8 +377,6 @@ static int exynos_adc_remove(struct platform_device *pdev)
>
>         device_for_each_child(&pdev->dev, NULL,
>                                 exynos_adc_remove_devices);
> -       iio_device_unregister(indio_dev);
> -       free_irq(info->irq, info);
>         writel(0, info->enable_reg);
>         clk_disable_unprepare(info->clk);
>         regulator_disable(info->vdd);
> --
> 1.7.9.5
>

I am not sure if this change is safe as it changes the order of execution.
With devm_request_irq, irq is freed only after the function completes, i.e.,
the order will now be something like

writel(0, info->enable_reg)
clk_disable_unprepare(info->clk);
regulator_disable(info->vdd);
iio_device_unregister
free_irq

which is not what the original code did.

-- 
Regards,
Sachin.

Re: [PATCH 4/5] iio: light: gp2ap020a00f: Use devm_* APIs

[Sachin Kamat]

On Wed, Jun 18, 2014 at 11:51 AM, Beomho Seo <beomho.seo@samsung.com> wrote:
> This patch changes APIs from request_threaded_irq()
> and iio_device_register() to devm_* APIs. Using them make simpler.
> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
> ---
>  drivers/iio/light/gp2ap020a00f.c |   10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
> index 221ed16..1745c6a 100644
> --- a/drivers/iio/light/gp2ap020a00f.c
> +++ b/drivers/iio/light/gp2ap020a00f.c
> @@ -1562,7 +1562,7 @@ static int gp2ap020a00f_probe(struct i2c_client *client,
>         }
>
>         /* This needs to be requested here for read_raw calls to work. */
> -       err = request_threaded_irq(client->irq, NULL,
> +       err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
>                                    &gp2ap020a00f_thresh_event_handler,
>                                    IRQF_TRIGGER_FALLING |
>                                    IRQF_ONESHOT,
> @@ -1581,10 +1581,10 @@ static int gp2ap020a00f_probe(struct i2c_client *client,
>         err = iio_trigger_register(data->trig);
>         if (err < 0) {
>                 dev_err(&client->dev, "Failed to register iio trigger.\n");
> -               goto error_free_irq;
> +               goto iio_uninit_buffer;
>         }
>
> -       err = iio_device_register(indio_dev);
> +       err = devm_iio_device_register(&client->dev, indio_dev);
>         if (err < 0)
>                 goto error_trigger_unregister;
>
> @@ -1592,8 +1592,6 @@ static int gp2ap020a00f_probe(struct i2c_client *client,
>
>  error_trigger_unregister:
>         iio_trigger_unregister(data->trig);
> -error_free_irq:
> -       free_irq(client->irq, indio_dev);
>  error_uninit_buffer:
>         iio_triggered_buffer_cleanup(indio_dev);
>  error_regulator_disable:
> @@ -1613,9 +1611,7 @@ static int gp2ap020a00f_remove(struct i2c_client *client)
>         if (err < 0)
>                 dev_err(&indio_dev->dev, "Failed to power off the device.\n");
>
> -       iio_device_unregister(indio_dev);
>         iio_trigger_unregister(data->trig);
> -       free_irq(client->irq, indio_dev);
>         iio_triggered_buffer_cleanup(indio_dev);
>         regulator_disable(data->vled_reg);

Same comments as patch 1, ordering issue?

-- 
Regards,
Sachin.

Re: [PATCH 2/5] iio: light: cm32181: Use devm_* APIs

[Sachin Kamat]

Hi Beomho,

On Wed, Jun 18, 2014 at 11:51 AM, Beomho Seo <beomho.seo@samsung.com> wrote:
> This patch changes API from iio_device_register() to devm_* API.
> Using API make code simpler.
>
> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
> ---
>  drivers/iio/light/cm32181.c |   11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
> index d976e6c..ad36b29 100644
> --- a/drivers/iio/light/cm32181.c
> +++ b/drivers/iio/light/cm32181.c
> @@ -331,7 +331,7 @@ static int cm32181_probe(struct i2c_client *client,
>                 return ret;
>         }
>
> -       ret = iio_device_register(indio_dev);
> +       ret = devm_iio_device_register(&client->dev, indio_dev);
>         if (ret) {
>                 dev_err(&client->dev,
>                         "%s: regist device failed\n",
> @@ -342,14 +342,6 @@ static int cm32181_probe(struct i2c_client *client,
>         return 0;
>  }
>
> -static int cm32181_remove(struct i2c_client *client)
> -{
> -       struct iio_dev *indio_dev = i2c_get_clientdata(client);
> -
> -       iio_device_unregister(indio_dev);
> -       return 0;
> -}
> -
>  static const struct i2c_device_id cm32181_id[] = {
>         { "cm32181", 0 },
>         { }
> @@ -370,7 +362,6 @@ static struct i2c_driver cm32181_driver = {
>         },
>         .id_table       = cm32181_id,
>         .probe          = cm32181_probe,
> -       .remove         = cm32181_remove,
>  };
>
>  module_i2c_driver(cm32181_driver);
> --
> 1.7.9.5
>

Looks good.
Reviewed-by: Sachin Kamat <sachin.kamat@samsung.com>

-- 
Regards,
Sachin.

Re: [PATCH 3/5] iio: light: cm36651: Use devm_* APIs

[Srinivas Pandruvada]

On 06/17/2014 11:21 PM, Beomho Seo wrote:
> This patch changes APIs from request_threaded_irq()
> and iio_device_register() to devm_* APIS.
> Using them make code simpler.
>
> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
> ---
>   drivers/iio/light/cm36651.c |   11 ++++-------
>   1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/light/cm36651.c b/drivers/iio/light/cm36651.c
> index 39fc67e..2fd9579 100644
> --- a/drivers/iio/light/cm36651.c
> +++ b/drivers/iio/light/cm36651.c
> @@ -679,7 +679,8 @@ static int cm36651_probe(struct i2c_client *client,
>   		goto error_i2c_unregister_ara;
>   	}
>
> -	ret = request_threaded_irq(client->irq, NULL, cm36651_irq_handler,
> +	ret = devm_request_threaded_irq(&client->dev, client->irq,
> +					NULL, cm36651_irq_handler,
>   					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
>   							"cm36651", indio_dev);
>   	if (ret) {
> @@ -687,16 +688,14 @@ static int cm36651_probe(struct i2c_client *client,
>   		goto error_i2c_unregister_ara;
>   	}
>
> -	ret = iio_device_register(indio_dev);
> +	ret = devm_iio_device_register(&client->dev, indio_dev);
May not be correct. As this driver needs to disable some regulator in 
remove path, it is suggested not to use devm_xx here,

Thanks,
Srinivas

>   	if (ret) {
>   		dev_err(&client->dev, "%s: regist device failed\n", __func__);
> -		goto error_free_irq;
> +		goto error_i2c_unregister_ara;
>   	}
>
>   	return 0;
>
> -error_free_irq:
> -	free_irq(client->irq, indio_dev);
>   error_i2c_unregister_ara:
>   	i2c_unregister_device(cm36651->ara_client);
>   error_i2c_unregister_ps:
> @@ -711,9 +710,7 @@ static int cm36651_remove(struct i2c_client *client)
>   	struct iio_dev *indio_dev = i2c_get_clientdata(client);
>   	struct cm36651_data *cm36651 = iio_priv(indio_dev);
>
> -	iio_device_unregister(indio_dev);
>   	regulator_disable(cm36651->vled_reg);
> -	free_irq(client->irq, indio_dev);
>   	i2c_unregister_device(cm36651->ps_client);
>   	i2c_unregister_device(cm36651->ara_client);
>
>

Re: [PATCH 4/5] iio: light: gp2ap020a00f: Use devm_* APIs

[Srinivas Pandruvada]

On 06/17/2014 11:21 PM, Beomho Seo wrote:
> This patch changes APIs from request_threaded_irq()
> and iio_device_register() to devm_* APIs. Using them make simpler.
> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
> ---
>   drivers/iio/light/gp2ap020a00f.c |   10 +++-------
>   1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
> index 221ed16..1745c6a 100644
> --- a/drivers/iio/light/gp2ap020a00f.c
> +++ b/drivers/iio/light/gp2ap020a00f.c
> @@ -1562,7 +1562,7 @@ static int gp2ap020a00f_probe(struct i2c_client *client,
>   	}
>
>   	/* This needs to be requested here for read_raw calls to work. */
> -	err = request_threaded_irq(client->irq, NULL,
> +	err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
>   				   &gp2ap020a00f_thresh_event_handler,
>   				   IRQF_TRIGGER_FALLING |
>   				   IRQF_ONESHOT,
> @@ -1581,10 +1581,10 @@ static int gp2ap020a00f_probe(struct i2c_client *client,
>   	err = iio_trigger_register(data->trig);
>   	if (err < 0) {
>   		dev_err(&client->dev, "Failed to register iio trigger.\n");
> -		goto error_free_irq;
> +		goto iio_uninit_buffer;
>   	}
>
> -	err = iio_device_register(indio_dev);
> +	err = devm_iio_device_register(&client->dev, indio_dev);
Same here. driver remove needs to do some shutdown, so devm_xx is 
probably not a good idea.

Thanks,
Srinivas
>   	if (err < 0)
>   		goto error_trigger_unregister;
>
> @@ -1592,8 +1592,6 @@ static int gp2ap020a00f_probe(struct i2c_client *client,
>
>   error_trigger_unregister:
>   	iio_trigger_unregister(data->trig);
> -error_free_irq:
> -	free_irq(client->irq, indio_dev);
>   error_uninit_buffer:
>   	iio_triggered_buffer_cleanup(indio_dev);
>   error_regulator_disable:
> @@ -1613,9 +1611,7 @@ static int gp2ap020a00f_remove(struct i2c_client *client)
>   	if (err < 0)
>   		dev_err(&indio_dev->dev, "Failed to power off the device.\n");
>
> -	iio_device_unregister(indio_dev);
>   	iio_trigger_unregister(data->trig);
> -	free_irq(client->irq, indio_dev);
>   	iio_triggered_buffer_cleanup(indio_dev);
>   	regulator_disable(data->vled_reg);
>
>

Re: [PATCH 1/5] iio: adc: exynos_adc: Use devm_* APIs

[Srinivas Pandruvada]

On 06/17/2014 11:35 PM, Sachin Kamat wrote:
> Hi Beomho,
>
> On Wed, Jun 18, 2014 at 11:51 AM, Beomho Seo <beomho.seo@samsung.com> wrote:
>> This patch changes APIs from request_irq() and iio_device_register()
>> to devm_* APIs. Using them, make code simpler.
>>
>> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
>> ---
>>   drivers/iio/adc/exynos_adc.c |   11 +++--------
>>   1 file changed, 3 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
>> index d325aea..37b6eb5 100644
>> --- a/drivers/iio/adc/exynos_adc.c
>> +++ b/drivers/iio/adc/exynos_adc.c
>> @@ -337,7 +337,7 @@ static int exynos_adc_probe(struct platform_device *pdev)
>>          else
>>                  indio_dev->num_channels = MAX_ADC_V2_CHANNELS;
>>
>> -       ret = request_irq(info->irq, exynos_adc_isr,
>> +       ret = devm_request_irq(&pdev->dev, info->irq, exynos_adc_isr,
>>                                          0, dev_name(&pdev->dev), info);
>>          if (ret < 0) {
>>                  dev_err(&pdev->dev, "failed requesting irq, irq = %d\n",
>> @@ -345,9 +345,9 @@ static int exynos_adc_probe(struct platform_device *pdev)
>>                  goto err_disable_clk;
>>          }
>>
>> -       ret = iio_device_register(indio_dev);
>> +       ret = devm_iio_device_register(&pdev->dev, indio_dev);
Same here. The remove operation needs some clock / regulator disable.
So devm_xx may be not be correct based on Jonathan's recent comments.

Thanks,
Srinivas
>>          if (ret)
>> -               goto err_irq;
>> +               goto err_disable_clk;
>>
>>          exynos_adc_hw_init(info);
>>
>> @@ -362,9 +362,6 @@ static int exynos_adc_probe(struct platform_device *pdev)
>>   err_of_populate:
>>          device_for_each_child(&pdev->dev, NULL,
>>                                  exynos_adc_remove_devices);
>> -       iio_device_unregister(indio_dev);
>> -err_irq:
>> -       free_irq(info->irq, info);
>>   err_disable_clk:
>>          writel(0, info->enable_reg);
>>          clk_disable_unprepare(info->clk);
>> @@ -380,8 +377,6 @@ static int exynos_adc_remove(struct platform_device *pdev)
>>
>>          device_for_each_child(&pdev->dev, NULL,
>>                                  exynos_adc_remove_devices);
>> -       iio_device_unregister(indio_dev);
>> -       free_irq(info->irq, info);
>>          writel(0, info->enable_reg);
>>          clk_disable_unprepare(info->clk);
>>          regulator_disable(info->vdd);
>> --
>> 1.7.9.5
>>
>
> I am not sure if this change is safe as it changes the order of execution.
> With devm_request_irq, irq is freed only after the function completes, i.e.,
> the order will now be something like
>
> writel(0, info->enable_reg)
> clk_disable_unprepare(info->clk);
> regulator_disable(info->vdd);
> iio_device_unregister
> free_irq
>
> which is not what the original code did.
>

Re: [PATCH 1/5] iio: adc: exynos_adc: Use devm_* APIs

[Jonathan Cameron]

On 18/06/14 15:53, Srinivas Pandruvada wrote:
> On 06/17/2014 11:35 PM, Sachin Kamat wrote:
>> Hi Beomho,
>>
>> On Wed, Jun 18, 2014 at 11:51 AM, Beomho Seo <beomho.seo@samsung.com> wrote:
>>> This patch changes APIs from request_irq() and iio_device_register()
>>> to devm_* APIs. Using them, make code simpler.
>>>
>>> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
>>> ---
>>>   drivers/iio/adc/exynos_adc.c |   11 +++--------
>>>   1 file changed, 3 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
>>> index d325aea..37b6eb5 100644
>>> --- a/drivers/iio/adc/exynos_adc.c
>>> +++ b/drivers/iio/adc/exynos_adc.c
>>> @@ -337,7 +337,7 @@ static int exynos_adc_probe(struct platform_device *pdev)
>>>          else
>>>                  indio_dev->num_channels = MAX_ADC_V2_CHANNELS;
>>>
>>> -       ret = request_irq(info->irq, exynos_adc_isr,
>>> +       ret = devm_request_irq(&pdev->dev, info->irq, exynos_adc_isr,
>>>                                          0, dev_name(&pdev->dev), info);
>>>          if (ret < 0) {
>>>                  dev_err(&pdev->dev, "failed requesting irq, irq = %d\n",
>>> @@ -345,9 +345,9 @@ static int exynos_adc_probe(struct platform_device *pdev)
>>>                  goto err_disable_clk;
>>>          }
>>>
>>> -       ret = iio_device_register(indio_dev);
>>> +       ret = devm_iio_device_register(&pdev->dev, indio_dev);
> Same here. The remove operation needs some clock / regulator disable.
> So devm_xx may be not be correct based on Jonathan's recent comments.
devm will be fine for anything before non devm elements that need to be
reversed.  After that the ordering gets messed up in the driver tear down
as described.
>
> Thanks,
> Srinivas
>>>          if (ret)
>>> -               goto err_irq;
>>> +               goto err_disable_clk;
>>>
>>>          exynos_adc_hw_init(info);
>>>
>>> @@ -362,9 +362,6 @@ static int exynos_adc_probe(struct platform_device *pdev)
>>>   err_of_populate:
>>>          device_for_each_child(&pdev->dev, NULL,
>>>                                  exynos_adc_remove_devices);
>>> -       iio_device_unregister(indio_dev);
>>> -err_irq:
>>> -       free_irq(info->irq, info);
>>>   err_disable_clk:
>>>          writel(0, info->enable_reg);
>>>          clk_disable_unprepare(info->clk);
>>> @@ -380,8 +377,6 @@ static int exynos_adc_remove(struct platform_device *pdev)
>>>
>>>          device_for_each_child(&pdev->dev, NULL,
>>>                                  exynos_adc_remove_devices);
>>> -       iio_device_unregister(indio_dev);
>>> -       free_irq(info->irq, info);
>>>          writel(0, info->enable_reg);
>>>          clk_disable_unprepare(info->clk);
>>>          regulator_disable(info->vdd);
>>> --
>>> 1.7.9.5
>>>
>>
>> I am not sure if this change is safe as it changes the order of execution.
>> With devm_request_irq, irq is freed only after the function completes, i.e.,
>> the order will now be something like
>>
>> writel(0, info->enable_reg)
>> clk_disable_unprepare(info->clk);
>> regulator_disable(info->vdd);
>> iio_device_unregister
>> free_irq
>>
>> which is not what the original code did.
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] iio: magnetometer: ak8975: Use devm_* APIs

[Jonathan Cameron]

On 18/06/14 07:21, Beomho Seo wrote:
> This patch use devm_* APIs and make driver simpler.
>
> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
Hi,

Comments in line.

J
> ---
>   drivers/iio/magnetometer/ak8975.c |   48 ++++++++-----------------------------
>   1 file changed, 10 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index 09ea5c4..ad71160 100644
> --- a/drivers/iio/magnetometer/ak8975.c
> +++ b/drivers/iio/magnetometer/ak8975.c
> @@ -165,7 +165,7 @@ static int ak8975_setup_irq(struct ak8975_data *data)
>   	else
>   		irq = gpio_to_irq(data->eoc_gpio);
>
> -	rc = request_irq(irq, ak8975_irq_handler,
> +	rc = devm_request_irq(&client->dev, irq, ak8975_irq_handler,
>   			 IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>   			 dev_name(&client->dev), data);
>   	if (rc < 0) {
> @@ -520,20 +520,21 @@ static int ak8975_probe(struct i2c_client *client,
>   	/* We may not have a GPIO based IRQ to scan, that is fine, we will
>   	   poll if so */
>   	if (gpio_is_valid(eoc_gpio)) {
> -		err = gpio_request_one(eoc_gpio, GPIOF_IN, "ak_8975");
> +		err = devm_gpio_request_one(&client->dev, eoc_gpio,
> +							GPIOF_IN, "ak_8975");
>   		if (err < 0) {
>   			dev_err(&client->dev,
>   				"failed to request GPIO %d, error %d\n",
>   							eoc_gpio, err);
> -			goto exit;
> +			return err;
>   		}
>   	}
>
>   	/* Register with IIO */
> -	indio_dev = iio_device_alloc(sizeof(*data));
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>   	if (indio_dev == NULL) {
>   		err = -ENOMEM;
> -		goto exit_gpio;
> +		return err;
return -ENOMEM;
>   	}
>   	data = iio_priv(indio_dev);
>   	i2c_set_clientdata(client, indio_dev);
> @@ -551,7 +552,7 @@ static int ak8975_probe(struct i2c_client *client,
>   		name = ak8975_match_acpi_device(&client->dev, &data->chipset);
>   	else {
>   		err = -ENOSYS;
> -		goto exit_free_iio;
> +		return err;
If you are going to do this, please just return -ENOSYS directly rather than
leaving the intermediate step.
>   	}
>   	dev_dbg(&client->dev, "Asahi compass chip %s\n", name);
>
> @@ -559,7 +560,7 @@ static int ak8975_probe(struct i2c_client *client,
>   	err = ak8975_setup(client);
>   	if (err < 0) {
>   		dev_err(&client->dev, "AK8975 initialization fails\n");
> -		goto exit_free_iio;
> +		return err;
>   	}
>
>   	data->client = client;
> @@ -571,37 +572,9 @@ static int ak8975_probe(struct i2c_client *client,
>   	indio_dev->info = &ak8975_info;
>   	indio_dev->modes = INDIO_DIRECT_MODE;
>   	indio_dev->name = name;
> -	err = iio_device_register(indio_dev);
> +	err = devm_iio_device_register(&client->dev, indio_dev);
>   	if (err < 0)
> -		goto exit_free_iio;
> -
> -	return 0;
> -
> -exit_free_iio:
> -	iio_device_free(indio_dev);
> -	if (data->eoc_irq)
> -		free_irq(data->eoc_irq, data);
> -exit_gpio:
> -	if (gpio_is_valid(eoc_gpio))
> -		gpio_free(eoc_gpio);
> -exit:
> -	return err;
> -}
> -
> -static int ak8975_remove(struct i2c_client *client)
> -{
> -	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> -	struct ak8975_data *data = iio_priv(indio_dev);
> -
> -	iio_device_unregister(indio_dev);
> -
> -	if (data->eoc_irq)
> -		free_irq(data->eoc_irq, data);
> -
> -	if (gpio_is_valid(data->eoc_gpio))
> -		gpio_free(data->eoc_gpio);
> -
> -	iio_device_free(indio_dev);
> +		return err;
>
>   	return 0;
>   }
> @@ -628,7 +601,6 @@ static struct i2c_driver ak8975_driver = {
>   		.acpi_match_table = ACPI_PTR(ak_acpi_match),
>   	},
>   	.probe		= ak8975_probe,
> -	.remove		= ak8975_remove,
>   	.id_table	= ak8975_id,
>   };
>   module_i2c_driver(ak8975_driver);
>

Re: [PATCH 2/5] iio: light: cm32181: Use devm_* APIs

[Jonathan Cameron]

On 18/06/14 09:20, Sachin Kamat wrote:
> Hi Beomho,
>
> On Wed, Jun 18, 2014 at 11:51 AM, Beomho Seo <beomho.seo@samsung.com> wrote:
>> This patch changes API from iio_device_register() to devm_* API.
>> Using API make code simpler.
>>
>> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
Hmm. Fair enough I suppose.

Applied to the togreg branch of iio.git

Jonathan
>> ---
>>   drivers/iio/light/cm32181.c |   11 +----------
>>   1 file changed, 1 insertion(+), 10 deletions(-)
>>
>> diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
>> index d976e6c..ad36b29 100644
>> --- a/drivers/iio/light/cm32181.c
>> +++ b/drivers/iio/light/cm32181.c
>> @@ -331,7 +331,7 @@ static int cm32181_probe(struct i2c_client *client,
>>                  return ret;
>>          }
>>
>> -       ret = iio_device_register(indio_dev);
>> +       ret = devm_iio_device_register(&client->dev, indio_dev);
>>          if (ret) {
>>                  dev_err(&client->dev,
>>                          "%s: regist device failed\n",
>> @@ -342,14 +342,6 @@ static int cm32181_probe(struct i2c_client *client,
>>          return 0;
>>   }
>>
>> -static int cm32181_remove(struct i2c_client *client)
>> -{
>> -       struct iio_dev *indio_dev = i2c_get_clientdata(client);
>> -
>> -       iio_device_unregister(indio_dev);
>> -       return 0;
>> -}
>> -
>>   static const struct i2c_device_id cm32181_id[] = {
>>          { "cm32181", 0 },
>>          { }
>> @@ -370,7 +362,6 @@ static struct i2c_driver cm32181_driver = {
>>          },
>>          .id_table       = cm32181_id,
>>          .probe          = cm32181_probe,
>> -       .remove         = cm32181_remove,
>>   };
>>
>>   module_i2c_driver(cm32181_driver);
>> --
>> 1.7.9.5
>>
>
> Looks good.
> Reviewed-by: Sachin Kamat <sachin.kamat@samsung.com>
>