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.

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

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

>       }
>  
>       /* setup fifo */
> @@ -594,13 +629,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