diff mbox series

[u-boot-marvell,1/5] serial: a37xx: Fix parent clock rate value and divider calculation

Message ID 20210525174242.27509-2-marek.behun@nic.cz
State Accepted
Commit 139d081384c2f613ac74b372cb7afe2386d556e4
Delegated to: Stefan Roese
Headers show
Series Support higher baudrates on Armada 3720 UART | expand

Commit Message

Marek Behún May 25, 2021, 5:42 p.m. UTC
From: Pali Rohár <pali@kernel.org>

UART parent clock is by default the platform's xtal clock, which is
25 MHz.

The value defined in the driver, though, is 25.8048 MHz. This is a hack
for the suboptimal divisor calculation
  Divisor = UART clock / (16 * baudrate)
which does not use rounding division, resulting in a suboptimal value
for divisor if the correct parent clock rate was used.

Change the code for divisor calculation to round to closest value, i.e.
  Divisor = Round(UART clock / (16 * baudrate))
and change the parent clock rate value to that returned by
get_ref_clk().

This makes A3720 UART stable at standard UART baudrates between 1800 and
230400.

Signed-off-by: Pali Rohár <pali@kernel.org>
Reviewed-by: Marek Behún <marek.behun@nic.cz>
---
 drivers/serial/serial_mvebu_a3700.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

Comments

