Re: [PATCH 2/2] scsi: ufs: core: Fix the code for entering hibernation

["Peter Wang (王信友)" <peter.wang@mediatek.com>]

On Thu, 2024-08-22 at 19:06 -0700, Bart Van Assche wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 8/22/24 4:34 PM, Bao D. Nguyen wrote:
> > Let's say you are sending a ufshcd_uic_pwr_ctrl() command. You will
> get 
> > 2 uic completion interrupts:
> > [1] ufshcd_uic_cmd_compl() is called for the first interrupt which 
> > happens to be UFSHCD_UIC_PWR_MASK only. At the end of the 
> > ufshcd_uic_pwr_ctrl(), you would set the hba->active_uic_cmd to
> NULL.
> 
> That's not correct. ufshcd_uic_pwr_ctrl() only clears
> hba->active_uic_cmd after the power mode change interrupt has been
> processed.
> 
> > [2]The second uic completion interrupt for UIC_COMMAND_COMP is
> delayed.
> > This interrupt is newly introduced by this patch.
> 
> If UIC_COMMAND_COMPL is delivered after UFSHCD_UIC_PWR_MASK then the
> UIC_COMMAND_COMPL interrupt will be ignored because hba-
> >active_uic_cmd
> is cleared by ufshcd_uic_pwr_ctrl() after it has processed the
> power mode change interrupt.
> 
> > Now let's say you have a new UIC command coming via 
> > ufshcd_send_uic_cmd(). The ufshcd_dispatch_uic_cmd() will update
> your 
> > hba->active_uic_cmd to the new uic_cmd.
> 
> UIC command processing is serialized by hba->uic_cmd_mutex. Hence
> only
> one UIC command is processed at any given time.
> 
> Does this address your concerns?
> 
> Thanks,
> 
> Bart.

Hi Bart,

You assume that the following steps are executed in sequence.
(Theread A) send ufshcd_uic_pwr_ctrl
(ISR) UIC_POWER_MODE A
	clear hab->active_uic_cmd
(ISR) UIC_COMMAND_COMPL A (step A)
	do nothing
(Thread B) ufshcd_send_uic_cmd set hab->active_uic_cmd (step B)
(ISR) UIC_COMMAND_COMPL B
	complte thread B's cmd

But actually step A ISR may come after step B and cause error.

(Theread A) send ufshcd_uic_pwr_ctrl
(ISR) UIC_POWER_MODE A
	clear hab->active_uic_cmd
(Thread B) ufshcd_send_uic_cmd set hab->active_uic_cmd (step B)
(ISR) UIC_COMMAND_COMPL A (step A)
	complete Thread A cmd	


Thanks.
Peter