mpc5200b/uart: improve baud rate calculation (reach high baud rates, better accuracy)

On the MPC5200B, make very high baud rates (e.g. 3 MBaud) accessible and achieve a higher precision for high baud rates in general. This is done by selecting the appropriate prescaler (/4 or /32). As to keep the code clean= ,
the getuartclk method has been dropped, and all calculations are done with = a
"virtual" /4 prescaler for all chips for maximum accuracy. A new set_divis= or
method scales down the divisor values found by the generic serial code appropriately.

Note: only "fsl,mpc5200b-psc-uart" compatible devices benefit from these improvements.

Tested on a custom 5200B based board, with up to 3 MBaud, and with both "fsl,mpc5200b-psc-uart" and "fsl,mpc5200-psc-uart" devices.

---

Changes vs. v.1: include improvements suggested by Wolfram and Grant (thank= s a
lot for your helpful input!!): drop getuartclk method and use the highest possible frequency for calculation, use new psc_ops for the 5200b, let the set_divisor method do all the dirty work, emit warnings if bad divisor valu= es
have been selected.
=20

--- linux-2.6.33-orig/drivers/serial/mpc52xx_uart.c 2010-02-24 19:52:17.000=
000000 +0100
+++ linux-2.6.33/drivers/serial/mpc52xx_uart.c 2010-03-03 10:52:04.00000000=
0 +0100
@@ -144,7 +144,8 @@ struct psc_ops {
unsigned char (*read_char)(struct uart_port *port);
void (*cw_disable_ints)(struct uart_port *port);
void (*cw_restore_ints)(struct uart_port *port);
- unsigned long (*getuartclk)(void *p);
+ void (*set_divisor)(struct uart_port *port,
+ unsigned int divisor);
};
=20
#ifdef CONFIG_PPC_MPC52xx
@@ -154,9 +155,6 @@ static void mpc52xx_psc_fifo_init(struct
struct mpc52xx_psc __iomem *psc =3D PSC(port);
struct mpc52xx_psc_fifo __iomem *fifo =3D FIFO_52xx(port);
=20
- /* /32 prescaler */
- out_be16(&psc->mpc52xx_psc_clock_select, 0xdd00);
-
out_8(&fifo->rfcntl, 0x00);
out_be16(&fifo->rfalarm, 0x1ff);
out_8(&fifo->tfcntl, 0x07);
@@ -245,15 +243,55 @@ static void mpc52xx_psc_cw_restore_ints(
out_be16(&PSC(port)->mpc52xx_psc_imr, port->read_status_mask);
}
=20
-/* Search for bus-frequency property in this node or a parent */ -static unsigned long mpc52xx_getuartclk(void *p)
+static void mpc52xx_psc_set_divisor(struct uart_port *port,
+ unsigned int divisor)
{
- /*
- * 5200 UARTs have a / 32 prescaler
- * but the generic serial code assumes 16
- * so return ipb freq / 2
- */
- return mpc5xxx_get_bus_frequency(p) / 2;
+ struct mpc52xx_psc __iomem *psc =3D PSC(port);
+
+ /* adjust divisor for a /32 prescaler; see note in
+ * mpc52xx_uart_of_probe() */
+ divisor =3D (divisor + 4) / 8;
+ if (divisor > 0xffff) {
+ pr_warning("%s: divisor overflow (%x), use 0xffff\n", __func__,
+ divisor);
+ divisor =3D 0xffff;
+ } else if (divisor =3D=3D 0) {
+ pr_warning("%s: divisor 0, use 1\n", __func__);
+ divisor =3D 1;
+ }
+
+ /* prescaler */
+ out_be16(&psc->mpc52xx_psc_clock_select, 0xdd00); /* /32 */
+
+ /* ctr */
+ out_8(&psc->ctur, divisor >> 8);
+ out_8(&psc->ctlr, divisor & 0xff);
+}
+
+static void mpc5200b_psc_set_divisor(struct uart_port *port,
+ unsigned int divisor)
+{
+ struct mpc52xx_psc __iomem *psc =3D PSC(port);
+
+ /* set prescaler; see note in mpc52xx_uart_of_probe() */
+ if (divisor > 0xffff) {
+ divisor =3D (divisor + 4) / 8;
+ out_be16(&psc->mpc52xx_psc_clock_select, 0xdd00); /* /32 */
+ } else
+ out_be16(&psc->mpc52xx_psc_clock_select, 0xff00); /* /4 */
+
+ if (divisor > 0xffff) {
+ pr_warning("%s: divisor overflow (%x), use 0xffff\n", __func__,
+ divisor);
+ divisor =3D 0xffff;
+ } else if (divisor =3D=3D 0) {
+ pr_warning("%s: divisor 0, use 1\n", __func__);
+ divisor =3D 1;
+ }
+
+ /* ctr */
+ out_8(&psc->ctur, divisor >> 8);
+ out_8(&psc->ctlr, divisor & 0xff);
}
=20
static struct psc_ops mpc52xx_psc_ops =3D {
@@ -272,7 +310,26 @@ static struct psc_ops mpc52xx_psc_ops =3D=20
.read_char =3D mpc52xx_psc_read_char,
.cw_disable_ints =3D mpc52xx_psc_cw_disable_ints, .cw_restore_ints =3D mpc52xx_psc_cw_restore_ints,
- .getuartclk =3D mpc52xx_getuartclk,
+ .set_divisor =3D mpc52xx_psc_set_divisor,
+};
+
+static struct psc_ops mpc5200b_psc_ops =3D {
+ .fifo_init =3D mpc52xx_psc_fifo_init,
+ .raw_rx_rdy =3D mpc52xx_psc_raw_rx_rdy,
+ .raw_tx_rdy =3D mpc52xx_psc_raw_tx_rdy,
+ .rx_rdy =3D mpc52xx_psc_rx_rdy,
+ .tx_rdy =3D mpc52xx_psc_tx_rdy,
+ .tx_empty =3D mpc52xx_psc_tx_empty,
+ .stop_rx =3D mpc52xx_psc_stop_rx,
+ .start_tx =3D mpc52xx_psc_start_tx,
+ .stop_tx =3D mpc52xx_psc_stop_tx,
+ .rx_clr_irq =3D mpc52xx_psc_rx_clr_irq,
+ .tx_clr_irq =3D mpc52xx_psc_tx_clr_irq,
+ .write_char =3D mpc52xx_psc_write_char,
+ .read_char =3D mpc52xx_psc_read_char,
+ .cw_disable_ints =3D mpc52xx_psc_cw_disable_ints,
+ .cw_restore_ints =3D mpc52xx_psc_cw_restore_ints,
+ .set_divisor =3D mpc5200b_psc_set_divisor,
};
=20
#endif /* CONFIG_MPC52xx */
@@ -388,9 +445,25 @@ static void mpc512x_psc_cw_restore_ints(
out_be32(&FIFO_512x(port)->rximr, port->read_status_mask & 0x7f);
}
=20
-static unsigned long mpc512x_getuartclk(void *p)
+static void mpc512x_psc_set_divisor(struct uart_port *port,
+ unsigned int divisor)
{
- return mpc5xxx_get_bus_frequency(p);
+ struct mpc52xx_psc __iomem *psc =3D PSC(port);
+
+ /* adjust divisor for a /16 prescaler; see note in
+ * mpc52xx_uart_of_probe() */
+ divisor =3D (divisor + 2) / 4;
+ if (divisor > 0xffff) {
+ pr_warning("%s: divisor overflow (%x), use 0xffff\n", __func__,
+ divisor);
+ divisor =3D 0xffff;
+ } else if (divisor =3D=3D 0) {
+ pr_warning("%s: divisor 0, use 1\n", __func__);
+ divisor =3D 1;
+ }
+
+ out_8(&psc->ctur, divisor >> 8);
+ out_8(&psc->ctlr, divisor & 0xff);
}
=20
static struct psc_ops mpc512x_psc_ops =3D {
@@ -409,7 +482,7 @@ static struct psc_ops mpc512x_psc_ops =3D=20
.read_char =3D mpc512x_psc_read_char,
.cw_disable_ints =3D mpc512x_psc_cw_disable_ints, .cw_restore_ints =3D mpc512x_psc_cw_restore_ints,
- .getuartclk =3D mpc512x_getuartclk,
+ .set_divisor =3D mpc512x_psc_set_divisor,
};
#endif
=20
@@ -564,7 +637,6 @@ mpc52xx_uart_set_termios(struct uart_por
struct mpc52xx_psc __iomem *psc =3D PSC(port);
unsigned long flags;
unsigned char mr1, mr2;
- unsigned short ctr;
unsigned int j, baud, quot;
=20
/* Prepare what we're gonna write */
@@ -604,7 +676,6 @@ mpc52xx_uart_set_termios(struct uart_por
=20
baud =3D uart_get_baud_rate(port, new, old, 0, port->uartclk/16); quot =3D uart_get_divisor(port, baud);
- ctr =3D quot & 0xffff;
=20
/* Get the lock */
spin_lock_irqsave(&port->lock, flags);
@@ -635,8 +706,7 @@ mpc52xx_uart_set_termios(struct uart_por
out_8(&psc->command, MPC52xx_PSC_SEL_MODE_REG_1); out_8(&psc->mode, mr1);
out_8(&psc->mode, mr2);
- out_8(&psc->ctur, ctr >> 8);
- out_8(&psc->ctlr, ctr & 0xff);
+ psc_ops->set_divisor(port, quot);
=20
if (UART_ENABLE_MS(port, new->c_cflag))
mpc52xx_uart_enable_ms(port);
@@ -1007,7 +1077,8 @@ mpc52xx_console_setup(struct console *co
return ret;
}
=20
- uartclk =3D psc_ops->getuartclk(np);
+ /* see remarks about the uart clock in mpc52xx_uart_of_probe() */
+ uartclk =3D mpc5xxx_get_bus_frequency(np) * 4;
if (uartclk =3D=3D 0) {
pr_debug("Could not find uart clock frequency!\n"); return -EINVAL;
@@ -1090,6 +1161,7 @@ static struct uart_driver mpc52xx_uart_d
=20
static struct of_device_id mpc52xx_uart_of_match[] =3D {
#ifdef CONFIG_PPC_MPC52xx
+ { .compatible =3D "fsl,mpc5200b-psc-uart", .data =3D &mpc5200b_psc_ops, }=
,
{ .compatible =3D "fsl,mpc5200-psc-uart", .data =3D &mpc52xx_psc_ops, }, /* binding used by old lite5200 device trees: */
{ .compatible =3D "mpc5200-psc-uart", .data =3D &mpc52xx_psc_ops, },
@@ -1122,7 +1194,24 @@ mpc52xx_uart_of_probe(struct of_device *
pr_debug("Found %s assigned to ttyPSC%x\n",
mpc52xx_uart_nodes[idx]->full_name, idx);
=20
- uartclk =3D psc_ops->getuartclk(op->node);
+ /*
+ * Note about the uart clock:
+ * This series of processors use the ipb clock frequency for the clock
+ * generation scaled down by prescalers and a 16-bit counter register:
+ * - the 5200 has a /32 prescaler
+ * - the 5200B has selectable /4 or /32 prescalers (i.e. the counter

+	 *   reg can be viewed as a 19-bit value, of which we can use either
+	 *   the upper or the lower 16 bits - in the latter case the three
+	 *   MSB's must of course be 0)

+ * - the 512x has a /16 prescaler
+ * The generic serial code assumes a prescaler of /16. As we want to
+ * achieve the maximum accuracy possible, we let the generic serial
+ * code perform all calculations with the /4 prescaler, i.e. we have
+ * to set the uart clock to ipb freq * 4 here. The set_divisor methods
+ * for the different chips are responsible for scaling down the divisor
+ * value appropriately.
+ */
+ uartclk =3D mpc5xxx_get_bus_frequency(op->node) * 4;
if (uartclk =3D=3D 0) {
dev_dbg(&op->dev, "Could not find uart clock frequency!\n"); return -EINVAL;

Hi Albrecht,

I like this version much better. Comments below...

e by

lean,

h a

ivisor

nks a

e

lues

:52:17.000000000 +0100

000000 +0100

0x7f);

0xffff\n", __func__,

__);

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).

lk/16);

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.

ncy!\n");

00b_psc_ops, },

xx_psc_ops, },

sc_ops, },

ck frequency!\n");

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Hi Grant:

Thanks a lot for your input!

[snip]

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

[snip]

tclk/16);

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

[snip]

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

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

On Thu, Mar 4, 2010 at 2:56 AM, Albrecht Dre=DF <[email protected]> w= rote:

? =A0It uses mpc5xxx_get_bus_frequency() as input for the baud rate calcula= tion, and if the serial code assumes /16 instead of /10, the result must be=
terribly off. =A0Or did I miss something here?

If you are, then I'm missing the same thing. Do you best to keep the 5121 calculation work out to the same value it uses now. We'll ask someone with a 5121 to test it out before I add the patch to my -next branch.

g.