[PATCH V2 0/2] Add q 12MHz internal clock rate to calculate

[PATCH V2 0/2] Add q 12MHz internal clock rate to calculate

[Yu Tu]

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

2.Added S4 SOC compatibility. 

Yu Tu (2):
  tty: serial: meson: Add a 12MHz internal clock rate to calculate baud
    rate in order to meet the baud rate requirements of special BT
    modules
  tty: serial: meson: Added S4 SOC compatibility

V1 -> V2: Change format as discussed in the email.

Link:https://lore.kernel.org/linux-amlogic/20220407081355.13602-3-yu.tu@amlogic.com/

 drivers/tty/serial/meson_uart.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)


base-commit: f398e0aa325c61fa20903833a5b534ecb8e6e418
-- 
2.33.1

[PATCH V2 0/2] Add q 12MHz internal clock rate to calculate

[Yu Tu]

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

2.Added S4 SOC compatibility. 

Yu Tu (2):
  tty: serial: meson: Add a 12MHz internal clock rate to calculate baud
    rate in order to meet the baud rate requirements of special BT
    modules
  tty: serial: meson: Added S4 SOC compatibility

V1 -> V2: Change format as discussed in the email.

Link:https://lore.kernel.org/linux-amlogic/20220407081355.13602-3-yu.tu@amlogic.com/

 drivers/tty/serial/meson_uart.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)


base-commit: f398e0aa325c61fa20903833a5b534ecb8e6e418
-- 
2.33.1


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

[PATCH V2 0/2] Add q 12MHz internal clock rate to calculate

[Yu Tu]

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

2.Added S4 SOC compatibility. 

Yu Tu (2):
  tty: serial: meson: Add a 12MHz internal clock rate to calculate baud
    rate in order to meet the baud rate requirements of special BT
    modules
  tty: serial: meson: Added S4 SOC compatibility

V1 -> V2: Change format as discussed in the email.

Link:https://lore.kernel.org/linux-amlogic/20220407081355.13602-3-yu.tu@amlogic.com/

 drivers/tty/serial/meson_uart.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)


base-commit: f398e0aa325c61fa20903833a5b534ecb8e6e418
-- 
2.33.1


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

[PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Yu Tu]

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.Especially 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 | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 8e59624935af..58bd2723c004 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;
+	const 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 && 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;
@@ -749,6 +761,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 = (void *)device_get_match_data(&pdev->dev);
 
 	meson_ports[pdev->id] = port;
 	platform_set_drvdata(pdev, port);
-- 
2.33.1

[PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Yu Tu]

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.Especially 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 | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 8e59624935af..58bd2723c004 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;
+	const 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 && 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;
@@ -749,6 +761,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 = (void *)device_get_match_data(&pdev->dev);
 
 	meson_ports[pdev->id] = port;
 	platform_set_drvdata(pdev, port);
-- 
2.33.1


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

[PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Yu Tu]

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.Especially 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 | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 8e59624935af..58bd2723c004 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;
+	const 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 && 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;
@@ -749,6 +761,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 = (void *)device_get_match_data(&pdev->dev);
 
 	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 V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Andy Shevchenko]

On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
>
> 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.Especially the BT module

'e. E' (mind the space)

> uses 3Mhz baud rate. 8Mhz calculations can lead to baud rate bias,
> causing some problems.

...

> +struct meson_uart_data {
> +       bool has_xtal_div2;

I would prefer to see this as an unsigned int and with a less
particular name, e.g. xtal_div would suffice.

> +};

...

> +               unsigned int xtal_div = 3;

> +               if (private_data && private_data->has_xtal_div2) {
> +                       xtal_div = 2;

Better to define privata data always


> +                       val |= AML_UART_BAUD_XTAL_DIV2;
> +               }
> +               val |= DIV_ROUND_CLOSEST(port->uartclk / xtal_div, baud) - 1;


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Andy Shevchenko]

On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
>
> 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.Especially the BT module

'e. E' (mind the space)

> uses 3Mhz baud rate. 8Mhz calculations can lead to baud rate bias,
> causing some problems.

...

> +struct meson_uart_data {
> +       bool has_xtal_div2;

I would prefer to see this as an unsigned int and with a less
particular name, e.g. xtal_div would suffice.

> +};

...

> +               unsigned int xtal_div = 3;

> +               if (private_data && private_data->has_xtal_div2) {
> +                       xtal_div = 2;

Better to define privata data always


> +                       val |= AML_UART_BAUD_XTAL_DIV2;
> +               }
> +               val |= DIV_ROUND_CLOSEST(port->uartclk / xtal_div, baud) - 1;


-- 
With Best Regards,
Andy Shevchenko

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

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Andy Shevchenko]

On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
>
> 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.Especially the BT module

'e. E' (mind the space)

> uses 3Mhz baud rate. 8Mhz calculations can lead to baud rate bias,
> causing some problems.

...

> +struct meson_uart_data {
> +       bool has_xtal_div2;

I would prefer to see this as an unsigned int and with a less
particular name, e.g. xtal_div would suffice.

> +};

...

> +               unsigned int xtal_div = 3;

