Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755819AbbLEBci (ORCPT ); Fri, 4 Dec 2015 20:32:38 -0500 Received: from mail-ig0-f173.google.com ([209.85.213.173]:37688 "EHLO mail-ig0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751942AbbLEBcg (ORCPT ); Fri, 4 Dec 2015 20:32:36 -0500 MIME-Version: 1.0 In-Reply-To: References: <20151118083455.331768508@telegraphics.com.au> <1449183781-2163-1-git-send-email-linux@rainbow-software.org> From: Julian Calaby Date: Sat, 5 Dec 2015 12:32:16 +1100 Message-ID: Subject: Re: [RFC PATCH 76/71] ncr5380: Enable PDMA for DTC chips To: Finn Thain Cc: Ondrej Zary , Michael Schmitz , linux-m68k@vger.kernel.org, linux-scsi , linux-kernel Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2437 Lines: 70 Hi Finn, On Fri, Dec 4, 2015 at 7:38 PM, Finn Thain wrote: > > On Fri, 4 Dec 2015, Julian Calaby wrote: > >> > - if (overrides[current_override].board == BOARD_NCR53C400A) { >> > + if (overrides[current_override].board == BOARD_NCR53C400A || >> > + overrides[current_override].board == BOARD_DTC3181E) { >> >> These if statements are starting to get a bit long, would it make >> sense to replace them with a flag or equivalent? > > To what end? Shorter lines? As in, Pretty much, each expression is quite long and they seem to be growing fairly rapidly as you and Ondrej discover similar boards. > > if (board_is_ncr53c400a || board_is_dtc3181e) { > /* ... */ > } > > I suppose that could be an improvement if new flags would entirely replace > the override.board struct member and the existing switch statement, > > switch (overrides[current_override].board) { > /* ... */ > } > > Or maybe you meant testing a new flag something like this, > > if (hostdata->ncr53c400_compatible) { > /* ... */ > } > > If your concern is the Don't Repeat Yourself rule, I'm not sure that new > flag would get tested more than once (?) And it would still have to be > assigned using an "objectionably" long expression, e.g. > > hostdata->ncr53c400_compatible = > overrides[current_override].board == BOARD_NCR53C400 || > overrides[current_override].board == BOARD_NCR53C400A || > overrides[current_override].board == BOARD_DTC3181E; > > Rather than add new flags, perhaps a 'switch' statement instead of an 'if' > statement would be shorter (if the size of the expression is the problem). I think switch statements would be cleaner in this particular instance. I was thinking something like: if (somthing->flags & NCR53C400_COMPATIBLE) { /* ... */ } but if it's only ever going to be used once, then it's pretty pointless and switch statements are cleaner. Thanks, -- Julian Calaby Email: julian.calaby@gmail.com Profile: http://www.google.com/profiles/julian.calaby/ -- 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/