2014-11-26 17:48 GMT+08:00 Tobias Klauser <tklauser@xxxxxxxxxx>: > On 2014-11-25 at 13:16:58 +0100, Chunyan Zhang <chunyan.zhang@xxxxxxxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html