Re: [PATCHv2] staging: iio: replace clk_get() with devm_clk_get()

From: Jonathan Cameron
Date: Sun Jan 24 2016 - 11:05:33 EST


On 21/01/16 09:55, Gujulan Elango, Hari Prasath (H.) wrote:
> From: Hari Prasath Gujulan Elango <hgujulan@xxxxxxxxxxx>
>
> This patch replaces the clk_get() with devm_clk_get().
> Accordingly,modified the error paths and removed clk_put() as well.
>
> Signed-off-by: Hari Prasath Gujulan Elango <hgujulan@xxxxxxxxxxx>
What about the clk_put in the remove function?
Drop that one as well as the managed handling will deal with that
anyway.

Also, I'd be happier if you reorganised the probe to do the clk stuff
before the of_iomap. That way it makes it readily apparent that we
haven't introduced a different in probe / remove (reversed) operations.

I'd largely forgotten about this driver after doing a cleanup set
back in 2014. Hartmut, you raised a few points back then about
some disagreements between the driver and the datasheet, otherwise
I'd probably have just shifted it out of staging back then.

Stefan has been quiet on this one for a while, I don't suppose anyone
actually has one of these to test? I was always nervous about making
the other changes Hartmut suggested without someone testing them.

Hari, are you doing this as generic cleanup (always good to have!) or
because you are working with the hardware and hence looking at the code?

Jonathan


> ---
> v2: email address was missing in the From field.
> ---
> drivers/staging/iio/adc/spear_adc.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/spear_adc.c b/drivers/staging/iio/adc/spear_adc.c
> index 712cae0..4ac0f54 100644
> --- a/drivers/staging/iio/adc/spear_adc.c
> +++ b/drivers/staging/iio/adc/spear_adc.c
> @@ -288,7 +288,7 @@ static int spear_adc_probe(struct platform_device *pdev)
> st->adc_base_spear3xx =
> (struct adc_regs_spear3xx __iomem *)st->adc_base_spear6xx;
>
> - st->clk = clk_get(dev, NULL);
> + st->clk = devm_clk_get(dev, NULL);
> if (IS_ERR(st->clk)) {
> dev_err(dev, "failed getting clock\n");
> goto errout1;
> @@ -297,7 +297,7 @@ static int spear_adc_probe(struct platform_device *pdev)
> ret = clk_prepare_enable(st->clk);
> if (ret) {
> dev_err(dev, "failed enabling clock\n");
> - goto errout2;
> + goto errout1;
> }
>
> irq = platform_get_irq(pdev, 0);
> @@ -356,8 +356,6 @@ static int spear_adc_probe(struct platform_device *pdev)
>
> errout3:
> clk_disable_unprepare(st->clk);
> -errout2:
> - clk_put(st->clk);
> errout1:
> iounmap(st->adc_base_spear6xx);
> return ret;
>