Re: pluart(4): change baud rate

2022-06-21 Thread Visa Hankala
On Mon, Jun 20, 2022 at 07:07:14PM +0200, Anton Lindqvist wrote:
> On Mon, Jun 20, 2022 at 02:42:52PM +, 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 
> > >  #include 
> > > +#include 
> > >  #include 
> > >  
> > >  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_ILPR0x20/* IrDA low-power 
> > > counter register */
> > >  #define UART_ILPR_ILPDVSR((x) & 0xf) /* IrDA low-power 
> > > divisor */
> > >  #define UART_IBRD0x24/* 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 0x.
> 
> Thanks, fixed.
> 
> > >  #define UART_FBRD0x28/* 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, >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
> > > +  */
> > > +  

Re: pluart(4): change baud rate

2022-06-20 Thread Anton Lindqvist
On Mon, Jun 20, 2022 at 02:42:52PM +, 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 
> >  #include 
> > +#include 
> >  #include 
> >  
> >  intpluart_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 0x.

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

Re: pluart(4): change baud rate

2022-06-20 Thread Visa Hankala
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 
>  #include 
> +#include 
>  #include 
>  
>  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_ILPR0x20/* IrDA low-power counter 
> register */
>  #define UART_ILPR_ILPDVSR((x) & 0xf) /* IrDA low-power divisor */
>  #define UART_IBRD0x24/* 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 0x.

>  #define UART_FBRD0x28/* 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, >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,
> +   

pluart(4): change baud rate

2022-06-19 Thread Anton Lindqvist
Hi,
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.

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 
 #include 
+#include 
 #include 
 
 intpluart_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 */
 #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, >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);
+
+   /*
+* 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);
}
 
/* 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);
-