On Thu, Dec 15, 2011 at 12:11:13AM +0200, Sakari Ailus wrote:
> Hi Martin,
>
> On Wed, Dec 14, 2011 at 07:58:45PM +0100, martin@xxxxxxxxxxxxxxxxxxxxxx wrote:
> ...
> > > > > > +static int mt9m032_setup_pll(struct mt9m032 *sensor)
> > > > > > +{
> > > > > > + struct mt9m032_platform_data* pdata = sensor->pdata;
> > > > > > + u16 reg_pll1;
> > > > > > + unsigned int pre_div;
> > > > > > + int res, ret;
> > > > > > +
> > > > > > + /* TODO: also support other pre-div values */
> > >
> > > I might already have mentioned this, but wouldn't it be time to work a on real
> > > PLL setup code that compute the pre-divisor, multiplier and output divisor
> > > dynamically from the input and output clock frequencies ?
> >
> > I'm not sure what the implications for quality and stability of such a
> > generic setup would be. My gut feeling is most users go with known working
> > hardcoded values.
>
> You'd get a lot better control of the sensor as a bonus in doing so. Also,
> you could program the sensor properly suitable for the host it is connected
> to, achieving optimal maximum frame rates for it.
>
> These values tend to be relatively board / bridge dependent. On one board
> some frequencies might not be usable even if they do not exceed the maximum
> for the bridge.
Yes, that's why i have exported almost all of the pll details i'm reasonanly sure
that they are settable in the platform data. I think this gives maximum
flexibility in the board configuration. Maybe something with less values to fiddle
with in the normal case would be better. But not really by a large amount.
static struct mt9m032_platform_data mt9m032_platform_data = {
.ext_clock = 13500000,
.pll_pre_div = 6,
.pll_mul = 120,
.pll_out_div = 5,
.invert_pixclock = 1,
};
I think what Laurent was talking about was moving to a setup where the board
doesn't need to specify the exact details. i.e. have pll_pre_div, pll_mul and
pll_out_div rolled into one. I'm not sure that's something that should be done.
And this driver does feel like it had it's share of tracking in development
interfaces.
>
> Please also see this:
>
> <URL:http://www.mail-archive.com/linux-media@xxxxxxxxxxxxxxx/msg39798.html>
Sounds sensible for the future. But let's not hold this driver until agreement is
reached about the details?
- Martin
--
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
