On Wed, 26 Jan 2022 at 03:13, <frank.ch...@sifive.com> wrote: > > From: Frank Chang <frank.ch...@sifive.com> > > In SPI-mode, CMD58 returns R3 response with the format: > > 39 32 31 0 > +------------+ +-----------------------------------+ > | R1 | | OCR | > +------------+ +-----------------------------------+ > > Where R1 has bits[0] indicating whether SD card is "in idle state". > However, according to SD card state transition table, CMD58 can only be > transited from trans to data state, which the "in idle state" bit should > not be set in CMD58's R3 response. > (But CMD8 should still have "in idle state" bit to be set in its > R7 response because it can only be transited from idle to idle state.) > > Signed-off-by: Frank Chang <frank.ch...@sifive.com> > --- > hw/sd/ssi-sd.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c > index 167c03b780..7faa969e82 100644 > --- a/hw/sd/ssi-sd.c > +++ b/hw/sd/ssi-sd.c > @@ -176,12 +176,17 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, > uint32_t val) > s->arglen = 1; > s->response[0] = 4; > DPRINTF("SD command failed\n"); > - } else if (s->cmd == 8 || s->cmd == 58) { > - /* CMD8/CMD58 returns R3/R7 response */ > - DPRINTF("Returned R3/R7\n"); > + } else if (s->cmd == 8) { > + /* CMD8 returns R7 response */ > + DPRINTF("Returned R7\n"); > s->arglen = 5; > s->response[0] = 1; > memcpy(&s->response[1], longresp, 4); > + } else if (s->cmd == 58) { > + /* CMD58 returns R3 response */ > + DPRINTF("Returned R3\n"); > + s->arglen = 5; > + memcpy(&s->response[1], longresp, 4); > } else if (s->arglen != 4) { > BADF("Unexpected response to cmd %d\n", s->cmd); > /* Illegal command is about as near as we can get. */
This demonstrates a problem with trying to implement SPI mode as "SD mode but the controller has to fix up the differences". Here CMD8 and CMD58 in SPI mode are supposed to return 5 bytes of data, of which one byte is format-R1 status information. But our SD card emulation returns SD mode R3 or R7 format, which don't include that information. In the current code in ssi-sd.c, which this patch is tweaking, we try to fake up a status byte. However, this isn't going to have the right behaviour, because this should count as a read that clears the "clear on read" status bits, and faking up the response byte won't do that. (For that matter, presumably the clear-on-read bits that are only present in the R2 2-byte response should not be cleared when the SPI command gets a 1-byte R1 response. I think that the other patch you've sent won't do that.) That all leaves me wondering if what we should really do here is make sd.c's SPI mode actually change the format of responses, rather than leaving it to the controller to try to do that. (This would also be useful if in future we need to model more than one controller that puts the card in SPI mode.) Side note, in sd.c sd_normal_command() we "return sd_r3" for the cmd58 case, so I think this will cause us to actually send the OCR data rather than the voltage-status data... thanks -- PMM