Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756268AbbDGOp7 (ORCPT ); Tue, 7 Apr 2015 10:45:59 -0400 Received: from mail-wg0-f45.google.com ([74.125.82.45]:35906 "EHLO mail-wg0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755538AbbDGOp0 (ORCPT ); Tue, 7 Apr 2015 10:45:26 -0400 Date: Tue, 7 Apr 2015 16:45:21 +0200 From: Ingo Molnar To: Daniel J Blueman Cc: "H. Peter Anvin" , Thomas Gleixner , Ingo Molnar , x86@kernel.org, linux-kernel@vger.kernel.org, Steffen Persvold Subject: Re: [PATCH] x86: Fix earlyprintk ttyS2/3 Message-ID: <20150407144520.GA18999@gmail.com> References: <1428416957-19278-1-git-send-email-daniel@numascale.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1428416957-19278-1-git-send-email-daniel@numascale.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1882 Lines: 67 * Daniel J Blueman wrote: > Quite a few platforms use ttyS2 for their serial-over-LAN, so fix early > printk support for ttyS2 and 3, avoiding the need to hard-code the IO port. Nice. > > Signed-off-by: Daniel J Blueman > --- > arch/x86/boot/early_serial_console.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/boot/early_serial_console.c b/arch/x86/boot/early_serial_console.c > index 5df2869..4f4c2e6 100644 > --- a/arch/x86/boot/early_serial_console.c > +++ b/arch/x86/boot/early_serial_console.c > @@ -71,15 +71,16 @@ static void parse_earlyprintk(void) > else > pos = e - arg; > } else if (!strncmp(arg + pos, "ttyS", 4)) { > - static const int bases[] = { 0x3f8, 0x2f8 }; > - int idx = 0; > + static const int bases[] = {0x3f8, 0x2f8, 0x3e8, 0x2e8}; > + unsigned idx = 0; > > if (!strncmp(arg + pos, "ttyS", 4)) > pos += 4; Btw., that's a weird pattern: why do the strncmp() twice? We already know that we matched, in this branch. Could be written as: pos += 4; ? > > - if (arg[pos++] == '1') > - idx = 1; > + if ((arg[pos] - '0') < (sizeof(bases) / sizeof(bases[0]))) > + idx = arg[pos] - '0'; Nit: use ARRAY_SIZE()? Not so nit: with your change, a typo in boot parameters like "ttyS-" would underflow the index and dereference randomly around bases[] with a negative index: Also, higher indices like "ttyS4" will overflow beyond the end of the array. Please add proper bounds checking and warning messages. > > + pos++; > port = bases[idx]; > } Thanks, Ingo -- 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/