| Message ID | 20210123104016.17485-10-bmeng.cn@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/riscv: sifive_u: Add missing SPI support | expand |
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; >
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; >> >
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
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
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;