Re: [PATCH v4 02/11] MmcDxe: move ECSD into CardInfo structure

[Leif Lindholm <leif.lindholm@linaro.org>]

On Tue, Nov 08, 2016 at 11:21:07PM +0800, Haojian Zhuang wrote:
> Since ECSD also describes the information of card, move it into
> structure CardInfo.

I like the fact that you have split out the "move ECSD to CARD_INFO"
and "declare ECSD structure", to keep the two logically separate
modifications apart.

However, it would be better if "declare ECSD structure" followed
directly after this one.

> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  EmbeddedPkg/Universal/MmcDxe/Mmc.h               | 1 +
>  EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 5 ++---
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> index 46a156c..4f132c6 100644
> --- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> +++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> @@ -138,6 +138,7 @@ typedef struct  {
>    OCR       OCRData;
>    CID       CIDData;
>    CSD       CSDData;
> +  UINT32    ECSD[128];      // MMC V4 extended card specific
>  } CARD_INFO;
>  
>  typedef struct _MMC_HOST_INSTANCE {
> diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> index 3f72b7f..578fcb6 100644
> --- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> +++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> @@ -39,7 +39,6 @@ EmmcIdentificationMode (
>    EFI_BLOCK_IO_MEDIA    *Media;
>    EFI_STATUS Status;
>    UINT32     RCA;
> -  UINT32     ECSD[128];
>  
>    Host  = MmcHostInstance->MmcHost;
>    Media = MmcHostInstance->BlockIo.Media;
> @@ -91,7 +90,7 @@ EmmcIdentificationMode (
>      DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD fetch error, Status=%r.\n", Status));
>    }
>  
> -  Status = Host->ReadBlockData (Host, 0, 512, ECSD);
> +  Status = Host->ReadBlockData (Host, 0, 512, (UINT32 *)&(MmcHostInstance->CardInfo.ECSD));
>    if (EFI_ERROR (Status)) {
>      DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD read error, Status=%r.\n", Status));
>      return Status;
> @@ -104,7 +103,7 @@ EmmcIdentificationMode (
>    Media->LogicalBlocksPerPhysicalBlock = 1;
>    Media->IoAlign = 4;
>    // Compute last block using bits [215:212] of the ECSD
> -  Media->LastBlock = ECSD[EMMC_ECSD_SIZE_OFFSET] - 1; // eMMC isn't supposed to report this for
> +  Media->LastBlock = MmcHostInstance->CardInfo.ECSD[EMMC_ECSD_SIZE_OFFSET] - 1; // eMMC isn't supposed to report this for
>    // Cards <2GB in size, but the model does.
>  
>    // Setup card type
> -- 
> 2.7.4
>