Re: [Patch] mpc5200b: improve baud rate calculation (reach high baud rates, better accuracy)

["Albrecht Dreß" <albrecht.dress@arcor.de>]

Hi Wolfram!

Thanks a lot for your comments!

[snip]
> > + * as the chip can be only either a 5200B or not. */
> > +static int is_mpc5200b =3D -1;
> > +
> > +
>=20
> One empty line too much. Maybe we can also get rid of the static later in
> the
> process, but first things first.

Ooops....

[snip]
> > +=09if (is_mpc5200b =3D=3D 1)
> > +=09=09return mpc5xxx_get_bus_frequency(p) * 4;
> > +=09else
> > +=09=09return mpc5xxx_get_bus_frequency(p) / 2;
>=20
> 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, w=
here I can choose to use either the lower or the higher 16 bits for the cou=
nter reg.  Remember also that using the higher 16 bits (/32 prescaler) is p=
robably the exceptional case - with an IPB frequency of 132 MHz this will h=
appen only for standard baud rates B300 and slower.

[snip]
> > +=09/* Check only once if we are running on a mpc5200b or not */
> > +=09if (is_mpc5200b =3D=3D -1) {
> > +=09=09struct device_node *np;
> > +
> > +=09=09np =3D of_find_compatible_node(NULL, NULL, "fsl,mpc5200b-immr");
>=20
> 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 intrusiv=
e.  CC'ing the device tree discussion list here... comments, please!!

> > +=09=09if (np) {
> > +=09=09=09is_mpc5200b =3D 1;
> > +=09=09=09dev_dbg(&op->dev, "mpc5200b: using /4 prescaler\n");
>=20
> Does this message respect the fallback case?

See comment above...

> 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', whe=
re only two functions differ from the generic 52xx struct as it is implemen=
ted now.  Using the static int needs less space.  However, in combination w=
ith the new compatible entry, it would of course make sense.

Again, any insight from the device tree gurus would be appreciated!

Thanks, Albrecht.

Tolle Dekollet=E9s oder scharfe Tatoos? Vote jetzt ... oder mach selbst mit=
 und zeige Deine Schokoladenseite
bei Topp oder Hopp von Arcor: http://www.arcor.de/rd/footer.toh