* [PATCH] tty/serial: atmel: fix fractional baud rate computation
@ 2016-09-21 10:44 Nicolas Ferre
2016-09-22 7:07 ` Uwe Kleine-König
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Nicolas Ferre @ 2016-09-21 10:44 UTC (permalink / raw)
To: linux-arm-kernel
From: Alexey Starikovskiy <aystarik@gmail.com>
The problem with previous code was it rounded values in wrong
place and produced wrong baud rate in some cases.
Signed-off-by: Alexey Starikovskiy <aystarik@gmail.com>
[nicolas.ferre at atmel.com: port to newer kernel and add commit log]
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
drivers/tty/serial/atmel_serial.c | 10 ++++++----
include/linux/atmel_serial.h | 1 +
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 5f550d9feed9..fd8aa1f4ba78 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -2170,13 +2170,15 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
* accurately. This feature is enabled only when using normal mode.
* baudrate = selected clock / (8 * (2 - OVER) * (CD + FP / 8))
* Currently, OVER is always set to 0 so we get
- * baudrate = selected clock (16 * (CD + FP / 8))
+ * baudrate = selected clock / (16 * (CD + FP / 8))
+ * then
+ * 8 CD + FP = selected clock / (2 * baudrate)
*/
if (atmel_port->has_frac_baudrate &&
(mode & ATMEL_US_USMODE) == ATMEL_US_USMODE_NORMAL) {
- div = DIV_ROUND_CLOSEST(port->uartclk, baud);
- cd = div / 16;
- fp = DIV_ROUND_CLOSEST(div % 16, 2);
+ div = DIV_ROUND_CLOSEST(port->uartclk, baud * 2);
+ cd = div >> 3;
+ fp = div & ATMEL_US_FP_MASK;
} else {
cd = uart_get_divisor(port, baud);
}
diff --git a/include/linux/atmel_serial.h b/include/linux/atmel_serial.h
index f8e452aa48d7..bd2560502f3c 100644
--- a/include/linux/atmel_serial.h
+++ b/include/linux/atmel_serial.h
@@ -119,6 +119,7 @@
#define ATMEL_US_BRGR 0x20 /* Baud Rate Generator Register */
#define ATMEL_US_CD GENMASK(15, 0) /* Clock Divider */
#define ATMEL_US_FP_OFFSET 16 /* Fractional Part */
+#define ATMEL_US_FP_MASK 0x7
#define ATMEL_US_RTOR 0x24 /* Receiver Time-out Register for USART */
#define ATMEL_UA_RTOR 0x28 /* Receiver Time-out Register for UART */
--
2.9.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH] tty/serial: atmel: fix fractional baud rate computation
2016-09-21 10:44 [PATCH] tty/serial: atmel: fix fractional baud rate computation Nicolas Ferre
@ 2016-09-22 7:07 ` Uwe Kleine-König
2016-09-22 7:39 ` Boris Brezillon
2016-09-25 12:14 ` Boris Brezillon
2016-09-26 6:17 ` Uwe Kleine-König
2 siblings, 1 reply; 7+ messages in thread
From: Uwe Kleine-König @ 2016-09-22 7:07 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Sep 21, 2016 at 12:44:14PM +0200, Nicolas Ferre wrote:
> From: Alexey Starikovskiy <aystarik@gmail.com>
>
> The problem with previous code was it rounded values in wrong
> place and produced wrong baud rate in some cases.
>
> Signed-off-by: Alexey Starikovskiy <aystarik@gmail.com>
> [nicolas.ferre at atmel.com: port to newer kernel and add commit log]
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
> drivers/tty/serial/atmel_serial.c | 10 ++++++----
> include/linux/atmel_serial.h | 1 +
> 2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 5f550d9feed9..fd8aa1f4ba78 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -2170,13 +2170,15 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
> * accurately. This feature is enabled only when using normal mode.
> * baudrate = selected clock / (8 * (2 - OVER) * (CD + FP / 8))
> * Currently, OVER is always set to 0 so we get
> - * baudrate = selected clock (16 * (CD + FP / 8))
> + * baudrate = selected clock / (16 * (CD + FP / 8))
> + * then
> + * 8 CD + FP = selected clock / (2 * baudrate)
> */
> if (atmel_port->has_frac_baudrate &&
> (mode & ATMEL_US_USMODE) == ATMEL_US_USMODE_NORMAL) {
> - div = DIV_ROUND_CLOSEST(port->uartclk, baud);
> - cd = div / 16;
> - fp = DIV_ROUND_CLOSEST(div % 16, 2);
> + div = DIV_ROUND_CLOSEST(port->uartclk, baud * 2);
> + cd = div >> 3;
> + fp = div & ATMEL_US_FP_MASK;
given baud = 115200 and uartclk = 5414300 this results in:
div = DIV_ROUND_CLOSEST(5414300, 115200 * 2) = 23
cd = 2
fp = 7
which yields a rate of 5414300 / 46 = 117702.17. With cd = 3 and fp = 0
however the resulting rate is 5414300 / 48 = 112797.92.
Which one is better?
> } else {
> cd = uart_get_divisor(port, baud);
> }
> diff --git a/include/linux/atmel_serial.h b/include/linux/atmel_serial.h
> index f8e452aa48d7..bd2560502f3c 100644
> --- a/include/linux/atmel_serial.h
> +++ b/include/linux/atmel_serial.h
> @@ -119,6 +119,7 @@
> #define ATMEL_US_BRGR 0x20 /* Baud Rate Generator Register */
> #define ATMEL_US_CD GENMASK(15, 0) /* Clock Divider */
> #define ATMEL_US_FP_OFFSET 16 /* Fractional Part */
> +#define ATMEL_US_FP_MASK 0x7
Is there another user of this header? If not, this can be folded into
the driver.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH] tty/serial: atmel: fix fractional baud rate computation
2016-09-22 7:07 ` Uwe Kleine-König
@ 2016-09-22 7:39 ` Boris Brezillon
2016-09-22 9:43 ` Boris Brezillon
0 siblings, 1 reply; 7+ messages in thread
From: Boris Brezillon @ 2016-09-22 7:39 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 22 Sep 2016 09:07:46 +0200
Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de> wrote:
> On Wed, Sep 21, 2016 at 12:44:14PM +0200, Nicolas Ferre wrote:
> > From: Alexey Starikovskiy <aystarik@gmail.com>
> >
> > The problem with previous code was it rounded values in wrong
> > place and produced wrong baud rate in some cases.
> >
> > Signed-off-by: Alexey Starikovskiy <aystarik@gmail.com>
> > [nicolas.ferre at atmel.com: port to newer kernel and add commit log]
> > Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> > ---
> > drivers/tty/serial/atmel_serial.c | 10 ++++++----
> > include/linux/atmel_serial.h | 1 +
> > 2 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> > index 5f550d9feed9..fd8aa1f4ba78 100644
> > --- a/drivers/tty/serial/atmel_serial.c
> > +++ b/drivers/tty/serial/atmel_serial.c
> > @@ -2170,13 +2170,15 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
> > * accurately. This feature is enabled only when using normal mode.
> > * baudrate = selected clock / (8 * (2 - OVER) * (CD + FP / 8))
> > * Currently, OVER is always set to 0 so we get
> > - * baudrate = selected clock (16 * (CD + FP / 8))
> > + * baudrate = selected clock / (16 * (CD + FP / 8))
> > + * then
> > + * 8 CD + FP = selected clock / (2 * baudrate)
> > */
> > if (atmel_port->has_frac_baudrate &&
> > (mode & ATMEL_US_USMODE) == ATMEL_US_USMODE_NORMAL) {
> > - div = DIV_ROUND_CLOSEST(port->uartclk, baud);
> > - cd = div / 16;
> > - fp = DIV_ROUND_CLOSEST(div % 16, 2);
> > + div = DIV_ROUND_CLOSEST(port->uartclk, baud * 2);
> > + cd = div >> 3;
> > + fp = div & ATMEL_US_FP_MASK;
>
> given baud = 115200 and uartclk = 5414300 this results in:
>
> div = DIV_ROUND_CLOSEST(5414300, 115200 * 2) = 23
> cd = 2
> fp = 7
How about:
div = DIV_ROUND_CLOSEST(port->uartclk, baud);
cd = div / 16;
fp = (div % 16) / 2;
best_baud = port->uartclk / ((16 * cd) + (8 * fp));
/* Check if we can get a better approximation by rounding up. */
if (div % 2) {
int alt_baud, alt_fp, alt_cd;
alt_fp = fp++;
alt_cd = cd;
if (alt_fp > 7) {
alt_cd++;
alt_fp = 0;
}
alt_baud = port->uartclk / ((16 * alt_cd) + (8 *alt_fp));
if (abs(best_baud - baud) > abs(alt_baud - baud)) {
best_baud = alt_baud;
fp = alt_fp;
cd = alt_cd;
}
}
>
> which yields a rate of 5414300 / 46 = 117702.17. With cd = 3 and fp = 0
> however the resulting rate is 5414300 / 48 = 112797.92.
>
> Which one is better?
>
> > } else {
> > cd = uart_get_divisor(port, baud);
> > }
> > diff --git a/include/linux/atmel_serial.h b/include/linux/atmel_serial.h
> > index f8e452aa48d7..bd2560502f3c 100644
> > --- a/include/linux/atmel_serial.h
> > +++ b/include/linux/atmel_serial.h
> > @@ -119,6 +119,7 @@
> > #define ATMEL_US_BRGR 0x20 /* Baud Rate Generator Register */
> > #define ATMEL_US_CD GENMASK(15, 0) /* Clock Divider */
> > #define ATMEL_US_FP_OFFSET 16 /* Fractional Part */
> > +#define ATMEL_US_FP_MASK 0x7
>
> Is there another user of this header? If not, this can be folded into
> the driver.
>
> Best regards
> Uwe
>
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH] tty/serial: atmel: fix fractional baud rate computation
2016-09-22 7:39 ` Boris Brezillon
@ 2016-09-22 9:43 ` Boris Brezillon
2016-09-25 12:13 ` Boris Brezillon
0 siblings, 1 reply; 7+ messages in thread
From: Boris Brezillon @ 2016-09-22 9:43 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 22 Sep 2016 09:39:04 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> On Thu, 22 Sep 2016 09:07:46 +0200
> Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de> wrote:
>
> > On Wed, Sep 21, 2016 at 12:44:14PM +0200, Nicolas Ferre wrote:
> > > From: Alexey Starikovskiy <aystarik@gmail.com>
> > >
> > > The problem with previous code was it rounded values in wrong
> > > place and produced wrong baud rate in some cases.
> > >
> > > Signed-off-by: Alexey Starikovskiy <aystarik@gmail.com>
> > > [nicolas.ferre at atmel.com: port to newer kernel and add commit log]
> > > Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> > > ---
> > > drivers/tty/serial/atmel_serial.c | 10 ++++++----
> > > include/linux/atmel_serial.h | 1 +
> > > 2 files changed, 7 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> > > index 5f550d9feed9..fd8aa1f4ba78 100644
> > > --- a/drivers/tty/serial/atmel_serial.c
> > > +++ b/drivers/tty/serial/atmel_serial.c
> > > @@ -2170,13 +2170,15 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
> > > * accurately. This feature is enabled only when using normal mode.
> > > * baudrate = selected clock / (8 * (2 - OVER) * (CD + FP / 8))
> > > * Currently, OVER is always set to 0 so we get
> > > - * baudrate = selected clock (16 * (CD + FP / 8))
> > > + * baudrate = selected clock / (16 * (CD + FP / 8))
> > > + * then
> > > + * 8 CD + FP = selected clock / (2 * baudrate)
> > > */
> > > if (atmel_port->has_frac_baudrate &&
> > > (mode & ATMEL_US_USMODE) == ATMEL_US_USMODE_NORMAL) {
> > > - div = DIV_ROUND_CLOSEST(port->uartclk, baud);
> > > - cd = div / 16;
> > > - fp = DIV_ROUND_CLOSEST(div % 16, 2);
> > > + div = DIV_ROUND_CLOSEST(port->uartclk, baud * 2);
> > > + cd = div >> 3;
> > > + fp = div & ATMEL_US_FP_MASK;
> >
> > given baud = 115200 and uartclk = 5414300 this results in:
> >
> > div = DIV_ROUND_CLOSEST(5414300, 115200 * 2) = 23
> > cd = 2
> > fp = 7
>
> How about:
>
> div = DIV_ROUND_CLOSEST(port->uartclk, baud);
> cd = div / 16;
> fp = (div % 16) / 2;
>
> best_baud = port->uartclk / ((16 * cd) + (8 * fp));
>
> /* Check if we can get a better approximation by rounding up. */
> if (div % 2) {
> int alt_baud, alt_fp, alt_cd;
>
> alt_fp = fp++;
> alt_cd = cd;
> if (alt_fp > 7) {
> alt_cd++;
> alt_fp = 0;
> }
>
> alt_baud = port->uartclk / ((16 * alt_cd) + (8 *alt_fp));
> if (abs(best_baud - baud) > abs(alt_baud - baud)) {
After a lengthy discussion that happened on IRC (#armlinux), Uwe
proved me wrong. This should actually be
/*
* Calculate the Error in the time domain:
* Error = (RealBaudPeriod - ExpectedBaudPeriod) /
* ExpectedBaudPeriod;
*
* which after conversion to the frequency domain gives:
* Error = 1 - (ExpectedBaudRate/RealBaudRate);
*
* and since we want to compare 2 errors and avoid
* approximation, we have:
*
* if (RealBaudRate2 * (RealBaudRate1 - ExpectedBaudRate) <
* RealBaudRate1 * (RealBaudRate2 - ExpectedBaudRate))
* ...
*
*/
if (alt_baud * abs(best_baud - baud) >
best_baud * abs(alt_baud - baud))
Thanks for your patience ;-).
> best_baud = alt_baud;
> fp = alt_fp;
> cd = alt_cd;
> }
> }
>
> >
> > which yields a rate of 5414300 / 46 = 117702.17. With cd = 3 and fp = 0
> > however the resulting rate is 5414300 / 48 = 112797.92.
> >
> > Which one is better?
> >
> > > } else {
> > > cd = uart_get_divisor(port, baud);
> > > }
> > > diff --git a/include/linux/atmel_serial.h b/include/linux/atmel_serial.h
> > > index f8e452aa48d7..bd2560502f3c 100644
> > > --- a/include/linux/atmel_serial.h
> > > +++ b/include/linux/atmel_serial.h
> > > @@ -119,6 +119,7 @@
> > > #define ATMEL_US_BRGR 0x20 /* Baud Rate Generator Register */
> > > #define ATMEL_US_CD GENMASK(15, 0) /* Clock Divider */
> > > #define ATMEL_US_FP_OFFSET 16 /* Fractional Part */
> > > +#define ATMEL_US_FP_MASK 0x7
> >
> > Is there another user of this header? If not, this can be folded into
> > the driver.
> >
> > Best regards
> > Uwe
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH] tty/serial: atmel: fix fractional baud rate computation
2016-09-22 9:43 ` Boris Brezillon
@ 2016-09-25 12:13 ` Boris Brezillon
0 siblings, 0 replies; 7+ messages in thread
From: Boris Brezillon @ 2016-09-25 12:13 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 22 Sep 2016 11:43:08 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> On Thu, 22 Sep 2016 09:39:04 +0200
> Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
>
> > On Thu, 22 Sep 2016 09:07:46 +0200
> > Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de> wrote:
> >
> > > On Wed, Sep 21, 2016 at 12:44:14PM +0200, Nicolas Ferre wrote:
> > > > From: Alexey Starikovskiy <aystarik@gmail.com>
> > > >
> > > > The problem with previous code was it rounded values in wrong
> > > > place and produced wrong baud rate in some cases.
> > > >
> > > > Signed-off-by: Alexey Starikovskiy <aystarik@gmail.com>
> > > > [nicolas.ferre at atmel.com: port to newer kernel and add commit log]
> > > > Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> > > > ---
> > > > drivers/tty/serial/atmel_serial.c | 10 ++++++----
> > > > include/linux/atmel_serial.h | 1 +
> > > > 2 files changed, 7 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> > > > index 5f550d9feed9..fd8aa1f4ba78 100644
> > > > --- a/drivers/tty/serial/atmel_serial.c
> > > > +++ b/drivers/tty/serial/atmel_serial.c
> > > > @@ -2170,13 +2170,15 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
> > > > * accurately. This feature is enabled only when using normal mode.
> > > > * baudrate = selected clock / (8 * (2 - OVER) * (CD + FP / 8))
> > > > * Currently, OVER is always set to 0 so we get
> > > > - * baudrate = selected clock (16 * (CD + FP / 8))
> > > > + * baudrate = selected clock / (16 * (CD + FP / 8))
> > > > + * then
> > > > + * 8 CD + FP = selected clock / (2 * baudrate)
> > > > */
> > > > if (atmel_port->has_frac_baudrate &&
> > > > (mode & ATMEL_US_USMODE) == ATMEL_US_USMODE_NORMAL) {
> > > > - div = DIV_ROUND_CLOSEST(port->uartclk, baud);
> > > > - cd = div / 16;
> > > > - fp = DIV_ROUND_CLOSEST(div % 16, 2);
> > > > + div = DIV_ROUND_CLOSEST(port->uartclk, baud * 2);
> > > > + cd = div >> 3;
> > > > + fp = div & ATMEL_US_FP_MASK;
> > >
> > > given baud = 115200 and uartclk = 5414300 this results in:
> > >
> > > div = DIV_ROUND_CLOSEST(5414300, 115200 * 2) = 23
> > > cd = 2
> > > fp = 7
> >
> > How about:
> >
> > div = DIV_ROUND_CLOSEST(port->uartclk, baud);
> > cd = div / 16;
> > fp = (div % 16) / 2;
> >
> > best_baud = port->uartclk / ((16 * cd) + (8 * fp));
> >
> > /* Check if we can get a better approximation by rounding up. */
> > if (div % 2) {
> > int alt_baud, alt_fp, alt_cd;
> >
> > alt_fp = fp++;
> > alt_cd = cd;
> > if (alt_fp > 7) {
> > alt_cd++;
> > alt_fp = 0;
> > }
> >
> > alt_baud = port->uartclk / ((16 * alt_cd) + (8 *alt_fp));
> > if (abs(best_baud - baud) > abs(alt_baud - baud)) {
>
> After a lengthy discussion that happened on IRC (#armlinux), Uwe
> proved me wrong. This should actually be
>
>
> /*
> * Calculate the Error in the time domain:
> * Error = (RealBaudPeriod - ExpectedBaudPeriod) /
> * ExpectedBaudPeriod;
> *
> * which after conversion to the frequency domain gives:
> * Error = 1 - (ExpectedBaudRate/RealBaudRate);
> *
> * and since we want to compare 2 errors and avoid
> * approximation, we have:
> *
> * if (RealBaudRate2 * (RealBaudRate1 - ExpectedBaudRate) <
> * RealBaudRate1 * (RealBaudRate2 - ExpectedBaudRate))
> * ...
> *
> */
> if (alt_baud * abs(best_baud - baud) >
> best_baud * abs(alt_baud - baud))
>
> Thanks for your patience ;-).
>
> > best_baud = alt_baud;
> > fp = alt_fp;
> > cd = alt_cd;
> > }
> > }
> >
> > >
> > > which yields a rate of 5414300 / 46 = 117702.17. With cd = 3 and fp = 0
> > > however the resulting rate is 5414300 / 48 = 112797.92.
> > >
> > > Which one is better?
Okay, it seems I was wrong here. It appears that 117702.17 is better
than 112797.92, and Alexey's patch is calculating the best cd and fp
values for all cases.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] tty/serial: atmel: fix fractional baud rate computation
2016-09-21 10:44 [PATCH] tty/serial: atmel: fix fractional baud rate computation Nicolas Ferre
2016-09-22 7:07 ` Uwe Kleine-König
@ 2016-09-25 12:14 ` Boris Brezillon
2016-09-26 6:17 ` Uwe Kleine-König
2 siblings, 0 replies; 7+ messages in thread
From: Boris Brezillon @ 2016-09-25 12:14 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 21 Sep 2016 12:44:14 +0200
Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
> From: Alexey Starikovskiy <aystarik@gmail.com>
>
> The problem with previous code was it rounded values in wrong
> place and produced wrong baud rate in some cases.
>
> Signed-off-by: Alexey Starikovskiy <aystarik@gmail.com>
> [nicolas.ferre at atmel.com: port to newer kernel and add commit log]
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> drivers/tty/serial/atmel_serial.c | 10 ++++++----
> include/linux/atmel_serial.h | 1 +
> 2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 5f550d9feed9..fd8aa1f4ba78 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -2170,13 +2170,15 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
> * accurately. This feature is enabled only when using normal mode.
> * baudrate = selected clock / (8 * (2 - OVER) * (CD + FP / 8))
> * Currently, OVER is always set to 0 so we get
> - * baudrate = selected clock (16 * (CD + FP / 8))
> + * baudrate = selected clock / (16 * (CD + FP / 8))
> + * then
> + * 8 CD + FP = selected clock / (2 * baudrate)
> */
> if (atmel_port->has_frac_baudrate &&
> (mode & ATMEL_US_USMODE) == ATMEL_US_USMODE_NORMAL) {
> - div = DIV_ROUND_CLOSEST(port->uartclk, baud);
> - cd = div / 16;
> - fp = DIV_ROUND_CLOSEST(div % 16, 2);
> + div = DIV_ROUND_CLOSEST(port->uartclk, baud * 2);
> + cd = div >> 3;
> + fp = div & ATMEL_US_FP_MASK;
> } else {
> cd = uart_get_divisor(port, baud);
> }
> diff --git a/include/linux/atmel_serial.h b/include/linux/atmel_serial.h
> index f8e452aa48d7..bd2560502f3c 100644
> --- a/include/linux/atmel_serial.h
> +++ b/include/linux/atmel_serial.h
> @@ -119,6 +119,7 @@
> #define ATMEL_US_BRGR 0x20 /* Baud Rate Generator Register */
> #define ATMEL_US_CD GENMASK(15, 0) /* Clock Divider */
> #define ATMEL_US_FP_OFFSET 16 /* Fractional Part */
> +#define ATMEL_US_FP_MASK 0x7
>
> #define ATMEL_US_RTOR 0x24 /* Receiver Time-out Register for USART */
> #define ATMEL_UA_RTOR 0x28 /* Receiver Time-out Register for UART */
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH] tty/serial: atmel: fix fractional baud rate computation
2016-09-21 10:44 [PATCH] tty/serial: atmel: fix fractional baud rate computation Nicolas Ferre
2016-09-22 7:07 ` Uwe Kleine-König
2016-09-25 12:14 ` Boris Brezillon
@ 2016-09-26 6:17 ` Uwe Kleine-König
2 siblings, 0 replies; 7+ messages in thread
From: Uwe Kleine-König @ 2016-09-26 6:17 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
On Wed, Sep 21, 2016 at 12:44:14PM +0200, Nicolas Ferre wrote:
> From: Alexey Starikovskiy <aystarik@gmail.com>
>
> The problem with previous code was it rounded values in wrong
> place and produced wrong baud rate in some cases.
>
> Signed-off-by: Alexey Starikovskiy <aystarik@gmail.com>
> [nicolas.ferre at atmel.com: port to newer kernel and add commit log]
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
I first thought this patch results in not always picking the optimal
divider in some cases. But given the right error function (i.e.
error(r) = abs(1/r_target - 1/r_actual) which minimizes the error in the
time domain and so guarantees the maximal count of matched samples) it
can be proved to result in the right values (assuming no overflow etc.).
As writing formulas in email is cumbersome, see the attachment for a
prove.
Reviewed-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: baudrate.pdf
Type: application/pdf
Size: 69704 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160926/02579e68/attachment-0001.pdf>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-09-26 6:17 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-21 10:44 [PATCH] tty/serial: atmel: fix fractional baud rate computation Nicolas Ferre
2016-09-22 7:07 ` Uwe Kleine-König
2016-09-22 7:39 ` Boris Brezillon
2016-09-22 9:43 ` Boris Brezillon
2016-09-25 12:13 ` Boris Brezillon
2016-09-25 12:14 ` Boris Brezillon
2016-09-26 6:17 ` Uwe Kleine-König
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).