> +               if (private_data && private_data->has_xtal_div2) {
> +                       xtal_div = 2;

Better to define privata data always


> +                       val |= AML_UART_BAUD_XTAL_DIV2;
> +               }
> +               val |= DIV_ROUND_CLOSEST(port->uartclk / xtal_div, baud) - 1;


-- 
With Best Regards,
Andy Shevchenko

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

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Yu Tu]

Hi Andy,
	Thank you for your advice.

On 2022/4/18 20:09, Andy Shevchenko wrote:
> [ EXTERNAL EMAIL ]
> 
> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
>>
>> 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.Especially the BT module
> 
> 'e. E' (mind the space)
My statement is a whole. There should be no spaces.
> 
>> uses 3Mhz baud rate. 8Mhz calculations can lead to baud rate bias,
>> causing some problems.
> 
> ...
> 
>> +struct meson_uart_data {
>> +       bool has_xtal_div2;
> 
> I would prefer to see this as an unsigned int and with a less
> particular name, e.g. xtal_div would suffice.
I don't have a problem with your suggestion.Let's see What Neil has to say.
> 
>> +};
> 
> ...
> 
>> +               unsigned int xtal_div = 3;
> 
>> +               if (private_data && private_data->has_xtal_div2) {
>> +                       xtal_div = 2;
> 
> Better to define privata data always
> 
> 
>> +                       val |= AML_UART_BAUD_XTAL_DIV2;
>> +               }
>> +               val |= DIV_ROUND_CLOSEST(port->uartclk / xtal_div, baud) - 1;
> 
> 

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Yu Tu]

Hi Andy,
	Thank you for your advice.

On 2022/4/18 20:09, Andy Shevchenko wrote:
> [ EXTERNAL EMAIL ]
> 
> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
>>
>> 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.Especially the BT module
> 
> 'e. E' (mind the space)
My statement is a whole. There should be no spaces.
> 
>> uses 3Mhz baud rate. 8Mhz calculations can lead to baud rate bias,
>> causing some problems.
> 
> ...
> 
>> +struct meson_uart_data {
>> +       bool has_xtal_div2;
> 
> I would prefer to see this as an unsigned int and with a less
> particular name, e.g. xtal_div would suffice.
I don't have a problem with your suggestion.Let's see What Neil has to say.
> 
>> +};
> 
> ...
> 
>> +               unsigned int xtal_div = 3;
> 
>> +               if (private_data && private_data->has_xtal_div2) {
>> +                       xtal_div = 2;
> 
> Better to define privata data always
> 
> 
>> +                       val |= AML_UART_BAUD_XTAL_DIV2;
>> +               }
>> +               val |= DIV_ROUND_CLOSEST(port->uartclk / xtal_div, baud) - 1;
> 
> 

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

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Yu Tu]

Hi Andy,
	Thank you for your advice.

On 2022/4/18 20:09, Andy Shevchenko wrote:
> [ EXTERNAL EMAIL ]
> 
> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
>>
>> 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.Especially the BT module
> 
> 'e. E' (mind the space)
My statement is a whole. There should be no spaces.
> 
>> uses 3Mhz baud rate. 8Mhz calculations can lead to baud rate bias,
>> causing some problems.
> 
> ...
> 
>> +struct meson_uart_data {
>> +       bool has_xtal_div2;
> 
> I would prefer to see this as an unsigned int and with a less
> particular name, e.g. xtal_div would suffice.
I don't have a problem with your suggestion.Let's see What Neil has to say.
> 
>> +};
> 
> ...
> 
>> +               unsigned int xtal_div = 3;
> 
>> +               if (private_data && private_data->has_xtal_div2) {
>> +                       xtal_div = 2;
> 
> Better to define privata data always
> 
> 
>> +                       val |= AML_UART_BAUD_XTAL_DIV2;
>> +               }
>> +               val |= DIV_ROUND_CLOSEST(port->uartclk / xtal_div, baud) - 1;
> 
> 

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

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Jiri Slaby]

Hi,

On 19. 04. 22, 9:29, Yu Tu wrote:
> Hi Andy,
>      Thank you for your advice.
> 
> On 2022/4/18 20:09, Andy Shevchenko wrote:
>> [ EXTERNAL EMAIL ]
>>
>> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
>>>
>>> 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.Especially the BT module
>>
>> 'e. E' (mind the space)
> My statement is a whole. There should be no spaces.

Period should be followed by a space, of course.

>>> uses 3Mhz baud rate. 8Mhz calculations can lead to baud rate bias,
>>> causing some problems.
>>
>> ...
>>
>>> +struct meson_uart_data {
>>> +       bool has_xtal_div2;
>>
>> I would prefer to see this as an unsigned int and with a less
>> particular name, e.g. xtal_div would suffice.
> I don't have a problem with your suggestion.Let's see What Neil has to say.

Actually why uint provided it's a boolean value? Or do you mean to store 
the divisor directly in this member, Andy?

thanks,
-- 
js
suse labs

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Jiri Slaby]

Hi,

On 19. 04. 22, 9:29, Yu Tu wrote:
> Hi Andy,
>      Thank you for your advice.
> 
> On 2022/4/18 20:09, Andy Shevchenko wrote:
>> [ EXTERNAL EMAIL ]
>>
>> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
>>>
>>> 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.Especially the BT module
>>
>> 'e. E' (mind the space)
> My statement is a whole. There should be no spaces.

