Re: [PATCH v3 2/2] SD : Updated CMD 6 implememtation.

[Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>]

Hi Leif,

I agree to contribute under TianoCore Contribution Agreement 1.1

Thanks,
Meenakshi

> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> Sent: Thursday, September 07, 2017 8:05 PM
> To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> Cc: jun.nie@linaro.org; haojian.zhuang@linaro.org; edk2-devel@lists.01.org
> Subject: Re: [PATCH v3 2/2] SD : Updated CMD 6 implememtation.
> 
> On Thu, Sep 07, 2017 at 07:47:53PM +0530, Meenakshi Aggarwal wrote:
> > For setting high speed in SD card,
> > First CMD 6 (Switch) is send to check if card supports High Speed and
> > Second command is send to switch card to high speed mode.
> >
> > In current inplementation, CMD 6 was sent only once to switch the card
> > into HS mode without checking if card supports HS or not, which is not
> > as per specification and also we are not setting the HS i.e. 50000000
> > but directly asking the card to switch to 26000000 which is incorrect
> > as SD card supports either 25000000 or 50000000.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> 
> Ok, I would say this one is good to go, but can you please confirm if you are
> happy to contribute under TianoCore Contribution Agreement 1.1, which is
> the version currently described in Contributions.txt?
> 
> If so, I can fold in this change and push.
> 
> Oh, you also appear to have left out edk2-devel@lists.01.org in this
> submission (added).
> 
> Best Regards,
> 
> Leif
> 
> > Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> > ---
> >  EmbeddedPkg/Universal/MmcDxe/Mmc.h               |  8 ++++
> >  EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 53
> > +++++++++++++++++++++---
> >  2 files changed, 56 insertions(+), 5 deletions(-)  mode change 100644
> > => 100755 EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> >
> > diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> > b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> > index f3e56ff..a77ba41 100644
> > --- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> > +++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> > @@ -64,6 +64,14 @@
> >  #define EMMC_CMD6_ARG_VALUE(x)              (((x) & 0xFF) << 8)
> >  #define EMMC_CMD6_ARG_CMD_SET(x)            (((x) & 0x7) << 0)
> >
> > +#define SWITCH_CMD_DATA_LENGTH              64
> > +#define SD_HIGH_SPEED_SUPPORTED             0x20000
> > +#define SD_DEFAULT_SPEED                    25000000
> > +#define SD_HIGH_SPEED                       50000000
> > +#define SWITCH_CMD_SUCCESS_MASK             0x0f000000
> > +
> > +#define BUSWIDTH_4                          4
> > +
> >  typedef enum {
> >    UNKNOWN_CARD,
> >    MMC_CARD,              //MMC card
> > diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > old mode 100644
> > new mode 100755
> > index 27e4444..f661a0c
> > --- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > +++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > @@ -318,6 +318,23 @@ InitializeEmmcDevice (  }
> >
> >  STATIC
> > +UINT32
> > +CreateSwitchCmdArgument (
> > +  IN  UINT32 Mode,
> > +  IN  UINT8  Group,
> > +  IN  UINT8  Value
> > +  )
> > +{
> > +  UINT32 Argument;
> > +
> > +  Argument = Mode << 31 | 0x00FFFFFF;  Argument &= ~(0xF << (Group *
> > + 4));  Argument |= Value << (Group * 4);
> > +
> > +  return Argument;
> > +}
> > +
> > +STATIC
> >  EFI_STATUS
> >  InitializeSdMmcDevice (
> >    IN  MMC_HOST_INSTANCE   *MmcHostInstance
> > @@ -326,6 +343,7 @@ InitializeSdMmcDevice (
> >    UINT32        CmdArg;
> >    UINT32        Response[4];
> >    UINT32        Buffer[128];
> > +  UINT32        Speed;
> >    UINTN         BlockSize;
> >    UINTN         CardSize;
> >    UINTN         NumBlocks;
> > @@ -334,6 +352,7 @@ InitializeSdMmcDevice (
> >    EFI_STATUS    Status;
> >    EFI_MMC_HOST_PROTOCOL     *MmcHost;
> >
> > +  Speed = SD_DEFAULT_SPEED;
> >    MmcHost = MmcHostInstance->MmcHost;
> >
> >    // Send a command to get Card specific data @@ -439,20 +458,44 @@
> > InitializeSdMmcDevice (
> >      }
> >    }
> >    if (CccSwitch) {
> > +    /* SD Switch, Mode:0, Group:0, Value:0 */
> > +    CmdArg = CreateSwitchCmdArgument(0, 0, 0);
> > +    Status = MmcHost->SendCommand (MmcHost, MMC_CMD6, CmdArg);
> > +    if (EFI_ERROR (Status)) {
> > +      DEBUG ((DEBUG_ERROR, "%a (MMC_CMD6): Error and Status = %r\n",
> __FUNCTION__, Status));
> > +       return Status;
> > +    } else {
> > +      Status = MmcHost->ReadBlockData (MmcHost, 0,
> SWITCH_CMD_DATA_LENGTH, Buffer);
> > +      if (EFI_ERROR (Status)) {
> > +        DEBUG ((DEBUG_ERROR, "%a (MMC_CMD6): ReadBlockData Error
> and Status = %r\n", __FUNCTION__, Status));
> > +        return Status;
> > +      }
> > +    }
> > +
> > +    if (!(Buffer[3] & SD_HIGH_SPEED_SUPPORTED)) {
> > +      DEBUG ((DEBUG_ERROR, "%a : High Speed not supported by Card
> %r\n", __FUNCTION__, Status));
> > +      return Status;
> > +    }
> > +
> > +    Speed = SD_HIGH_SPEED;
> > +
> >      /* SD Switch, Mode:1, Group:0, Value:1 */
> > -    CmdArg = 1 << 31 | 0x00FFFFFF;
> > -    CmdArg &= ~(0xF << (0 * 4));
> > -    CmdArg |= 1 << (0 * 4);
> > +    CmdArg = CreateSwitchCmdArgument(1, 0, 1);
> >      Status = MmcHost->SendCommand (MmcHost, MMC_CMD6, CmdArg);
> >      if (EFI_ERROR (Status)) {
> >        DEBUG ((DEBUG_ERROR, "%a (MMC_CMD6): Error and Status = %r\n",
> __FUNCTION__, Status));
> >         return Status;
> >      } else {
> > -      Status = MmcHost->ReadBlockData (MmcHost, 0, 64, Buffer);
> > +      Status = MmcHost->ReadBlockData (MmcHost, 0,
> > + SWITCH_CMD_DATA_LENGTH, Buffer);
> >        if (EFI_ERROR (Status)) {
> >          DEBUG ((DEBUG_ERROR, "%a (MMC_CMD6): ReadBlockData Error and
> Status = %r\n", __FUNCTION__, Status));
> >          return Status;
> >        }
> > +
> > +      if ((Buffer[4] & SWITCH_CMD_SUCCESS_MASK) != 0x01000000) {
> > +        DEBUG((DEBUG_ERROR, "Problem switching SD card into high-speed
> mode\n"));
> > +        return Status;
> > +      }
> >      }
> >    }
> >    if (Scr.SD_BUS_WIDTHS & SD_BUS_WIDTH_4BIT) { @@ -470,7 +513,7 @@
> > InitializeSdMmcDevice (
> >      }
> >    }
> >    if (MMC_HOST_HAS_SETIOS(MmcHost)) {
> > -    Status = MmcHost->SetIos (MmcHost, 26 * 1000 * 1000, 4,
> EMMCBACKWARD);
> > +    Status = MmcHost->SetIos (MmcHost, Speed, BUSWIDTH_4,
> > + EMMCBACKWARD);
> >      if (EFI_ERROR (Status)) {
> >        DEBUG ((DEBUG_ERROR, "%a (SetIos): Error and Status = %r\n",
> __FUNCTION__, Status));
> >        return Status;
> > --
> > 1.9.1
> >