Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751608AbbEZPIf (ORCPT ); Tue, 26 May 2015 11:08:35 -0400 Received: from mga14.intel.com ([192.55.52.115]:19694 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750726AbbEZPIb (ORCPT ); Tue, 26 May 2015 11:08:31 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,498,1427785200"; d="scan'208";a="731863179" From: "Shevchenko, Andriy" To: "alan@linux.intel.com" CC: "manabian@gmail.com" , "linux-kernel@vger.kernel.org" , "blogic@openwrt.org" , "bigeasy@linutronix.de" , "alan@lxorguk.ukuu.org.uk" , "ricardo.ribalda@gmail.com" , "yamada.masahiro@socionext.com" , "linux-serial@vger.kernel.org" , "gregkh@linuxfoundation.org" , "jslaby@suse.cz" , "matthias.bgg@gmail.com" , "peter@hurleysoftware.com" , "linux-arm-kernel@lists.infradead.org" Subject: Re: [PATCH v6] serial: 8250_uniphier: add UniPhier serial driver Thread-Topic: [PATCH v6] serial: 8250_uniphier: add UniPhier serial driver Thread-Index: AQHQl8BBSViTbfCFTEeK18jo2NRp8Z2OS10A Date: Tue, 26 May 2015 15:08:27 +0000 Message-ID: <1432652906.8736.44.camel@intel.com> References: <1432525477-14051-1-git-send-email-yamada.masahiro@socionext.com> <1432545245.8736.5.camel@intel.com> <1432650518.13900.72.camel@linux.intel.com> In-Reply-To: <1432650518.13900.72.camel@linux.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.237.72.90] Content-Type: text/plain; charset="utf-8" Content-ID: MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by nfs id t4QF93vR012498 Content-Length: 1835 Lines: 45 On Tue, 2015-05-26 at 15:28 +0100, Alan Cox wrote: > > > + > > > +#define UNIPHIER_UART_CHAR_FCR 3 /* Character / FIFO Control Register */ > > > +#define UNIPHIER_UART_LCR_MCR 4 /* Line/Modem Control Register */ > > > +#define UNIPHIER_UART_LCR_SHIFT 8 > > > > Indentation problem, needs to be fixed. > > If you are going to review a patch set at least look at the previous > reviews - the indentation was already discussed and is done that way to > show (as many drivers do) which are fields for which registers This is not exactly the field, the way how to get the field. In some cases it is even better to define something like _LCR(x) ((x) << 8) > > > > +static unsigned int uniphier_serial_in(struct uart_port *p, int offset) > > > +{ > > > + int valshift = 0; > > > > Perhaps unsigned int? > > Why ? even if it mattered gcc is already realising that the value can > only be 0 or 8 and will be generating whatever works best for that. It's not about how gcc does, it's about what assumptions can be made from the reading of the source code. I think if we do a counter of shift value it would be nice to set an unsigned type explicitly. -- Andy Shevchenko Intel Finland Oy --------------------------------------------------------------------- Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?