1. Patchew
  2. QEMU
  3. [PATCH v2 00/25] hw/riscv: sifive_u: Add missing SPI support
  4. View patch

[PATCH v2 09/25] hw/sd: ssi-sd: Fix the wrong command index for STOP_TRANSMISSION

Bin Meng posted 25 patches 5 years ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Alistair Francis <alistair@alistair23.me>, Alistair Francis <Alistair.Francis@wdc.com>, Palmer Dabbelt <palmer@dabbelt.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Max Reitz <mreitz@redhat.com>
There is a newer version of this series
[PATCH v2 09/25] hw/sd: ssi-sd: Fix the wrong command index for STOP_TRANSMISSION
Posted by Bin Meng 5 years ago 
From: Bin Meng <bin.meng@windriver.com>

This fixes the wrong command index for STOP_TRANSMISSION, the
required command to interrupt the multiple block read command,
in the old codes. It should be CMD12 (0x4c), not CMD13 (0x4d).

Fixes: 775616c3ae8c ("Partial SD card SPI mode support")
Signed-off-by: Bin Meng <bin.meng@windriver.com>

---

Changes in v2:
- Make this fix a separate patch from the CMD18 support

 hw/sd/ssi-sd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 5763afeba0..9e2f13374a 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -83,7 +83,7 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
     ssi_sd_state *s = SSI_SD(dev);
 
     /* Special case: allow CMD12 (STOP TRANSMISSION) while reading data.  */
