On Wed, 2018-08-01 at 11:04 +-0300, Avri Altman wrote: +AD4- +- wait+AF8-event(hba-+AD4-tm+AF8-tag+AF8-wq, ufshcd+AF8-get+AF8-tm+AF8-free+AF8-slot(hba, +ACY-free+AF8-slot))+ADs- The above is the weirdest API I have seen so far for tag allocation. Why does ufshcd+AF8-get+AF8-tm+AF8-free+AF8-slot() does a linear search through a bitmap instead of using the sbitmap data structure? +AD4- +- ufshcd+AF8-writel(hba, 1 +ADwAPA- free+AF8-slot, REG+AF8-UTP+AF8-TASK+AF8-REQ+AF8-DOOR+AF8-BELL)+ADs- +AD4- +- /+ACo- Make sure that doorbell is committed immediately +ACo-/ +AD4- +- wmb()+ADs- This is the first time that I see someone claiming that issuing a write memory barrier causes the write to be executed faster than without memory barrier. Are you sure that comment is correct and that that wmb() is necessary? +AD4- +- spin+AF8-unlock+AF8-irqrestore(host-+AD4-host+AF8-lock, flags)+ADs- +AD4- +- +AD4- +- /+ACo- wait until the task management command is completed +ACo-/ +AD4- +- err +AD0- wait+AF8-event+AF8-timeout(hba-+AD4-tm+AF8-wq, +AD4- +- test+AF8-bit(free+AF8-slot, +ACY-hba-+AD4-tm+AF8-condition), +AD4- +- msecs+AF8-to+AF8-jiffies(TM+AF8-CMD+AF8-TIMEOUT))+ADs- Did you perhaps start implementing the ufshcd+AF8-issue+AF8-tm+AF8-upiu+AF8-cmd() function by copy/pasting ufshcd+AF8-issue+AF8-tm+AF8-cmd()? Please don't do that and instead avoid code duplication by moving shared code in a new function. +AD4- +- /+ACo- ignore the returning value here - ufshcd+AF8-check+AF8-query+AF8-response is +AD4- +- +ACo- bound to fail since dev+AF8-cmd.query and dev+AF8-cmd.type were left empty. +AD4- +- +ACo- read the response directly ignoring all errors. +AD4- +- +ACo-/ Have you verified this patch with checkpatch? This is not the proper kernel comment style. Thanks, Bart.
