linux-kernel - Re: [PATCH] tty/8250_early: Prevent rounding error in uartclk to baud ratio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 29 Sep 2012 12:10:28 +0530
From:	Vineet Gupta <Vineet.Gupta1@...opsys.com>
To:	Jiri Slaby <jslaby@...e.cz>
CC:	Alexey Brodkin <Alexey.Brodkin@...opsys.com>,
	<gregkh@...uxfoundation.org>, <alan@...ux.intel.com>,
	<linux-serial@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<Vineet.Gupta1@...opsys.com>
Subject: Re: [PATCH] tty/8250_early: Prevent rounding error in uartclk to
 baud ratio

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@...opsys.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/