> -----Original Message----- > From: Christian Löhle <CLoehle@xxxxxxxxxxxxxx> > Sent: Tuesday, December 13, 2022 5:26 PM > To: Avri Altman <Avri.Altman@xxxxxxx> > Cc: linux-mmc@xxxxxxxxxxxxxxx; ulf.hansson@xxxxxxxxxx > Subject: RE: [PATCH] mmc-utils: Add basic erase error check > > CAUTION: This email originated from outside of Western Digital. Do not click > on links or open attachments unless you recognize the sender and know that > the content is safe. > > > >> > For SD a CMD13 after CMD38 is required, too. > >> > I guess I can add that. > >> > >> Just realized that sending CMD13 is not sufficient as the kernel will > >> poll because of R1B and clear the error flag. > >> Anyway I have this kernel patch for a write flag bit that aggregates > >> errors during polling until card is in TRAN again. > >> I will send it then, since I don't think there is a good way of > >> solving this for SD in mmc-utils, please consider this patch on its own. > > Leaving SD aside for now - I Still wasn't able to get an expert opinion - > holiday season etc. Got it now. Although table 70 seems simple and definitive - Apparently I misinterpret it. Sorry about that. > > While waiting however, looking in Table 70 - Device Status field/command - > cross reference, I can see that : > > - ERASE_RESET - is not reported for any of cmd35, cmd36, and cmd38 > > - ERASE_PARAM - is 'X' for cmd35 only > > - ERASE_SEQ_ ERROR - is 'R' for cmd35 and cmd36 > > > > So potentially only ERASE_SEQ_ ERROR may reside in those commands > responses. > > But mmc-utils uses multi-ioctl for that, so there couldn't be any mis- > ordering? > > Which error bits you see in which command responses? > > > > Thanks, > > Avri > > Hey Avri, > Thanks for having a look at this. > Yeah I have indeed only seen ERASE_SEQ_ERROR, but added the rest > because it doesn't hurt. ERASE_SEQ_ERROR - It is set when e.g. CMD36 is given before CMD35 e.g. the following CMDs are issued CMD36 -> CMD35 -> CMD38 In the response of CMD36, ERASE_SEQ_ERROR error bit will be reported. This can't be happening for mmc-utils because it send the correct sequence using multi-ioctl - Which assure it can't be interrupted by any other command. The trace that you attached looks like a fw bug to me. > Why would you say ERASE_PARAM shouldn't be checked? Or are you arguing > it should only be checked at CMD38 (i.e. the X of CMD35)? ERASE_PARAM - It is set when wrong address is given for first erase group in CMD35, And is reported in the response of CMD36. This is actually the only error bit we should look for. > (In which case I agree, just didn't want more than one error mask) > Seeing a ERASE_PARAM would definitely make the erase not happen (that's > all mmc-utils should really care about). > ERASE_RESET may be removed for sure, could be checked when a SD ioctl > erase fix like I described is in the kernel. ERASE_RESET is set when any other commands apart from CMD35/36/38/13 is sent during the erase sequence. This as aforesaid, can't be happening for mmc-utils. > Then an ideal mmc-utils erase operation checks that no relevant R1 bits are > set at CMD38 RSP and all CMD13 until card leaves PRG and stops signalling > busy. Are you suggesting that we should include cmd13 as the 4th command in the erase sequence? I am not sure we need it. To conclude, IMO only ERASE_PARAM should be checked in the response of CMD36. Thanks, Avri > > > For an erase call like this: > mmc-utils erase legacy 0 0xFFFFFFFF /dev/mmcblk2 > > the MMC trace (according to spec and most cards I tried, this is one of them) > looks like this: > Format: timestamp,type,CMDOPCODE, > for type 1 (CMD) and type 2 (Resp48) then the next field is arg/response in > hex. > I guess rest is more or less self-explanatory / not relevant. > > 325533.758261654,1,13,00010000 > 325533.758282029,2,13,00000900, READY_FOR_DATA, TRANS_STATE > 325534.549850485,1,08,00000000 > 325534.549874693,2,08,00000900, READY_FOR_DATA, TRANS_STATE > 325534.550132818,4,08,0,512 > 325534.550132818,5,08,00000000000000000000000... REDACTED FOR SIZE > 325534.550693693,1,35,00000000 > 325534.550710026,2,35,00000900, READY_FOR_DATA, TRANS_STATE > 325534.550761651,1,36,ffffffff > 325534.550774485,2,36,80000900, ADDRESS_OUT_OF_RANGE, > READY_FOR_DATA, TRANS_STATE > 325534.552136276,1,38,00000000 > 325534.552164568,2,38,10000900, ERASE_SEQ_ERROR, READY_FOR_DATA, > TRANS_STATE > 325534.552227568,1,13,00010000 > 325534.552241276,2,13,00000900, READY_FOR_DATA, TRANS_STATE > > Hope that helps. > > Regards, > Christian > > Hyperstone GmbH | Reichenaustr. 39a | 78467 Konstanz > Managing Director: Dr. Jan Peter Berns. > Commercial register of local courts: Freiburg HRB381782
