Messages in this thread | | | Date | Thu, 27 Nov 2014 19:39:36 +0800 | Subject | Re: [PATCH v3 5/5] tty/serial: Add Spreadtrum sc9836-uart driver support | From | Lyra Zhang <> |
| |
2014-11-26 17:48 GMT+08:00 Tobias Klauser <tklauser@distanz.ch>: > On 2014-11-25 at 13:16:58 +0100, Chunyan Zhang <chunyan.zhang@spreadtrum.com> wrote:
>> --- [...] >> + >> +config SERIAL_SPRD_CONSOLE >> + bool "SPRD UART console support" >> + depends on SERIAL_SPRD=y >> + select SERIAL_CORE_CONSOLE >> + select SERIAL_EARLYCON >> + help >> + Support for early debug console using Spreadtrum's serial. This enables >> + the console before standard serial driver is probed. This is enabled >> + with "earlycon" on the kernel command line. The console is >> + enabled when early_param is processed. >> + >> endmenu > > Please consistently use tabs instead of spaces for indentation. The help > text should be indented by one tabe + 2 spaces. >
OK, I will note it later. [...]
>> +static inline int handle_lsr_errors(struct uart_port *port, >> + unsigned int *flag, unsigned int *lsr) > > This line should be aligned with the opening ( above. > OK, will change.
>> +static inline void sprd_rx(int irq, void *dev_id) >> +{ >> + struct uart_port *port = (struct uart_port *)dev_id; > > No need to cast a void pointer. >
>> +static void sprd_console_write(struct console *co, const char *s, >> + unsigned int count) >> +{ >> + struct uart_port *port = (struct uart_port *)sprd_port[co->index]; > > Better explicitly access the .port member of sprd_port[co->index] here > instead of casting. >
>> + port = (struct uart_port *)sprd_port[co->index]; > > Same here, use the .port member of struct sprd_port[co->index]. > >> + if (port == NULL) { >> + pr_info("srial port %d not yet initialized\n", co->index); > > Typo: should be serial instead of srial. >
>> + up->mapbase = mem->start; >> + up->membase = ioremap(mem->start, resource_size(mem)); > > Return value of ioremap() should be checked for NULL. > >> +static int sprd_resume(struct platform_device *dev) >> +{ >> + int id = dev->id; >> + struct uart_port *port = (struct uart_port *)sprd_port[id]; > > Access the .port member instead of the cast. > OK, will change all of the problems you pointed out in v4. Thanks for your review.
Chunyan
| |