omap3isp clock problems on Beagleboard xM.

  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?
  

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.