* [PATCH] tty/8250_early: Prevent rounding error in uartclk to baud ratio
@ 2012-09-28 12:19 Alexey Brodkin
2012-09-28 13:02 ` Jiri Slaby
2012-09-28 13:03 ` Alan Cox
0 siblings, 2 replies; 8+ messages in thread
From: Alexey Brodkin @ 2012-09-28 12:19 UTC (permalink / raw)
To: gregkh, alan; +Cc: linux-serial, linux-kernel, Vineet.Gupta1, Alexey.Brodkin
Modify divisor to select the nearest baud rate divider rather than the
lowest. It minimizes baud rate errors especially on low UART clock
frequencies.
For example, if uartclk is 33000000 and baud is 115200 the ratio is
about 17.9 The current code selects 17 (5% error) but should select 18
(0.5% error).
On the same lines as following:
http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.9-rc3/2.6.9-rc3-mm2/broken-out/serial-pick-nearest-baud-rate-divider.patch
Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
---
drivers/tty/serial/8250_early.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250_early.c b/drivers/tty/serial/8250_early.c
index eaafb98..cfc46b7 100644
--- a/drivers/tty/serial/8250_early.c
+++ b/drivers/tty/serial/8250_early.c
@@ -140,7 +140,7 @@ static void __init init_port(struct early_serial8250_device *device)
serial_out(port, UART_FCR, 0); /* no fifo */
serial_out(port, UART_MCR, 0x3); /* DTR + RTS */
- divisor = port->uartclk / (16 * device->baud);
+ divisor = (port->uartclk + (8 * device->baud)) / (16 * device->baud);
c = serial_in(port, UART_LCR);
serial_out(port, UART_LCR, c | UART_LCR_DLAB);
serial_out(port, UART_DLL, divisor & 0xff);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] tty/8250_early: Prevent rounding error in uartclk to baud ratio
2012-09-28 12:19 [PATCH] tty/8250_early: Prevent rounding error in uartclk to baud ratio Alexey Brodkin
@ 2012-09-28 13:02 ` Jiri Slaby
2012-09-29 6:40 ` Vineet Gupta
2012-09-28 13:03 ` Alan Cox
1 sibling, 1 reply; 8+ messages in thread
From: Jiri Slaby @ 2012-09-28 13:02 UTC (permalink / raw)
To: Alexey Brodkin; +Cc: gregkh, alan, linux-serial, linux-kernel, Vineet.Gupta1
On 09/28/2012 02:19 PM, Alexey Brodkin wrote:
> Modify divisor to select the nearest baud rate divider rather than the
> lowest. It minimizes baud rate errors especially on low UART clock
> frequencies.
>
> For example, if uartclk is 33000000 and baud is 115200 the ratio is
> about 17.9 The current code selects 17 (5% error) but should select 18
> (0.5% error).
>
> On the same lines as following:
> http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.9-rc3/2.6.9-rc3-mm2/broken-out/serial-pick-nearest-baud-rate-divider.patch
>
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> ---
> drivers/tty/serial/8250_early.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250_early.c b/drivers/tty/serial/8250_early.c
> index eaafb98..cfc46b7 100644
> --- a/drivers/tty/serial/8250_early.c
> +++ b/drivers/tty/serial/8250_early.c
> @@ -140,7 +140,7 @@ static void __init init_port(struct early_serial8250_device *device)
> serial_out(port, UART_FCR, 0); /* no fifo */
> serial_out(port, UART_MCR, 0x3); /* DTR + RTS */
>
> - divisor = port->uartclk / (16 * device->baud);
> + divisor = (port->uartclk + (8 * device->baud)) / (16 * device->baud);
So this should be in fact DIV_ROUND_CLOSEST(), right?
But anyway I'm missing any explanation of *why* this is needed? This
should be part of the commit log. Is there any bug you are fixing here?
thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tty/8250_early: Prevent rounding error in uartclk to baud ratio
2012-09-28 13:02 ` Jiri Slaby
@ 2012-09-29 6:40 ` Vineet Gupta
2012-09-29 7:07 ` Jiri Slaby
0 siblings, 1 reply; 8+ messages in thread
From: Vineet Gupta @ 2012-09-29 6:40 UTC (permalink / raw)
To: Jiri Slaby
Cc: Alexey Brodkin, gregkh, alan, linux-serial, linux-kernel,
Vineet.Gupta1
On Friday 28 September 2012 06:32 PM, Jiri Slaby wrote:
> On 09/28/2012 02:19 PM, Alexey Brodkin wrote:
>> Modify divisor to select the nearest baud rate divider rather than the
>> lowest. It minimizes baud rate errors especially on low UART clock
>> frequencies.
>>
>> For example, if uartclk is 33000000 and baud is 115200 the ratio is
>> about 17.9 The current code selects 17 (5% error) but should select 18
>> (0.5% error).
>>
>> On the same lines as following:
>> http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.9-rc3/2.6.9-rc3-mm2/broken-out/serial-pick-nearest-baud-rate-divider.patch
>>
>> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
>> ---
>> drivers/tty/serial/8250_early.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/8250_early.c b/drivers/tty/serial/8250_early.c
>> index eaafb98..cfc46b7 100644
>> --- a/drivers/tty/serial/8250_early.c
>> +++ b/drivers/tty/serial/8250_early.c
>> @@ -140,7 +140,7 @@ static void __init init_port(struct early_serial8250_device *device)
>> serial_out(port, UART_FCR, 0); /* no fifo */
>> serial_out(port, UART_MCR, 0x3); /* DTR + RTS */
>>
>> - divisor = port->uartclk / (16 * device->baud);
>> + divisor = (port->uartclk + (8 * device->baud)) / (16 * device->baud);
> So this should be in fact DIV_ROUND_CLOSEST(), right?
>
> But anyway I'm missing any explanation of *why* this is needed? This
> should be part of the commit log. Is there any bug you are fixing here?
Actually the issue showed up when using the stock 8250 driver for
Synopsys DW UART. This was on a FPGA with ~50MHz clk. When we enabled
early serial, we saw garbage which Alexey narrowed down to the rounding
error. So the bug had been latent and it only showed up with such low
clk rates.
Thx,
Vineet
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tty/8250_early: Prevent rounding error in uartclk to baud ratio
2012-09-29 6:40 ` Vineet Gupta
@ 2012-09-29 7:07 ` Jiri Slaby
2012-09-29 11:04 ` Vineet Gupta
0 siblings, 1 reply; 8+ messages in thread
From: Jiri Slaby @ 2012-09-29 7:07 UTC (permalink / raw)
To: Vineet Gupta; +Cc: Alexey Brodkin, gregkh, alan, linux-serial, linux-kernel
On 09/29/2012 08:40 AM, Vineet Gupta wrote:
> On Friday 28 September 2012 06:32 PM, Jiri Slaby wrote:
>> On 09/28/2012 02:19 PM, Alexey Brodkin wrote:
>>> Modify divisor to select the nearest baud rate divider rather than the
>>> lowest. It minimizes baud rate errors especially on low UART clock
>>> frequencies.
>>>
>>> For example, if uartclk is 33000000 and baud is 115200 the ratio is
>>> about 17.9 The current code selects 17 (5% error) but should select 18
>>> (0.5% error).
>>>
>>> On the same lines as following:
>>> http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.9-rc3/2.6.9-rc3-mm2/broken-out/serial-pick-nearest-baud-rate-divider.patch
>>>
>>> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
>>> ---
>>> drivers/tty/serial/8250_early.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/tty/serial/8250_early.c b/drivers/tty/serial/8250_early.c
>>> index eaafb98..cfc46b7 100644
>>> --- a/drivers/tty/serial/8250_early.c
>>> +++ b/drivers/tty/serial/8250_early.c
>>> @@ -140,7 +140,7 @@ static void __init init_port(struct early_serial8250_device *device)
>>> serial_out(port, UART_FCR, 0); /* no fifo */
>>> serial_out(port, UART_MCR, 0x3); /* DTR + RTS */
>>>
>>> - divisor = port->uartclk / (16 * device->baud);
>>> + divisor = (port->uartclk + (8 * device->baud)) / (16 * device->baud);
>> So this should be in fact DIV_ROUND_CLOSEST(), right?
>>
>> But anyway I'm missing any explanation of *why* this is needed? This
>> should be part of the commit log. Is there any bug you are fixing here?
>
> Actually the issue showed up when using the stock 8250 driver for
> Synopsys DW UART. This was on a FPGA with ~50MHz clk. When we enabled
> early serial, we saw garbage which Alexey narrowed down to the rounding
> error. So the bug had been latent and it only showed up with such low
> clk rates.
Yes, this is a perfect argument I was asking for and should have been a
part of the commit...
thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tty/8250_early: Prevent rounding error in uartclk to baud ratio
2012-09-29 7:07 ` Jiri Slaby
@ 2012-09-29 11:04 ` Vineet Gupta
0 siblings, 0 replies; 8+ messages in thread
From: Vineet Gupta @ 2012-09-29 11:04 UTC (permalink / raw)
To: Jiri Slaby; +Cc: Alexey Brodkin, gregkh, alan, linux-serial, linux-kernel
On Saturday 29 September 2012 12:37 PM, Jiri Slaby wrote:
> On 09/29/2012 08:40 AM, Vineet Gupta wrote:
>> On Friday 28 September 2012 06:32 PM, Jiri Slaby wrote:
>>> On 09/28/2012 02:19 PM, Alexey Brodkin wrote:
>>>> Modify divisor to select the nearest baud rate divider rather than the
>>>> lowest. It minimizes baud rate errors especially on low UART clock
>>>> frequencies.
>>>>
>>>> For example, if uartclk is 33000000 and baud is 115200 the ratio is
>>>> about 17.9 The current code selects 17 (5% error) but should select 18
>>>> (0.5% error).
>>>>
>>>> On the same lines as following:
>>>> http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.9-rc3/2.6.9-rc3-mm2/broken-out/serial-pick-nearest-baud-rate-divider.patch
>>>>
>>>> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
>>>> ---
>>>> drivers/tty/serial/8250_early.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/tty/serial/8250_early.c b/drivers/tty/serial/8250_early.c
>>>> index eaafb98..cfc46b7 100644
>>>> --- a/drivers/tty/serial/8250_early.c
>>>> +++ b/drivers/tty/serial/8250_early.c
>>>> @@ -140,7 +140,7 @@ static void __init init_port(struct early_serial8250_device *device)
>>>> serial_out(port, UART_FCR, 0); /* no fifo */
>>>> serial_out(port, UART_MCR, 0x3); /* DTR + RTS */
>>>>
>>>> - divisor = port->uartclk / (16 * device->baud);
>>>> + divisor = (port->uartclk + (8 * device->baud)) / (16 * device->baud);
>>> So this should be in fact DIV_ROUND_CLOSEST(), right?
>>>
>>> But anyway I'm missing any explanation of *why* this is needed? This
>>> should be part of the commit log. Is there any bug you are fixing here?
>> Actually the issue showed up when using the stock 8250 driver for
>> Synopsys DW UART. This was on a FPGA with ~50MHz clk. When we enabled
>> early serial, we saw garbage which Alexey narrowed down to the rounding
>> error. So the bug had been latent and it only showed up with such low
>> clk rates.
> Yes, this is a perfect argument I was asking for and should have been a
> part of the commit...
First timer's anxiety I suppose ! Per your DIV_ROUND_CLOSEST()
suggestion, Alexey has already sent out a v2 patch. I presume he needs
to respin a v3 to beef up the commitlog along the lines of above !
Thx,
-Vineet
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tty/8250_early: Prevent rounding error in uartclk to baud ratio
2012-09-28 12:19 [PATCH] tty/8250_early: Prevent rounding error in uartclk to baud ratio Alexey Brodkin
2012-09-28 13:02 ` Jiri Slaby
@ 2012-09-28 13:03 ` Alan Cox
1 sibling, 0 replies; 8+ messages in thread
From: Alan Cox @ 2012-09-28 13:03 UTC (permalink / raw)
To: Alexey Brodkin; +Cc: gregkh, linux-serial, linux-kernel, Vineet.Gupta1
On Fri, 28 Sep 2012 16:19:08 +0400
Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote:
> Modify divisor to select the nearest baud rate divider rather than the
> lowest. It minimizes baud rate errors especially on low UART clock
> frequencies.
>
> For example, if uartclk is 33000000 and baud is 115200 the ratio is
> about 17.9 The current code selects 17 (5% error) but should select 18
> (0.5% error).
>
> On the same lines as following:
> http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.9-rc3/2.6.9-rc3-mm2/broken-out/serial-pick-nearest-baud-rate-divider.patch
>
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
Seems sensible to me.
Acked-by: Alan Cox <alan@linux.intel.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] tty/8250_early: Prevent rounding error in uartclk to baud ratio
@ 2012-09-28 12:03 Alexey Brodkin
2012-09-28 13:01 ` Jiri Slaby
0 siblings, 1 reply; 8+ messages in thread
From: Alexey Brodkin @ 2012-09-28 12:03 UTC (permalink / raw)
To: gregkh, alan, linux-serial, linux-kernel, Vineet.Gupta1,
Alexey.Brodkin
Modify divisor to select the nearest baud rate divider rather than the
lowest. It minimizes baud rate errors especially on low UART clock
frequencies.
For example, if uartclk is 33000000 and baud is 115200 the ratio is
about 17.9 The current code selects 17 (5% error) but should select 18
(0.5% error).
On the same lines as following:
http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.9-rc3/2.6.9-rc3-mm2/broken-out/serial-pick-nearest-baud-rate-divider.patch
Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
---
drivers/tty/serial/8250_early.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250_early.c b/drivers/tty/serial/8250_early.c
index eaafb98..cfc46b7 100644
--- a/drivers/tty/serial/8250_early.c
+++ b/drivers/tty/serial/8250_early.c
@@ -140,7 +140,7 @@ static void __init init_port(struct early_serial8250_device *device)
serial_out(port, UART_FCR, 0); /* no fifo */
serial_out(port, UART_MCR, 0x3); /* DTR + RTS */
- divisor = port->uartclk / (16 * device->baud);
+ divisor = (port->uartclk + (8 * device->baud)) / (16 * device->baud);
c = serial_in(port, UART_LCR);
serial_out(port, UART_LCR, c | UART_LCR_DLAB);
serial_out(port, UART_DLL, divisor & 0xff);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] tty/8250_early: Prevent rounding error in uartclk to baud ratio
2012-09-28 12:03 Alexey Brodkin
@ 2012-09-28 13:01 ` Jiri Slaby
0 siblings, 0 replies; 8+ messages in thread
From: Jiri Slaby @ 2012-09-28 13:01 UTC (permalink / raw)
To: Alexey Brodkin; +Cc: gregkh, alan, linux-serial, linux-kernel, Vineet.Gupta1
On 09/28/2012 02:03 PM, Alexey Brodkin wrote:
> Modify divisor to select the nearest baud rate divider rather than the
> lowest. It minimizes baud rate errors especially on low UART clock
> frequencies.
>
> For example, if uartclk is 33000000 and baud is 115200 the ratio is
> about 17.9 The current code selects 17 (5% error) but should select 18
> (0.5% error).
>
> On the same lines as following:
> http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.9-rc3/2.6.9-rc3-mm2/broken-out/serial-pick-nearest-baud-rate-divider.patch
>
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> ---
> drivers/tty/serial/8250_early.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250_early.c b/drivers/tty/serial/8250_early.c
> index eaafb98..cfc46b7 100644
> --- a/drivers/tty/serial/8250_early.c
> +++ b/drivers/tty/serial/8250_early.c
> @@ -140,7 +140,7 @@ static void __init init_port(struct early_serial8250_device *device)
> serial_out(port, UART_FCR, 0); /* no fifo */
> serial_out(port, UART_MCR, 0x3); /* DTR + RTS */
>
> - divisor = port->uartclk / (16 * device->baud);
> + divisor = (port->uartclk + (8 * device->baud)) / (16 * device->baud);
So this should be in fact DIV_ROUND_CLOSEST(), right?
But anyway I'm missing any explanation of *why* this is needed? This
should be part of the commit log. Is there any bug you are fixing here?
thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-09-29 11:04 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-28 12:19 [PATCH] tty/8250_early: Prevent rounding error in uartclk to baud ratio Alexey Brodkin
2012-09-28 13:02 ` Jiri Slaby
2012-09-29 6:40 ` Vineet Gupta
2012-09-29 7:07 ` Jiri Slaby
2012-09-29 11:04 ` Vineet Gupta
2012-09-28 13:03 ` Alan Cox
-- strict thread matches above, loose matches on Subject: below --
2012-09-28 12:03 Alexey Brodkin
2012-09-28 13:01 ` Jiri Slaby
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).