[PATCH 0/3] Use DIV_ROUND_CLOSEST to calculate baud rate,

[PATCH 0/3] Use DIV_ROUND_CLOSEST to calculate baud rate,

[Yu Tu]

1.Use DIV_ROUND_CLOSEST to calculate baud rates.

2.Added 12Mhz as the clock source for calculating baud rate.

3.Added S4 SOC compatibility.

Yu Tu (3):
  tty: serial: meson: Use DIV_ROUND_CLOSEST to calculate baud rates
  tty: serial: meson: Added 12Mhz as the clock source for calculating
    baud rate
  tty: serial: meson: Added S4 SOC compatibility

 drivers/tty/serial/meson_uart.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)


base-commit: 3123109284176b1532874591f7c81f3837bbdc17
-- 
2.33.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 1/3] tty: serial: meson: Use DIV_ROUND_CLOSEST to calculate baud rates

[Yu Tu]

Due to chip process differences, chip designers recommend using baud
rates as close to and larger as possible in order to reduce clock
errors.

Signed-off-by: Yu Tu <yu.tu@amlogic.com>
---
 drivers/tty/serial/meson_uart.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 2bf1c57e0981..8e59624935af 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -299,10 +299,10 @@ static void meson_uart_change_speed(struct uart_port *port, unsigned long baud)
 		cpu_relax();
 
 	if (port->uartclk == 24000000) {
-		val = ((port->uartclk / 3) / baud) - 1;
+		val = DIV_ROUND_CLOSEST(port->uartclk / 3, baud) - 1;
 		val |= AML_UART_BAUD_XTAL;
 	} else {
-		val = ((port->uartclk * 10 / (baud * 4) + 5) / 10) - 1;
+		val =  DIV_ROUND_CLOSEST(port->uartclk / 4, baud) - 1;
 	}
 	val |= AML_UART_BAUD_USE;
 	writel(val, port->membase + AML_UART_REG5);
-- 
2.33.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 1/3] tty: serial: meson: Use DIV_ROUND_CLOSEST to calculate baud rates

[Neil Armstrong]

On 07/04/2022 10:13, Yu Tu wrote:
> Due to chip process differences, chip designers recommend using baud
> rates as close to and larger as possible in order to reduce clock
> errors.
> 
> Signed-off-by: Yu Tu <yu.tu@amlogic.com>
> ---
>   drivers/tty/serial/meson_uart.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 2bf1c57e0981..8e59624935af 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -299,10 +299,10 @@ static void meson_uart_change_speed(struct uart_port *port, unsigned long baud)
>   		cpu_relax();
>   
>   	if (port->uartclk == 24000000) {
> -		val = ((port->uartclk / 3) / baud) - 1;
> +		val = DIV_ROUND_CLOSEST(port->uartclk / 3, baud) - 1;
>   		val |= AML_UART_BAUD_XTAL;
>   	} else {
> -		val = ((port->uartclk * 10 / (baud * 4) + 5) / 10) - 1;
> +		val =  DIV_ROUND_CLOSEST(port->uartclk / 4, baud) - 1;
>   	}
>   	val |= AML_UART_BAUD_USE;
>   	writel(val, port->membase + AML_UART_REG5);

I check the calculations, and with DIV_ROUND_CLOSEST(), result is always
closer to the required baudrate.

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 2/3] tty: serial: meson: Added 12Mhz as the clock source for calculating baud rate

[Yu Tu]

Starting with the g12A chip, add a 12Mhz clock to calculate the baud
rate, since the BT module uses 3Mhz baud rate. 8Mhz calculations can
lead to baud rate bias, causing some problems.

Signed-off-by: Yu Tu <yu.tu@amlogic.com>
---
 drivers/tty/serial/meson_uart.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 8e59624935af..7e77693a1318 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -68,6 +68,7 @@
 #define AML_UART_BAUD_MASK		0x7fffff
 #define AML_UART_BAUD_USE		BIT(23)
 #define AML_UART_BAUD_XTAL		BIT(24)
+#define AML_UART_BAUD_XTAL_DIV2		BIT(27)
 
 #define AML_UART_PORT_NUM		12
 #define AML_UART_PORT_OFFSET		6
@@ -80,6 +81,10 @@ static struct uart_driver meson_uart_driver;
 
 static struct uart_port *meson_ports[AML_UART_PORT_NUM];
 
