Re: [PATCH 04/15] i2c: busses: exynos5: Use devm_clk_get_enabled()

Andi Shyti
2023-06-23T12:31:01Z

Hi Krzysztof,

On Fri, Jun 23, 2023 at 02:18:32PM +0200, Krzysztof Kozlowski wrote:
> On 23/06/2023 13:48, Andi Shyti wrote:
> > On Fri, Jun 23, 2023 at 12:03:56PM +0200, Wolfram Sang wrote:
> >>
> >>> - clk_unprepare(i2c->clk);
> >>> - clk_unprepare(i2c->pclk);
> >>
> >> Are you sure we can use devm_clk_get_enabled() here which calls
> >> clk_disable_unprepare() on remove and not clk_unprepare()?
> >
> > Unless I am missing something, I think so. This is what the
> > driver does, gets the clock and enables it.
> >
> > I don't know why there is just unprepare() and not
> > disable_unprepare() in this function.
>
> Your code is not equivalent and does not explain why. Pure
> devm_clk_get_enabled() conversion should be equivalent.
>
> If original code was correct, your patch will cause double clk disable.
> If original code was not correct, your patch silently fixes it without
> explaining that there was a bug.
>
> Did you test the patch, including the unbind path?

you are right! My code, indeed, doesn't do a 1to1 conversion.

Will prepare it again, maybe in two patches, one for the
conversion and one for clearing the removal path.

Thanks for dropping in,
Andi