On Mon, Jun 20, 2022 at 02:42:52PM +0000, Visa Hankala wrote:
> On Sun, Jun 19, 2022 at 03:06:47PM +0200, Anton Lindqvist wrote:
> > This allows the pluart baud rate to be changed. There's one potential
> > pitfall with this change as users will have the wrong baud rate in their
> > /etc/ttys if not installed after revision 1.11 of dev/ic/pluart.c which
> > landed today. This will make the serial console unusable until the
> > expected baud rate in /etc/ttys is changed to 115200.
> 
> An upgrade note would be good.

I can prepare something for current.html.

> > Comments? OK?
> > 
> > diff --git sys/dev/fdt/pluart_fdt.c sys/dev/fdt/pluart_fdt.c
> > index 969018eccdc..ac2467bdf47 100644
> > --- sys/dev/fdt/pluart_fdt.c
> > +++ sys/dev/fdt/pluart_fdt.c
> > @@ -27,6 +27,7 @@
> >  
> >  #include <dev/ofw/fdt.h>
> >  #include <dev/ofw/openfirm.h>
> > +#include <dev/ofw/ofw_clock.h>
> >  #include <dev/ofw/ofw_pinctrl.h>
> >  
> >  int        pluart_fdt_match(struct device *, void *, void *);
> > @@ -70,8 +71,12 @@ pluart_fdt_attach(struct device *parent, struct device 
> > *self, void *aux)
> >             return;
> >     }
> >  
> > -   if (OF_is_compatible(faa->fa_node, "arm,sbsa-uart"))
> > +   if (OF_is_compatible(faa->fa_node, "arm,sbsa-uart")) {
> >             sc->sc_hwflags |= COM_HW_SBSA;
> > +   } else {
> > +           clock_enable_all(faa->fa_node);
> > +           sc->sc_clkfreq = clock_get_frequency(faa->fa_node, "uartclk");
> > +   }
> >  
> >     periphid = OF_getpropint(faa->fa_node, "arm,primecell-periphid", 0);
> >     if (periphid != 0)
> > diff --git sys/dev/ic/pluart.c sys/dev/ic/pluart.c
> > index 40e2b1976fb..aa4301e8fb0 100644
> > --- sys/dev/ic/pluart.c
> > +++ sys/dev/ic/pluart.c
> > @@ -71,9 +71,9 @@
> >  #define UART_ILPR          0x20            /* IrDA low-power counter 
> > register */
> >  #define UART_ILPR_ILPDVSR  ((x) & 0xf)     /* IrDA low-power divisor */
> >  #define UART_IBRD          0x24            /* Integer baud rate register */
> > -#define UART_IBRD_DIVINT   ((x) & 0xff)    /* Integer baud rate divisor */
> > +#define UART_IBRD_DIVINT(x)        ((x) & 0xff)    /* Integer baud rate 
> > divisor */
> 
> This mask should be 0xffff.

Thanks, fixed.

