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

[Bart Van Assche <bvanassche@acm.org>]

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.

> 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.
- 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.

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.

Thanks,

Bart.