| Message ID | 1304603588-3178-1-git-send-email-javier.martin@vista-silicon.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
Hi Javier, On Thursday 05 May 2011 15:53:08 Javier Martin wrote: > Do not use omap3isp_get() when what we really want to do is just > enable clocks, since omap3isp_get() has additional, unwanted, side > effects as an increase of the counter. > > This prevented omap3isp of working with Beagleboard xM and it has > been tested only with that platform + mt9p031 sensor. > > Signed-off-by: Javier Martin <javier.martin@vista-silicon.com> > --- > drivers/media/video/omap3isp/isp.c | 8 +++++--- > 1 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/video/omap3isp/isp.c > b/drivers/media/video/omap3isp/isp.c index 472a693..ca0831f 100644 > --- a/drivers/media/video/omap3isp/isp.c > +++ b/drivers/media/video/omap3isp/isp.c > @@ -85,9 +85,11 @@ module_param(autoidle, int, 0444); > MODULE_PARM_DESC(autoidle, "Enable OMAP3ISP AUTOIDLE support"); > > static void isp_save_ctx(struct isp_device *isp); > - > static void isp_restore_ctx(struct isp_device *isp); > > +static int isp_enable_clocks(struct isp_device *isp); > +static void isp_disable_clocks(struct isp_device *isp); > + > static const struct isp_res_mapping isp_res_maps[] = { > { > .isp_rev = ISP_REVISION_2_0, > @@ -239,10 +241,10 @@ 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); This won't work. Let's assume the following sequence of events: - Userspace opens the sensor subdev device node - The sensor driver calls to board code to turn the sensor clock on - Board code calls to the ISP driver to turn XCLK on - The ISP driver calls isp_enable_clocks() ... - Userspace opens an ISP video device node - The ISP driver calls isp_get(), incrementing the reference count - Userspace closes the ISP video device node - The ISP driver calls isp_put(), decrementing the reference count - The reference count reaches 0, the ISP driver calls isp_disable_clocks() The sensor will then loose its clock, even though the sensor subdev device node is still opened. Could you please explain why the existing code doesn't work on your hardware ? > /* Stopping the clock. */ > else if (divisor < 2 && isp->xclk_divisor[xclksel - 1] >= 2) > - omap3isp_put(isp); > + isp_disable_clocks(isp); > > isp->xclk_divisor[xclksel - 1] = divisor;
diff --git a/drivers/media/video/omap3isp/isp.c b/drivers/media/video/omap3isp/isp.c index 472a693..ca0831f 100644 --- a/drivers/media/video/omap3isp/isp.c +++ b/drivers/media/video/omap3isp/isp.c @@ -85,9 +85,11 @@ module_param(autoidle, int, 0444); MODULE_PARM_DESC(autoidle, "Enable OMAP3ISP AUTOIDLE support"); static void isp_save_ctx(struct isp_device *isp); - static void isp_restore_ctx(struct isp_device *isp); +static int isp_enable_clocks(struct isp_device *isp); +static void isp_disable_clocks(struct isp_device *isp); + static const struct isp_res_mapping isp_res_maps[] = { { .isp_rev = ISP_REVISION_2_0, @@ -239,10 +241,10 @@ 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); + isp_disable_clocks(isp); isp->xclk_divisor[xclksel - 1] = divisor;
Do not use omap3isp_get() when what we really want to do is just enable clocks, since omap3isp_get() has additional, unwanted, side effects as an increase of the counter. This prevented omap3isp of working with Beagleboard xM and it has been tested only with that platform + mt9p031 sensor. Signed-off-by: Javier Martin <javier.martin@vista-silicon.com> --- drivers/media/video/omap3isp/isp.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)