-    if (s->mode == SSI_SD_DATA_READ && val == 0x4d) {
+    if (s->mode == SSI_SD_DATA_READ && val == 0x4c) {
         s->mode = SSI_SD_CMD;
         /* There must be at least one byte delay before the card responds.  */
         s->stopping = 1;
-- 
2.25.1
Re: [PATCH v2 09/25] hw/sd: ssi-sd: Fix the wrong command index for STOP_TRANSMISSION
Posted by Philippe Mathieu-Daudé 5 years ago 
On 1/23/21 11:40 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> This fixes the wrong command index for STOP_TRANSMISSION, the
> required command to interrupt the multiple block read command,
> in the old codes. It should be CMD12 (0x4c), not CMD13 (0x4d).
> 
> Fixes: 775616c3ae8c ("Partial SD card SPI mode support")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> 
> ---
> 
> Changes in v2:
> - Make this fix a separate patch from the CMD18 support
> 
>  hw/sd/ssi-sd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
> index 5763afeba0..9e2f13374a 100644
> --- a/hw/sd/ssi-sd.c
> +++ b/hw/sd/ssi-sd.c
> @@ -83,7 +83,7 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
>      ssi_sd_state *s = SSI_SD(dev);
>  
>      /* Special case: allow CMD12 (STOP TRANSMISSION) while reading data.  */
> -    if (s->mode == SSI_SD_DATA_READ && val == 0x4d) {
> +    if (s->mode == SSI_SD_DATA_READ && val == 0x4c) {

Patch is correct, but I wonder if we couldn't improve using instead:

     if (s->mode == SSI_SD_DATA_READ && ((val & 0x3f) == 12)) {

>          s->mode = SSI_SD_CMD;
>          /* There must be at least one byte delay before the card responds.  */
>          s->stopping = 1;
>
Re: [PATCH v2 09/25] hw/sd: ssi-sd: Fix the wrong command index for STOP_TRANSMISSION
Posted by Philippe Mathieu-Daudé 5 years ago 
On 1/24/21 6:33 PM, Philippe Mathieu-Daudé wrote:
> On 1/23/21 11:40 AM, Bin Meng wrote:
>> From: Bin Meng <bin.meng@windriver.com>
>>
>> This fixes the wrong command index for STOP_TRANSMISSION, the
>> required command to interrupt the multiple block read command,
>> in the old codes. It should be CMD12 (0x4c), not CMD13 (0x4d).
>>
>> Fixes: 775616c3ae8c ("Partial SD card SPI mode support")
>> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>>
>> ---
>>
>> Changes in v2:
>> - Make this fix a separate patch from the CMD18 support
>>
>>  hw/sd/ssi-sd.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
>> index 5763afeba0..9e2f13374a 100644
>> --- a/hw/sd/ssi-sd.c
>> +++ b/hw/sd/ssi-sd.c
>> @@ -83,7 +83,7 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
>>      ssi_sd_state *s = SSI_SD(dev);
>>  
>>      /* Special case: allow CMD12 (STOP TRANSMISSION) while reading data.  */
>> -    if (s->mode == SSI_SD_DATA_READ && val == 0x4d) {
>> +    if (s->mode == SSI_SD_DATA_READ && val == 0x4c) {
> 
> Patch is correct,

Forgot to amend:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> but I wonder if we couldn't improve using instead:
> 
>      if (s->mode == SSI_SD_DATA_READ && ((val & 0x3f) == 12)) {
> 
>>          s->mode = SSI_SD_CMD;
>>          /* There must be at least one byte delay before the card responds.  */
>>          s->stopping = 1;
>>
>
Re: [PATCH v2 09/25] hw/sd: ssi-sd: Fix the wrong command index for STOP_TRANSMISSION
Posted by Bin Meng 5 years ago 
On Mon, Jan 25, 2021 at 1:33 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 1/23/21 11:40 AM, Bin Meng wrote:
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > This fixes the wrong command index for STOP_TRANSMISSION, the
> > required command to interrupt the multiple block read command,
> > in the old codes. It should be CMD12 (0x4c), not CMD13 (0x4d).
> >
> > Fixes: 775616c3ae8c ("Partial SD card SPI mode support")
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> >
> > ---
> >
> > Changes in v2:
> > - Make this fix a separate patch from the CMD18 support
> >
> >  hw/sd/ssi-sd.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
> > index 5763afeba0..9e2f13374a 100644
> > --- a/hw/sd/ssi-sd.c
> > +++ b/hw/sd/ssi-sd.c
> > @@ -83,7 +83,7 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
> >      ssi_sd_state *s = SSI_SD(dev);
> >
> >      /* Special case: allow CMD12 (STOP TRANSMISSION) while reading data.  */
> > -    if (s->mode == SSI_SD_DATA_READ && val == 0x4d) {
> > +    if (s->mode == SSI_SD_DATA_READ && val == 0x4c) {
>
> Patch is correct, but I wonder if we couldn't improve using instead:
>
>      if (s->mode == SSI_SD_DATA_READ && ((val & 0x3f) == 12)) {
>

(val & 0x3f == 12) isn't correct because it can catch values equal to
0x7c or 0xfc.

Regards,
Bin
Re: [PATCH v2 09/25] hw/sd: ssi-sd: Fix the wrong command index for STOP_TRANSMISSION
Posted by Bin Meng 5 years ago 
On Mon, Jan 25, 2021 at 8:33 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Mon, Jan 25, 2021 at 1:33 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >
> > On 1/23/21 11:40 AM, Bin Meng wrote:
> > > From: Bin Meng <bin.meng@windriver.com>
> > >
> > > This fixes the wrong command index for STOP_TRANSMISSION, the
> > > required command to interrupt the multiple block read command,
> > > in the old codes. It should be CMD12 (0x4c), not CMD13 (0x4d).
> > >
> > > Fixes: 775616c3ae8c ("Partial SD card SPI mode support")
> > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > >
> > > ---
> > >
> > > Changes in v2:
> > > - Make this fix a separate patch from the CMD18 support
> > >
> > >  hw/sd/ssi-sd.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
> > > index 5763afeba0..9e2f13374a 100644
> > > --- a/hw/sd/ssi-sd.c
> > > +++ b/hw/sd/ssi-sd.c
> > > @@ -83,7 +83,7 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
> > >      ssi_sd_state *s = SSI_SD(dev);
> > >
> > >      /* Special case: allow CMD12 (STOP TRANSMISSION) while reading data.  */
> > > -    if (s->mode == SSI_SD_DATA_READ && val == 0x4d) {
> > > +    if (s->mode == SSI_SD_DATA_READ && val == 0x4c) {
> >
> > Patch is correct, but I wonder if we couldn't improve using instead:
> >
> >      if (s->mode == SSI_SD_DATA_READ && ((val & 0x3f) == 12)) {
> >
>
> (val & 0x3f == 12) isn't correct because it can catch values equal to
> 0x7c or 0xfc.

Sorry, wrong calculation. It should read:

It can catch values 0x0c, 0x8c, 0xcc.

Regards,
Bin

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.