> >  #define UART_FBRD          0x28            /* Fractional baud rate 
> > register */
> > -#define UART_FBRD_DIVFRAC  ((x) & 0x3f)    /* Fractional baud rate divisor 
> > */
> > +#define UART_FBRD_DIVFRAC(x)       ((x) & 0x3f)    /* Fractional baud rate 
> > divisor */
> >  #define UART_LCR_H         0x2c            /* Line control register */
> >  #define UART_LCR_H_BRK             (1 << 0)        /* Send break */
> >  #define UART_LCR_H_PEN             (1 << 1)        /* Parity enable */
> > @@ -338,7 +338,9 @@ pluart_param(struct tty *tp, struct termios *t)
> >             /* lower dtr */
> >     }
> >  
> > -   if (ospeed != 0) {
> > +   if (ospeed != 0 && sc->sc_clkfreq != 0 && tp->t_ospeed != ospeed) {
> > +           int div, lcr;
> > +
> >             while (ISSET(tp->t_state, TS_BUSY)) {
> >                     ++sc->sc_halt;
> >                     error = ttysleep(tp, &tp->t_outq,
> > @@ -349,7 +351,40 @@ pluart_param(struct tty *tp, struct termios *t)
> >                             return (error);
> >                     }
> >             }
> > -           /* set speed */
> > +
> > +           /*
> > +            * Writes to IBRD and FBRD are made effective first when LCR_H
> > +            * is written.
> > +            */
> > +           lcr = bus_space_read_4(sc->sc_iot, sc->sc_ioh, UART_LCR_H);
> > +
> > +           /* The UART must be disabled while changing the baud rate. */
> > +           bus_space_write_4(sc->sc_iot, sc->sc_ioh, UART_CR, 0);
> 
> I think this should save original CR for restoring later, and set CR
> with UARTEN masked out.
> 
>               cr = bus_space_read_4(sc->sc_iot, sc->sc_ioh, UART_CR);
>               bus_space_write_4(sc->sc_iot, sc->sc_ioh, UART_CR,
>                   cr & ~UART_CR_UARTEN);
> 
> The PL011 manual says that reserved bits in CR should not be modified.
> 
> > +
> > +           /*
> > +            * The baud rate divisor is expressed relative to the UART clock
> > +            * frequency where IBRD represents the quotient using 16 bits
> > +            * and FBRD the remainder using 6 bits. The PL011 specification
> > +            * provides the following formula:
> > +            *
> > +            *      uartclk/(16 * baudrate)
> > +            *
> > +            * The formula can be estimated by scaling it with the
> > +            * precision 64 (2^6) and letting the resulting upper 16 bits
> > +            * represents the quotient and the lower 6 bits the remainder:
> > +            *
> > +            *      64 * uartclk/(16 * baudrate) = 4 * uartclk/baudrate
> > +            */
> > +           div = 4 * sc->sc_clkfreq / ospeed;
> > +           bus_space_write_4(sc->sc_iot, sc->sc_ioh, UART_IBRD,
> > +               UART_IBRD_DIVINT(div >> 6));
> > +           bus_space_write_4(sc->sc_iot, sc->sc_ioh, UART_FBRD,
> > +               UART_FBRD_DIVFRAC(div));
> > +           /* Commit baud rate change. */
> > +           bus_space_write_4(sc->sc_iot, sc->sc_ioh, UART_LCR_H, lcr);
> > +           /* Enable UART. */
> > +           bus_space_write_4(sc->sc_iot, sc->sc_ioh,
> > +               UART_CR, UART_CR_UARTEN | UART_CR_TXE | UART_CR_RXE);
> 
> Restore CR here.

Also addressed in the revised diff below.

diff --git sys/dev/fdt/pluart_fdt.c sys/dev/fdt/pluart_fdt.c
index 969018eccdc..ac2467bdf47 100644
--- sys/dev/fdt/pluart_fdt.c
+++ sys/dev/fdt/pluart_fdt.c
@@ -27,6 +27,7 @@
 
 #include <dev/ofw/fdt.h>
 #include <dev/ofw/openfirm.h>
+#include <dev/ofw/ofw_clock.h>
 #include <dev/ofw/ofw_pinctrl.h>
 
 int    pluart_fdt_match(struct device *, void *, void *);
@@ -70,8 +71,12 @@ pluart_fdt_attach(struct device *parent, struct device 
*self, void *aux)
                return;
        }
 
-       if (OF_is_compatible(faa->fa_node, "arm,sbsa-uart"))
+       if (OF_is_compatible(faa->fa_node, "arm,sbsa-uart")) {
                sc->sc_hwflags |= COM_HW_SBSA;
+       } else {
+               clock_enable_all(faa->fa_node);
+               sc->sc_clkfreq = clock_get_frequency(faa->fa_node, "uartclk");
+       }
 
        periphid = OF_getpropint(faa->fa_node, "arm,primecell-periphid", 0);
        if (periphid != 0)
diff --git sys/dev/ic/pluart.c sys/dev/ic/pluart.c
index 40e2b1976fb..aba78c550ea 100644
--- sys/dev/ic/pluart.c
+++ sys/dev/ic/pluart.c
@@ -71,9 +71,9 @@
 #define UART_ILPR              0x20            /* IrDA low-power counter 
register */
 #define UART_ILPR_ILPDVSR      ((x) & 0xf)     /* IrDA low-power divisor */
 #define UART_IBRD              0x24            /* Integer baud rate register */
-#define UART_IBRD_DIVINT       ((x) & 0xff)    /* Integer baud rate divisor */
+#define UART_IBRD_DIVINT(x)    ((x) & 0xffff)  /* Integer baud rate divisor */
 #define UART_FBRD              0x28            /* Fractional baud rate 
register */
-#define UART_FBRD_DIVFRAC      ((x) & 0x3f)    /* Fractional baud rate divisor 
*/
+#define UART_FBRD_DIVFRAC(x)   ((x) & 0x3f)    /* Fractional baud rate divisor 
*/
 #define UART_LCR_H             0x2c            /* Line control register */
 #define UART_LCR_H_BRK         (1 << 0)        /* Send break */
 #define UART_LCR_H_PEN         (1 << 1)        /* Parity enable */
@@ -338,7 +338,9 @@ pluart_param(struct tty *tp, struct termios *t)
                /* lower dtr */
        }
 
