On 27/10/23 12:41, Ulf Hansson wrote: > On Thu, 26 Oct 2023 at 12:07, Avri Altman <Avri.Altman@xxxxxxx> wrote: >> >>> On 25/10/23 14:30, Avri Altman wrote: >>>> Field Firmware Update (ffu) may use close-ended or open ended sequence. >>>> Each such sequence is comprised of a write commands enclosed between 2 >>>> switch commands - to and from ffu mode. >>>> >>>> Some platforms generate auto command error interrupt when it shouldn't, >>>> e.g. auto-cmd12 while in close-ended ffu sequence. I encountered this >>>> issue while testing fwupd (github.com/fwupd/fwupd) on HP Chromebook >>> x2, >>>> a qualcomm based QC-7c, code name - strongbad. Instead of a quirk, make >>>> sure it disable auto-cmd12 while close-ended ffu is in progress. >>> >>> I think I misunderstood this because I was thinking that auto-cmd12 >>> was being used with an open-ended sequence, and that it wasn't >>> working with FFU. However it seems mmc-utils is using a closed-ended >>> sequence. >> Yes, mmc-utils, fwupd, as well as others - uses close-ended, >> And unlike rpmb - it sends cmd23 as part of the ffu sequence. >> >>> >>> It looks like the the host controller driver doesn't know that, >>> because the ioctl interface does not use mrq.sbc and the >>> SET_BLOCK_COUNT command is sent separately. Then when the >>> MULTI_WRITE >>> command is issued, the host controller driver treats it as open-ended >>> and will enable auto-cmd12 if the controller supports it. >>> >>> If that is the case, it would be better to fix the ioctl handling >>> and make it use mrq.sbc instead of issuing SET_BLOCK_COUNT separately. >> We can do that. >> On the other hand, this doesn't happen on other platforms. >> Fwupd has just recently switched to close-ended, but mmc-utils is using close-ended mode for many years, >> Performing ffu successfully on many different platforms. >> My understanding is, that the hw should realize that cmd23 has just sent prior to cmd25 and avoid this auto-cmd12. > > Yes, in principle that's correct. > > In fact, I think that most host drivers should already support this > behavior, although it relies on the CMD23 to be incorporated within > the same mmc request (mrq) as the CMD25. We use "mrq.sbc" for this and > the host driver uses MMC_CAP_CMD23 to inform the MMC core whether it > supports this or not. > >> >> Going back to your proposal, we can ignore cmd23 in close-ended ffu, but eventually, >> we will need to change mmc-utils and fwupd to stop send cmd23. > > This is not what we proposed, at least if I understood Adrian correctly. > > Instead, the idea that could make better sense, is to fix the mmc > ioctl handling in the mmc core, so that it can discover that a CMD23 > command is followed by another CMD18/25 (multiple read/write). And in > this case, it should boundle the commands together, using mrq.sbc so > that one request gets sent to the mmc host driver instead of two. Yes that is what I was thinking. Perhaps look at __mmc_blk_ioctl_cmd() first. It doesn't have enough information to decide what to do, so either something needs to be added to struct mmc_blk_ioc_data and set up before hand, or it needs to be passed struct mmc_queue_req *mq_rq. > > In this way, there should be no need for any specific changes to any > of the host drivers (assuming they have the CMD23 support implemented > correctly), they should just work. > > [...] > > Kind regards > Uffe
