Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754821AbaK0Ljl (ORCPT ); Thu, 27 Nov 2014 06:39:41 -0500 Received: from mail-wi0-f172.google.com ([209.85.212.172]:58134 "EHLO mail-wi0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754102AbaK0Lji (ORCPT ); Thu, 27 Nov 2014 06:39:38 -0500 MIME-Version: 1.0 In-Reply-To: <20141126094838.GB17980@distanz.ch> References: <1416917818-10506-1-git-send-email-chunyan.zhang@spreadtrum.com> <1416917818-10506-6-git-send-email-chunyan.zhang@spreadtrum.com> <20141126094838.GB17980@distanz.ch> Date: Thu, 27 Nov 2014 19:39:36 +0800 Message-ID: Subject: Re: [PATCH v3 5/5] tty/serial: Add Spreadtrum sc9836-uart driver support From: Lyra Zhang To: Tobias Klauser Cc: Chunyan Zhang , Grant Likely , "robh+dt@kernel.org" , Catalin Marinas , "gregkh@linuxfoundation.org" , "ijc+devicetree@hellion.org.uk" , "jslaby@suse.cz" , Kumar Gala , Mark Brown , Mark Rutland , "m-karicheri2@ti.com" , Pawel Moll , Ramkumar Ramachandra , "rrichter@cavium.com" , Will Deacon , Arnd Bergmann , gnomes@lxorguk.ukuu.org.uk, Jonathan Corbet , jason@lakedaemon.net, Mark Brown , =?UTF-8?Q?Heiko_St=C3=BCbner?= , shawn.guo@freescale.com, florian.vaussard@epfl.ch, andrew@lunn.ch, Hayato Suzuki , Orson Zhai , "geng.ren@spreadtrum.com" , "zhizhou.zhang" , lanqing.liu@spreadtrum.com, =?UTF-8?B?V2VpIFFpYW8gKOS5lOS8nyk=?= , "devicetree@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "sprdlinux@freelists.org" , linux-doc@vger.kernel.org, linux-serial@vger.kernel.org, linux-api@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2014-11-26 17:48 GMT+08:00 Tobias Klauser : > On 2014-11-25 at 13:16:58 +0100, Chunyan Zhang 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-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/