-       if (ospeed != 0) {
+       if (sc->sc_clkfreq != 0 && ospeed != 0 && ospeed != tp->t_ospeed) {
+               int cr, div, lcr;
+
                while (ISSET(tp->t_state, TS_BUSY)) {
                        ++sc->sc_halt;
                        error = ttysleep(tp, &tp->t_outq,
@@ -349,7 +351,41 @@ pluart_param(struct tty *tp, struct termios *t)
                                return (error);
                        }
                }
-               /* set speed */
+
+               /*
+                * Writes to IBRD and FBRD are made effective first when LCR_H
+                * is written.
+                */
+               lcr = bus_space_read_4(sc->sc_iot, sc->sc_ioh, UART_LCR_H);
+
+               /* The UART must be disabled while changing the baud rate. */
+               cr = bus_space_read_4(sc->sc_iot, sc->sc_ioh, UART_CR);
+               bus_space_write_4(sc->sc_iot, sc->sc_ioh, UART_CR,
+                   cr & ~UART_CR_UARTEN);
+
+               /*
+                * The baud rate divisor is expressed relative to the UART clock
+                * frequency where IBRD represents the quotient using 16 bits
+                * and FBRD the remainder using 6 bits. The PL011 specification
+                * provides the following formula:
+                *
+                *      uartclk/(16 * baudrate)
+                *
+                * The formula can be estimated by scaling it with the
+                * precision 64 (2^6) and letting the resulting upper 16 bits
+                * represents the quotient and the lower 6 bits the remainder:
+                *
+                *      64 * uartclk/(16 * baudrate) = 4 * uartclk/baudrate
+                */
+               div = 4 * sc->sc_clkfreq / ospeed;
+               bus_space_write_4(sc->sc_iot, sc->sc_ioh, UART_IBRD,
+                   UART_IBRD_DIVINT(div >> 6));
+               bus_space_write_4(sc->sc_iot, sc->sc_ioh, UART_FBRD,
+                   UART_FBRD_DIVFRAC(div));
+               /* Commit baud rate change. */
+               bus_space_write_4(sc->sc_iot, sc->sc_ioh, UART_LCR_H, lcr);
+               /* Enable UART. */
+               bus_space_write_4(sc->sc_iot, sc->sc_ioh, UART_CR, cr);
        }
 
        /* setup fifo */
@@ -594,13 +630,6 @@ pluartopen(dev_t dev, int flag, int mode, struct proc *p)
                    5 << IMXUART_FCR_RFDIV_SH |
                    1 << IMXUART_FCR_RXTL_SH);
 
-               bus_space_write_4(iot, ioh, IMXUART_UBIR,
-                   (pluartdefaultrate / 100) - 1);
-
-               /* formula: clk / (rfdiv * 1600) */
-               bus_space_write_4(iot, ioh, IMXUART_UBMR,
-                   (clk_get_rate(sc->sc_clk) * 1000) / 1600);
-
                SET(sc->sc_ucr1, IMXUART_CR1_EN|IMXUART_CR1_RRDYEN);
                SET(sc->sc_ucr2, IMXUART_CR2_TXEN|IMXUART_CR2_RXEN);
                bus_space_write_4(iot, ioh, IMXUART_UCR1, sc->sc_ucr1);
diff --git sys/dev/ic/pluartvar.h sys/dev/ic/pluartvar.h
index 492c2a60b2c..47c64b9cc88 100644
--- sys/dev/ic/pluartvar.h
+++ sys/dev/ic/pluartvar.h
@@ -48,6 +48,7 @@ struct pluart_softc {
 #define COM_SW_PPS      0x10
        int             sc_fifolen;
        int             sc_imsc;
+       int             sc_clkfreq;
 
        u_int8_t        sc_initialize;
        u_int8_t        sc_cua;

Reply via email to