Period should be followed by a space, of course.

>>> uses 3Mhz baud rate. 8Mhz calculations can lead to baud rate bias,
>>> causing some problems.
>>
>> ...
>>
>>> +struct meson_uart_data {
>>> +       bool has_xtal_div2;
>>
>> I would prefer to see this as an unsigned int and with a less
>> particular name, e.g. xtal_div would suffice.
> I don't have a problem with your suggestion.Let's see What Neil has to say.

Actually why uint provided it's a boolean value? Or do you mean to store 
the divisor directly in this member, Andy?

thanks,
-- 
js
suse labs

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

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Jiri Slaby]

Hi,

On 19. 04. 22, 9:29, Yu Tu wrote:
> Hi Andy,
>      Thank you for your advice.
> 
> On 2022/4/18 20:09, Andy Shevchenko wrote:
>> [ EXTERNAL EMAIL ]
>>
>> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
>>>
>>> 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.Especially the BT module
>>
>> 'e. E' (mind the space)
> My statement is a whole. There should be no spaces.

Period should be followed by a space, of course.

>>> uses 3Mhz baud rate. 8Mhz calculations can lead to baud rate bias,
>>> causing some problems.
>>
>> ...
>>
>>> +struct meson_uart_data {
>>> +       bool has_xtal_div2;
>>
>> I would prefer to see this as an unsigned int and with a less
>> particular name, e.g. xtal_div would suffice.
> I don't have a problem with your suggestion.Let's see What Neil has to say.

Actually why uint provided it's a boolean value? Or do you mean to store 
the divisor directly in this member, Andy?

thanks,
-- 
js
suse labs

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

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Andy Shevchenko]

On Tue, Apr 19, 2022 at 10:38 AM Jiri Slaby <jirislaby@kernel.org> wrote:
> On 19. 04. 22, 9:29, Yu Tu wrote:
> > On 2022/4/18 20:09, Andy Shevchenko wrote:
> >> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:

...

