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: cor...@lwn.net; linux-media@vger.kernel.org; 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