+struct meson_uart_data {
+	bool has_xtal_div2;
+};
+
 static void meson_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
 }
@@ -293,13 +298,20 @@ static int meson_uart_startup(struct uart_port *port)
 
 static void meson_uart_change_speed(struct uart_port *port, unsigned long baud)
 {
-	u32 val;
+	struct meson_uart_data *private_data = port->private_data;
+	u32 val = 0;
 
 	while (!meson_uart_tx_empty(port))
 		cpu_relax();
 
 	if (port->uartclk == 24000000) {
-		val = DIV_ROUND_CLOSEST(port->uartclk / 3, baud) - 1;
+		unsigned int xtal_div = 3;
+
+		if (private_data->has_xtal_div2) {
+			xtal_div = 2;
+			val |= AML_UART_BAUD_XTAL_DIV2;
+		}
+		val |= DIV_ROUND_CLOSEST(port->uartclk / xtal_div, baud) - 1;
 		val |= AML_UART_BAUD_XTAL;
 	} else {
 		val =  DIV_ROUND_CLOSEST(port->uartclk / 4, baud) - 1;
@@ -691,6 +703,7 @@ static int meson_uart_probe_clocks(struct platform_device *pdev,
 
 static int meson_uart_probe(struct platform_device *pdev)
 {
+	struct meson_uart_data *private_data;
 	struct resource *res_mem;
 	struct uart_port *port;
 	u32 fifosize = 64; /* Default is 64, 128 for EE UART_0 */
@@ -733,6 +746,13 @@ static int meson_uart_probe(struct platform_device *pdev)
 	if (!port)
 		return -ENOMEM;
 
+	private_data = devm_kzalloc(&pdev->dev, sizeof(struct meson_uart_data), GFP_KERNEL);
+	if (!private_data)
+		return -ENOMEM;
+
+	if ((bool)device_get_match_data(&pdev->dev))
+		private_data->has_xtal_div2 = true;
+
 	ret = meson_uart_probe_clocks(pdev, port);
 	if (ret)
 		return ret;
@@ -749,6 +769,7 @@ static int meson_uart_probe(struct platform_device *pdev)
 	port->x_char = 0;
 	port->ops = &meson_uart_ops;
 	port->fifosize = fifosize;
+	port->private_data = private_data;
 
 	meson_ports[pdev->id] = port;
 	platform_set_drvdata(pdev, port);
-- 
2.33.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 2/3] tty: serial: meson: Added 12Mhz as the clock source for calculating baud rate

[Neil Armstrong]

On 07/04/2022 10:13, Yu Tu wrote:
> Starting with the g12A chip, add a 12Mhz clock to calculate the baud
> rate, since the BT module uses 3Mhz baud rate. 8Mhz calculations can
> lead to baud rate bias, causing some problems.

The commit message isn't clear enough, 12MHz is a new intermediate clock
rate, not a new clock source of the UART module.

Please explain a /2 divider over XTAL was introduced since G12A, and
is preferred to be used over the still present /3 divider since it
provides much closer frequencies vs the request baudrate.

> 
> Signed-off-by: Yu Tu <yu.tu@amlogic.com>
> ---
>   drivers/tty/serial/meson_uart.c | 25 +++++++++++++++++++++++--
>   1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 8e59624935af..7e77693a1318 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -68,6 +68,7 @@
>   #define AML_UART_BAUD_MASK		0x7fffff
>   #define AML_UART_BAUD_USE		BIT(23)
>   #define AML_UART_BAUD_XTAL		BIT(24)
> +#define AML_UART_BAUD_XTAL_DIV2		BIT(27)
>   
>   #define AML_UART_PORT_NUM		12
>   #define AML_UART_PORT_OFFSET		6
> @@ -80,6 +81,10 @@ static struct uart_driver meson_uart_driver;
>   
>   static struct uart_port *meson_ports[AML_UART_PORT_NUM];
>   
> +struct meson_uart_data {
> +	bool has_xtal_div2;
> +};
> +
>   static void meson_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
>   {
>   }
> @@ -293,13 +298,20 @@ static int meson_uart_startup(struct uart_port *port)
>   
>   static void meson_uart_change_speed(struct uart_port *port, unsigned long baud)
>   {
> -	u32 val;
> +	struct meson_uart_data *private_data = port->private_data;
> +	u32 val = 0;
>   
>   	while (!meson_uart_tx_empty(port))
>   		cpu_relax();
>   
>   	if (port->uartclk == 24000000) {
> -		val = DIV_ROUND_CLOSEST(port->uartclk / 3, baud) - 1;
> +		unsigned int xtal_div = 3;
> +
> +		if (private_data->has_xtal_div2) {
> +			xtal_div = 2;
> +			val |= AML_UART_BAUD_XTAL_DIV2;
> +		}
> +		val |= DIV_ROUND_CLOSEST(port->uartclk / xtal_div, baud) - 1;
>   		val |= AML_UART_BAUD_XTAL;
>   	} else {
>   		val =  DIV_ROUND_CLOSEST(port->uartclk / 4, baud) - 1;
> @@ -691,6 +703,7 @@ static int meson_uart_probe_clocks(struct platform_device *pdev,
>   
>   static int meson_uart_probe(struct platform_device *pdev)
>   {
> +	struct meson_uart_data *private_data;
>   	struct resource *res_mem;
>   	struct uart_port *port;
>   	u32 fifosize = 64; /* Default is 64, 128 for EE UART_0 */
> @@ -733,6 +746,13 @@ static int meson_uart_probe(struct platform_device *pdev)
>   	if (!port)
>   		return -ENOMEM;
>   
> +	private_data = devm_kzalloc(&pdev->dev, sizeof(struct meson_uart_data), GFP_KERNEL);
> +	if (!private_data)
> +		return -ENOMEM;
> +
> +	if ((bool)device_get_match_data(&pdev->dev))
> +		private_data->has_xtal_div2 = true;

It should be much cleaner to pass meson_uart_data to meson_uart_dt_match .data,
and then you can retrieve device_get_match_data() and put the result into
port->private_data.
This will avoid a devm_kzalloc().

Then in meson_uart_change_speed() change the test to check for NULL private_data:
if (private_data && private_data->has_xtal_div2)

> +
>   	ret = meson_uart_probe_clocks(pdev, port);
>   	if (ret)
>   		return ret;
> @@ -749,6 +769,7 @@ static int meson_uart_probe(struct platform_device *pdev)
>   	port->x_char = 0;
>   	port->ops = &meson_uart_ops;
>   	port->fifosize = fifosize;
> +	port->private_data = private_data;
>   
>   	meson_ports[pdev->id] = port;
>   	platform_set_drvdata(pdev, port);

Neil

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 2/3] tty: serial: meson: Added 12Mhz as the clock source for calculating baud rate

[Yu Tu]

Hi Neil,
	Thank you for your reply.

On 2022/4/7 22:03, Neil Armstrong wrote:
> [ EXTERNAL EMAIL ]
> 
> On 07/04/2022 10:13, Yu Tu wrote:
>> Starting with the g12A chip, add a 12Mhz clock to calculate the baud
>> rate, since the BT module uses 3Mhz baud rate. 8Mhz calculations can
>> lead to baud rate bias, causing some problems.
> 
> The commit message isn't clear enough, 12MHz is a new intermediate clock
> rate, not a new clock source of the UART module.
> 
> Please explain a /2 divider over XTAL was introduced since G12A, and
> is preferred to be used over the still present /3 divider since it
> provides much closer frequencies vs the request baudrate.
> 
I will prepare the next version of the code as you requested.
>>
>> Signed-off-by: Yu Tu <yu.tu@amlogic.com>
>> ---
>>   drivers/tty/serial/meson_uart.c | 25 +++++++++++++++++++++++--
>>   1 file changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/serial/meson_uart.c 
>> b/drivers/tty/serial/meson_uart.c
>> index 8e59624935af..7e77693a1318 100644
>> --- a/drivers/tty/serial/meson_uart.c
>> +++ b/drivers/tty/serial/meson_uart.c
>> @@ -68,6 +68,7 @@
>>   #define AML_UART_BAUD_MASK        0x7fffff
>>   #define AML_UART_BAUD_USE        BIT(23)
>>   #define AML_UART_BAUD_XTAL        BIT(24)
>> +#define AML_UART_BAUD_XTAL_DIV2        BIT(27)
>>   #define AML_UART_PORT_NUM        12
>>   #define AML_UART_PORT_OFFSET        6
>> @@ -80,6 +81,10 @@ static struct uart_driver meson_uart_driver;
>>   static struct uart_port *meson_ports[AML_UART_PORT_NUM];
>> +struct meson_uart_data {
>> +    bool has_xtal_div2;
>> +};
>> +
>>   static void meson_uart_set_mctrl(struct uart_port *port, unsigned 
>> int mctrl)
>>   {
>>   }
>> @@ -293,13 +298,20 @@ static int meson_uart_startup(struct uart_port 
>> *port)
>>   static void meson_uart_change_speed(struct uart_port *port, unsigned 
>> long baud)
>>   {
>> -    u32 val;
>> +    struct meson_uart_data *private_data = port->private_data;
>> +    u32 val = 0;
>>       while (!meson_uart_tx_empty(port))
>>           cpu_relax();
>>       if (port->uartclk == 24000000) {
>> -        val = DIV_ROUND_CLOSEST(port->uartclk / 3, baud) - 1;
>> +        unsigned int xtal_div = 3;
>> +
>> +        if (private_data->has_xtal_div2) {
>> +            xtal_div = 2;
>> +            val |= AML_UART_BAUD_XTAL_DIV2;
>> +        }
>> +        val |= DIV_ROUND_CLOSEST(port->uartclk / xtal_div, baud) - 1;
>>           val |= AML_UART_BAUD_XTAL;
>>       } else {
>>           val =  DIV_ROUND_CLOSEST(port->uartclk / 4, baud) - 1;
>> @@ -691,6 +703,7 @@ static int meson_uart_probe_clocks(struct 
>> platform_device *pdev,
>>   static int meson_uart_probe(struct platform_device *pdev)
>>   {
>> +    struct meson_uart_data *private_data;
>>       struct resource *res_mem;
>>       struct uart_port *port;
>>       u32 fifosize = 64; /* Default is 64, 128 for EE UART_0 */
>> @@ -733,6 +746,13 @@ static int meson_uart_probe(struct 
>> platform_device *pdev)
>>       if (!port)
>>           return -ENOMEM;
>> +    private_data = devm_kzalloc(&pdev->dev, sizeof(struct 
>> meson_uart_data), GFP_KERNEL);
>> +    if (!private_data)
>> +        return -ENOMEM;
>> +
>> +    if ((bool)device_get_match_data(&pdev->dev))
>> +        private_data->has_xtal_div2 = true;
> 
> It should be much cleaner to pass meson_uart_data to meson_uart_dt_match 
> .data,
> and then you can retrieve device_get_match_data() and put the result into
> port->private_data.
> This will avoid a devm_kzalloc().
> 
> Then in meson_uart_change_speed() change the test to check for NULL 
> private_data:
> if (private_data && private_data->has_xtal_div2)
> 
>> +
>>       ret = meson_uart_probe_clocks(pdev, port);
>>       if (ret)
>>           return ret;
>> @@ -749,6 +769,7 @@ static int meson_uart_probe(struct platform_device 
>> *pdev)
>>       port->x_char = 0;
>>       port->ops = &meson_uart_ops;
>>       port->fifosize = fifosize;
>> +    port->private_data = private_data;
>>       meson_ports[pdev->id] = port;
>>       platform_set_drvdata(pdev, port);
> 
> Neil
>
I will prepare the next version of the code as you requested.

> .

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 3/3] tty: serial: meson: Added S4 SOC compatibility

[Yu Tu]

Make UART driver compatible with S4 SOC UART. Meanwhile, the S4 SOC
UART uses 12MHz as the clock source for baud rate calculations.

Signed-off-by: Yu Tu <yu.tu@amlogic.com>
---
 drivers/tty/serial/meson_uart.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 7e77693a1318..03c9a305d805 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -803,6 +803,10 @@ static const struct of_device_id meson_uart_dt_match[] = {
 	{ .compatible = "amlogic,meson8-uart" },
 	{ .compatible = "amlogic,meson8b-uart" },
 	{ .compatible = "amlogic,meson-gx-uart" },
+	{
+		.compatible = "amlogic,meson-s4-uart",
+		.data = (void *)true,
+	},
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, meson_uart_dt_match);
-- 
2.33.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic