Patchwork [U-Boot] Possible MMC subsystem bug

login
register
mail settings
Submitter Marek Vasut
Date Jan. 27, 2013, 4:32 a.m.
Message ID <201301270532.12393.marex@denx.de>
Download mbox | patch
Permalink /patch/215943/
State Rejected
Delegated to: Andy Fleming
Headers show

Comments

Marek Vasut - Jan. 27, 2013, 4:32 a.m.
Hi Andy,

I was going through the MMC code, trying to get Phison 8007 SD-to-NAND bridge 
working (don't ask please, this chip's sole existence defies any logic).

So, I found the following code and I was wondering if the following patch is not 
needed. Maybe my brain is just giving up though. Give it some thought please and 
let me know, thanks!
Andy Fleming - Jan. 27, 2013, 4:44 a.m.
I'll need to double-check the spec, but I believe the BUSY bit has the opposite meaning of common sense.

On Jan 26, 2013, at 22:32, "Marek Vasut" <marex@denx.de> wrote:

> Hi Andy,
> 
> I was going through the MMC code, trying to get Phison 8007 SD-to-NAND bridge 
> working (don't ask please, this chip's sole existence defies any logic).
> 
> So, I found the following code and I was wondering if the following patch is not 
> needed. Maybe my brain is just giving up though. Give it some thought please and 
> let me know, thanks!
> 
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 72e8ce6..94926ca 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -475,8 +474,11 @@ static int sd_send_op_cond(struct mmc *mmc)
>                if (err)
>                        return err;
> 
> +               if (!(cmd.response[0] & OCR_BUSY))
> +                       break;
> +
>                udelay(1000);
> -       } while ((!(cmd.response[0] & OCR_BUSY)) && timeout--);
> +       } while (timeout--);
> 
>        if (timeout <= 0)
>                return UNUSABLE_ERR;
>
Marek Vasut - Jan. 27, 2013, 4:49 a.m.
Dear Fleming Andy-AFLEMING,

> I'll need to double-check the spec, but I believe the BUSY bit has the
> opposite meaning of common sense.

I tihnk my brain is maybe just giving up really ... I think the code is right 
after some quick poking through the spec.

> On Jan 26, 2013, at 22:32, "Marek Vasut" <marex@denx.de> wrote:
> > Hi Andy,
> > 
> > I was going through the MMC code, trying to get Phison 8007 SD-to-NAND
> > bridge working (don't ask please, this chip's sole existence defies any
> > logic).
> > 
> > So, I found the following code and I was wondering if the following patch
> > is not needed. Maybe my brain is just giving up though. Give it some
> > thought please and let me know, thanks!
> > 
> > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> > index 72e8ce6..94926ca 100644
> > --- a/drivers/mmc/mmc.c
> > +++ b/drivers/mmc/mmc.c
> > @@ -475,8 +474,11 @@ static int sd_send_op_cond(struct mmc *mmc)
> > 
> >                if (err)
> >                
> >                        return err;
> > 
> > +               if (!(cmd.response[0] & OCR_BUSY))
> > +                       break;
> > +
> > 
> >                udelay(1000);
> > 
> > -       } while ((!(cmd.response[0] & OCR_BUSY)) && timeout--);
> > +       } while (timeout--);
> > 
> >        if (timeout <= 0)
> >        
> >                return UNUSABLE_ERR;

Best regards,
Marek Vasut

Patch

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 72e8ce6..94926ca 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -475,8 +474,11 @@  static int sd_send_op_cond(struct mmc *mmc)
                if (err)
                        return err;
 
+               if (!(cmd.response[0] & OCR_BUSY))
+                       break;
+
                udelay(1000);
-       } while ((!(cmd.response[0] & OCR_BUSY)) && timeout--);
+       } while (timeout--);
 
        if (timeout <= 0)
                return UNUSABLE_ERR;