From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752313AbdJSIot (ORCPT ); Thu, 19 Oct 2017 04:44:49 -0400 Received: from mga03.intel.com ([134.134.136.65]:1664 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751681AbdJSIop (ORCPT ); Thu, 19 Oct 2017 04:44:45 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,400,1503385200"; d="scan'208";a="325063311" Subject: Re: [PATCH] mmc: sdhci-cadence: use bitfield access macros for cleanup To: Masahiro Yamada , linux-mmc@vger.kernel.org Cc: Piotr Sroka , linux-kernel@vger.kernel.org, Ulf Hansson References: <1507899031-1245-1-git-send-email-yamada.masahiro@socionext.com> From: Adrian Hunter Organization: Intel Finland Oy, Registered Address: PL 281, 00181 Helsinki, Business Identity Code: 0357606 - 4, Domiciled in Helsinki Message-ID: <2ed276f9-5869-93ed-766f-a122f4942591@intel.com> Date: Thu, 19 Oct 2017 11:37:53 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <1507899031-1245-1-git-send-email-yamada.masahiro@socionext.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 13/10/17 15:50, Masahiro Yamada wrote: > Accessing register fields generally need mask and shift part. > Defining them separately, like SDHCI_CDNS_HRS06_TUNE_{SHIFT,MASK}, > is tedious. > > Register fields can be always defined by GENMASK (or, BIT if it it > a single bit). They are nicely handled by FIELD_* macros. > > Signed-off-by: Masahiro Yamada Acked-by: Adrian Hunter > --- > > drivers/mmc/host/sdhci-cadence.c | 28 ++++++++++++++-------------- > 1 file changed, 14 insertions(+), 14 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c > index 56529c3..0f589e2 100644 > --- a/drivers/mmc/host/sdhci-cadence.c > +++ b/drivers/mmc/host/sdhci-cadence.c > @@ -13,6 +13,7 @@ > * GNU General Public License for more details. > */ > > +#include > #include > #include > #include > @@ -27,15 +28,14 @@ > #define SDHCI_CDNS_HRS04_ACK BIT(26) > #define SDHCI_CDNS_HRS04_RD BIT(25) > #define SDHCI_CDNS_HRS04_WR BIT(24) > -#define SDHCI_CDNS_HRS04_RDATA_SHIFT 16 > -#define SDHCI_CDNS_HRS04_WDATA_SHIFT 8 > -#define SDHCI_CDNS_HRS04_ADDR_SHIFT 0 > +#define SDHCI_CDNS_HRS04_RDATA GENMASK(23, 16) > +#define SDHCI_CDNS_HRS04_WDATA GENMASK(15, 8) > +#define SDHCI_CDNS_HRS04_ADDR GENMASK(5, 0) > > #define SDHCI_CDNS_HRS06 0x18 /* eMMC control */ > #define SDHCI_CDNS_HRS06_TUNE_UP BIT(15) > -#define SDHCI_CDNS_HRS06_TUNE_SHIFT 8 > -#define SDHCI_CDNS_HRS06_TUNE_MASK 0x3f > -#define SDHCI_CDNS_HRS06_MODE_MASK 0x7 > +#define SDHCI_CDNS_HRS06_TUNE GENMASK(13, 8) > +#define SDHCI_CDNS_HRS06_MODE GENMASK(2, 0) > #define SDHCI_CDNS_HRS06_MODE_SD 0x0 > #define SDHCI_CDNS_HRS06_MODE_MMC_SDR 0x2 > #define SDHCI_CDNS_HRS06_MODE_MMC_DDR 0x3 > @@ -105,8 +105,8 @@ static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, > u32 tmp; > int ret; > > - tmp = (data << SDHCI_CDNS_HRS04_WDATA_SHIFT) | > - (addr << SDHCI_CDNS_HRS04_ADDR_SHIFT); > + tmp = FIELD_PREP(SDHCI_CDNS_HRS04_WDATA, data) | > + FIELD_PREP(SDHCI_CDNS_HRS04_ADDR, addr); > writel(tmp, reg); > > tmp |= SDHCI_CDNS_HRS04_WR; > @@ -189,8 +189,8 @@ static void sdhci_cdns_set_emmc_mode(struct sdhci_cdns_priv *priv, u32 mode) > > /* The speed mode for eMMC is selected by HRS06 register */ > tmp = readl(priv->hrs_addr + SDHCI_CDNS_HRS06); > - tmp &= ~SDHCI_CDNS_HRS06_MODE_MASK; > - tmp |= mode; > + tmp &= ~SDHCI_CDNS_HRS06_MODE; > + tmp |= FIELD_PREP(SDHCI_CDNS_HRS06_MODE, mode); > writel(tmp, priv->hrs_addr + SDHCI_CDNS_HRS06); > } > > @@ -199,7 +199,7 @@ static u32 sdhci_cdns_get_emmc_mode(struct sdhci_cdns_priv *priv) > u32 tmp; > > tmp = readl(priv->hrs_addr + SDHCI_CDNS_HRS06); > - return tmp & SDHCI_CDNS_HRS06_MODE_MASK; > + return FIELD_GET(SDHCI_CDNS_HRS06_MODE, tmp); > } > > static void sdhci_cdns_set_uhs_signaling(struct sdhci_host *host, > @@ -254,12 +254,12 @@ static int sdhci_cdns_set_tune_val(struct sdhci_host *host, unsigned int val) > void __iomem *reg = priv->hrs_addr + SDHCI_CDNS_HRS06; > u32 tmp; > > - if (WARN_ON(val > SDHCI_CDNS_HRS06_TUNE_MASK)) > + if (WARN_ON(!FIELD_FIT(SDHCI_CDNS_HRS06_TUNE, val))) > return -EINVAL; > > tmp = readl(reg); > - tmp &= ~(SDHCI_CDNS_HRS06_TUNE_MASK << SDHCI_CDNS_HRS06_TUNE_SHIFT); > - tmp |= val << SDHCI_CDNS_HRS06_TUNE_SHIFT; > + tmp &= ~SDHCI_CDNS_HRS06_TUNE; > + tmp |= FIELD_PREP(SDHCI_CDNS_HRS06_TUNE, val); > tmp |= SDHCI_CDNS_HRS06_TUNE_UP; > writel(tmp, reg); > >