Re: [PATCH 2/4] mmc: sdhci-esdhc-imx: Fix DAT line software reset

From: Adrian Hunter
Date: Mon May 29 2017 - 04:08:48 EST


On 03/05/17 13:05, BenoÃt ThÃbaudeau wrote:
> On i.MX25, the eSDHC DAT line software reset (SYSCTL.RSTD) unexpectedly
> clears at least the data transfer width (PROCTL.DTW), which then results
> in data CRC errors. This behavior is not documented, but it has actually
> been observed. Consequently, the DAT line software resets triggered by
> sdhci.c in case of errors caused unrecoverable errors.
>
> Fix this by making sure that the DAT line software reset does not alter
> the Host Control register. This behavior being undocumented, it may also
> be present on other i.MX SoCs, so apply this fix for the whole i.MX
> family.
>
> Signed-off-by: BenoÃt ThÃbaudeau <benoit@xxxxxxxxxxx>

I would expect to see Acks from other sdhci-esdhc-imx users. Nevertheless,
for sdhci:

Acked-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>

> ---
> drivers/mmc/host/sdhci-esdhc-imx.c | 59 ++++++++++++++++++++++++--------------
> 1 file changed, 38 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 23d8b8a..4ee82e1 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -579,7 +579,7 @@ static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg)
> {
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
> - u32 new_val;
> + u32 new_val = 0;
> u32 mask;
>
> switch (reg) {
> @@ -610,29 +610,46 @@ static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg)
>
> esdhc_clrset_le(host, mask, new_val, reg);
> return;
> + case SDHCI_SOFTWARE_RESET:
> + if (val & SDHCI_RESET_DATA)
> + new_val = readl(host->ioaddr + SDHCI_HOST_CONTROL);
> + break;
> }
> esdhc_clrset_le(host, 0xff, val, reg);
>
> - /*
> - * The esdhc has a design violation to SDHC spec which tells
> - * that software reset should not affect card detection circuit.
> - * But esdhc clears its SYSCTL register bits [0..2] during the
> - * software reset. This will stop those clocks that card detection
> - * circuit relies on. To work around it, we turn the clocks on back
> - * to keep card detection circuit functional.
> - */
> - if ((reg == SDHCI_SOFTWARE_RESET) && (val & 1)) {
> - esdhc_clrset_le(host, 0x7, 0x7, ESDHC_SYSTEM_CONTROL);
> - /*
> - * The reset on usdhc fails to clear MIX_CTRL register.
> - * Do it manually here.
> - */
> - if (esdhc_is_usdhc(imx_data)) {
> - /* the tuning bits should be kept during reset */
> - new_val = readl(host->ioaddr + ESDHC_MIX_CTRL);
> - writel(new_val & ESDHC_MIX_CTRL_TUNING_MASK,
> - host->ioaddr + ESDHC_MIX_CTRL);
> - imx_data->is_ddr = 0;
> + if (reg == SDHCI_SOFTWARE_RESET) {
> + if (val & SDHCI_RESET_ALL) {
> + /*
> + * The esdhc has a design violation to SDHC spec which
> + * tells that software reset should not affect card
> + * detection circuit. But esdhc clears its SYSCTL
> + * register bits [0..2] during the software reset. This
> + * will stop those clocks that card detection circuit
> + * relies on. To work around it, we turn the clocks on
> + * back to keep card detection circuit functional.
> + */
> + esdhc_clrset_le(host, 0x7, 0x7, ESDHC_SYSTEM_CONTROL);
> + /*
> + * The reset on usdhc fails to clear MIX_CTRL register.
> + * Do it manually here.
> + */
> + if (esdhc_is_usdhc(imx_data)) {
> + /*
> + * the tuning bits should be kept during reset
> + */
> + new_val = readl(host->ioaddr + ESDHC_MIX_CTRL);
> + writel(new_val & ESDHC_MIX_CTRL_TUNING_MASK,
> + host->ioaddr + ESDHC_MIX_CTRL);
> + imx_data->is_ddr = 0;
> + }
> + } else if (val & SDHCI_RESET_DATA) {
> + /*
> + * The eSDHC DAT line software reset clears at least the
> + * data transfer width on i.MX25, so make sure that the
> + * Host Control register is unaffected.
> + */
> + esdhc_clrset_le(host, 0xff, new_val,
> + SDHCI_HOST_CONTROL);
> }
> }
> }
>