[PATCH] arm: amlogic: add setbrg op to serial device

Implement setbrg in amlogic/meson serial device with driver model similar to how the meson_uart.c driver does it in Linux. Also configure (probe) the serial device with the new reg5 register.
Signed-off-by: Edoardo Tomelleri e.tomell@gmail.com ---
drivers/serial/serial_meson.c | 70 +++++++++++++++++++++++++++++++++++ include/configs/meson64.h | 7 ++++ 2 files changed, 77 insertions(+)
diff --git a/drivers/serial/serial_meson.c b/drivers/serial/serial_meson.c index d69ec221..90c370cf 100644 --- a/drivers/serial/serial_meson.c +++ b/drivers/serial/serial_meson.c @@ -7,9 +7,11 @@ #include <dm.h> #include <errno.h> #include <fdtdec.h> +#include <linux/kernel.h> #include <linux/bitops.h> #include <linux/compiler.h> #include <serial.h> +#include <clk.h>
struct meson_uart { u32 wfifo; @@ -17,6 +19,7 @@ struct meson_uart { u32 control; u32 status; u32 misc; + u32 reg5; /* New baud control register */ };
struct meson_serial_plat { @@ -42,6 +45,35 @@ struct meson_serial_plat { #define AML_UART_RX_RST BIT(23) #define AML_UART_CLR_ERR BIT(24)
+/* AML_UART_REG5 bits */ +#define AML_UART_REG5_XTAL_DIV2 BIT(27) +#define AML_UART_REG5_XTAL_CLK_SEL BIT(26) /* default 0 (div by 3), 1 for no div */ +#define AML_UART_REG5_USE_XTAL_CLK BIT(24) /* default 1 (use crystal as clock source) */ +#define AML_UART_REG5_USE_NEW_BAUD BIT(23) /* default 1 (use new baud rate register) */ +#define AML_UART_REG5_BAUD_MASK 0x7fffff + +static u32 meson_calc_baud_divisor(ulong src_rate, u32 baud) +{ + /* + * Usually src_rate is 24 MHz (from crystal) as clock source for serial + * device. Since 8 Mb/s is the maximum supported baud rate, use div by 3 + * to derive baud rate. This choice is used also in meson_serial_setbrg. + */ + return DIV_ROUND_CLOSEST(src_rate / 3, baud) - 1; +} + +static void meson_serial_set_baud(struct meson_uart *uart, ulong src_rate, u32 baud) +{ + /* + * Set crystal divided by 3 (regardless of device tree clock property) + * as clock source and the corresponding divisor to approximate baud + */ + u32 divisor = meson_calc_baud_divisor(src_rate, baud); + u32 val = AML_UART_REG5_USE_XTAL_CLK | AML_UART_REG5_USE_NEW_BAUD | + (divisor & AML_UART_REG5_BAUD_MASK); + writel(val, &uart->reg5); +} + static void meson_serial_init(struct meson_uart *uart) { u32 val; @@ -59,7 +91,14 @@ static int meson_serial_probe(struct udevice *dev) { struct meson_serial_plat *plat = dev_get_plat(dev); struct meson_uart *const uart = plat->reg; + struct clk per_clk; + int ret = clk_get_by_name(dev, "baud", &per_clk); + + if (ret) + return ret; + ulong rate = clk_get_rate(&per_clk);
+ meson_serial_set_baud(uart, rate, CONFIG_BAUDRATE); meson_serial_init(uart);
return 0; @@ -111,6 +150,36 @@ static int meson_serial_putc(struct udevice *dev, const char ch) return 0; }
+static int meson_serial_setbrg(struct udevice *dev, const int baud) +{ + /* + * Change device baud rate if baud is reasonable (considering a 23 bit + * counter with an 8 MHz clock input) and the actual baud + * rate is within 2% of the requested value (2% is arbitrary). + */ + if (baud < 1 || baud > 8000000) + return -EINVAL; + + struct meson_serial_plat *const plat = dev_get_plat(dev); + struct meson_uart *const uart = plat->reg; + struct clk per_clk; + int ret = clk_get_by_name(dev, "baud", &per_clk); + + if (ret) + return ret; + ulong rate = clk_get_rate(&per_clk); + u32 divisor = meson_calc_baud_divisor(rate, baud); + u32 calc_baud = (rate / 3) / (divisor + 1); + u32 calc_err = baud > calc_baud ? baud - calc_baud : calc_baud - baud; + + if (((calc_err * 100) / baud) > 2) + return -EINVAL; + + meson_serial_set_baud(uart, rate, baud); + + return 0; +} + static int meson_serial_pending(struct udevice *dev, bool input) { struct meson_serial_plat *plat = dev_get_plat(dev); @@ -154,6 +223,7 @@ static const struct dm_serial_ops meson_serial_ops = { .putc = meson_serial_putc, .pending = meson_serial_pending, .getc = meson_serial_getc, + .setbrg = meson_serial_setbrg, };
static const struct udevice_id meson_serial_ids[] = { diff --git a/include/configs/meson64.h b/include/configs/meson64.h index 196e58ed..01413a02 100644 --- a/include/configs/meson64.h +++ b/include/configs/meson64.h @@ -16,6 +16,13 @@ #define GICC_BASE 0xc4302000 #endif
+/* Serial drivers */ +/* The following table includes the supported baudrates */ +#define CONFIG_SYS_BAUDRATE_TABLE \ + {300, 600, 1200, 2400, 4800, 9600, 19200, 38400, 57600, 115200, \ + 230400, 250000, 460800, 500000, 1000000, 2000000, 4000000, \ + 8000000 } + /* For splashscreen */ #ifdef CONFIG_DM_VIDEO #define STDOUT_CFG "vidconsole,serial"

Hi,
Sorry for the delay...
On 18/09/2022 18:17, Edoardo Tomelleri wrote:
Implement setbrg in amlogic/meson serial device with driver model similar to how the meson_uart.c driver does it in Linux. Also configure (probe) the serial device with the new reg5 register.
Signed-off-by: Edoardo Tomelleri e.tomell@gmail.com
drivers/serial/serial_meson.c | 70 +++++++++++++++++++++++++++++++++++ include/configs/meson64.h | 7 ++++ 2 files changed, 77 insertions(+)
diff --git a/drivers/serial/serial_meson.c b/drivers/serial/serial_meson.c index d69ec221..90c370cf 100644 --- a/drivers/serial/serial_meson.c +++ b/drivers/serial/serial_meson.c @@ -7,9 +7,11 @@ #include <dm.h> #include <errno.h> #include <fdtdec.h> +#include <linux/kernel.h> #include <linux/bitops.h> #include <linux/compiler.h> #include <serial.h> +#include <clk.h>
struct meson_uart { u32 wfifo; @@ -17,6 +19,7 @@ struct meson_uart { u32 control; u32 status; u32 misc;
u32 reg5; /* New baud control register */ };
struct meson_serial_plat {
@@ -42,6 +45,35 @@ struct meson_serial_plat { #define AML_UART_RX_RST BIT(23) #define AML_UART_CLR_ERR BIT(24)
+/* AML_UART_REG5 bits */ +#define AML_UART_REG5_XTAL_DIV2 BIT(27) +#define AML_UART_REG5_XTAL_CLK_SEL BIT(26) /* default 0 (div by 3), 1 for no div */ +#define AML_UART_REG5_USE_XTAL_CLK BIT(24) /* default 1 (use crystal as clock source) */ +#define AML_UART_REG5_USE_NEW_BAUD BIT(23) /* default 1 (use new baud rate register) */ +#define AML_UART_REG5_BAUD_MASK 0x7fffff
+static u32 meson_calc_baud_divisor(ulong src_rate, u32 baud) +{
- /*
* Usually src_rate is 24 MHz (from crystal) as clock source for serial
* device. Since 8 Mb/s is the maximum supported baud rate, use div by 3
* to derive baud rate. This choice is used also in meson_serial_setbrg.
*/
- return DIV_ROUND_CLOSEST(src_rate / 3, baud) - 1;
+}
+static void meson_serial_set_baud(struct meson_uart *uart, ulong src_rate, u32 baud) +{
- /*
* Set crystal divided by 3 (regardless of device tree clock property)
* as clock source and the corresponding divisor to approximate baud
*/
- u32 divisor = meson_calc_baud_divisor(src_rate, baud);
- u32 val = AML_UART_REG5_USE_XTAL_CLK | AML_UART_REG5_USE_NEW_BAUD |
(divisor & AML_UART_REG5_BAUD_MASK);
- writel(val, &uart->reg5);
+}
- static void meson_serial_init(struct meson_uart *uart) { u32 val;
@@ -59,7 +91,14 @@ static int meson_serial_probe(struct udevice *dev) { struct meson_serial_plat *plat = dev_get_plat(dev); struct meson_uart *const uart = plat->reg;
struct clk per_clk;
int ret = clk_get_by_name(dev, "baud", &per_clk);
if (ret)
return ret;
ulong rate = clk_get_rate(&per_clk);
meson_serial_set_baud(uart, rate, CONFIG_BAUDRATE); meson_serial_init(uart);
return 0;
@@ -111,6 +150,36 @@ static int meson_serial_putc(struct udevice *dev, const char ch) return 0; }
+static int meson_serial_setbrg(struct udevice *dev, const int baud) +{
- /*
* Change device baud rate if baud is reasonable (considering a 23 bit
* counter with an 8 MHz clock input) and the actual baud
* rate is within 2% of the requested value (2% is arbitrary).
*/
- if (baud < 1 || baud > 8000000)
return -EINVAL;
- struct meson_serial_plat *const plat = dev_get_plat(dev);
- struct meson_uart *const uart = plat->reg;
- struct clk per_clk;
- int ret = clk_get_by_name(dev, "baud", &per_clk);
- if (ret)
return ret;
- ulong rate = clk_get_rate(&per_clk);
- u32 divisor = meson_calc_baud_divisor(rate, baud);
- u32 calc_baud = (rate / 3) / (divisor + 1);
- u32 calc_err = baud > calc_baud ? baud - calc_baud : calc_baud - baud;
- if (((calc_err * 100) / baud) > 2)
return -EINVAL;
- meson_serial_set_baud(uart, rate, baud);
- return 0;
+}
- static int meson_serial_pending(struct udevice *dev, bool input) { struct meson_serial_plat *plat = dev_get_plat(dev);
@@ -154,6 +223,7 @@ static const struct dm_serial_ops meson_serial_ops = { .putc = meson_serial_putc, .pending = meson_serial_pending, .getc = meson_serial_getc,
.setbrg = meson_serial_setbrg, };
static const struct udevice_id meson_serial_ids[] = {
diff --git a/include/configs/meson64.h b/include/configs/meson64.h index 196e58ed..01413a02 100644 --- a/include/configs/meson64.h +++ b/include/configs/meson64.h @@ -16,6 +16,13 @@ #define GICC_BASE 0xc4302000 #endif
+/* Serial drivers */ +/* The following table includes the supported baudrates */ +#define CONFIG_SYS_BAUDRATE_TABLE \
- {300, 600, 1200, 2400, 4800, 9600, 19200, 38400, 57600, 115200, \
230400, 250000, 460800, 500000, 1000000, 2000000, 4000000, \
8000000 }
- /* For splashscreen */ #ifdef CONFIG_DM_VIDEO #define STDOUT_CFG "vidconsole,serial"
Looks good !
If someone can test it ? otherwise I'll take it for next release.
Neil

Hello, Il giorno gio 22 set 2022 alle ore 18:51 Neil Armstrong neil.armstrong@linaro.org ha scritto:
Hi,
Sorry for the delay...
No problem, don't worry
Looks good !
If someone can test it ? otherwise I'll take it for next release.
...does it count if I've tested it? I have a radxa zero board (S905Y2 chip), this started as an experimentation with editing register values in memory "for fun" and I decided to try to make a patch out of it. I do not have an oscilloscope so I can not accurately measure the baud rate, but the logic is the same as in linux and using an USB to UART (PL2303) adapter console input and loady command work fine. I've just repeated a small test: I'm using picocom and lrzsz in ymodem mode under WSL, the usb adapter is forwarded to WSL using usbip, but this should not change anything at this kind of speed (I hope). Here are some results sending a 1 MB test file (created once from /dev/urandom): picocom v3.1
port is : /dev/ttyUSB0 flowcontrol : none baudrate is : 115200 parity is : none databits are : 8 stopbits are : 1 [...] send_cmd is : sz --ymodem -vv receive_cmd is : rz -vv -E [...] @115200 Bytes Sent:1048576 BPS:7762 @230400 Bytes Sent:1048576 BPS:16372 @460800 Bytes Sent:1048576 BPS:26681 @500000 Bytes Sent:1048576 BPS:27689 @500000 (2nd run) Bytes Sent:1048576 BPS:27207
At this point I stopped testing, I think my adapter is not good enough and that the logic is correct (I've read amlogic's public datasheets [1], but I've not found a formula specified there), Reading address (for hardware device UARTAO1, that _of course_ is not listed there) md.l 0xff803014 1 ff803014: 0180000f This corresponds to REG5 of device UARTAO1, with bits 24 (USE_XTAL_CLK), 23 (use new baud rate) set, and bits 27 and 26 clear (so 24 MHz crystal clock is divided by 3), NEW_BAUD_RATE is f, so baud rate should be (8 MHz / (15 + 1)) = 500K.
Other than that, I noticed that setting the env variable baudrate is restricted to values allowed in a compile time config, loady is not and loady ignores the return value of meson_serial_setbrg (it returns -EINVAL if for example a baudrate higher than 8'000'000 is requested), here's a test (with env variable baudrate=500000): => loady 0x01000000 8000001 ## Switch baudrate to 8000001 bps and press ENTER ... ## Ready for binary (ymodem) download to 0x01000000 at 8000001 bps... C *** file: ./dest $ sz --ymodem -vv ./dest Sending: dest Bytes Sent:1048576 BPS:27781 Sending: Ymodem sectors/kbytes sent: 0/ 0k Transfer complete
*** exit status: 0 *** ## Total Size = 0x00100000 = 1048576 Bytes ## Start Addr = 0x01000000 ## Switch baudrate to 500000 bps and press ESC ... =>
setbrg fails, the baud rate register is left at the 500'000 setting and that's it. I think the "for fun" experiment has gone a bit too far.
Edoardo [1] https://dl.radxa.com/zero/docs/hw/IO%20Interface%ef%bc%8dS905Y2%20Datasheet....

Hi,
On 22/09/2022 20:51, Edoardo Tomelleri wrote:
Hello, Il giorno gio 22 set 2022 alle ore 18:51 Neil Armstrong neil.armstrong@linaro.org ha scritto:
Hi,
Sorry for the delay...
No problem, don't worry
Looks good !
If someone can test it ? otherwise I'll take it for next release.
...does it count if I've tested it? I have a radxa zero board (S905Y2 chip), this started as an experimentation with editing register values in memory "for fun" and I decided to try to make a patch out of it.
Yes it's perfect, thanks a lot for the test results !
I do not have an oscilloscope so I can not accurately measure the baud rate, but the logic is the same as in linux and using an USB to UART (PL2303) adapter console input and loady command work fine. I've just repeated a small test: I'm using picocom and lrzsz in ymodem mode under WSL, the usb adapter is forwarded to WSL using usbip, but this should not change anything at this kind of speed (I hope). Here are some results sending a 1 MB test file (created once from /dev/urandom): picocom v3.1
port is : /dev/ttyUSB0 flowcontrol : none baudrate is : 115200 parity is : none databits are : 8 stopbits are : 1 [...] send_cmd is : sz --ymodem -vv receive_cmd is : rz -vv -E [...] @115200 Bytes Sent:1048576 BPS:7762 @230400 Bytes Sent:1048576 BPS:16372 @460800 Bytes Sent:1048576 BPS:26681 @500000 Bytes Sent:1048576 BPS:27689 @500000 (2nd run) Bytes Sent:1048576 BPS:27207
At this point I stopped testing, I think my adapter is not good enough and that the logic is correct (I've read amlogic's public datasheets [1], but I've not found a formula specified there), Reading address (for hardware device UARTAO1, that _of course_ is not listed there) md.l 0xff803014 1 ff803014: 0180000f This corresponds to REG5 of device UARTAO1, with bits 24 (USE_XTAL_CLK), 23 (use new baud rate) set, and bits 27 and 26 clear (so 24 MHz crystal clock is divided by 3), NEW_BAUD_RATE is f, so baud rate should be (8 MHz / (15 + 1)) = 500K.
Other than that, I noticed that setting the env variable baudrate is restricted to values allowed in a compile time config, loady is not and loady ignores the return value of meson_serial_setbrg (it returns -EINVAL if for example a baudrate higher than 8'000'000 is requested), here's a test (with env variable baudrate=500000): => loady 0x01000000 8000001 ## Switch baudrate to 8000001 bps and press ENTER ... ## Ready for binary (ymodem) download to 0x01000000 at 8000001 bps... C *** file: ./dest $ sz --ymodem -vv ./dest Sending: dest Bytes Sent:1048576 BPS:27781 Sending: Ymodem sectors/kbytes sent: 0/ 0k Transfer complete
*** exit status: 0 *** ## Total Size = 0x00100000 = 1048576 Bytes ## Start Addr = 0x01000000 ## Switch baudrate to 500000 bps and press ESC ... =>
setbrg fails, the baud rate register is left at the 500'000 setting and that's it. I think the "for fun" experiment has gone a bit too far.
Edoardo [1] https://dl.radxa.com/zero/docs/hw/IO%20Interface%ef%bc%8dS905Y2%20Datasheet....
I'll apply it then.
Acked-by: Neil Armstrong narmstrong@baylibre.com
Please send more patches with such quality !
Thanks, Neil

Hi,
On Sun, 18 Sep 2022 18:17:01 +0200, Edoardo Tomelleri wrote:
Implement setbrg in amlogic/meson serial device with driver model similar to how the meson_uart.c driver does it in Linux. Also configure (probe) the serial device with the new reg5 register.
Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-amlogic (u-boot-amlogic-test)
[1/1] arm: amlogic: add setbrg op to serial device https://source.denx.de/u-boot/custodians/u-boot-amlogic/-/commit/7802c3ac52a...
participants (3)
-
Edoardo Tomelleri
-
Neil Armstrong
-
neil.armstrong@linaro.org