On Wed, 2018-08-01 at 11:04 +0300, Avri Altman wrote: > + wait_event(hba->tm_tag_wq, ufshcd_get_tm_free_slot(hba, &free_slot));
The above is the weirdest API I have seen so far for tag allocation. Why does ufshcd_get_tm_free_slot() does a linear search through a bitmap instead of using the sbitmap data structure? > + ufshcd_writel(hba, 1 << free_slot, REG_UTP_TASK_REQ_DOOR_BELL); > + /* Make sure that doorbell is committed immediately */ > + wmb(); 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? > + spin_unlock_irqrestore(host->host_lock, flags); > + > + /* wait until the task management command is completed */ > + err = wait_event_timeout(hba->tm_wq, > + test_bit(free_slot, &hba->tm_condition), > + msecs_to_jiffies(TM_CMD_TIMEOUT)); Did you perhaps start implementing the ufshcd_issue_tm_upiu_cmd() function by copy/pasting ufshcd_issue_tm_cmd()? Please don't do that and instead avoid code duplication by moving shared code in a new function. > + /* ignore the returning value here - ufshcd_check_query_response is > + * bound to fail since dev_cmd.query and dev_cmd.type were left empty. > + * read the response directly ignoring all errors. > + */ Have you verified this patch with checkpatch? This is not the proper kernel comment style. Thanks, Bart.
