| Message ID | 1307503005-3969-1-git-send-email-richard.zhu@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 08, 2011 at 11:16:45AM +0800, Richard Zhu wrote: > Eanble the ADMA2 mode on freescale esdhc imx driver, > tested on MX51 and MX53. What does the new, vendor specific bit in the interrupt register cover what the old ADMA_ERROR bit did not cover? And is the old one the same as in mx25, so the new bit can be seen as the fixup which makes ADMA work only on mx51/53? Information like this should be added to the define, so people will clearly know later. > > Only ADMA2 mode is enabled, MX25/35 can't support the ADMA2 mode. > So this patch is only used for MX51/53. > The ADMA2 mode supported or not can be distinguished by the > Capability Register(offset 0x40) of eSDHC module. > Up to now, only MX51/MX53 set the ADMA2 supported bit(Bit20) in the > Capability Register. > > Signed-off-by: Richard Zhu <richard.zhu@linaro.org> > --- > drivers/mmc/host/sdhci-esdhc-imx.c | 36 +++++++++++++++++++++++++++++++----- > 1 files changed, 31 insertions(+), 5 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c > index a19967d..8015e1a 100644 > --- a/drivers/mmc/host/sdhci-esdhc-imx.c > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > @@ -31,6 +31,8 @@ > #define SDHCI_VENDOR_SPEC 0xC0 > #define SDHCI_VENDOR_SPEC_SDIO_QUIRK 0x00000002 > > +#define SDHCI_SPEC_INT_ADMA_ERR 0x10000000 There is no register SDHCI_SPEC_*. I'd suggest "SDHCI_INT_VENDOR_SPEC_ADMA_ERROR". > + if (unlikely(reg == SDHCI_CAPABILITIES)) { > + if (val & SDHCI_CAN_DO_ADMA1) { > + val &= ~SDHCI_CAN_DO_ADMA1; > + val |= SDHCI_CAN_DO_ADMA2; Why is this needed? Above you say, MX51/53 has ADMA2 bit already set? And you would set ADMA2 for mx25/35 now? > @@ -154,7 +177,7 @@ static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg) > > static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg) > { > - u32 new_val; > + u16 new_val; Why? esdhc_clrset_le wants a u32. > > switch (reg) { > case SDHCI_POWER_CONTROL: > @@ -168,10 +191,12 @@ static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg) > new_val = val & (SDHCI_CTRL_LED | SDHCI_CTRL_4BITBUS); > /* ensure the endianess */ > new_val |= ESDHC_HOST_CONTROL_LE; > - /* DMA mode bits are shifted */ > - new_val |= (val & SDHCI_CTRL_DMA_MASK) << 5; > + if (val & SDHCI_CTRL_DMA_MASK) > + /* DMA mode bits are shifted */ > + new_val |= (val & SDHCI_CTRL_DMA_MASK) << 5; I think the if doesn't save much than just doing this simple operation; it also adds another line and level of indentation. Regards, Wolfram
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c index a19967d..8015e1a 100644 --- a/drivers/mmc/host/sdhci-esdhc-imx.c +++ b/drivers/mmc/host/sdhci-esdhc-imx.c @@ -31,6 +31,8 @@ #define SDHCI_VENDOR_SPEC 0xC0 #define SDHCI_VENDOR_SPEC_SDIO_QUIRK 0x00000002 +#define SDHCI_SPEC_INT_ADMA_ERR 0x10000000 + #define ESDHC_FLAG_GPIO_FOR_CD_WP (1 << 0) /* * The CMDTYPE of the CMD register (offset 0xE) should be set to @@ -80,6 +82,20 @@ static u32 esdhc_readl_le(struct sdhci_host *host, int reg) val |= SDHCI_CARD_PRESENT; } + if (unlikely(reg == SDHCI_CAPABILITIES)) { + if (val & SDHCI_CAN_DO_ADMA1) { + val &= ~SDHCI_CAN_DO_ADMA1; + val |= SDHCI_CAN_DO_ADMA2; + } + } + + if (unlikely(reg == SDHCI_INT_STATUS)) { + if (val & SDHCI_SPEC_INT_ADMA_ERR) { + val &= ~SDHCI_SPEC_INT_ADMA_ERR; + val |= SDHCI_INT_ADMA_ERROR; + } + } + return val; } @@ -105,6 +121,13 @@ static void esdhc_writel_le(struct sdhci_host *host, u32 val, int reg) writel(v, host->ioaddr + SDHCI_VENDOR_SPEC); } + if (unlikely((reg == SDHCI_INT_ENABLE) + || (reg == SDHCI_SIGNAL_ENABLE))) { + if (val & SDHCI_INT_ADMA_ERROR) { + val &= ~SDHCI_INT_ADMA_ERROR; + val |= SDHCI_SPEC_INT_ADMA_ERR; + } + } writel(val, host->ioaddr + reg); } @@ -154,7 +177,7 @@ static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg) static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg) { - u32 new_val; + u16 new_val; switch (reg) { case SDHCI_POWER_CONTROL: @@ -168,10 +191,12 @@ static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg) new_val = val & (SDHCI_CTRL_LED | SDHCI_CTRL_4BITBUS); /* ensure the endianess */ new_val |= ESDHC_HOST_CONTROL_LE; - /* DMA mode bits are shifted */ - new_val |= (val & SDHCI_CTRL_DMA_MASK) << 5; + if (val & SDHCI_CTRL_DMA_MASK) + /* DMA mode bits are shifted */ + new_val |= (val & SDHCI_CTRL_DMA_MASK) << 5; esdhc_clrset_le(host, 0xffff, new_val, reg); + return; } esdhc_clrset_le(host, 0xff, val, reg); @@ -322,9 +347,10 @@ static void esdhc_pltfm_exit(struct sdhci_host *host) } struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = { - .quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_ADMA + .quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_NO_HISPD_BIT + | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC + | SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC | SDHCI_QUIRK_BROKEN_CARD_DETECTION, - /* ADMA has issues. Might be fixable */ .ops = &sdhci_esdhc_ops, .init = esdhc_pltfm_init, .exit = esdhc_pltfm_exit,
Eanble the ADMA2 mode on freescale esdhc imx driver, tested on MX51 and MX53. Only ADMA2 mode is enabled, MX25/35 can't support the ADMA2 mode. So this patch is only used for MX51/53. The ADMA2 mode supported or not can be distinguished by the Capability Register(offset 0x40) of eSDHC module. Up to now, only MX51/MX53 set the ADMA2 supported bit(Bit20) in the Capability Register. Signed-off-by: Richard Zhu <richard.zhu@linaro.org> --- drivers/mmc/host/sdhci-esdhc-imx.c | 36 +++++++++++++++++++++++++++++++----- 1 files changed, 31 insertions(+), 5 deletions(-)