On Tue, 27 Nov 2012, Libin Yang wrote:
> Hello Guennadi,
>
> Please see my comments below.
>
> Best Regards,
> Libin
>
> >-----Original Message-----
> >From: Albert Wang
> >Sent: Tuesday, November 27, 2012 7:21 PM
> >To: Guennadi Liakhovetski
> >Cc: corbet@xxxxxxx; linux-media@xxxxxxxxxxxxxxx; Libin Yang
> >Subject: RE: [PATCH 02/15] [media] marvell-ccic: add MIPI support for marvell-ccic driver
> >
> >Hi, Guennadi
> >
> >We will update the patch by following your good suggestion! :)
> >
>
> [snip]
>
> >>> + pll1 = clk_get(dev, "pll1");
> >>> + if (IS_ERR(pll1)) {
> >>> + dev_err(dev, "Could not get pll1 clock\n");
> >>> + return;
> >>> + }
> >>> +
> >>> + tx_clk_esc = clk_get_rate(pll1) / 1000000 / 12;
> >>> + clk_put(pll1);
> >>
> >>Once you release your clock per "clk_put()" its rate can be changed by some other user,
> >>so, your tx_clk_esc becomes useless. Better keep the reference to the clock until clean up.
> >>Maybe you can also use
> >>devm_clk_get() to simplify the clean up.
> >>
> >That's a good suggestion.
> >
> [Libin] In our code design, the pll1 will never be changed after the system boots up. Camera and other components can only get the clk without modifying it.
This doesn't matter. We have a standard API and we have to abide to its
rules. Your driver can be reused or its code can be copied by others. I
don't think it should be too difficult to just issue devm_clk_get() once
and then just forget about it.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
