Hi Sascha,
Thanks for your comments.
On Mon, May 17, 2010 at 09:27:20AM +0200, Sascha Hauer wrote:
> On Wed, May 12, 2010 at 09:02:29PM +0200, Guennadi Liakhovetski wrote:
> > Hi Baruch
> >
> > Thanks for eventually mainlining this driver! A couple of comments below.
> > Sascha, would be great, if you could get it tested on imx27 with and
> > without emma.
>
> I will see what I can do. Testing and probably breathing life into a
> camera driver usually takes me two days given that the platform support
> is very outdated. I hope our customer is interested in this, then it
> would be possible to test it.
>
> > BTW, if you say, that you use emma to avoid using the
> > standard DMA controller, why would anyone want not to use emma? Resource
> > conflict? There is also a question for you down in the comments, please,
> > skim over.
>
> I originally did not know how all the components should work together.
> Now I think it's the right way to use the EMMA to be able to scale
> images and to do colour conversions (which does not work with our Bayer
> format cameras, so I cannot test it).
So can I remove the non EMMA code from this driver? This will simplify the
code quite a bit.
[snip]
> > > +static int mclk_get_divisor(struct mx2_camera_dev *pcdev)
> > > +{
> > > + dev_info(pcdev->dev, "%s not implemented. Running at max speed\n",
> > > + __func__);
> >
> > Hm, why is this unimplemented?
> >
> > > +
> > > +#if 0
> > > + unsigned int mclk = pcdev->pdata->clk_csi;
> > > + unsigned int pclk = clk_get_rate(pcdev->clk_csi);
> > > + int i;
> > > +
> > > + dev_dbg(pcdev->dev, "%s: %ld %ld\n", __func__, mclk, pclk);
> > > +
> > > + for (i = 0; i < 0xf; i++)
> > > + if ((i + 1) * 2 * mclk <= pclk)
> > > + break;
> >
> > This doesn't look right. You increment the counter i, and terminate the
> > loop as soon as "(i + 1) * 2 * mclk <= pclk". Obviously, if 2 * mclk <= pclk,
> > this will terminate immediately, otherwise it will run until the end and
> > return 0xf without satisfying the condition. What exactly are you trying to
> > achieve? Find the _largest_ i, such that "(i + 1) * 2 * mclk <= pclk"? Then
> > why not just do "i = pclk / 2 / mclk - 1"?
> >
> > > + return i;
> > > +#endif
> > > + return 0;
> > > +}
Can you shed some light on this?
Thanks,
baruch
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@xxxxxxxxxx - tel: +972.2.679.5364, http://www.tkos.co.il -
--
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