> >>> +struct meson_uart_data {
> >>> +       bool has_xtal_div2;
> >>
> >> I would prefer to see this as an unsigned int and with a less
> >> particular name, e.g. xtal_div would suffice.
> > I don't have a problem with your suggestion.Let's see What Neil has to say.
>
> Actually why uint provided it's a boolean value? Or do you mean to store
> the divisor directly in this member, Andy?

Yes I was thinking to provide the value and then always provide the
private data. In such cases we don't need an additional condition.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Andy Shevchenko]

On Tue, Apr 19, 2022 at 10:38 AM Jiri Slaby <jirislaby@kernel.org> wrote:
> On 19. 04. 22, 9:29, Yu Tu wrote:
> > On 2022/4/18 20:09, Andy Shevchenko wrote:
> >> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:

...

> >>> +struct meson_uart_data {
> >>> +       bool has_xtal_div2;
> >>
> >> I would prefer to see this as an unsigned int and with a less
> >> particular name, e.g. xtal_div would suffice.
> > I don't have a problem with your suggestion.Let's see What Neil has to say.
>
> Actually why uint provided it's a boolean value? Or do you mean to store
> the divisor directly in this member, Andy?

Yes I was thinking to provide the value and then always provide the
private data. In such cases we don't need an additional condition.

-- 
With Best Regards,
Andy Shevchenko

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

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Andy Shevchenko]

On Tue, Apr 19, 2022 at 10:38 AM Jiri Slaby <jirislaby@kernel.org> wrote:
> On 19. 04. 22, 9:29, Yu Tu wrote:
> > On 2022/4/18 20:09, Andy Shevchenko wrote:
> >> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:

...

> >>> +struct meson_uart_data {
> >>> +       bool has_xtal_div2;
> >>
> >> I would prefer to see this as an unsigned int and with a less
> >> particular name, e.g. xtal_div would suffice.
> > I don't have a problem with your suggestion.Let's see What Neil has to say.
>
> Actually why uint provided it's a boolean value? Or do you mean to store
> the divisor directly in this member, Andy?

Yes I was thinking to provide the value and then always provide the
private data. In such cases we don't need an additional condition.

-- 
With Best Regards,
Andy Shevchenko

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

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Neil Armstrong]

Hi Andy,

On 19/04/2022 10:21, Andy Shevchenko wrote:
> On Tue, Apr 19, 2022 at 10:38 AM Jiri Slaby <jirislaby@kernel.org> wrote:
>> On 19. 04. 22, 9:29, Yu Tu wrote:
>>> On 2022/4/18 20:09, Andy Shevchenko wrote:
>>>> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
> 
> ...
> 
>>>>> +struct meson_uart_data {
>>>>> +       bool has_xtal_div2;
>>>>
>>>> I would prefer to see this as an unsigned int and with a less
>>>> particular name, e.g. xtal_div would suffice.
>>> I don't have a problem with your suggestion.Let's see What Neil has to say.
>>
>> Actually why uint provided it's a boolean value? Or do you mean to store
>> the divisor directly in this member, Andy?
> 
> Yes I was thinking to provide the value and then always provide the
> private data. In such cases we don't need an additional condition.
> 

Actually, the original boolean "has_xtal_div2" is right because it encodes
if the HW has an internal /2 divider for the XTAL clock input path.

The HW historically has a /3 divider on the same path, and new HW now has both.

So the boolean indicates if the /2 divider is present so it can be used.

So I'm in favour of keeping the boolean type.
For the naming, it seems appropriate for me.

Neil

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Neil Armstrong]

Hi Andy,

On 19/04/2022 10:21, Andy Shevchenko wrote:
> On Tue, Apr 19, 2022 at 10:38 AM Jiri Slaby <jirislaby@kernel.org> wrote:
>> On 19. 04. 22, 9:29, Yu Tu wrote:
>>> On 2022/4/18 20:09, Andy Shevchenko wrote:
>>>> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
> 
> ...
> 
>>>>> +struct meson_uart_data {
>>>>> +       bool has_xtal_div2;
>>>>
>>>> I would prefer to see this as an unsigned int and with a less
>>>> particular name, e.g. xtal_div would suffice.
>>> I don't have a problem with your suggestion.Let's see What Neil has to say.
>>
>> Actually why uint provided it's a boolean value? Or do you mean to store
>> the divisor directly in this member, Andy?
> 
> Yes I was thinking to provide the value and then always provide the
> private data. In such cases we don't need an additional condition.
> 

Actually, the original boolean "has_xtal_div2" is right because it encodes
if the HW has an internal /2 divider for the XTAL clock input path.

The HW historically has a /3 divider on the same path, and new HW now has both.

So the boolean indicates if the /2 divider is present so it can be used.

So I'm in favour of keeping the boolean type.
For the naming, it seems appropriate for me.

Neil

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

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Neil Armstrong]

Hi Andy,

On 19/04/2022 10:21, Andy Shevchenko wrote:
> On Tue, Apr 19, 2022 at 10:38 AM Jiri Slaby <jirislaby@kernel.org> wrote:
>> On 19. 04. 22, 9:29, Yu Tu wrote:
>>> On 2022/4/18 20:09, Andy Shevchenko wrote:
>>>> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
> 
> ...
> 
>>>>> +struct meson_uart_data {
>>>>> +       bool has_xtal_div2;
>>>>
>>>> I would prefer to see this as an unsigned int and with a less
>>>> particular name, e.g. xtal_div would suffice.
>>> I don't have a problem with your suggestion.Let's see What Neil has to say.
>>
>> Actually why uint provided it's a boolean value? Or do you mean to store
>> the divisor directly in this member, Andy?
> 
> Yes I was thinking to provide the value and then always provide the
> private data. In such cases we don't need an additional condition.
> 

Actually, the original boolean "has_xtal_div2" is right because it encodes
if the HW has an internal /2 divider for the XTAL clock input path.

The HW historically has a /3 divider on the same path, and new HW now has both.

So the boolean indicates if the /2 divider is present so it can be used.

So I'm in favour of keeping the boolean type.
For the naming, it seems appropriate for me.

Neil

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

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Yu Tu]

Hi Jiri,
	Thanks for the tip.

On 2022/4/19 15:38, Jiri Slaby wrote:
> [ EXTERNAL EMAIL ]
> 
> Hi,
> 
> On 19. 04. 22, 9:29, Yu Tu wrote:
>> Hi Andy,
>>      Thank you for your advice.
>>
>> On 2022/4/18 20:09, Andy Shevchenko wrote:
>>> [ EXTERNAL EMAIL ]
>>>
>>> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
>>>>
>>>> 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.Especially the BT module
>>>
>>> 'e. E' (mind the space)
>> My statement is a whole. There should be no spaces.
> 
> Period should be followed by a space, of course.
I got it .This will be corrected in the next version.

> 
>>>> uses 3Mhz baud rate. 8Mhz calculations can lead to baud rate bias,
>>>> causing some problems.
>>>
>>> ...
>>>
>>>> +struct meson_uart_data {
>>>> +       bool has_xtal_div2;
>>>
>>> I would prefer to see this as an unsigned int and with a less
>>> particular name, e.g. xtal_div would suffice.
>> I don't have a problem with your suggestion.Let's see What Neil has to 
>> say.
> 
> Actually why uint provided it's a boolean value? Or do you mean to store 
> the divisor directly in this member, Andy?
> 
> thanks,

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Yu Tu]

Hi Jiri,
	Thanks for the tip.

On 2022/4/19 15:38, Jiri Slaby wrote:
> [ EXTERNAL EMAIL ]
> 
> Hi,
> 
> On 19. 04. 22, 9:29, Yu Tu wrote:
>> Hi Andy,
>>      Thank you for your advice.
>>
>> On 2022/4/18 20:09, Andy Shevchenko wrote:
>>> [ EXTERNAL EMAIL ]
>>>
>>> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
>>>>
>>>> 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.Especially the BT module
>>>
>>> 'e. E' (mind the space)
>> My statement is a whole. There should be no spaces.
> 
> Period should be followed by a space, of course.
I got it .This will be corrected in the next version.

> 
>>>> uses 3Mhz baud rate. 8Mhz calculations can lead to baud rate bias,
>>>> causing some problems.
>>>
>>> ...
>>>
>>>> +struct meson_uart_data {
>>>> +       bool has_xtal_div2;
>>>
>>> I would prefer to see this as an unsigned int and with a less
>>> particular name, e.g. xtal_div would suffice.
>> I don't have a problem with your suggestion.Let's see What Neil has to 
>> say.
> 
> Actually why uint provided it's a boolean value? Or do you mean to store 
> the divisor directly in this member, Andy?
> 
> thanks,

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

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Yu Tu]

Hi Jiri,
	Thanks for the tip.

On 2022/4/19 15:38, Jiri Slaby wrote:
> [ EXTERNAL EMAIL ]
> 
> Hi,
> 
> On 19. 04. 22, 9:29, Yu Tu wrote:
>> Hi Andy,
>>      Thank you for your advice.
>>
>> On 2022/4/18 20:09, Andy Shevchenko wrote:
>>> [ EXTERNAL EMAIL ]
>>>
>>> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
>>>>
>>>> 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.Especially the BT module
>>>
>>> 'e. E' (mind the space)
>> My statement is a whole. There should be no spaces.
> 
> Period should be followed by a space, of course.
I got it .This will be corrected in the next version.

> 
>>>> uses 3Mhz baud rate. 8Mhz calculations can lead to baud rate bias,
>>>> causing some problems.
>>>
>>> ...
>>>
>>>> +struct meson_uart_data {
>>>> +       bool has_xtal_div2;
>>>
>>> I would prefer to see this as an unsigned int and with a less
>>> particular name, e.g. xtal_div would suffice.
>> I don't have a problem with your suggestion.Let's see What Neil has to 
>> say.
> 
> Actually why uint provided it's a boolean value? Or do you mean to store 
> the divisor directly in this member, Andy?
> 
> thanks,

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

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Neil Armstrong]

Hi Andy,

On 18/04/2022 14:09, Andy Shevchenko wrote:
> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
>>
>> 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.Especially the BT module
> 
> 'e. E' (mind the space)
> 
>> uses 3Mhz baud rate. 8Mhz calculations can lead to baud rate bias,
>> causing some problems.
> 
> ...
> 
>> +struct meson_uart_data {
>> +       bool has_xtal_div2;
> 
> I would prefer to see this as an unsigned int and with a less
> particular name, e.g. xtal_div would suffice.
> 
>> +};
> 
> ...
> 
>> +               unsigned int xtal_div = 3;
> 
>> +               if (private_data && private_data->has_xtal_div2) {
>> +                       xtal_div = 2;
> 
> Better to define privata data always

While I'm in favor of defining private data, here 3 and 2 are actually the values
2 and 3 used to divide.

The code is easy to read and we quickly understand this value is the clock divider.

> 
> 
>> +                       val |= AML_UART_BAUD_XTAL_DIV2;
>> +               }
>> +               val |= DIV_ROUND_CLOSEST(port->uartclk / xtal_div, baud) - 1;
> 
> 

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Neil Armstrong]

