linux-kernel - Re: [PATCH] use callbacks to access UART_DLL/UART

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]

Message-ID: <yw1xoadpcbbn.fsf@unicorn.mansr.com>
Date:	Thu, 17 Dec 2015 16:02:52 +0000
From:	Måns Rullgård <mans@...sr.com>
To:	Sebastian Frias <sf84@...oste.net>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Magnus Damm <damm@...nsource.se>, linux-serial@...r.kernel.org,
	LKML <linux-kernel@...r.kernel.org>, Mason <slash.tmp@...e.fr>
Subject: Re: [PATCH] use callbacks to access UART_DLL/UART_DLM

Sebastian Frias <sf84@...oste.net> writes:

> ---
> resending as plain-text
> ---
>   drivers/tty/serial/8250/8250_core.c |   17 +++++++----------
>   1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_core.c 
> b/drivers/tty/serial/8250/8250_core.c
> index 2c46a21..9ca863c 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -791,22 +791,19 @@ static int size_fifo(struct uart_8250_port *up)
>    */
>   static unsigned int autoconfig_read_divisor_id(struct uart_8250_port *p)

What tree are you working against?  Current mainline has that function
in 8250_port.c.

>   {
> -    unsigned char old_dll, old_dlm, old_lcr;
> +    unsigned char old_lcr;
>       unsigned int id;
> +    unsigned int old_dl;

Your patch is messed up.  Consider using "git send-email" instead.

>       old_lcr = serial_in(p, UART_LCR);
> -    serial_out(p, UART_LCR, UART_LCR_CONF_MODE_A);
> -
> -    old_dll = serial_in(p, UART_DLL);
> -    old_dlm = serial_in(p, UART_DLM);
>
> -    serial_out(p, UART_DLL, 0);
> -    serial_out(p, UART_DLM, 0);
> +    serial_out(p, UART_LCR, UART_LCR_CONF_MODE_A);
>
> -    id = serial_in(p, UART_DLL) | serial_in(p, UART_DLM) << 8;
> +    old_dl = serial_dl_read(p);
> +    serial_dl_write(p, 0);
> +    id = serial_dl_read(p);
> +    serial_dl_write(p, old_dl);
>
> -    serial_out(p, UART_DLL, old_dll);
> -    serial_out(p, UART_DLM, old_dlm);
>       serial_out(p, UART_LCR, old_lcr);
>
>       return id;
> -- 

If you left the blank lines alone, the patch would end up much easier to
understand.  In this diff, some of the lines listed as added or removed
have actually not changed, and that's not immediately obvious.

-- 
Måns Rullgård
--
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/