diff mbox

[U-Boot] mmc: add host_caps checking avoid switch card improperly

Message ID 1321522272-22149-1-git-send-email-macpaul@andestech.com
State Changes Requested
Delegated to: Andy Fleming
Headers show

Commit Message

Macpaul Lin Nov. 17, 2011, 9:31 a.m. UTC
Add a host capability checking to avoid the mmc stack
switch the card to HIGHSPEED mode when the card supports
HIGHSPEED while the host doesn't.

This patch avoid furthur transaction problem when the
mmc/sd card runs different mode to the host.

Signed-off-by: Macpaul Lin <macpaul@andestech.com>
---
 drivers/mmc/mmc.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

Comments

Andy Fleming Nov. 25, 2011, 11:36 p.m. UTC | #1
On Thu, Nov 17, 2011 at 3:31 AM, Macpaul Lin <macpaul@andestech.com> wrote:
> Add a host capability checking to avoid the mmc stack
> switch the card to HIGHSPEED mode when the card supports
> HIGHSPEED while the host doesn't.
>
> This patch avoid furthur transaction problem when the
> mmc/sd card runs different mode to the host.
>
> Signed-off-by: Macpaul Lin <macpaul@andestech.com>
> ---
>  drivers/mmc/mmc.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 21665ec..ce34d05 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -785,6 +785,16 @@ retry_scr:
>        if (!(__be32_to_cpu(switch_status[3]) & SD_HIGHSPEED_SUPPORTED))
>                return 0;
>
> +       /*
> +        * If the host doesn't support SD_HIGHSPEED, do not switch card to
> +        * HIGHSPEED mode even if the card support SD_HIGHSPPED.
> +        * This can avoid furthur problem when the card runs in different
> +        * mode between the host.
> +        */
> +       if (!((mmc->host_caps & MMC_MODE_HS_52MHz) ||
> +               (mmc->host_caps & MMC_MODE_HS)))
> +               return 0;
> +

Isn't this the wrong logic? It seems like you don't want to switch
unless both support high speed. But this logic says to switch if
either one does.

Shouldn't it be:

if (!((mmc->host_caps & MMC_MODE_HS_52MHz) &&
       (mmc->host_caps & MMC_MODE_HS)))
    return 0;

?

Andy
>        err = sd_switch(mmc, SD_SWITCH_SWITCH, 0, 1, (u8 *)switch_status);
>
>        if (err)
> --
> 1.7.3.5
>
>
Macpaul Lin Nov. 28, 2011, 5:51 a.m. UTC | #2
Hi Andy,

2011/11/26 Andy Fleming <afleming@gmail.com>

> On Thu, Nov 17, 2011 at 3:31 AM, Macpaul Lin <macpaul@andestech.com>
> wrote:
> > Add a host capability checking to avoid the mmc stack
> > switch the card to HIGHSPEED mode when the card supports
> > HIGHSPEED while the host doesn't.
> > +       if (!((mmc->host_caps & MMC_MODE_HS_52MHz) ||
> > +               (mmc->host_caps & MMC_MODE_HS)))
> > +               return 0;
> > +
>

I wrote this is because I've thought that SD cards usually work at 50MHz
when they under
high speed mode (MMC_MODE_HS), and MMC cards usually work at 52MHz when
they are under high speed mode.
I'm not sure if one controller support HS_52MHz for MMC cards will
definitely
support SD cards under 50MHz.
If I'm wrong for the above please correct me.

Hence I wrote this to force them working under low speed to avoid problem
occur.

But I think you're correct, we should use "&&" logic here until there is
really a problem for
the unbelievable situation I mentioned above that happened. :-p


>  Isn't this the wrong logic? It seems like you don't want to switch
> unless both support high speed. But this logic says to switch if
> either one does.
>
> Shouldn't it be:
>
> if (!((mmc->host_caps & MMC_MODE_HS_52MHz) &&
>       (mmc->host_caps & MMC_MODE_HS)))
>    return 0;
>
> ?
>
>
I'll send patch v2 by using your logic later.
Thanks!
diff mbox

Patch

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 21665ec..ce34d05 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -785,6 +785,16 @@  retry_scr:
 	if (!(__be32_to_cpu(switch_status[3]) & SD_HIGHSPEED_SUPPORTED))
 		return 0;
 
+	/*
+	 * If the host doesn't support SD_HIGHSPEED, do not switch card to
+	 * HIGHSPEED mode even if the card support SD_HIGHSPPED.
+	 * This can avoid furthur problem when the card runs in different
+	 * mode between the host.
+	 */
+	if (!((mmc->host_caps & MMC_MODE_HS_52MHz) ||
+		(mmc->host_caps & MMC_MODE_HS)))
+		return 0;
+
 	err = sd_switch(mmc, SD_SWITCH_SWITCH, 0, 1, (u8 *)switch_status);
 
 	if (err)