On Mon, Jun 20, 2022 at 07:07:14PM +0200, Anton Lindqvist wrote:
> 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.
OK visa@