Stefan Roese May 27, 2021, 6:13 a.m. UTC | #1
On 25.05.21 19:42, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> UART parent clock is by default the platform's xtal clock, which is
> 25 MHz.
> 
> The value defined in the driver, though, is 25.8048 MHz. This is a hack
> for the suboptimal divisor calculation
>    Divisor = UART clock / (16 * baudrate)
> which does not use rounding division, resulting in a suboptimal value
> for divisor if the correct parent clock rate was used.
> 
> Change the code for divisor calculation to round to closest value, i.e.
>    Divisor = Round(UART clock / (16 * baudrate))
> and change the parent clock rate value to that returned by
> get_ref_clk().
> 
> This makes A3720 UART stable at standard UART baudrates between 1800 and
> 230400.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Marek Behún <marek.behun@nic.cz>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/serial/serial_mvebu_a3700.c | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/serial/serial_mvebu_a3700.c b/drivers/serial/serial_mvebu_a3700.c
> index 8f404879a5..9e7e479f80 100644
> --- a/drivers/serial/serial_mvebu_a3700.c
> +++ b/drivers/serial/serial_mvebu_a3700.c
> @@ -7,6 +7,7 @@
>   #include <dm.h>
>   #include <serial.h>
>   #include <asm/io.h>
> +#include <asm/arch/cpu.h>
>   
>   struct mvebu_plat {
>   	void __iomem *base;
> @@ -29,8 +30,6 @@ struct mvebu_plat {
>   #define UART_CTRL_RXFIFO_RESET	0x4000
>   #define UART_CTRL_TXFIFO_RESET	0x8000
>   
> -#define CONFIG_UART_BASE_CLOCK	25804800
> -
>   static int mvebu_serial_putc(struct udevice *dev, const char ch)
>   {
>   	struct mvebu_plat *plat = dev_get_plat(dev);
> @@ -75,12 +74,15 @@ static int mvebu_serial_setbrg(struct udevice *dev, int baudrate)
>   {
>   	struct mvebu_plat *plat = dev_get_plat(dev);
>   	void __iomem *base = plat->base;
> +	u32 parent_rate, divider;
>   
>   	/*
>   	 * Calculate divider
>   	 * baudrate = clock / 16 / divider
>   	 */
> -	writel(CONFIG_UART_BASE_CLOCK / baudrate / 16, base + UART_BAUD_REG);
> +	parent_rate = get_ref_clk() * 1000000;
> +	divider = DIV_ROUND_CLOSEST(parent_rate, baudrate * 16);
> +	writel(divider, base + UART_BAUD_REG);
>   
>   	/*
>   	 * Set Programmable Oversampling Stack to 0,
> @@ -144,6 +146,7 @@ U_BOOT_DRIVER(serial_mvebu) = {
>   static inline void _debug_uart_init(void)
>   {
>   	void __iomem *base = (void __iomem *)CONFIG_DEBUG_UART_BASE;
> +	u32 baudrate, parent_rate, divider;
>   
>   	/* reset FIFOs */
>   	writel(UART_CTRL_RXFIFO_RESET | UART_CTRL_TXFIFO_RESET,
> @@ -156,7 +159,10 @@ static inline void _debug_uart_init(void)
>   	 * Calculate divider
>   	 * baudrate = clock / 16 / divider
>   	 */
> -	writel(CONFIG_UART_BASE_CLOCK / 115200 / 16, base + UART_BAUD_REG);
> +	baudrate = 115200;
> +	parent_rate = get_ref_clk() * 1000000;
> +	divider = DIV_ROUND_CLOSEST(parent_rate, baudrate * 16);
> +	writel(divider, base + UART_BAUD_REG);
>   
>   	/*
>   	 * Set Programmable Oversampling Stack to 0,
> 


Viele Grüße,
Stefan
diff mbox series

Patch

diff --git a/drivers/serial/serial_mvebu_a3700.c b/drivers/serial/serial_mvebu_a3700.c
index 8f404879a5..9e7e479f80 100644
--- a/drivers/serial/serial_mvebu_a3700.c
+++ b/drivers/serial/serial_mvebu_a3700.c
@@ -7,6 +7,7 @@ 
 #include <dm.h>
 #include <serial.h>
 #include <asm/io.h>
+#include <asm/arch/cpu.h>
 
 struct mvebu_plat {
 	void __iomem *base;
@@ -29,8 +30,6 @@  struct mvebu_plat {
 #define UART_CTRL_RXFIFO_RESET	0x4000
 #define UART_CTRL_TXFIFO_RESET	0x8000
 
-#define CONFIG_UART_BASE_CLOCK	25804800
-
 static int mvebu_serial_putc(struct udevice *dev, const char ch)
 {
 	struct mvebu_plat *plat = dev_get_plat(dev);
@@ -75,12 +74,15 @@  static int mvebu_serial_setbrg(struct udevice *dev, int baudrate)
 {
 	struct mvebu_plat *plat = dev_get_plat(dev);
 	void __iomem *base = plat->base;
+	u32 parent_rate, divider;
 
 	/*
 	 * Calculate divider
 	 * baudrate = clock / 16 / divider
 	 */
-	writel(CONFIG_UART_BASE_CLOCK / baudrate / 16, base + UART_BAUD_REG);
+	parent_rate = get_ref_clk() * 1000000;
+	divider = DIV_ROUND_CLOSEST(parent_rate, baudrate * 16);
+	writel(divider, base + UART_BAUD_REG);
 
 	/*
 	 * Set Programmable Oversampling Stack to 0,
@@ -144,6 +146,7 @@  U_BOOT_DRIVER(serial_mvebu) = {
 static inline void _debug_uart_init(void)
 {
 	void __iomem *base = (void __iomem *)CONFIG_DEBUG_UART_BASE;
+	u32 baudrate, parent_rate, divider;
 
 	/* reset FIFOs */
 	writel(UART_CTRL_RXFIFO_RESET | UART_CTRL_TXFIFO_RESET,
@@ -156,7 +159,10 @@  static inline void _debug_uart_init(void)
 	 * Calculate divider
 	 * baudrate = clock / 16 / divider
 	 */
-	writel(CONFIG_UART_BASE_CLOCK / 115200 / 16, base + UART_BAUD_REG);
+	baudrate = 115200;
+	parent_rate = get_ref_clk() * 1000000;
+	divider = DIV_ROUND_CLOSEST(parent_rate, baudrate * 16);
+	writel(divider, base + UART_BAUD_REG);
 
 	/*
 	 * Set Programmable Oversampling Stack to 0,