[PATCH] serial: tegra: Fixes lower tolerance baud rate limit for older tegra chips introduced by d781ec21bae6

[PATCH] serial: tegra: Fixes lower tolerance baud rate limit for older tegra chips introduced by d781ec21bae6

[Patrik John]

The current implementation uses 0 as lower limit for the baud rate tolerance which contradicts the initial commit description (https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/commit/drivers/tty/serial/serial-tegra.c?h=for-next&id=d781ec21bae6ff8f9e07682e8947a654484611f5) of +4/-4% tolerance for older tegra chips other than Tegra186 and Tegra194.
This causes issues on UART initilization as soon as the actual baud rate clock is slightly lower than required which we have seen on the Tegra124-based Toradex Apalis TK1 which also uses tegra30-hsuart as compatible in the DT serial node (for reference line 1540ff https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/tree/arch/arm/boot/dts/tegra124-apalis-v1.2.dtsi?h=for-next)

The standard baud rate tolerance limits are also stated in the tegra20-hsuart driver description (https://www.kernel.org/doc/Documentation/devicetree/bindings/serial/nvidia%2Ctegra20-hsuart.txt).

The previously introduced check_rate_in_range() always fails due to the lower limit set to 0 even if the actual baud rate is within the required -4% tolerance.

static int tegra_check_rate_in_range(struct tegra_uart_port *tup)
{
    long diff;
    diff = ((long)(tup->configured_rate - tup->required_rate) * 10000)
        / tup->required_rate;
    if (diff < (tup->cdata->error_tolerance_low_range * 100) ||
        diff > (tup->cdata->error_tolerance_high_range * 100)) {
        dev_err(tup->uport.dev,
            "configured baud rate is out of range by %ld", diff);
        return -EIO;
    }
    return 0;
}

Changing the lower tolerance limit to the actual -4% resolved the issues for the Tegra124 and should resolve potential issues for other Tegra20/Tegra30 based platforms as well.

Signed-off-by: Patrik John <patrik.john@u-blox.com>
---
 drivers/tty/serial/serial-tegra.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index 45e2e4109acd..b6223fab0687 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -1506,7 +1506,7 @@ static struct tegra_uart_chip_data tegra20_uart_chip_data = {
 	.fifo_mode_enable_status	= false,
 	.uart_max_port			= 5,
 	.max_dma_burst_bytes		= 4,
-	.error_tolerance_low_range	= 0,
+	.error_tolerance_low_range	= -4,
 	.error_tolerance_high_range	= 4,
 };
 
@@ -1517,7 +1517,7 @@ static struct tegra_uart_chip_data tegra30_uart_chip_data = {
 	.fifo_mode_enable_status	= false,
 	.uart_max_port			= 5,
 	.max_dma_burst_bytes		= 4,
-	.error_tolerance_low_range	= 0,
+	.error_tolerance_low_range	= -4,
 	.error_tolerance_high_range	= 4,
 };
 
-- 
2.25.1

Re: [PATCH] serial: tegra: Fixes lower tolerance baud rate limit for older tegra chips introduced by d781ec21bae6

[Greg KH]

On Mon, Nov 22, 2021 at 01:44:26PM +0100, Patrik John wrote:
> The current implementation uses 0 as lower limit for the baud rate tolerance which contradicts the initial commit description (https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/commit/drivers/tty/serial/serial-tegra.c?h=for-next&id=d781ec21bae6ff8f9e07682e8947a654484611f5) of +4/-4% tolerance for older tegra chips other than Tegra186 and Tegra194.
> This causes issues on UART initilization as soon as the actual baud rate clock is slightly lower than required which we have seen on the Tegra124-based Toradex Apalis TK1 which also uses tegra30-hsuart as compatible in the DT serial node (for reference line 1540ff https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/tree/arch/arm/boot/dts/tegra124-apalis-v1.2.dtsi?h=for-next)

All of these links will break in a few days.

And a line number is not "1540ff" :(

> 
> The standard baud rate tolerance limits are also stated in the tegra20-hsuart driver description (https://www.kernel.org/doc/Documentation/devicetree/bindings/serial/nvidia%2Ctegra20-hsuart.txt).

You can just reference a file in the kernel source tree directly, no
need to go back to kernel.org

> 
> The previously introduced check_rate_in_range() always fails due to the lower limit set to 0 even if the actual baud rate is within the required -4% tolerance.

Can you please wrap your changelog text at 72 columns like git asked you
to when you committed the change to your local tree?

> 
> static int tegra_check_rate_in_range(struct tegra_uart_port *tup)
> {
>     long diff;
>     diff = ((long)(tup->configured_rate - tup->required_rate) * 10000)
>         / tup->required_rate;
>     if (diff < (tup->cdata->error_tolerance_low_range * 100) ||
>         diff > (tup->cdata->error_tolerance_high_range * 100)) {
>         dev_err(tup->uport.dev,
>             "configured baud rate is out of range by %ld", diff);
>         return -EIO;
>     }
>     return 0;
> }

I do not understand, why is this code in the changelog?

> 
> Changing the lower tolerance limit to the actual -4% resolved the issues for the Tegra124 and should resolve potential issues for other Tegra20/Tegra30 based platforms as well.
> 
> Signed-off-by: Patrik John <patrik.john@u-blox.com>

What commit does this fix?  Should it have a "Fixes:" tag in it?

And should it go to stable kernel(s)?

Also, this is a v2 patch, please include below the --- line what changed
from the previous version when you resend v3.

thanks,

greg k-h

Re: [PATCH] serial: tegra: Fixes lower tolerance baud rate limit for older tegra chips introduced by d781ec21bae6

[Michał Mirosław]

On Mon, Nov 22, 2021 at 01:44:26PM +0100, Patrik John wrote:
> The current implementation uses 0 as lower limit for the baud rate tolerance which contradicts the initial commit description (https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/commit/drivers/tty/serial/serial-tegra.c?h=for-next&id=d781ec21bae6ff8f9e07682e8947a654484611f5) of +4/-4% tolerance for older tegra chips other than Tegra186 and Tegra194.
> This causes issues on UART initilization as soon as the actual baud rate clock is slightly lower than required which we have seen on the Tegra124-based Toradex Apalis TK1 which also uses tegra30-hsuart as compatible in the DT serial node (for reference line 1540ff https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/tree/arch/arm/boot/dts/tegra124-apalis-v1.2.dtsi?h=for-next)
> 
> The standard baud rate tolerance limits are also stated in the tegra20-hsuart driver description (https://www.kernel.org/doc/Documentation/devicetree/bindings/serial/nvidia%2Ctegra20-hsuart.txt).
> 
> The previously introduced check_rate_in_range() always fails due to the lower limit set to 0 even if the actual baud rate is within the required -4% tolerance.
> 
[...]

I have a same patch waiting in my tree [1]. Feel free to use the commit
message and to add:

Reviewed-and-tested-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

[1] https://rere.qmqm.pl/git/?p=linux;a=commitdiff;h=b658dcd83d0db777410fe960721193d35a38115a

Best Regards
Michał Mirosław

Re: [PATCH] serial: tegra: Fixes lower tolerance baud rate limit for older tegra chips introduced by d781ec21bae6

[Dmitry Osipenko]

22.11.2021 15:44, Patrik John пишет:
> The current implementation uses 0 as lower limit for the baud rate tolerance which contradicts the initial commit description (https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/commit/drivers/tty/serial/serial-tegra.c?h=for-next&id=d781ec21bae6ff8f9e07682e8947a654484611f5) of +4/-4% tolerance for older tegra chips other than Tegra186 and Tegra194.
> This causes issues on UART initilization as soon as the actual baud rate clock is slightly lower than required which we have seen on the Tegra124-based Toradex Apalis TK1 which also uses tegra30-hsuart as compatible in the DT serial node (for reference line 1540ff https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/tree/arch/arm/boot/dts/tegra124-apalis-v1.2.dtsi?h=for-next)
> 
> The standard baud rate tolerance limits are also stated in the tegra20-hsuart driver description (https://www.kernel.org/doc/Documentation/devicetree/bindings/serial/nvidia%2Ctegra20-hsuart.txt).
> 
> The previously introduced check_rate_in_range() always fails due to the lower limit set to 0 even if the actual baud rate is within the required -4% tolerance.
> 
> static int tegra_check_rate_in_range(struct tegra_uart_port *tup)
> {
>     long diff;
>     diff = ((long)(tup->configured_rate - tup->required_rate) * 10000)
>         / tup->required_rate;
>     if (diff < (tup->cdata->error_tolerance_low_range * 100) ||
>         diff > (tup->cdata->error_tolerance_high_range * 100)) {
>         dev_err(tup->uport.dev,
>             "configured baud rate is out of range by %ld", diff);
>         return -EIO;
>     }
>     return 0;
> }
> 
> Changing the lower tolerance limit to the actual -4% resolved the issues for the Tegra124 and should resolve potential issues for other Tegra20/Tegra30 based platforms as well.


Hi,

1. The text of commit message should be wrapped around 75 chars. Please
run "./scripts/checkpatch.pl example.patch" and fix all reported
problems before sending out the patch.

2. This patch should have v2 in the title. Please use the "git
format-patch -v3" next time, it will generate patch with v3 in the title.

3. Use "Link" tag and put all http links here, before the Signed-off-by
tag, like this:

Link:
https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/commit/drivers/tty/serial/serial-tegra.c?h=for-next&id=d781ec21bae6ff8f9e07682e8947a654484611f5
Link:
https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/tree/arch/arm/boot/dts/tegra124-apalis-v1.2.dtsi?h=for-next
Link:
https://www.kernel.org/doc/Documentation/devicetree/bindings/serial/nvidia%2Ctegra20-hsuart.txt

> Signed-off-by: Patrik John <patrik.john@u-blox.com>
> ---
>  drivers/tty/serial/serial-tegra.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
> index 45e2e4109acd..b6223fab0687 100644
> --- a/drivers/tty/serial/serial-tegra.c
> +++ b/drivers/tty/serial/serial-tegra.c
> @@ -1506,7 +1506,7 @@ static struct tegra_uart_chip_data tegra20_uart_chip_data = {
>  	.fifo_mode_enable_status	= false,
>  	.uart_max_port			= 5,
>  	.max_dma_burst_bytes		= 4,
> -	.error_tolerance_low_range	= 0,
> +	.error_tolerance_low_range	= -4,
>  	.error_tolerance_high_range	= 4,
>  };
>  
> @@ -1517,7 +1517,7 @@ static struct tegra_uart_chip_data tegra30_uart_chip_data = {
>  	.fifo_mode_enable_status	= false,
>  	.uart_max_port			= 5,
>  	.max_dma_burst_bytes		= 4,
> -	.error_tolerance_low_range	= 0,
> +	.error_tolerance_low_range	= -4,
>  	.error_tolerance_high_range	= 4,
>  };
>  
> 

Re: [PATCH] serial: tegra: Fixes lower tolerance baud rate limit for older tegra chips introduced by d781ec21bae6

[Dmitry Osipenko]

28.11.2021 02:19, Michał Mirosław пишет:
> On Mon, Nov 22, 2021 at 01:44:26PM +0100, Patrik John wrote:
>> The current implementation uses 0 as lower limit for the baud rate tolerance which contradicts the initial commit description (https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/commit/drivers/tty/serial/serial-tegra.c?h=for-next&id=d781ec21bae6ff8f9e07682e8947a654484611f5) of +4/-4% tolerance for older tegra chips other than Tegra186 and Tegra194.
>> This causes issues on UART initilization as soon as the actual baud rate clock is slightly lower than required which we have seen on the Tegra124-based Toradex Apalis TK1 which also uses tegra30-hsuart as compatible in the DT serial node (for reference line 1540ff https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/tree/arch/arm/boot/dts/tegra124-apalis-v1.2.dtsi?h=for-next)
>>
>> The standard baud rate tolerance limits are also stated in the tegra20-hsuart driver description (https://www.kernel.org/doc/Documentation/devicetree/bindings/serial/nvidia%2Ctegra20-hsuart.txt).
>>
>> The previously introduced check_rate_in_range() always fails due to the lower limit set to 0 even if the actual baud rate is within the required -4% tolerance.
>>
> [...]
> 
> I have a same patch waiting in my tree [1]. Feel free to use the commit
> message and to add:
> 
> Reviewed-and-tested-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> 
> [1] https://rere.qmqm.pl/git/?p=linux;a=commitdiff;h=b658dcd83d0db777410fe960721193d35a38115a

The Reviewed-and-tested-by isn't a valid tag, should be two separate tags:

Reviewed-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Tested-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

Re: [PATCH] serial: tegra: Fixes lower tolerance baud rate limit for older tegra chips introduced by d781ec21bae6

[Dmitry Osipenko]

29.11.2021 15:32, Dmitry Osipenko пишет:
> 3. Use "Link" tag and put all http links here, before the Signed-off-by
> tag, like this:
> 
> Link:
> https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/commit/drivers/tty/serial/serial-tegra.c?h=for-next&id=d781ec21bae6ff8f9e07682e8947a654484611f5
> Link:
> https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/tree/arch/arm/boot/dts/tegra124-apalis-v1.2.dtsi?h=for-next
> Link:
> https://www.kernel.org/doc/Documentation/devicetree/bindings/serial/nvidia%2Ctegra20-hsuart.txt

Actually, it should be like this:

Link: https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/commit/drivers/tty/serial/serial-tegra.c?h=for-next&id=d781ec21bae6ff8f9e07682e8947a654484611f5
Link: https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/tree/arch/arm/boot/dts/tegra124-apalis-v1.2.dtsi?h=for-next
Link: https://www.kernel.org/doc/Documentation/devicetree/bindings/serial/nvidia%2Ctegra20-hsuart.txt

I turned off line wrapping for this email.

Re: [PATCH] serial: tegra: Fixes lower tolerance baud rate limit for older tegra chips introduced by d781ec21bae6

[Dmitry Osipenko]

29.11.2021 15:36, Dmitry Osipenko пишет:
> 29.11.2021 15:32, Dmitry Osipenko пишет:
>> 3. Use "Link" tag and put all http links here, before the Signed-off-by
>> tag, like this:
>>
>> Link:
>> https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/commit/drivers/tty/serial/serial-tegra.c?h=for-next&id=d781ec21bae6ff8f9e07682e8947a654484611f5
>> Link:
>> https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/tree/arch/arm/boot/dts/tegra124-apalis-v1.2.dtsi?h=for-next
>> Link:
>> https://www.kernel.org/doc/Documentation/devicetree/bindings/serial/nvidia%2Ctegra20-hsuart.txt
> 
> Actually, it should be like this:
> 
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/commit/drivers/tty/serial/serial-tegra.c?h=for-next&id=d781ec21bae6ff8f9e07682e8947a654484611f5
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git/tree/arch/arm/boot/dts/tegra124-apalis-v1.2.dtsi?h=for-next
> Link: https://www.kernel.org/doc/Documentation/devicetree/bindings/serial/nvidia%2Ctegra20-hsuart.txt
> 
> I turned off line wrapping for this email.
> 

For the reference, I just found that there was v3 already on the list:

https://patchwork.ozlabs.org/project/linux-tegra/patch/sig.19614244f8.20211123132737.88341-1-patrik.john@u-blox.com/