On Sun, Dec 6, 2015 at 10:03 PM, Andrew Baumann <andrew.baum...@microsoft.com> wrote: > Hi Peter, > >> From: Peter Crosthwaite [mailto:crosthwaitepe...@gmail.com] >> Sent: Sunday, 6 December 2015 15:47 >> On Sun, Dec 6, 2015 at 1:20 PM, Andrew Baumann >> <andrew.baum...@microsoft.com> wrote: >> >> From: Peter Crosthwaite [mailto:crosthwaitepe...@gmail.com] >> >> Sent: Saturday, 5 December 2015 22:27 >> >> On Fri, Dec 4, 2015 at 1:16 PM, Andrew Baumann >> >> <andrew.baum...@microsoft.com> wrote: >> >> > @@ -1563,6 +1592,11 @@ void sd_write_data(SDState *sd, uint8_t >> value) >> >> > sd->data_offset = 0; >> >> > sd->csd[14] |= 0x40; >> >> > >> >> > + if (sd->multi_blk_active) { >> >> > + assert(sd->multi_blk_cnt > 0); >> >> > + sd->multi_blk_cnt--; >> >> >> >> So reading the spec, it says that this command is an alternative to a >> >> host issued CMD12. Does this mean completion of the length-specified >> >> read should move back to sd_transfer_state the same way CMD12 does? >> > >> > That was my initial assumption (and implementation) too, but no: table 4- >> 34 in the spec shows that you stay in transfer mode, and UEFI issues CMD12 >> after each multiple-block (CMD23) I/O, which doesn't work if you've already >> left the transfer state. >> >> So table 4-34 doesn't really help as that is showing the state >> following a command itself, whereas an automatic CMD12 would happen >> following the end of a data payload (not an explicit command). It >> would if anything be a form of "Operation complete" (first line in >> 4-34) which would be a state transition. You mention that CMD12 >> doesn't work if you have left transfer_state, and that is accurate. >> However CMD12 normally doesn't transfer out of the transfer_state, >> instead it transfers from data->tran or rcv->prog (note rcv->prog is >> as good as rcv->tran for us as we model prog state as instantaneous). >> If your original implementation transferred to something that is not >> tran that would fail. >> >> I have both the eMMC and SD specs infront of me. Here is what I have for >> eMMC: >> >> " >> Multiple block read with pre-defined block count: >> >> The card will transfer the requested number of data blocks, terminate >> the transaction and return to transfer state. Stop command is not >> required at the end of this type of multiple block read, unless >> terminated with an error. In order to start a multiple block read with >> pre-defined block count the host must use the SET_BLOCK_COUNT >> command >> (CMD23) immediately preceding the READ_MULTIPLE_BLOCK (CMD18) >> command. >> Otherwise the card will start an open-ended multiple block read which >> can be stopped using the STOP_TRANSMISION command. >> " >> >> That to me suggests that CMD12 should not be needed at all, but does >> mention CMD12 is needed in error paths. >> >> SD spec has this: >> >> " >> CMD12 has been used to stop multiple-block Read / Write operation. >> However, CMD12 is timing dependent and it is difficult to control >> timing to issue CMD12 at exact timing. As UHS104 card has large delay >> variation between clock and data, CMD23 is useful for the host to stop >> multiple read / write operation instead of CMD12. Host is not >> necessary to control timing of CMD12. This command is applicable to >> always 512-byte block length read/write operation and then SDSC card >> does not support this command. Support of CMD23 is mandatory for >> UHS104 card. >> >> Support of CMD23 is defined in SCR. The response type of CMD23 is R1 >> and busy is not indicated. CMD23 is accepted in transfer state and >> effective to the multiple-block read/write command (CMD18 or CMD25) >> just behind CMD23. If another command follows CMD23, set block count >> is canceled (including CMD13). If command CRC error occurs, the card >> does not return R1 response for CMD23. In this case, Set block count >> is not valid and retry of CMD23 is required. If multiple CMD23 are >> issued, the last one is valid. >> >> Figure 4-58 shows the definition of CMD23. If block count in the >> argument is set to 0, CMD23 has no effect. The block count value set >> by CMD23 is not checked by the card and then CMD23 does not indicate >> any error in the response (A previous command error is indicated in >> the response of CMD23). If illegal block count is set, out of range >> error will be indicated during read/write operation (For example, data >> transfer is stopped at user area boundary). Host needs to issue CMD12 >> if any error is detected in the CMD18 and CMD25 operations. If a CMD25 >> is aborted and the amount of data transferred is less >> than the amount of data indicated by the preceding CMD23, then the >> area specified by CMD23 that is unwritten may contain undefined data. >> If the amount of data transferred is greater than the amount of >> data indicated by the preceding CMD23, then the extra data is not written. >> " >> >> which is significantly less clear but still suggest that CMD12 is not >> needed. The last bit ("If the amount of data transferred is greater >> than the amount of data indicated by the preceding CMD23, then the >> extra data is not written") is what you have implemented very >> directly, but would also be a consequence of an automatic CMD12 at the >> end of a data-phase. >> >> Some possibilities: >> >> 1: The SD card vendors don't follow the spec (as shown in 4-34) and >> just ignore a spurious CMD12 when you have already returned to >> transfer state. This would explain everything. >> 2: UEFI is using early termination in the normal code path. >> >> Can you give me a source code pointer to the CMD12 in UEFI by any >> chance? I'd like to have a look at the driver logic. > > Thanks for the detailed investigation of this. I now actually think you're > probably correct. The MMC spec is certainly clear, much more so than the SD > spec which I was looking at until now. > > I went looking for the UEFI code. It turns out that this use of CMD23 is not > in the mainline version of EDK2; it appears to have been added somewhere > along the way to building the Pi2 bootloader (I'm not sure why), so we > probably shouldn't place a huge amount of faith in it.
Is that in a git repo that we could run a git blame on? Regards, Peter >It also doesn't check for the success/failure of the CMD12 which it issues. >After my original implementation, I saw a console full of "CMD12 in a wrong >state" printfs and immediately assumed I was doing something wrong, but I now >think my first guess was correct and I should have silenced the printf >instead. I'll dust that code off and resubmit the patch, assuming it tests ok. > > Andrew
