On 16/11/2021 23:53, Bart Van Assche wrote: > On 11/16/21 12:16, Adrian Hunter wrote: >> On 16/11/2021 18:08, Bart Van Assche wrote: >>> On 11/16/21 1:07 AM, Peter Wang wrote: >>>> Should we add unmap? >>> >>> Hi Peter, >>> >>> I will add DMA unmapping code in the abort handler. >> >> I would note that __ufshcd_transfer_req_compl() does that, as well as providing >> a matching ufshcd_release() for the ufshcd_hold() in ufshcd_queuecommand(), so >> do consider __ufshcd_transfer_req_compl(). >> >> Using __ufshcd_transfer_req_compl() seems consistent with the error handler which >> uses __ufshcd_transfer_req_compl() via ufshcd_complete_requests(), which will pick >> up all the requests that the error handler has just aborted via >> ufshcd_try_to_abort_task(). Also ufshcd_host_reset_and_restore() uses >> __ufshcd_transfer_req_compl() via ufshcd_complete_requests(), which will pick >> up anything still in outstanding_reqs because the doorbell has become zero at >> that point. > > Hi Adrian, > > Although I agree with minimizing code duplication, I'm not sure that __ufshcd_transfer_req_compl() should be called from inside ufshcd_abort(). __ufshcd_transfer_req_compl() also calls ufshcd_update_monitor() and ufshcd_add_command_trace(hba, index, UFS_CMD_COMP). Neither function should be called when aborting a command. But we should trace the abort I would have thought - seems like a separate issue though.
