On Tue, 9 Mar 2021 at 14:18, Luca Porzio <porzio@xxxxxxxxx> wrote: > > >> >> This means adding a new path for when the host needs to get locked >> (claimed), which is the opposite direction of what we have been >> working on for SD/eMMC during the last couple of years. >> >> Please have a look at mmc_blk_issue_drv_op(), where you can find how >> these kinds of requests are being funneled through the mmc block >> device layer instead. This is the preferred option. >> >> That said, I am actually wondering if we perhaps could manage the >> enable/disable of CQE "automagically" for FFU, along the lines of what >> we do for RPMB already. In fact, maybe the best/easiest approach is to >> *always* disable CQE when there is a request being received through >> the mmc ioctl interface. Of course, then if we disabled CQE we should >> also re-enable it when the ioctl request has been completed. >> >> What do you think? >> >> > > Thanks a lot for your feedback! > > The reason why this is an RFC patch and not a clear patch is exactly what you > say here. > During the FFU (as well as other sequence of commands issued through > multi cmd() ioctl feature, is that sometimes it may require an emmc reboot or > some other legacy command subsequent sequence (= which needs to be > sent with two ioctl() multi_cmd sequences) and the legacy mode need to > survive across reboots or partition changes. The reboot is kind of a separate problem, even if it's related, isn't it? What you propose is to add a sysfs node to let userspace enable/disable CQE. I was under the impression that this was solely to allow the FFU process to be completed, no? Are you saying that we may want CQE to stay disabled beyond the FFU process, to allow the reboot to be completed safely? > Also we need to claim the host to make sure all the pending commands > in the queue are completed successfully before disabling. Yes, of course. It sounds like you may have misinterpreted my proposals. The problem is not that we need to claim the host, but that you add an entirely new path to do it. *If* we conclude that we should add a sysfs node to control CQE, we should create a mmc blk request for it (which will manage the claim for us as well). I suggest you have a closer look at power_ro_lock_store(), which should be the equivalent to what we need to implement here. > > I can rethink the patch to implement a specific iotcl() request which disables > CMDQ if you think that is a better implementation but in the end it will still > require the host claim. > > Any feedback or suggestion is appreciated. To be clear, I am not proposing to add a new ioctl for mmc. Those we have today, should be sufficient I think. However, rather than adding a new sysfs node to control CQE, perhaps we can parse the received ioctl data structure, to find out if the command/request is for FFU and then take specific actions. Along the lines of what we do for mmc_sanitize(), for example. Another option, rather than parsing ioctl data for the FFU command/request, is to always temporarily disable CQE while processing an mmc ioctl request. Would this work? Kind regards Uffe
