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

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

On Wed, 2024-08-28 at 10:10 -0400, 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/28/24 9:46 AM, Peter Wang (王信友) wrote:
> > 1. If only UFS controller core driver should do this,
> >     What about registers that are not REG_INTERRUPT_ENABLE?
> >     Since ufshcd_writel has been exported, shouldn't the host
> >     driver have the authority to control all Host REGs?
> 
> It is not because host drivers can change any host controller
> register
> that host drivers should take the freedom to modify all host
> controller
> registers. Modifying host controller registers that are vendor
> specific from a host driver seems totally fine to me. I think that
> standardized host controller registers should only be modified from
> the
> UFS driver core. Otherwise the UFS core driver cannot be understood
> nor
> verified without deep understanding of all the host drivers.
> 

Hi Bart,

But what we are discussing is a 'workaround', which might allow 
modifications to standardized host controller registers, right?



> > 2. Set REG_INTERRUPT_ENABLE only when hibernate exit?
> >     If cause the UniPro link to exit, then it should still be
> correct,
> >     just exiting hibernate early?
> 
> This approach has two disadvantages:
> - It requires that even more state information is tracked in struct
>    ufs_hba.

It could utilize a host private structure, like ufs_mtk_host.

> - This approach is probably incompatible with the power management
> core.
>    I think that there is code in the power management core for
> disabling
>    interrupts during suspend and reenabling interrupts during resume.
>    Enabling an interrupt that is already enabled is not allowed.
> 

Power management enable or disable interrupt should through 
device driver hook, such as suspend/resume callback function? 
I am not sure because MediaTek's power management does 
not directly control interrupts.


> In general, disabling / reenabling interrupts is something that
> should
> be avoided because it is not compatible with multithreading. The
> reason
> why it works in the UFS driver for UIC commands is because these are
> serialized.

Yes, but entering and exiting hibernate are protected by 
clock gating or the runtime/system PM framework. There shouldn't 
be issues with multiple threads entering or exiting hibernate?

Thanks.
Peter

> 
> Thanks,
> 
> Bart.