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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> ---
> 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 <[email protected]>
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 <[email protected]>
>> ---
>> 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.
> .