All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Albrecht Dreß" <albrecht.dress@arcor.de>
To: grant.likely@secretlab.ca, albrecht.dress@arcor.de
Cc: linuxppc-dev@ozlabs.org
Subject: Re: [Patch v.2] mpc5200b/uart: improve baud rate calculation (reach high baud rates, better accuracy)
Date: Thu, 4 Mar 2010 10:56:46 +0100 (CET)	[thread overview]
Message-ID: <16827431.1267696606015.JavaMail.ngmail@webmail13.arcor-online.net> (raw)
In-Reply-To: <fa686aa41003031307j79e004cfk49297e419a65f5da@mail.gmail.com>

Hi Grant:

Thanks a lot for your input!

[snip]
> Save yourself some duplicated code here.  The above 14 lines can be
> shared between the 512x, 52xx and 5200b versions.  Create yourself an
> internal __mpc5xxx_psc_set_divisor() function that is passed the *psc,
> the divisor, and the clock select register setting (both the 5200 and
> the 5121 have the clock select register).

Hmm, yes, that's true.  Will look into that.

[snip]
> > @@ -604,7 +676,6 @@ mpc52xx_uart_set_termios(struct uart_por
> >
> > =A0 =A0 =A0 =A0baud =3D uart_get_baud_rate(port, new, old, 0, port->uar=
tclk/16);
>=20
> I'm probably nitpicking, because I don't know if the io pin will
> handle this speed but uartclk/16 is no longer the maximum baudrate if
> a /4 prescaler is used.

Yes, you are right. Must of course be fixed.

[snip]
> > @@ -635,8 +706,7 @@ mpc52xx_uart_set_termios(struct uart_por
> > =A0 =A0 =A0 =A0out_8(&psc->command, MPC52xx_PSC_SEL_MODE_REG_1);
> > =A0 =A0 =A0 =A0out_8(&psc->mode, mr1);
> > =A0 =A0 =A0 =A0out_8(&psc->mode, mr2);
> > - =A0 =A0 =A0 out_8(&psc->ctur, ctr >> 8);
> > - =A0 =A0 =A0 out_8(&psc->ctlr, ctr & 0xff);
> > + =A0 =A0 =A0 psc_ops->set_divisor(port, quot);
>=20
> Hmmm.  The divisor calculations have some tricky bits to them.  I
> would consider changing the set_divisor() function to accept a baud
> rate, and modify the set_divisor function to call uart_get_divisor().

That sounds like a good idea to me.  I will change the code that way.

> That way each set_divisor() can do whatever makes the most sense for
> the divisors available to it.  The 5121 for example has both a /10 and
> a /32 divisor, plus it can use an external clock.

Ouch.  I don't have a 512x, but isn't the current code plain wrong then?  I=
t uses mpc5xxx_get_bus_frequency() as input for the baud rate calculation, =
and if the serial code assumes /16 instead of /10, the result must be terri=
bly off.  Or did I miss something here?

Best, 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

  reply	other threads:[~2010-03-04  9:56 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-03 18:23 [Patch v.2] mpc5200b/uart: improve baud rate calculation (reach high baud rates, better accuracy) Albrecht Dreß
2010-03-03 21:07 ` Grant Likely
2010-03-04  9:56   ` Albrecht Dreß [this message]
2010-03-04 13:27     ` Grant Likely

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=16827431.1267696606015.JavaMail.ngmail@webmail13.arcor-online.net \
    --to=albrecht.dress@arcor.de \
    --cc=grant.likely@secretlab.ca \
    --cc=linuxppc-dev@ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.