Hi Andy,

On 18/04/2022 14:09, Andy Shevchenko wrote:
> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
>>
>> 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.Especially the BT module
> 
> 'e. E' (mind the space)
> 
>> uses 3Mhz baud rate. 8Mhz calculations can lead to baud rate bias,
>> causing some problems.
> 
> ...
> 
>> +struct meson_uart_data {
>> +       bool has_xtal_div2;
> 
> I would prefer to see this as an unsigned int and with a less
> particular name, e.g. xtal_div would suffice.
> 
>> +};
> 
> ...
> 
>> +               unsigned int xtal_div = 3;
> 
>> +               if (private_data && private_data->has_xtal_div2) {
>> +                       xtal_div = 2;
> 
> Better to define privata data always

While I'm in favor of defining private data, here 3 and 2 are actually the values
2 and 3 used to divide.

The code is easy to read and we quickly understand this value is the clock divider.

> 
> 
>> +                       val |= AML_UART_BAUD_XTAL_DIV2;
>> +               }
>> +               val |= DIV_ROUND_CLOSEST(port->uartclk / xtal_div, baud) - 1;
> 
> 


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

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Neil Armstrong]

Hi Andy,

On 18/04/2022 14:09, Andy Shevchenko wrote:
> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
>>
>> 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.Especially the BT module
> 
> 'e. E' (mind the space)
> 
>> uses 3Mhz baud rate. 8Mhz calculations can lead to baud rate bias,
>> causing some problems.
> 
> ...
> 
>> +struct meson_uart_data {
>> +       bool has_xtal_div2;
> 
> I would prefer to see this as an unsigned int and with a less
> particular name, e.g. xtal_div would suffice.
> 
>> +};
> 
> ...
> 
>> +               unsigned int xtal_div = 3;
> 
>> +               if (private_data && private_data->has_xtal_div2) {
>> +                       xtal_div = 2;
> 
> Better to define privata data always

While I'm in favor of defining private data, here 3 and 2 are actually the values
2 and 3 used to divide.

The code is easy to read and we quickly understand this value is the clock divider.

> 
> 
>> +                       val |= AML_UART_BAUD_XTAL_DIV2;
>> +               }
>> +               val |= DIV_ROUND_CLOSEST(port->uartclk / xtal_div, baud) - 1;
> 
> 


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

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Yu Tu]

