From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161395AbcFMXcI (ORCPT ); Mon, 13 Jun 2016 19:32:08 -0400 Received: from mail-io0-f196.google.com ([209.85.223.196]:33216 "EHLO mail-io0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753315AbcFMXcF (ORCPT ); Mon, 13 Jun 2016 19:32:05 -0400 MIME-Version: 1.0 In-Reply-To: References: From: Julian Calaby Date: Tue, 14 Jun 2016 09:31:44 +1000 Message-ID: Subject: Re: [linux-sunxi] [PATCH v3 07/13] spi: sunxi: rename constants to match between sun4i and sun6i To: Michal Suchanek Cc: linux-sunxi , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Maxime Ripard , Chen-Yu Tsai , Russell King , Mark Brown , Arnd Bergmann , Olof Johansson , Krzysztof Kozlowski , Javier Martinez Canillas , Simon Horman , Sjoerd Simons , Thierry Reding , Alison Wang , Timo Sigurdsson , Jonathan Liu , Gerhard Bertelsmann , Priit Laes , devicetree , "Mailing List, Arm" , "linux-kernel@vger.kernel.org" , linux-spi Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Michal, On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek wrote: > SUNXI_CTL_ -> SUNXI_TFR_CTL_ > SUNXI_TFR_CTL_LMTF -> SUNXI_TFR_CTL_FBS I don't know these abbreviations, are they both referring to the same thing? > SUNXI_TFR_CTL_CS_ACTIVE_LOW -> SUNXI_TFR_CTL_SPOL It looks like you're making the constant name less descriptive here. Is the old version (CS_ACTIVE_LOW) incorrect? > and some SUNXI_???_CTL_ -> SUNXI_CTL_ > for constants migrated to different registers between sun4i and sun6i > > No functional change. > > Signed-off-by: Michal Suchanek > --- > drivers/spi/spi-sun4i.c | 68 ++++++++++++++++++++++++------------------------- > drivers/spi/spi-sun6i.c | 14 +++++----- > 2 files changed, 41 insertions(+), 41 deletions(-) > > diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c > index 155d720..b7f8de1 100644 > --- a/drivers/spi/spi-sun4i.c > +++ b/drivers/spi/spi-sun4i.c > @@ -28,21 +28,21 @@ > > #define SUNXI_TXDATA_REG 0x04 > > -#define SUNXI_CTL_REG 0x08 > +#define SUNXI_TFR_CTL_REG 0x08 > #define SUNXI_CTL_ENABLE BIT(0) > #define SUNXI_CTL_MASTER BIT(1) > -#define SUNXI_CTL_CPHA BIT(2) > -#define SUNXI_CTL_CPOL BIT(3) > -#define SUNXI_CTL_CS_ACTIVE_LOW BIT(4) > -#define SUNXI_CTL_LMTF BIT(6) > +#define SUNXI_TFR_CTL_CPHA BIT(2) > +#define SUNXI_TFR_CTL_CPOL BIT(3) > +#define SUNXI_TFR_CTL_SPOL BIT(4) > +#define SUNXI_TFR_CTL_FBS BIT(6) > #define SUNXI_CTL_TF_RST BIT(8) > #define SUNXI_CTL_RF_RST BIT(9) > -#define SUNXI_CTL_XCH BIT(10) > -#define SUNXI_CTL_CS_MASK 0x3000 > -#define SUNXI_CTL_CS(cs) (((cs) << 12) & SUNXI_CTL_CS_MASK) > -#define SUNXI_CTL_DHB BIT(15) > -#define SUNXI_CTL_CS_MANUAL BIT(16) > -#define SUNXI_CTL_CS_LEVEL BIT(17) > +#define SUNXI_TFR_CTL_XCH BIT(10) > +#define SUNXI_TFR_CTL_CS_MASK 0x3000 > +#define SUNXI_TFR_CTL_CS(cs) (((cs) << 12) & SUNXI_TFR_CTL_CS_MASK) > +#define SUNXI_TFR_CTL_DHB BIT(15) > +#define SUNXI_TFR_CTL_CS_MANUAL BIT(16) > +#define SUNXI_TFR_CTL_CS_LEVEL BIT(17) > #define SUNXI_CTL_TP BIT(18) > > #define SUNXI_INT_CTL_REG 0x0c > diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c > index a27bf8f..f26b52a 100644 > --- a/drivers/spi/spi-sun6i.c > +++ b/drivers/spi/spi-sun6i.c > @@ -26,9 +26,9 @@ > #define SUNXI_FIFO_DEPTH 128 > > #define SUNXI_GBL_CTL_REG 0x04 > -#define SUNXI_GBL_CTL_BUS_ENABLE BIT(0) > -#define SUNXI_GBL_CTL_MASTER BIT(1) > -#define SUNXI_GBL_CTL_TP BIT(7) > +#define SUNXI_CTL_ENABLE BIT(0) > +#define SUNXI_CTL_MASTER BIT(1) > +#define SUNXI_CTL_TP BIT(7) If these are bit definitions for the GBL register, why throw that information away? > #define SUNXI_GBL_CTL_RST BIT(31) > > #define SUNXI_TFR_CTL_REG 0x08 > @@ -50,8 +50,8 @@ > #define SUNXI_INT_STA_REG 0x14 > > #define SUNXI_FIFO_CTL_REG 0x18 > -#define SUNXI_FIFO_CTL_RF_RST BIT(15) > -#define SUNXI_FIFO_CTL_TF_RST BIT(31) > +#define SUNXI_CTL_RF_RST BIT(15) > +#define SUNXI_CTL_TF_RST BIT(31) Same here with FIFO. > > #define SUNXI_FIFO_STA_REG 0x1c > #define SUNXI_FIFO_STA_RF_CNT_MASK 0x7f My gut feeling on this is that we have a lot of cases of a definition of a register offset, then definitions of the bits in that register with that register encoded into the constant's name. You appear to be throwing a lot of that information away which makes me worry. Thanks, -- Julian Calaby Email: julian.calaby@gmail.com Profile: http://www.google.com/profiles/julian.calaby/