diff mbox

[edk2,v2,8/9] MmcDxe: set iospeed and bus width in SD stack

Message ID CAD0U-h+7AmMXnPVj+TugFv2EmQ2+fHa-5p7FcbnyQgZe4V8ZEQ@mail.gmail.com
State New
Headers show

Commit Message

Ryan Harkin April 5, 2016, 9:49 a.m. UTC 
On 5 April 2016 at 09:37, Ryan Harkin <ryan.harkin@linaro.org> wrote:
> On 5 April 2016 at 03:57, Haojian Zhuang <haojian.zhuang@linaro.org> wrote:

>> On 5 April 2016 at 01:17, Ryan Harkin <ryan.harkin@linaro.org> wrote:

>>> Hi Haojian,

>>>

>>> I've had time to investigate where TC2 is hanging with your patches

>>> applied and narrowed it down to the single line of code marked below.

>>>

>>> I'm going to read the code now and see if I can work out what it's

>>> trying to do, but I thought I'd tell you sooner because you might have

>>> a better idea.

>>>

>>> On 22 March 2016 at 12:48, Haojian Zhuang <haojian.zhuang@linaro.org> wrote:

>>>> Add more SD commands to support 4-bit bus width & iospeed. It's not

>>>> formal code. And it needs to be updated later.

>>>>

>>>> Contributed-under: TianoCore Contribution Agreement 1.0

>>>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

>>>> ---

>>>>  EmbeddedPkg/Include/Protocol/MmcHost.h           |   3 +

>>>>  EmbeddedPkg/Universal/MmcDxe/Mmc.h               |  17 +++

>>>>  EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 138 ++++++++++++++++++++---

>>>>  3 files changed, 142 insertions(+), 16 deletions(-)

>>>>

>>>> diff --git a/EmbeddedPkg/Include/Protocol/MmcHost.h b/EmbeddedPkg/Include/Protocol/MmcHost.h

>>>> index 5e3a2b7..e9a74f0 100644

>>>> --- a/EmbeddedPkg/Include/Protocol/MmcHost.h

>>>> +++ b/EmbeddedPkg/Include/Protocol/MmcHost.h

>>>> @@ -64,11 +64,14 @@ typedef UINT32 MMC_CMD;

>>>>  #define MMC_CMD24             (MMC_INDX(24) | MMC_CMD_WAIT_RESPONSE)

>>>>  #define MMC_CMD55             (MMC_INDX(55) | MMC_CMD_WAIT_RESPONSE)

>>>>  #define MMC_ACMD41            (MMC_INDX(41) | MMC_CMD_WAIT_RESPONSE | MMC_CMD_NO_CRC_RESPONSE)

>>>> +#define MMC_ACMD51            (MMC_INDX(51) | MMC_CMD_WAIT_RESPONSE)

>>>>

>>>>  // Valid responses for CMD1 in eMMC

>>>>  #define EMMC_CMD1_CAPACITY_LESS_THAN_2GB 0x00FF8080 // Capacity <= 2GB, byte addressing used

>>>>  #define EMMC_CMD1_CAPACITY_GREATER_THAN_2GB 0x40FF8080 // Capacity > 2GB, 512-byte sector addressing used

>>>>

>>>> +#define MMC_STATUS_APP_CMD    (1 << 5)

>>>> +