Hi Neil,

On 2022/4/21 16:46, Neil Armstrong wrote:
> [ EXTERNAL EMAIL ]
> 
> Hi Andy,
> 
> On 18/04/2022 14:09, Andy Shevchenko wrote:
>> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
>>>
>>> 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.Especially the BT module
>>
>> 'e. E' (mind the space)
>>
>>> uses 3Mhz baud rate. 8Mhz calculations can lead to baud rate bias,
>>> causing some problems.
>>
>> ...
>>
>>> +struct meson_uart_data {
>>> +       bool has_xtal_div2;
>>
>> I would prefer to see this as an unsigned int and with a less
>> particular name, e.g. xtal_div would suffice.
>>
>>> +};
>>
>> ...
>>
>>> +               unsigned int xtal_div = 3;
>>
>>> +               if (private_data && private_data->has_xtal_div2) {
>>> +                       xtal_div = 2;
>>
>> Better to define privata data always
> 
> While I'm in favor of defining private data, here 3 and 2 are actually 
> the values
> 2 and 3 used to divide.
> 
> The code is easy to read and we quickly understand this value is the 
> clock divider.
> 
Therefore, this place should be modified according to your previous 
suggestion.I will prepare the next version.
>>
>>
>>> +                       val |= AML_UART_BAUD_XTAL_DIV2;
>>> +               }
>>> +               val |= DIV_ROUND_CLOSEST(port->uartclk / xtal_div, 
>>> baud) - 1;
>>
>>
> 
> .

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Yu Tu]

Hi Neil,

On 2022/4/21 16:46, Neil Armstrong wrote:
> [ EXTERNAL EMAIL ]
> 
> Hi Andy,
> 
> On 18/04/2022 14:09, Andy Shevchenko wrote:
>> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
>>>
>>> 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.Especially the BT module
>>
>> 'e. E' (mind the space)
>>
>>> uses 3Mhz baud rate. 8Mhz calculations can lead to baud rate bias,
>>> causing some problems.
>>
>> ...
>>
>>> +struct meson_uart_data {
>>> +       bool has_xtal_div2;
>>
>> I would prefer to see this as an unsigned int and with a less
>> particular name, e.g. xtal_div would suffice.
>>
>>> +};
>>
>> ...
>>
>>> +               unsigned int xtal_div = 3;
>>
>>> +               if (private_data && private_data->has_xtal_div2) {
>>> +                       xtal_div = 2;
>>
>> Better to define privata data always
> 
> While I'm in favor of defining private data, here 3 and 2 are actually 
> the values
> 2 and 3 used to divide.
> 
> The code is easy to read and we quickly understand this value is the 
> clock divider.
> 
Therefore, this place should be modified according to your previous 
suggestion.I will prepare the next version.
>>
>>
>>> +                       val |= AML_UART_BAUD_XTAL_DIV2;
>>> +               }
>>> +               val |= DIV_ROUND_CLOSEST(port->uartclk / xtal_div, 
>>> baud) - 1;
>>
>>
> 
> .

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

Re: [PATCH V2 1/2] tty: serial: meson: Add a 12MHz internal clock rate to calculate baud rate in order to meet the baud rate requirements of special BT modules

[Yu Tu]

Hi Neil,

On 2022/4/21 16:46, Neil Armstrong wrote:
> [ EXTERNAL EMAIL ]
> 
> Hi Andy,
> 
> On 18/04/2022 14:09, Andy Shevchenko wrote:
>> On Mon, Apr 18, 2022 at 8:50 AM Yu Tu <yu.tu@amlogic.com> wrote:
>>>
>>> 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.Especially the BT module
>>
>> 'e. E' (mind the space)
>>
>>> uses 3Mhz baud rate. 8Mhz calculations can lead to baud rate bias,
>>> causing some problems.
>>
>> ...
>>
>>> +struct meson_uart_data {
>>> +       bool has_xtal_div2;
>>
>> I would prefer to see this as an unsigned int and with a less
>> particular name, e.g. xtal_div would suffice.
>>
>>> +};
>>
>> ...
>>
>>> +               unsigned int xtal_div = 3;
>>
>>> +               if (private_data && private_data->has_xtal_div2) {
>>> +                       xtal_div = 2;
>>
>> Better to define privata data always
> 
> While I'm in favor of defining private data, here 3 and 2 are actually 
> the values
> 2 and 3 used to divide.
> 
> The code is easy to read and we quickly understand this value is the 
> clock divider.
> 
Therefore, this place should be modified according to your previous 
suggestion.I will prepare the next version.
>>
>>
>>> +                       val |= AML_UART_BAUD_XTAL_DIV2;
>>> +               }
>>> +               val |= DIV_ROUND_CLOSEST(port->uartclk / xtal_div, 
>>> baud) - 1;
>>
>>
> 
> .

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

