[PATCH] mmc: sdhci-esdhc-imx: Do not set MMC_CAP_1_8V_DDR in the 'no-1-8-v' case

[PATCH] mmc: sdhci-esdhc-imx: Do not set MMC_CAP_1_8V_DDR in the 'no-1-8-v' case

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

Since commit e2997c944dbdff3f ("mmc: sdhci-esdhc-imx: add MMC_CAP_1_8V_DDR
for mx6") the driver sets the MMC_CAP_1_8V_DDR flag unconditionally on
mx6, but we should not do this when the 'no-1-8-v' property is passed via 
device tree.

This causes the following errors when accessing eMMC on a mx6sl board:

mmc0: MAN_BKOPS_EN bit is not set
mmc0: power class selection to bus width 8 ddr 4 failed
mmc0: error -110 whilst initialising MMC card

So only set the MMC_CAP_1_8V_DDR flag when the 'no-1-8-v' property is
absent.

With this fix in place it is possible to successfully mount the rootfs
from the emmc on a mx6sl board which has 'no-1-8-v' property passed in the
device tree.

Fixes: e2997c944dbdff3f ("mmc: sdhci-esdhc-imx: add MMC_CAP_1_8V_DDR for mx6")
Cc: stable@vger.kernel.org
Reported-by: Kevin Lemoi <kevin.lemoi@savant.com>
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index faf0cb9..b562faf 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1005,7 +1005,6 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	if (esdhc_is_usdhc(imx_data)) {
 		writel(0x08100810, host->ioaddr + ESDHC_WTMK_LVL);
 		host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
-		host->mmc->caps |= MMC_CAP_1_8V_DDR;
 
 		if (!(imx_data->socdata->flags & ESDHC_FLAG_HS200))
 			host->quirks2 |= SDHCI_QUIRK2_BROKEN_HS200;
@@ -1094,6 +1093,7 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	/* sdr50 and sdr104 needs work on 1.8v signal voltage */
 	if ((boarddata->support_vsel) && esdhc_is_usdhc(imx_data) &&
 	    !IS_ERR(imx_data->pins_default)) {
+		host->mmc->caps |= MMC_CAP_1_8V_DDR;
 		imx_data->pins_100mhz = pinctrl_lookup_state(imx_data->pinctrl,
 						ESDHC_PINCTRL_STATE_100MHZ);
 		imx_data->pins_200mhz = pinctrl_lookup_state(imx_data->pinctrl,
-- 
1.9.1

Re: [PATCH] mmc: sdhci-esdhc-imx: Do not set MMC_CAP_1_8V_DDR in the 'no-1-8-v' case

[Dong Aisheng]

Hi Fabio,

On Thu, Jun 11, 2015 at 07:08:23PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> Since commit e2997c944dbdff3f ("mmc: sdhci-esdhc-imx: add MMC_CAP_1_8V_DDR
> for mx6") the driver sets the MMC_CAP_1_8V_DDR flag unconditionally on
> mx6, but we should not do this when the 'no-1-8-v' property is passed via 
> device tree.
> 
> This causes the following errors when accessing eMMC on a mx6sl board:
> 
> mmc0: MAN_BKOPS_EN bit is not set
> mmc0: power class selection to bus width 8 ddr 4 failed
> mmc0: error -110 whilst initialising MMC card
> 
> So only set the MMC_CAP_1_8V_DDR flag when the 'no-1-8-v' property is
> absent.
> 
> With this fix in place it is possible to successfully mount the rootfs
> from the emmc on a mx6sl board which has 'no-1-8-v' property passed in the
> device tree.
> 
> Fixes: e2997c944dbdff3f ("mmc: sdhci-esdhc-imx: add MMC_CAP_1_8V_DDR for mx6")
> Cc: stable@vger.kernel.org
> Reported-by: Kevin Lemoi <kevin.lemoi@savant.com>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>

I think this patch will break the eMMC DDR 3.3v mode support.

Looking at EXT_CSD_CARD_TYPE_DDR_1_8V definition,
#define EXT_CSD_CARD_TYPE_DDR_1_8V  (1<<2)   /* Card can run at 52MHz */
                                             /* DDR mode @1.8V or 3V I/O */
it means the card can  run at DDR mode on either 3V or 1.8V IO.

This patch disable the DDR capability unconditionally if no 1.8v IO support.

But there's indeed some confusion in mmc core that comments of MMC_CAP_1_8V_DDR
only indicates 1.8V DDR mode support which probably could be improved.
#define MMC_CAP_1_8V_DDR        (1 << 11)       /* can support */
                                                /* DDR mode at 1.8V */

I guess why you issue passed with this change is because the DDR
mode is disabled.
Probably you need figure out why 3.3V DDR mode can't work for the board
in the issue, usually it's caused by timing, io pad setting and etc.

Regards
Dong Aisheng

> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index faf0cb9..b562faf 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -1005,7 +1005,6 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
>  	if (esdhc_is_usdhc(imx_data)) {
>  		writel(0x08100810, host->ioaddr + ESDHC_WTMK_LVL);
>  		host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> -		host->mmc->caps |= MMC_CAP_1_8V_DDR;
>  
>  		if (!(imx_data->socdata->flags & ESDHC_FLAG_HS200))
>  			host->quirks2 |= SDHCI_QUIRK2_BROKEN_HS200;
> @@ -1094,6 +1093,7 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
>  	/* sdr50 and sdr104 needs work on 1.8v signal voltage */
>  	if ((boarddata->support_vsel) && esdhc_is_usdhc(imx_data) &&
>  	    !IS_ERR(imx_data->pins_default)) {
> +		host->mmc->caps |= MMC_CAP_1_8V_DDR;
>  		imx_data->pins_100mhz = pinctrl_lookup_state(imx_data->pinctrl,
>  						ESDHC_PINCTRL_STATE_100MHZ);
>  		imx_data->pins_200mhz = pinctrl_lookup_state(imx_data->pinctrl,
> -- 
> 1.9.1
> 

Re: [PATCH] mmc: sdhci-esdhc-imx: Do not set MMC_CAP_1_8V_DDR in the 'no-1-8-v' case

[Fabio Estevam]

Hi Dong,

On Fri, Jun 12, 2015 at 12:22 AM, Dong Aisheng <b29396@freescale.com> wrote:

> I think this patch will break the eMMC DDR 3.3v mode support.

I don't think so.

This patch only changes the behaviour when 'no-1-8-v' property is used.

This custom board has an eMMC that can operate at 1.8V, but the board
itself can only provide 3.3V.

In this case we need to pass 'no-1-8-v' as explained in
Documentation/devicetree/bindings/mmc/mmc.txt:

"- no-1-8-v: when present, denotes that 1.8v card voltage is not supported on
  this system, even if the controller claims it is."

Then in this case we should not set MMC_CAP_1_8V_DDR, which will make
the card to work at 3.3V.

Regards,

Fabio Estevam

Re: [PATCH] mmc: sdhci-esdhc-imx: Do not set MMC_CAP_1_8V_DDR in the 'no-1-8-v' case

[Fabio Estevam]

On Fri, Jun 12, 2015 at 12:22 AM, Dong Aisheng <b29396@freescale.com> wrote:

> I think this patch will break the eMMC DDR 3.3v mode support.

I think you are right.

On a imx6q-sabresd which has 'no-1-8-v' in the emmc node we have originally:

mmc2: new DDR MMC card at address 0001

After applying this patch I get:

mmc2: new high speed MMC card at address 0001

I will try to come up with a different solution.

Thanks