* [PATCH] serial: sh-sci: Fix HSCIF RX sampling point calculation
@ 2019-04-01 11:25 Geert Uytterhoeven
2019-04-01 12:12 ` Mukesh Ojha
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Geert Uytterhoeven @ 2019-04-01 11:25 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Ulrich Hecht
Cc: Eugeniu Rosca, Dirk Behme, linux-serial, linux-kernel,
Geert Uytterhoeven
There are several issues with the formula used for calculating the
deviation from the intended rate:
1. While min_err and last_stop are signed, srr and baud are unsigned.
Hence the signed values are promoted to unsigned, which will lead
to a bogus value of deviation if min_err is negative,
2. Srr is the register field value, which is one less than the actual
sampling rate factor,
3. The divisions do not use rounding.
Fix this by casting unsigned variables to int, adding one to srr, and
using a single DIV_ROUND_CLOSEST().
Fixes: 63ba1e00f178a448 ("serial: sh-sci: Support for HSCIF RX sampling point adjustment")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Anyone with a good test setup for verifying this feature actually works
as advertised?
---
drivers/tty/serial/sh-sci.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 2bdaeba5d527a6ce..8dea59edde34c224 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2522,7 +2522,9 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
* center of the last stop bit in sampling clocks.
*/
int last_stop = bits * 2 - 1;
- int deviation = min_err * srr * last_stop / 2 / baud;
+ int deviation = DIV_ROUND_CLOSEST(min_err * last_stop *
+ (int)(srr + 1),
+ 2 * (int)baud);
if (abs(deviation) >= 2) {
/* At least two sampling clocks off at the
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] serial: sh-sci: Fix HSCIF RX sampling point calculation
2019-04-01 11:25 [PATCH] serial: sh-sci: Fix HSCIF RX sampling point calculation Geert Uytterhoeven
@ 2019-04-01 12:12 ` Mukesh Ojha
2019-04-01 15:05 ` Ulrich Hecht
2019-04-04 9:45 ` Eugeniu Rosca
2 siblings, 0 replies; 5+ messages in thread
From: Mukesh Ojha @ 2019-04-01 12:12 UTC (permalink / raw)
To: Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby, Ulrich Hecht
Cc: Eugeniu Rosca, Dirk Behme, linux-serial, linux-kernel
On 4/1/2019 4:55 PM, Geert Uytterhoeven wrote:
> There are several issues with the formula used for calculating the
> deviation from the intended rate:
> 1. While min_err and last_stop are signed, srr and baud are unsigned.
> Hence the signed values are promoted to unsigned, which will lead
> to a bogus value of deviation if min_err is negative,
> 2. Srr is the register field value, which is one less than the actual
> sampling rate factor,
> 3. The divisions do not use rounding.
>
> Fix this by casting unsigned variables to int, adding one to srr, and
> using a single DIV_ROUND_CLOSEST().
>
> Fixes: 63ba1e00f178a448 ("serial: sh-sci: Support for HSCIF RX sampling point adjustment")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>
Cheers,
-Mukesh
> ---
> Anyone with a good test setup for verifying this feature actually works
> as advertised?
> ---
> drivers/tty/serial/sh-sci.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 2bdaeba5d527a6ce..8dea59edde34c224 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -2522,7 +2522,9 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
> * center of the last stop bit in sampling clocks.
> */
> int last_stop = bits * 2 - 1;
> - int deviation = min_err * srr * last_stop / 2 / baud;
> + int deviation = DIV_ROUND_CLOSEST(min_err * last_stop *
> + (int)(srr + 1),
> + 2 * (int)baud);
>
> if (abs(deviation) >= 2) {
> /* At least two sampling clocks off at the
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] serial: sh-sci: Fix HSCIF RX sampling point calculation
2019-04-01 11:25 [PATCH] serial: sh-sci: Fix HSCIF RX sampling point calculation Geert Uytterhoeven
2019-04-01 12:12 ` Mukesh Ojha
@ 2019-04-01 15:05 ` Ulrich Hecht
2019-04-04 9:45 ` Eugeniu Rosca
2 siblings, 0 replies; 5+ messages in thread
From: Ulrich Hecht @ 2019-04-01 15:05 UTC (permalink / raw)
To: Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby, Ulrich Hecht
Cc: Eugeniu Rosca, Dirk Behme, linux-serial, linux-kernel
> On April 1, 2019 at 1:25 PM Geert Uytterhoeven <geert+renesas@glider.be> wrote:
>
>
> There are several issues with the formula used for calculating the
> deviation from the intended rate:
> 1. While min_err and last_stop are signed, srr and baud are unsigned.
> Hence the signed values are promoted to unsigned, which will lead
> to a bogus value of deviation if min_err is negative,
> 2. Srr is the register field value, which is one less than the actual
> sampling rate factor,
> 3. The divisions do not use rounding.
>
> Fix this by casting unsigned variables to int, adding one to srr, and
> using a single DIV_ROUND_CLOSEST().
>
> Fixes: 63ba1e00f178a448 ("serial: sh-sci: Support for HSCIF RX sampling point adjustment")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>
CU
Uli
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] serial: sh-sci: Fix HSCIF RX sampling point calculation
2019-04-01 11:25 [PATCH] serial: sh-sci: Fix HSCIF RX sampling point calculation Geert Uytterhoeven
@ 2019-04-04 9:45 ` Eugeniu Rosca
2019-04-01 15:05 ` Ulrich Hecht
2019-04-04 9:45 ` Eugeniu Rosca
2 siblings, 0 replies; 5+ messages in thread
From: Eugeniu Rosca @ 2019-04-04 9:45 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Greg Kroah-Hartman, Jiri Slaby, Ulrich Hecht, Dirk Behme,
linux-serial, linux-kernel, Simon Herkenhoff, Eugeniu Rosca
Hi Geert,
On Mon, Apr 01, 2019 at 01:25:10PM +0200, Geert Uytterhoeven wrote:
> There are several issues with the formula used for calculating the
> deviation from the intended rate:
> 1. While min_err and last_stop are signed, srr and baud are unsigned.
> Hence the signed values are promoted to unsigned, which will lead
> to a bogus value of deviation if min_err is negative,
> 2. Srr is the register field value, which is one less than the actual
> sampling rate factor,
> 3. The divisions do not use rounding.
>
> Fix this by casting unsigned variables to int, adding one to srr, and
> using a single DIV_ROUND_CLOSEST().
>
> Fixes: 63ba1e00f178a448 ("serial: sh-sci: Support for HSCIF RX sampling point adjustment")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Anyone with a good test setup for verifying this feature actually works
> as advertised?
> ---
> drivers/tty/serial/sh-sci.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 2bdaeba5d527a6ce..8dea59edde34c224 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -2522,7 +2522,9 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
> * center of the last stop bit in sampling clocks.
> */
> int last_stop = bits * 2 - 1;
> - int deviation = min_err * srr * last_stop / 2 / baud;
> + int deviation = DIV_ROUND_CLOSEST(min_err * last_stop *
> + (int)(srr + 1),
> + 2 * (int)baud);
>
> if (abs(deviation) >= 2) {
> /* At least two sampling clocks off at the
Just wanted to say thank you for your tremendous commitment and
continuity in supporting the R-Car platform (and beyond that), which
allowed us to gracefully overcome the problems encountered in the
project in the best possible time, specifically thanks to this patch.
Best regards,
Eugeniu.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] serial: sh-sci: Fix HSCIF RX sampling point calculation
@ 2019-04-04 9:45 ` Eugeniu Rosca
0 siblings, 0 replies; 5+ messages in thread
From: Eugeniu Rosca @ 2019-04-04 9:45 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Greg Kroah-Hartman, Jiri Slaby, Ulrich Hecht, Dirk Behme,
linux-serial, linux-kernel, Simon Herkenhoff, Eugeniu Rosca
Hi Geert,
On Mon, Apr 01, 2019 at 01:25:10PM +0200, Geert Uytterhoeven wrote:
> There are several issues with the formula used for calculating the
> deviation from the intended rate:
> 1. While min_err and last_stop are signed, srr and baud are unsigned.
> Hence the signed values are promoted to unsigned, which will lead
> to a bogus value of deviation if min_err is negative,
> 2. Srr is the register field value, which is one less than the actual
> sampling rate factor,
> 3. The divisions do not use rounding.
>
> Fix this by casting unsigned variables to int, adding one to srr, and
> using a single DIV_ROUND_CLOSEST().
>
> Fixes: 63ba1e00f178a448 ("serial: sh-sci: Support for HSCIF RX sampling point adjustment")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Anyone with a good test setup for verifying this feature actually works
> as advertised?
> ---
> drivers/tty/serial/sh-sci.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 2bdaeba5d527a6ce..8dea59edde34c224 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -2522,7 +2522,9 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
> * center of the last stop bit in sampling clocks.
> */
> int last_stop = bits * 2 - 1;
> - int deviation = min_err * srr * last_stop / 2 / baud;
> + int deviation = DIV_ROUND_CLOSEST(min_err * last_stop *
> + (int)(srr + 1),
> + 2 * (int)baud);
>
> if (abs(deviation) >= 2) {
> /* At least two sampling clocks off at the
Just wanted to say thank you for your tremendous commitment and
continuity in supporting the R-Car platform (and beyond that), which
allowed us to gracefully overcome the problems encountered in the
project in the best possible time, specifically thanks to this patch.
Best regards,
Eugeniu.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-04-04 9:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-01 11:25 [PATCH] serial: sh-sci: Fix HSCIF RX sampling point calculation Geert Uytterhoeven
2019-04-01 12:12 ` Mukesh Ojha
2019-04-01 15:05 ` Ulrich Hecht
2019-04-04 9:45 ` Eugeniu Rosca
2019-04-04 9:45 ` Eugeniu Rosca
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.