>>>>  typedef enum _MMC_STATE {

>>>>      MmcInvalidState = 0,

>>>>      MmcHwInitializationState,

>>>> diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h b/EmbeddedPkg/Universal/MmcDxe/Mmc.h

>>>> index 0ccbc80..a62ba32 100644

>>>> --- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h

>>>> +++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h

>>>> @@ -75,6 +75,23 @@ typedef struct {

>>>>    UINT32  PowerUp:     1; // This bit is set to LOW if the card has not finished the power up routine

>>>>  } OCR;

>>>>

>>>> +/* For little endian CPU */

>>>> +typedef struct {

>>>> +  UINT8   SD_SPEC:               4; // SD Memory Card - Spec. Version [59:56]

>>>> +  UINT8   SCR_STRUCTURE:         4; // SCR Structure [63:60]

>>>> +  UINT8   SD_BUS_WIDTHS:         4; // DAT Bus widths supported [51:48]

>>>> +  UINT8   DATA_STAT_AFTER_ERASE: 1; // Data Status after erases [55]

>>>> +  UINT8   SD_SECURITY:           3; // CPRM Security Support [54:52]

>>>> +  UINT8   EX_SECURITY_1:         1; // Extended Security Support [43]

>>>> +  UINT8   SD_SPEC4:              1; // Spec. Version 4.00 or higher [42]

>>>> +  UINT8   RESERVED_1:            2; // Reserved [41:40]

>>>> +  UINT8   SD_SPEC3:              1; // Spec. Version 3.00 or higher [47]

>>>> +  UINT8   EX_SECURITY_2:         3; // Extended Security Support [46:44]

>>>> +  UINT8   CMD_SUPPORT:           4; // Command Support bits [35:32]

>>>> +  UINT8   RESERVED_2:            4; // Reserved [39:36]

>>>> +  UINT32  RESERVED_3;               // Manufacturer Usage [31:0]

>>>> +} SCR;

>>>> +

>>>>  typedef struct {

>>>>    UINT32  NOT_USED;   // 1 [0:0]

>>>>    UINT32  CRC;        // CRC7 checksum [7:1]

>>>> diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c

>>>> index f806bfc..125d3f9 100644

>>>> --- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c

>>>> +++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c

>>>> @@ -12,6 +12,9 @@

>>>>  *

>>>>  **/

>>>>

>>>> +#include <Library/BaseMemoryLib.h>

>>>> +#include <Library/TimerLib.h>

>>>> +

>>>>  #include "Mmc.h"

>>>>

>>>>  typedef union {

>>>> @@ -41,6 +44,11 @@ typedef union {

>>>>

>>>>  #define EMMC_SWITCH_ERROR       (1 << 7)

>>>>

>>>> +#define SD_BUS_WIDTH_1BIT       (1 << 0)

>>>> +#define SD_BUS_WIDTH_4BIT       (1 << 2)

>>>> +

>>>> +#define SD_CCC_SWITCH           (1 << 10)

>>>> +

>>>>  #define DEVICE_STATE(x)         (((x) >> 9) & 0xf)

>>>>  typedef enum _EMMC_DEVICE_STATE {

>>>>    EMMC_IDLE_STATE = 0,

>>>> @@ -69,28 +77,30 @@ EmmcGetDeviceState (

>>>>  {

>>>>    EFI_MMC_HOST_PROTOCOL *Host;

>>>>    EFI_STATUS Status;

>>>> -  UINT32     Data, RCA;

>>>> +  UINT32     Rsp[4], RCA;

>>>>

>>>>    if (State == NULL)

>>>>      return EFI_INVALID_PARAMETER;

>>>>

>>>>    Host  = MmcHostInstance->MmcHost;

>>>>    RCA = MmcHostInstance->CardInfo.RCA << RCA_SHIFT_OFFSET;

>>>> -  Status = Host->SendCommand (Host, MMC_CMD13, RCA);

>>>> -  if (EFI_ERROR (Status)) {

>>>> -    DEBUG ((EFI_D_ERROR, "EmmcGetDeviceState(): Failed to get card status, Status=%r.\n", Status));

>>>> -    return Status;

>>>> -  }

>>>> -  Status = Host->ReceiveResponse (Host, MMC_RESPONSE_TYPE_R1, &Data);

>>>> -  if (EFI_ERROR (Status)) {

>>>> -    DEBUG ((EFI_D_ERROR, "EmmcGetDeviceState(): Failed to get response of CMD13, Status=%r.\n", Status));

>>>> -    return Status;

>>>> -  }

>>>> -  if (Data & EMMC_SWITCH_ERROR) {

>>>> -    DEBUG ((EFI_D_ERROR, "EmmcGetDeviceState(): Failed to switch expected mode, Status=%r.\n", Status));

>>>> -    return EFI_DEVICE_ERROR;

>>>> -  }

>>>> -  *State = DEVICE_STATE(Data);

>>>> +  do {

>>>> +    Status = Host->SendCommand (Host, MMC_CMD13, RCA);

>>>> +    if (EFI_ERROR (Status)) {

>>>> +      DEBUG ((EFI_D_ERROR, "EmmcGetDeviceState(): Failed to get card status, Status=%r.\n", Status));

>>>> +      return Status;

>>>> +    }

>>>> +    Status = Host->ReceiveResponse (Host, MMC_RESPONSE_TYPE_R1, Rsp);

>>>> +    if (EFI_ERROR (Status)) {

>>>> +      DEBUG ((EFI_D_ERROR, "EmmcGetDeviceState(): Failed to get response of CMD13, Status=%r.\n", Status));

>>>> +      return Status;

>>>> +    }

>>>> +    if (Rsp[0] & EMMC_SWITCH_ERROR) {

>>>> +      DEBUG ((EFI_D_ERROR, "EmmcGetDeviceState(): Failed to switch expected mode, Status=%r.\n", Status));

>>>> +      return EFI_DEVICE_ERROR;

>>>> +    }

>>>> +  } while (!(Rsp[0] & MMC_R0_READY_FOR_DATA));

>>>> +  *State = MMC_R0_CURRENTSTATE(Rsp);

>>>>    return EFI_SUCCESS;

>>>>  }

>>>>

>>>> @@ -305,9 +315,12 @@ InitializeSdMmcDevice (

>>>>  {

>>>>    UINT32        CmdArg;

>>>>    UINT32        Response[4];

>>>> +  UINT32        Buffer[128];

>>>>    UINTN         BlockSize;

>>>>    UINTN         CardSize;

>>>>    UINTN         NumBlocks;

>>>> +  BOOLEAN       CccSwitch;

>>>> +  SCR           Scr;

>>>>    EFI_STATUS    Status;

>>>>    EFI_MMC_HOST_PROTOCOL     *MmcHost;

>>>>

>>>> @@ -328,6 +341,10 @@ InitializeSdMmcDevice (

>>>>      return Status;

>>>>    }

>>>>    PrintCSD (Response);

>>>> +  if (MMC_CSD_GET_CCC(Response) & SD_CCC_SWITCH)

>>>> +    CccSwitch = TRUE;

>>>> +  else

>>>> +    CccSwitch = FALSE;

>>>>

>>>>    if (MmcHostInstance->CardInfo.CardType == SD_CARD_2_HIGH) {

>>>>      CardSize = HC_MMC_CSD_GET_DEVICESIZE (Response);

>>>> @@ -358,6 +375,95 @@ InitializeSdMmcDevice (

>>>>      return Status;

>>>>    }

>>>>

>>>> +  Status = MmcHost->SendCommand (MmcHost, MMC_CMD55, CmdArg);

>>>> +  if (EFI_ERROR (Status)) {

>>>> +    DEBUG ((EFI_D_ERROR, "%a(MMC_CMD55): Error and Status = %r\n", Status));

>>>> +    return Status;

>>>> +  }

>>>> +  Status = MmcHost->ReceiveResponse (MmcHost, MMC_RESPONSE_TYPE_R1, Response);

>>>> +  if (EFI_ERROR (Status)) {

>>>> +    DEBUG ((EFI_D_ERROR, "%a(MMC_CMD55): Error and Status = %r\n", Status));

>>>> +    return Status;

>>>> +  }

>>>> +  if ((Response[0] & MMC_STATUS_APP_CMD) == 0)

>>>> +    return EFI_SUCCESS;

>>>> +

>>>> +  /* SCR */

>>>> +  Status = MmcHost->SendCommand (MmcHost, MMC_ACMD51, 0);

>>>> +  if (EFI_ERROR (Status)) {

>>>> +    DEBUG ((EFI_D_ERROR, "%a(MMC_ACMD51): Error and Status = %r\n", __func__, Status));

>>>> +    return Status;

>>>> +  } else {

>>>> +    Status = MmcHost->ReadBlockData (MmcHost, 0, 8, Buffer);

>>>

>>> ^^ TC2 hangs at this line

>>>

>>

>> Ryan,

>>

>> Could you help to check where it hangs in

>> ArmPlatformPkg/Drivers/PL180MciDxe/PL180Mci.c?

>>

>

> I guess you mean, "where in function MciReadBlockData does it hang?".

>

> I'll have a look this asap.

>


I made this mod to the code to add DEBUG:



And after the InitializeSdMmcDevice debug, I see this output continuously:

RMH: MciReadBlockData(234): 0x40

If I add an MmioWrite32 to clear the flags at the start of the read
block function, I see the same behaviour, except it reads 0x00
continuously instead of 0x40.


>

>> I checked that PL180 supports SD v0.96. But I only found SD v1.10. In

>> SD v1.10, SCR (ACMD51)

>> is defined. So I also assume that SCR should be supported in PL180.

>>

>> Best Regards

>> Haojian

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Haojian Zhuang April 5, 2016, 10:18 a.m. UTC | #1 
On 5 April 2016 at 17:49, Ryan Harkin <ryan.harkin@linaro.org> wrote:
> On 5 April 2016 at 09:37, Ryan Harkin <ryan.harkin@linaro.org> wrote:

>> On 5 April 2016 at 03:57, Haojian Zhuang <haojian.zhuang@linaro.org> wrote:

>>> On 5 April 2016 at 01:17, Ryan Harkin <ryan.harkin@linaro.org> wrote:

>>>> Hi Haojian,

>>>>

>>>> I've had time to investigate where TC2 is hanging with your patches

>>>> applied and narrowed it down to the single line of code marked below.

>>>>

>>>> I'm going to read the code now and see if I can work out what it's

>>>> trying to do, but I thought I'd tell you sooner because you might have

>>>> a better idea.

>>>>

>>>> On 22 March 2016 at 12:48, Haojian Zhuang <haojian.zhuang@linaro.org> wrote:

>>>>> Add more SD commands to support 4-bit bus width & iospeed. It's not

>>>>> formal code. And it needs to be updated later.

>>>>>

>>>>> Contributed-under: TianoCore Contribution Agreement 1.0

>>>>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

>>>>> ---

>>>>>  EmbeddedPkg/Include/Protocol/MmcHost.h           |   3 +

>>>>>  EmbeddedPkg/Universal/MmcDxe/Mmc.h               |  17 +++

>>>>>  EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 138 ++++++++++++++++++++---

>>>>>  3 files changed, 142 insertions(+), 16 deletions(-)

>>>>>

>>>>> diff --git a/EmbeddedPkg/Include/Protocol/MmcHost.h b/EmbeddedPkg/Include/Protocol/MmcHost.h

>>>>> index 5e3a2b7..e9a74f0 100644

>>>>> --- a/EmbeddedPkg/Include/Protocol/MmcHost.h

>>>>> +++ b/EmbeddedPkg/Include/Protocol/MmcHost.h


>>>>> +

>>>>> +  /* SCR */

>>>>> +  Status = MmcHost->SendCommand (MmcHost, MMC_ACMD51, 0);

>>>>> +  if (EFI_ERROR (Status)) {

>>>>> +    DEBUG ((EFI_D_ERROR, "%a(MMC_ACMD51): Error and Status = %r\n", __func__, Status));

>>>>> +    return Status;

>>>>> +  } else {

>>>>> +    Status = MmcHost->ReadBlockData (MmcHost, 0, 8, Buffer);

>>>>

>>>> ^^ TC2 hangs at this line

>>>>

>>>

>>> Ryan,

>>>

>>> Could you help to check where it hangs in

>>> ArmPlatformPkg/Drivers/PL180MciDxe/PL180Mci.c?

>>>

>>

>> I guess you mean, "where in function MciReadBlockData does it hang?".

>>

>> I'll have a look this asap.

>>

>

> I made this mod to the code to add DEBUG:

>

> diff --git a/ArmPlatformPkg/Drivers/PL180MciDxe/PL180Mci.c

> b/ArmPlatformPkg/Drivers/PL180MciDxe/PL180Mci.c

> index 5526aac..7ddcf46 100644

> --- a/ArmPlatformPkg/Drivers/PL180MciDxe/PL180Mci.c

> +++ b/ArmPlatformPkg/Drivers/PL180MciDxe/PL180Mci.c

> @@ -231,6 +231,7 @@ MciReadBlockData (

>    do {

>      // Read the Status flags

>      Status = MmioRead32 (MCI_STATUS_REG);

> +DEBUG ((EFI_D_ERROR, "RMH: MciReadBlockData(%d): 0x%x\n", __LINE__, Status));^M

>

>      // Do eight reads if possible else a single read

>      if (Status & MCI_STATUS_CMD_RXFIFOHALFFULL) {

>

>

> And after the InitializeSdMmcDevice debug, I see this output continuously:

>

> RMH: MciReadBlockData(234): 0x40

>

> If I add an MmioWrite32 to clear the flags at the start of the read

> block function, I see the same behaviour, except it reads 0x00

> continuously instead of 0x40.

>


There's a loop count in MciReadBlockData(). Could you print the value
of variable Loop and
variable Finish? Then I could know whether it reads any data from FIFO.

Regards
Haojian
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ryan Harkin April 5, 2016, 3:07 p.m. UTC | #2 
On 5 April 2016 at 11:18, Haojian Zhuang <haojian.zhuang@linaro.org> wrote:
> On 5 April 2016 at 17:49, Ryan Harkin <ryan.harkin@linaro.org> wrote:

>> On 5 April 2016 at 09:37, Ryan Harkin <ryan.harkin@linaro.org> wrote:

>>> On 5 April 2016 at 03:57, Haojian Zhuang <haojian.zhuang@linaro.org> wrote:

>>>> On 5 April 2016 at 01:17, Ryan Harkin <ryan.harkin@linaro.org> wrote:

>>>>> Hi Haojian,

>>>>>

>>>>> I've had time to investigate where TC2 is hanging with your patches

>>>>> applied and narrowed it down to the single line of code marked below.

>>>>>

>>>>> I'm going to read the code now and see if I can work out what it's

>>>>> trying to do, but I thought I'd tell you sooner because you might have

>>>>> a better idea.

>>>>>

>>>>> On 22 March 2016 at 12:48, Haojian Zhuang <haojian.zhuang@linaro.org> wrote:

>>>>>> Add more SD commands to support 4-bit bus width & iospeed. It's not

>>>>>> formal code. And it needs to be updated later.

>>>>>>

>>>>>> Contributed-under: TianoCore Contribution Agreement 1.0

>>>>>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

>>>>>> ---

>>>>>>  EmbeddedPkg/Include/Protocol/MmcHost.h           |   3 +

>>>>>>  EmbeddedPkg/Universal/MmcDxe/Mmc.h               |  17 +++

>>>>>>  EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 138 ++++++++++++++++++++---

>>>>>>  3 files changed, 142 insertions(+), 16 deletions(-)

>>>>>>

>>>>>> diff --git a/EmbeddedPkg/Include/Protocol/MmcHost.h b/EmbeddedPkg/Include/Protocol/MmcHost.h

>>>>>> index 5e3a2b7..e9a74f0 100644

>>>>>> --- a/EmbeddedPkg/Include/Protocol/MmcHost.h

>>>>>> +++ b/EmbeddedPkg/Include/Protocol/MmcHost.h

>

>>>>>> +

>>>>>> +  /* SCR */

>>>>>> +  Status = MmcHost->SendCommand (MmcHost, MMC_ACMD51, 0);

>>>>>> +  if (EFI_ERROR (Status)) {

>>>>>> +    DEBUG ((EFI_D_ERROR, "%a(MMC_ACMD51): Error and Status = %r\n", __func__, Status));

>>>>>> +    return Status;

>>>>>> +  } else {

>>>>>> +    Status = MmcHost->ReadBlockData (MmcHost, 0, 8, Buffer);

>>>>>

>>>>> ^^ TC2 hangs at this line

>>>>>

>>>>

>>>> Ryan,

>>>>

>>>> Could you help to check where it hangs in

>>>> ArmPlatformPkg/Drivers/PL180MciDxe/PL180Mci.c?

>>>>

>>>

>>> I guess you mean, "where in function MciReadBlockData does it hang?".

>>>

>>> I'll have a look this asap.

>>>

>>

>> I made this mod to the code to add DEBUG:

>>

>> diff --git a/ArmPlatformPkg/Drivers/PL180MciDxe/PL180Mci.c

>> b/ArmPlatformPkg/Drivers/PL180MciDxe/PL180Mci.c

>> index 5526aac..7ddcf46 100644

>> --- a/ArmPlatformPkg/Drivers/PL180MciDxe/PL180Mci.c

>> +++ b/ArmPlatformPkg/Drivers/PL180MciDxe/PL180Mci.c

>> @@ -231,6 +231,7 @@ MciReadBlockData (

>>    do {

>>      // Read the Status flags

>>      Status = MmioRead32 (MCI_STATUS_REG);

>> +DEBUG ((EFI_D_ERROR, "RMH: MciReadBlockData(%d): 0x%x\n", __LINE__, Status));^M

>>

>>      // Do eight reads if possible else a single read

>>      if (Status & MCI_STATUS_CMD_RXFIFOHALFFULL) {

>>

>>

>> And after the InitializeSdMmcDevice debug, I see this output continuously:

>>

>> RMH: MciReadBlockData(234): 0x40

>>

>> If I add an MmioWrite32 to clear the flags at the start of the read

>> block function, I see the same behaviour, except it reads 0x00

>> continuously instead of 0x40.

>>

>

> There's a loop count in MciReadBlockData(). Could you print the value

> of variable Loop and

> variable Finish? Then I could know whether it reads any data from FIFO.

>


Sorry for the delay.  I added those two variables to the output and
they never change; no data is read:

RMH: MciReadBlockData(237): Status 0x0 Loop 0 Finish 128


> Regards

> Haojian

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox

Patch

diff --git a/ArmPlatformPkg/Drivers/PL180MciDxe/PL180Mci.c
b/ArmPlatformPkg/Drivers/PL180MciDxe/PL180Mci.c
index 5526aac..7ddcf46 100644
--- a/ArmPlatformPkg/Drivers/PL180MciDxe/PL180Mci.c
+++ b/ArmPlatformPkg/Drivers/PL180MciDxe/PL180Mci.c
@@ -231,6 +231,7 @@  MciReadBlockData (
   do {
     // Read the Status flags
     Status = MmioRead32 (MCI_STATUS_REG);
+DEBUG ((EFI_D_ERROR, "RMH: MciReadBlockData(%d): 0x%x\n", __LINE__, Status));^M

     // Do eight reads if possible else a single read
     if (Status & MCI_STATUS_CMD_RXFIFOHALFFULL) {