On Tue, 2024-08-13 at 21:47 +0800, ZhangHui wrote:
> From: zhanghui <zhanghui31@xxxxxxxxxx>
>
> If the SSU CMD completion flow in UFS resume and the CMD timeout flow
> occur
> simultaneously, the timestamp attribute command will be sent to the
> device
>
Hi Zhanghui,
If the timeout command is SSU?
In resume flow, if SSU command timeout, ufshcd_eh_host_reset_handler
invoke ufshcd_link_recovery only, not schedule eh work?
Thanks.
Peter
> during the UFS resume flow, while the UFS controller performs a reset
> in
> the timeout error handling flow.
>
> In this scenario, a bus timeout will occur because the UFS resume
> flow
> will attempt to access the UFS host controller registers while the
> UFS
> controller is in a reset state or power cycle.
>
> Call trace:
> arm64_serror_panic+0x6c/0x94
> do_serror+0xbc/0xc8
> el1h_64_error_handler+0x34/0x48
> el1h_64_error+0x68/0x6c
> _raw_spin_unlock+0x18/0x44
> ufshcd_send_command+0x1c0/0x380
> ufshcd_exec_dev_cmd+0x21c/0x29c
> ufshcd_set_timestamp_attr+0x78/0xdc
> __ufshcd_wl_resume+0x228/0x48c
> ufshcd_wl_resume+0x5c/0x1b4
> scsi_bus_resume+0x58/0xa0
> dpm_run_callback+0x6c/0x23c
> __device_resume+0x298/0x46c
> async_resume+0x24/0x3c
> async_run_entry_fn+0x44/0x150
> process_scheduled_works+0x254/0x4f4
> worker_thread+0x244/0x334
> kthread+0x124/0x1f0
> ret_from_fork+0x10/0x20
>
> This patch fixes the issue by adding a check of the UFS controller
> register states before sending the device command.
>
> Signed-off-by: zhanghui <zhanghui31@xxxxxxxxxx>
> ---
> drivers/ufs/core/ufshcd.c | 14 ++++++++++++++
> include/ufs/ufshcd.h | 13 +++++++++++++
> 2 files changed, 27 insertions(+)
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 5e3c67e96956..e5e3e0277d43 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -3291,6 +3291,8 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba
> *hba,
> struct ufshcd_lrb *lrbp = &hba->lrb[tag];
> int err;
>
> + if (hba->ufshcd_reg_state == UFSHCD_REG_RESET)
> + return -EBUSY;
> /* Protects use of hba->reserved_slot. */
> lockdep_assert_held(&hba->dev_cmd.lock);
>
> @@ -4857,6 +4859,7 @@ static int ufshcd_hba_execute_hce(struct
> ufs_hba *hba)
> int ufshcd_hba_enable(struct ufs_hba *hba)
> {
> int ret;
> + unsigned long flags;
>
> if (hba->quirks & UFSHCI_QUIRK_BROKEN_HCE) {
> ufshcd_set_link_off(hba);
> @@ -4881,6 +4884,13 @@ int ufshcd_hba_enable(struct ufs_hba *hba)
> ret = ufshcd_hba_execute_hce(hba);
> }
>
> + spin_lock_irqsave(hba->host->host_lock, flags);
> + if (ret)
> + hba->ufshcd_reg_state = UFSHCD_REG_RESET;
> + else
> + hba->ufshcd_reg_state = UFSHCD_REG_OPERATIONAL;
> + spin_unlock_irqrestore(hba->host->host_lock, flags);
> +
> return ret;
> }
> EXPORT_SYMBOL_GPL(ufshcd_hba_enable);
> @@ -7693,7 +7703,11 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
> static int ufshcd_host_reset_and_restore(struct ufs_hba *hba)
> {
> int err;
> + unsigned long flags;
>
> + spin_lock_irqsave(hba->host->host_lock, flags);
> + hba->ufshcd_reg_state = UFSHCD_REG_RESET;
> + spin_unlock_irqrestore(hba->host->host_lock, flags);
> /*
> * Stop the host controller and complete the requests
> * cleared by h/w
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index cac0cdb9a916..e5af4c2114ce 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -523,6 +523,18 @@ enum ufshcd_state {
> UFSHCD_STATE_ERROR,
> };
>
> +/**
> + * enum ufshcd_state - UFS host controller state
> + * @UFSHCD_REG_OPERATIONAL: UFS host controller is enabled, the host
> controller
> + * register can be accessed.
> + * @UFSHCD_REG_RESET: UFS host controller registers will be reset,
> can't access
> + * any ufs host controller register.
> + */
> +enum ufshcd_reg_state {
> + UFSHCD_REG_OPERATIONAL,
> + UFSHCD_REG_RESET,
> +};
> +
> enum ufshcd_quirks {
> /* Interrupt aggregation support is broken */
> UFSHCD_QUIRK_BROKEN_INTR_AGGR = 1 << 0,
> @@ -1020,6 +1032,7 @@ struct ufs_hba {
> struct completion *uic_async_done;
>
> enum ufshcd_state ufshcd_state;
> + enum ufshcd_reg_state ufshcd_reg_state;
> u32 eh_flags;
> u32 intr_mask;
> u16 ee_ctrl_mask;
