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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx> 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@xxxxxxxxxxx>
---
  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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux