Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756441AbcJHNLe (ORCPT ); Sat, 8 Oct 2016 09:11:34 -0400 Received: from mail-qt0-f193.google.com ([209.85.216.193]:34997 "EHLO mail-qt0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752511AbcJHNLY (ORCPT ); Sat, 8 Oct 2016 09:11:24 -0400 MIME-Version: 1.0 In-Reply-To: <1475794457-17993-1-git-send-email-aaron.brice@datasoft.com> References: <1475794457-17993-1-git-send-email-aaron.brice@datasoft.com> From: Dong Aisheng Date: Sat, 8 Oct 2016 21:10:37 +0800 Message-ID: Subject: Re: [PATCH] sdhci-esdhc-imx: Correct two register accesses To: Aaron Brice Cc: david.russell@datasoft.com, Adrian Hunter , Ulf Hansson , Dong Aisheng , "linux-mmc@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" 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: 3337 Lines: 86 On Fri, Oct 7, 2016 at 6:54 AM, Aaron Brice wrote: > - The DMA error interrupt bit is in a different position as > compared to the sdhci standard. This is accounted for in > many cases, but not handled in the case of clearing the > INT_STATUS register by writing a 1 to that location. > - The HOST_CONTROL register is very different as compared to > the sdhci standard. This is accounted for in the write > case, but not when read back out (which it is in the sdhci > code). > > Signed-off-by: Dave Russell > Signed-off-by: Aaron Brice > --- > drivers/mmc/host/sdhci-esdhc-imx.c | 23 ++++++++++++++++++++++- > 1 file changed, 22 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c > index 1f54fd8..d61ef16 100644 > --- a/drivers/mmc/host/sdhci-esdhc-imx.c > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > @@ -346,7 +346,8 @@ static void esdhc_writel_le(struct sdhci_host *host, u32 val, int reg) > struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host); > u32 data; > > - if (unlikely(reg == SDHCI_INT_ENABLE || reg == SDHCI_SIGNAL_ENABLE)) { > + if (unlikely(reg == SDHCI_INT_ENABLE || reg == SDHCI_SIGNAL_ENABLE || > + reg == SDHCI_INT_STATUS)) { > if ((val & SDHCI_INT_CARD_INT) && !esdhc_is_usdhc(imx_data)) { > /* > * Clear and then set D3CD bit to avoid missing the > @@ -555,6 +556,25 @@ static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg) > esdhc_clrset_le(host, 0xffff, val, reg); > } > > +static u8 esdhc_readb_le(struct sdhci_host *host, int reg) > +{ > + u8 ret; > + u32 long_val; > + > + switch (reg) { > + case SDHCI_HOST_CONTROL: > + long_val = readl(host->ioaddr + reg); > + > + ret = long_val & SDHCI_CTRL_LED; > + ret |= (long_val >> 5) & SDHCI_CTRL_DMA_MASK; > + ret |= (long_val & ESDHC_CTRL_4BITBUS); > + ret |= (long_val & ESDHC_CTRL_8BITBUS) << 3; > + return ret; Thanks for the effort. One nitpick: would be more like to use 'val' instead of 'long_val' to be consistent with exist using. (i saw a few 'new_val' as well, maybe we could clean up them in the future, but at least we could avoid inventing more from now) Otherwise, Acked-by: Dong Aisheng Regards Dong Aisheng > + } > + > + return readb(host->ioaddr + reg); > +} > + > static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg) > { > struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > @@ -947,6 +967,7 @@ static void esdhc_set_timeout(struct sdhci_host *host, struct mmc_command *cmd) > static struct sdhci_ops sdhci_esdhc_ops = { > .read_l = esdhc_readl_le, > .read_w = esdhc_readw_le, > + .read_b = esdhc_readb_le, > .write_l = esdhc_writel_le, > .write_w = esdhc_writew_le, > .write_b = esdhc_writeb_le, > -- > 2.7.4 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel