[Patch] mpc5200b: improve baud rate calculation (reach high baud rates, better accuracy)
Wolfram Sang
w.sang at pengutronix.de
Tue Mar 2 19:28:58 EST 2010
> [snip]
> > > + if (is_mpc5200b == 1)
> > > + return mpc5xxx_get_bus_frequency(p) * 4;
> > > + else
> > > + return mpc5xxx_get_bus_frequency(p) / 2;
> >
> > Isn't this wrong? You can also have /32 on the 5200B (the fallback).
>
> Yes, but I do all /calculations/ with the /4 prescaler for higher accuracy.
> If the divisor exceeds the available 16 bits of the counter reg, I round
> (divisor / 8) to use the /32 prescaler. Think of a 19-bit counter value,
> where I can choose to use either the lower or the higher 16 bits for the
> counter reg.
Okay, now I got it. (Maybe this is an indication for another comment above the
set divisor function?)
> Remember also that using the higher 16 bits (/32 prescaler) is
> probably the exceptional case - with an IPB frequency of 132 MHz this will
> happen only for standard baud rates B300 and slower.
Even the rare cases have to be correct ;)
> [snip]
> > > + /* Check only once if we are running on a mpc5200b or not */
> > > + if (is_mpc5200b == -1) {
> > > + struct device_node *np;
> > > +
> > > + np = of_find_compatible_node(NULL, NULL, "fsl,mpc5200b-immr");
> >
> > This should be handled using a new compatible-entry
> > "fsl,mpc5200b-psc-uart".
>
> I agree that this would be a lot cleaner, but it's also a lot more intrusive.
> CC'ing the device tree discussion list here... comments, please!!
Why intrusive? Maybe I miss something?
> > You could also have a set_divisor-function for 5200 and 5200B and set it
> > here in the function struct (one reason less for the static ;))
>
> Hmmm, but then I would need a 'static struct psc_ops mpc5200b_psc_ops', where
> only two functions differ from the generic 52xx struct as it is implemented
> now. Using the static int needs less space. However, in combination with
> the new compatible entry, it would of course make sense.
Leave those two function pointers empty and fill them during probe (probe has
access to the compatible-property it was matched against, see its arguments).
So it should be a matter of:
if (matched_property == 5200b)
ops->func = this_one;
else
ops->func = that_one;
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20100302/43c507a8/attachment.pgp>
More information about the Linuxppc-dev
mailing list