| Message ID | BANLkTinRqcFj5doua4r6d-vwPAym=JGvDw@mail.gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
On Thu, 5 May 2011, javier Martin wrote: > Hi, > as you know I'm currently working on submitting mt9p031 driver to > mainline, testing it with my Beagleboard xM. > While I was trying to clean Guennadi's patches I ran into the attached > patch which changes a call to "omap3isp_get(isp);" into > "isp_enable_clocks(isp);". > > I don't think this is clean since it would unbalance the number of > omap3isp_get() vs omap3isp_put() and we probably don't want that. > What seems clear is if we don't apply this patch the clock is not > actually enabled. > > According to my debugging results "isp_disable_clocks()" is never > called, so, after the first call to "isp_enable_clocks()" there > shouldn't be any need to enable the clocks again. > > Guennadi, do you know what is the cause of the problem? I don't remember exactly, but it didn't work without this patch. I know it is not clean and shouldn't be needed, so, if now it works also without it - perfect! You can start, stop, and restart streaming without this patch and it all works? Then certainly it should be dropped. 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Javier, On Thursday 05 May 2011 12:17:12 Guennadi Liakhovetski wrote: > On Thu, 5 May 2011, javier Martin wrote: > > Hi, > > as you know I'm currently working on submitting mt9p031 driver to > > mainline, testing it with my Beagleboard xM. > > While I was trying to clean Guennadi's patches I ran into the attached > > patch which changes a call to "omap3isp_get(isp);" into > > "isp_enable_clocks(isp);". > > > > I don't think this is clean since it would unbalance the number of > > omap3isp_get() vs omap3isp_put() and we probably don't want that. > > What seems clear is if we don't apply this patch the clock is not > > actually enabled. > > > > According to my debugging results "isp_disable_clocks()" is never > > called, so, after the first call to "isp_enable_clocks()" there > > shouldn't be any need to enable the clocks again. > > > > Guennadi, do you know what is the cause of the problem? > > I don't remember exactly, but it didn't work without this patch. I know it > is not clean and shouldn't be needed, so, if now it works also without it > - perfect! You can start, stop, and restart streaming without this patch > and it all works? Then certainly it should be dropped. And otherwise you can work on a fix ;-) I unfortunately have no sensor module for the Beagleboard xM so I can't really test this.
Thank you two guys for your answer. > I don't remember exactly, but it didn't work without this patch. I know it > is not clean and shouldn't be needed, so, if now it works also without it > - perfect! You can start, stop, and restart streaming without this patch > and it all works? Then certainly it should be dropped. No, sorry, what I meant is although, according to my debugging results the patch shouldn't be needed, it still does not work without it. I'll try to track down the issue and I'll work on a fix myself.
diff --git a/drivers/media/video/omap3isp/isp.c b/drivers/media/video/omap3isp/isp.c index 472a693..6a6ea86 100644 --- a/drivers/media/video/omap3isp/isp.c +++ b/drivers/media/video/omap3isp/isp.c @@ -177,6 +177,8 @@ static void isp_disable_interrupts(struct isp_device *isp) isp_reg_writel(isp, 0, OMAP3_ISP_IOMEM_MAIN, ISP_IRQ0ENABLE); } +static int isp_enable_clocks(struct isp_device *isp); + /** * isp_set_xclk - Configures the specified cam_xclk to the desired frequency. * @isp: OMAP3 ISP device @@ -239,7 +241,7 @@ static u32 isp_set_xclk(struct isp_device *isp, u32 xclk, u8 xclksel) /* Do we go from stable whatever to clock? */ if (divisor >= 2 && isp->xclk_divisor[xclksel - 1] < 2) - omap3isp_get(isp); + isp_enable_clocks(isp); /* Stopping the clock. */ else if (divisor < 2 && isp->xclk_divisor[xclksel - 1] >= 2) omap3isp_put(isp);