[PATCH V2 2/2] 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 | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 58bd2723c004..43941f21735f 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -790,11 +790,19 @@ static int meson_uart_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static struct meson_uart_data s4_uart_date = {
+	.has_xtal_div2 = true,
+};
+
 static const struct of_device_id meson_uart_dt_match[] = {
 	{ .compatible = "amlogic,meson6-uart" },
 	{ .compatible = "amlogic,meson8-uart" },
 	{ .compatible = "amlogic,meson8b-uart" },
 	{ .compatible = "amlogic,meson-gx-uart" },
+	{
+		.compatible = "amlogic,meson-s4-uart",
+		.data = (void *)&s4_uart_date,
+	},
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, meson_uart_dt_match);
-- 
2.33.1

[PATCH V2 2/2] 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 | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 58bd2723c004..43941f21735f 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -790,11 +790,19 @@ static int meson_uart_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static struct meson_uart_data s4_uart_date = {
+	.has_xtal_div2 = true,
+};
+
 static const struct of_device_id meson_uart_dt_match[] = {
 	{ .compatible = "amlogic,meson6-uart" },
 	{ .compatible = "amlogic,meson8-uart" },
 	{ .compatible = "amlogic,meson8b-uart" },
 	{ .compatible = "amlogic,meson-gx-uart" },
+	{
+		.compatible = "amlogic,meson-s4-uart",
+		.data = (void *)&s4_uart_date,
+	},
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, meson_uart_dt_match);
-- 
2.33.1


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

[PATCH V2 2/2] 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 | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 58bd2723c004..43941f21735f 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -790,11 +790,19 @@ static int meson_uart_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static struct meson_uart_data s4_uart_date = {
+	.has_xtal_div2 = true,
+};
+
 static const struct of_device_id meson_uart_dt_match[] = {
 	{ .compatible = "amlogic,meson6-uart" },
 	{ .compatible = "amlogic,meson8-uart" },
 	{ .compatible = "amlogic,meson8b-uart" },
 	{ .compatible = "amlogic,meson-gx-uart" },
+	{
+		.compatible = "amlogic,meson-s4-uart",
+		.data = (void *)&s4_uart_date,
+	},
 	{ /* 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

Re: [PATCH V2 2/2] tty: serial: meson: Added S4 SOC compatibility

[Neil Armstrong]

On 18/04/2022 07:32, Yu Tu wrote:
> 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 | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 58bd2723c004..43941f21735f 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -790,11 +790,19 @@ static int meson_uart_remove(struct platform_device *pdev)
>   	return 0;
>   }
>   
> +static struct meson_uart_data s4_uart_date = {

Should be s4_uart_data instead of s4_uart_date

> +	.has_xtal_div2 = true,
> +};
> +
>   static const struct of_device_id meson_uart_dt_match[] = {
>   	{ .compatible = "amlogic,meson6-uart" },
>   	{ .compatible = "amlogic,meson8-uart" },
>   	{ .compatible = "amlogic,meson8b-uart" },
>   	{ .compatible = "amlogic,meson-gx-uart" },
> +	{
> +		.compatible = "amlogic,meson-s4-uart",
> +		.data = (void *)&s4_uart_date,

Here same

> +	},
>   	{ /* sentinel */ },
>   };
>   MODULE_DEVICE_TABLE(of, meson_uart_dt_match);
With this change, it's fine for me.

Neil

Re: [PATCH V2 2/2] tty: serial: meson: Added S4 SOC compatibility

[Neil Armstrong]

On 18/04/2022 07:32, Yu Tu wrote:
> 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 | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 58bd2723c004..43941f21735f 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -790,11 +790,19 @@ static int meson_uart_remove(struct platform_device *pdev)
>   	return 0;
>   }
>   
> +static struct meson_uart_data s4_uart_date = {

Should be s4_uart_data instead of s4_uart_date

> +	.has_xtal_div2 = true,
> +};
> +
>   static const struct of_device_id meson_uart_dt_match[] = {
>   	{ .compatible = "amlogic,meson6-uart" },
>   	{ .compatible = "amlogic,meson8-uart" },
>   	{ .compatible = "amlogic,meson8b-uart" },
>   	{ .compatible = "amlogic,meson-gx-uart" },
> +	{
> +		.compatible = "amlogic,meson-s4-uart",
> +		.data = (void *)&s4_uart_date,

Here same

> +	},
>   	{ /* sentinel */ },
>   };
>   MODULE_DEVICE_TABLE(of, meson_uart_dt_match);
With this change, it's fine for me.

Neil

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

Re: [PATCH V2 2/2] tty: serial: meson: Added S4 SOC compatibility

[Neil Armstrong]

On 18/04/2022 07:32, Yu Tu wrote:
> 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 | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 58bd2723c004..43941f21735f 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -790,11 +790,19 @@ static int meson_uart_remove(struct platform_device *pdev)
>   	return 0;
>   }
>   
> +static struct meson_uart_data s4_uart_date = {

Should be s4_uart_data instead of s4_uart_date

> +	.has_xtal_div2 = true,
> +};
> +
>   static const struct of_device_id meson_uart_dt_match[] = {
>   	{ .compatible = "amlogic,meson6-uart" },
>   	{ .compatible = "amlogic,meson8-uart" },
>   	{ .compatible = "amlogic,meson8b-uart" },
>   	{ .compatible = "amlogic,meson-gx-uart" },
> +	{
> +		.compatible = "amlogic,meson-s4-uart",
> +		.data = (void *)&s4_uart_date,

Here same

> +	},
>   	{ /* sentinel */ },
>   };
>   MODULE_DEVICE_TABLE(of, meson_uart_dt_match);
With this change, it's fine for me.

