EmbeddedPkg/MmcDxe: Correct argument of ECSD read

[edk2] [PATCH] EmbeddedPkg/MmcDxe: Correct argument of ECSD read

Posted by Jun Nie 6 years, 9 months ago

The argument of CMD8 should be stuff bits according to standard
JESD84-A44.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
index 4ce0ddd..c28207e 100644
--- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
+++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
@@ -210,7 +210,7 @@ EmmcIdentificationMode (
   }
 
   // Fetch ECSD
-  Status = Host->SendCommand (Host, MMC_CMD8, RCA);
+  Status = Host->SendCommand (Host, MMC_CMD8, 0);
   if (EFI_ERROR (Status)) {
     DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD fetch error, Status=%r.\n", Status));
   }
-- 
1.9.1

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

Re: [edk2] [PATCH] EmbeddedPkg/MmcDxe: Correct argument of ECSD read

Posted by Leif Lindholm 6 years, 9 months ago

On Thu, Jun 29, 2017 at 05:02:05PM +0800, Jun Nie wrote:
> The argument of CMD8 should be stuff bits according to standard
> JESD84-A44.

OK, I realise that "stuff bits" is a term used by the spec, so that is
probably sufficient explanation even though the term was known to me.
And the MdeModulePkg driver seems to agree on the technical point.
My question is why zeroes is the correct "stuff bits" value?

> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> index 4ce0ddd..c28207e 100644
> --- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> +++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> @@ -210,7 +210,7 @@ EmmcIdentificationMode (
>    }
>  
>    // Fetch ECSD
> -  Status = Host->SendCommand (Host, MMC_CMD8, RCA);
> +  Status = Host->SendCommand (Host, MMC_CMD8, 0);
>    if (EFI_ERROR (Status)) {
>      DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD fetch error, Status=%r.\n", Status));
>    }
> -- 
> 1.9.1
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] EmbeddedPkg/MmcDxe: Correct argument of ECSD read

Posted by Jun Nie 6 years, 9 months ago

2017-06-29 20:09 GMT+08:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Thu, Jun 29, 2017 at 05:02:05PM +0800, Jun Nie wrote:
>> The argument of CMD8 should be stuff bits according to standard
>> JESD84-A44.
>
> OK, I realise that "stuff bits" is a term used by the spec, so that is
> probably sufficient explanation even though the term was known to me.
> And the MdeModulePkg driver seems to agree on the technical point.
> My question is why zeroes is the correct "stuff bits" value?

Yes, it is defined in page 2 in spec. I guess 0 is best filling value
than other value when we do not need a real value.
stuff bit: filling 0 bits to ensure fixed length frames for commands
and responses.

>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>> ---
>>  EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
>> index 4ce0ddd..c28207e 100644
>> --- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
>> +++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
>> @@ -210,7 +210,7 @@ EmmcIdentificationMode (
>>    }
>>
>>    // Fetch ECSD
>> -  Status = Host->SendCommand (Host, MMC_CMD8, RCA);
>> +  Status = Host->SendCommand (Host, MMC_CMD8, 0);
>>    if (EFI_ERROR (Status)) {
>>      DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD fetch error, Status=%r.\n", Status));
>>    }
>> --
>> 1.9.1
>>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] EmbeddedPkg/MmcDxe: Correct argument of ECSD read

Posted by Leif Lindholm 6 years, 9 months ago

On Thu, Jun 29, 2017 at 10:29:08PM +0800, Jun Nie wrote:
> 2017-06-29 20:09 GMT+08:00 Leif Lindholm <leif.lindholm@linaro.org>:
> > On Thu, Jun 29, 2017 at 05:02:05PM +0800, Jun Nie wrote:
> >> The argument of CMD8 should be stuff bits according to standard
> >> JESD84-A44.
> >
> > OK, I realise that "stuff bits" is a term used by the spec, so that is
> > probably sufficient explanation even though the term was known to me.
> > And the MdeModulePkg driver seems to agree on the technical point.
> > My question is why zeroes is the correct "stuff bits" value?
> 
> Yes, it is defined in page 2 in spec. I guess 0 is best filling value
> than other value when we do not need a real value.
> stuff bit: filling 0 bits to ensure fixed length frames for commands
> and responses.

Yeah, fair enough.
It literally is just "nothing".
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

Pushed as 0ad564ffe7.
(Oh, and "Add non-DDR timing mode support" which I had seen as a
companion patch to the ECSD alignment fix also pushed, as 44f4ff6257.)

/
    Leif

> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> >> ---
> >>  EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> >> index 4ce0ddd..c28207e 100644
> >> --- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> >> +++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> >> @@ -210,7 +210,7 @@ EmmcIdentificationMode (
> >>    }
> >>
> >>    // Fetch ECSD
> >> -  Status = Host->SendCommand (Host, MMC_CMD8, RCA);
> >> +  Status = Host->SendCommand (Host, MMC_CMD8, 0);
> >>    if (EFI_ERROR (Status)) {
> >>      DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD fetch error, Status=%r.\n", Status));
> >>    }
> >> --
> >> 1.9.1
> >>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel