Hi Guennadi, >-----Original Message----- >From: Guennadi Liakhovetski [mailto:g.liakhovetski@xxxxxx] >Sent: Wednesday, November 28, 2012 3:14 PM >To: Libin Yang >Cc: Albert Wang; corbet@xxxxxxx; linux-media@xxxxxxxxxxxxxxx >Subject: RE: [PATCH 02/15] [media] marvell-ccic: add MIPI support for marvell-ccic driver > >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. [Libin] Yes, you are right. We should consider the driver may be reused. I didn't realize it. Another question is: If we use devm_clk_get(), what I understand, the clk will be put when the device is being released. It means the driver will hold the clk all the time the driver is in the kernel. What do you think if we get the clk when opening the camera, and put it in the close? > >Thanks >Guennadi >--- >Guennadi Liakhovetski, Ph.D. >Freelance Open-Source Software Developer >http://www.open-technology.de/ Best Regard, Libin -- 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