Neil

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

Re: [PATCH V2 2/2] tty: serial: meson: Added S4 SOC compatibility

[Yu Tu]

On 2022/4/21 16:49, Neil Armstrong wrote:
> [ EXTERNAL EMAIL ]
> 
> On 18/04/2022 07:32, Yu Tu wrote:
>> 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 | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/tty/serial/meson_uart.c 
>> b/drivers/tty/serial/meson_uart.c
>> index 58bd2723c004..43941f21735f 100644
>> --- a/drivers/tty/serial/meson_uart.c
>> +++ b/drivers/tty/serial/meson_uart.c
>> @@ -790,11 +790,19 @@ static int meson_uart_remove(struct 
>> platform_device *pdev)
>>       return 0;
>>   }
>> +static struct meson_uart_data s4_uart_date = {
> 
> Should be s4_uart_data instead of s4_uart_date
> 
I will prepare the next version and  correct it.
>> +    .has_xtal_div2 = true,
>> +};
>> +
>>   static const struct of_device_id meson_uart_dt_match[] = {
>>       { .compatible = "amlogic,meson6-uart" },
>>       { .compatible = "amlogic,meson8-uart" },
>>       { .compatible = "amlogic,meson8b-uart" },
>>       { .compatible = "amlogic,meson-gx-uart" },
>> +    {
>> +        .compatible = "amlogic,meson-s4-uart",
>> +        .data = (void *)&s4_uart_date,
> 
> Here same
> 
>> +    },
>>       { /* sentinel */ },
>>   };
>>   MODULE_DEVICE_TABLE(of, meson_uart_dt_match);
> With this change, it's fine for me.
> 
> Neil
> 
> .

Re: [PATCH V2 2/2] tty: serial: meson: Added S4 SOC compatibility

[Yu Tu]

On 2022/4/21 16:49, Neil Armstrong wrote:
> [ EXTERNAL EMAIL ]
> 
> On 18/04/2022 07:32, Yu Tu wrote:
>> 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 | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/tty/serial/meson_uart.c 
>> b/drivers/tty/serial/meson_uart.c
>> index 58bd2723c004..43941f21735f 100644
>> --- a/drivers/tty/serial/meson_uart.c
>> +++ b/drivers/tty/serial/meson_uart.c
>> @@ -790,11 +790,19 @@ static int meson_uart_remove(struct 
>> platform_device *pdev)
>>       return 0;
>>   }
>> +static struct meson_uart_data s4_uart_date = {
> 
> Should be s4_uart_data instead of s4_uart_date
> 
I will prepare the next version and  correct it.
>> +    .has_xtal_div2 = true,
>> +};
>> +
>>   static const struct of_device_id meson_uart_dt_match[] = {
>>       { .compatible = "amlogic,meson6-uart" },
>>       { .compatible = "amlogic,meson8-uart" },
>>       { .compatible = "amlogic,meson8b-uart" },
>>       { .compatible = "amlogic,meson-gx-uart" },
>> +    {
>> +        .compatible = "amlogic,meson-s4-uart",
>> +        .data = (void *)&s4_uart_date,
> 
> Here same
> 
>> +    },
>>       { /* sentinel */ },
>>   };
>>   MODULE_DEVICE_TABLE(of, meson_uart_dt_match);
> With this change, it's fine for me.
> 
> Neil
> 
> .

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

Re: [PATCH V2 2/2] tty: serial: meson: Added S4 SOC compatibility

[Yu Tu]

On 2022/4/21 16:49, Neil Armstrong wrote:
> [ EXTERNAL EMAIL ]
> 
> On 18/04/2022 07:32, Yu Tu wrote:
>> 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 | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/tty/serial/meson_uart.c 
>> b/drivers/tty/serial/meson_uart.c
>> index 58bd2723c004..43941f21735f 100644
>> --- a/drivers/tty/serial/meson_uart.c
>> +++ b/drivers/tty/serial/meson_uart.c
>> @@ -790,11 +790,19 @@ static int meson_uart_remove(struct 
>> platform_device *pdev)
>>       return 0;
>>   }
>> +static struct meson_uart_data s4_uart_date = {
> 
> Should be s4_uart_data instead of s4_uart_date
> 
I will prepare the next version and  correct it.
>> +    .has_xtal_div2 = true,
>> +};
>> +
>>   static const struct of_device_id meson_uart_dt_match[] = {
>>       { .compatible = "amlogic,meson6-uart" },
>>       { .compatible = "amlogic,meson8-uart" },
>>       { .compatible = "amlogic,meson8b-uart" },
>>       { .compatible = "amlogic,meson-gx-uart" },
>> +    {
>> +        .compatible = "amlogic,meson-s4-uart",
>> +        .data = (void *)&s4_uart_date,
> 
> Here same
> 
>> +    },
>>       { /* sentinel */ },
>>   };
>>   MODULE_DEVICE_TABLE(of, meson_uart_dt_match);
> With this change, it's fine for me.
> 
> Neil
> 
> .

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