[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <4BA7514D.3020901@evidence.eu.com>
Date: Mon, 22 Mar 2010 12:15:25 +0100
From: Claudio Scordino <claudio@...dence.eu.com>
To: Ryan Mallon <ryan@...ewatersys.com>
CC: Linux Kernel <linux-kernel@...r.kernel.org>,
Rick Bronson <rick@....org>, John Nicholls <john@...nlinx.com>,
Sebastian Heutling <Sebastian.Heutling@...-ing.de>,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
michael trimarchi <michael@...dence.eu.com>,
alan@...rguk.ukuu.org.uk
Subject: Re: [PATCH] atmel_serial: Atmel RS485 support
Hi Ryan,
first of all, thank you for your feedback.
To me, all comments seem to be reasonable, except maybe the following one:
>>
>> +static void atmel_start_rx(struct uart_port *port);
>> +static void atmel_stop_rx(struct uart_port *port);
>
> Can you move these functions, so that these declarations are not needed?
Actually, atmel_stop_rx is already defined, and I prefer to not move it
(since I change it, people may get confused by the diff...).
I also think that atmel_start_rx (which is added by the patch) should be
near to atmel_stop_rx...
>> + UART_PUT_IDR(port, ATMEL_US_ENDRX | ATMEL_US_TIMEOUT |
>> port->read_status_mask);
>
> This line, and a couple of others, extend over 80 characters. Could you
> please split them up.
Of course. I'm going to split them up and use a backslash at the end of
the original lines.
I will create a new patch according to your comments, test it and submit
again to the mailing list.
Many thanks again,
Claudio
--
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/
Powered by blists - more mailing lists