On Tue, 2024-08-27 at 11:42 -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/26/24 6:39 PM, Peter Wang (王信友) wrote:
> > It is not a new vendor hook, ufshcd_vops_hibern8_notify is exist
> > in current kernel.
> Hi Peter,
>
> Is something like the untested patch below perhaps what you have in
> mind?
>
> Thanks,
>
> Bart.
>
Hi Bart,
No, I means you can reference ufs-sprd.c driver. which may have the
same issue?
/*
* Disable UIC COMPL INTR to prevent access to
UFSHCI after
* checking HCS.UPMCRS
*/
ufs_sprd_ctrl_uic_compl(hba, false);
Then after enter hibernte, you can prevent access to UFSHCI.
After exit hibernate, enable uic complete interrupt again for
workaround.
Thanks.
Peter
>
> diff --git a/drivers/ufs/core/ufshcd-priv.h
> b/drivers/ufs/core/ufshcd-priv.h
> index ce36154ce963..69ee49a75c04 100644
> --- a/drivers/ufs/core/ufshcd-priv.h
> +++ b/drivers/ufs/core/ufshcd-priv.h
> @@ -176,12 +176,14 @@ static inline void
> ufshcd_vops_setup_task_mgmt(struct ufs_hba *hba,
> return hba->vops->setup_task_mgmt(hba, tag, tm_function);
> }
>
> -static inline void ufshcd_vops_hibern8_notify(struct ufs_hba *hba,
> -enum uic_cmd_dme cmd,
> -enum ufs_notify_change_status status)
> +static inline void
> +ufshcd_vops_hibern8_notify(struct ufs_hba *hba, enum uic_cmd_dme
> cmd,
> + enum ufs_notify_change_status status,
> + bool *disable_uic_compl_intr)
> {
> if (hba->vops && hba->vops->hibern8_notify)
> -return hba->vops->hibern8_notify(hba, cmd, status);
> +return hba->vops->hibern8_notify(hba, cmd, status,
> + disable_uic_compl_intr);
> }
>
> static inline int ufshcd_vops_apply_dev_quirks(struct ufs_hba *hba)
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index e13b9ac145f6..614b24f2eb7f 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -2541,9 +2541,8 @@ ufshcd_wait_for_uic_cmd(struct ufs_hba *hba,
> struct uic_command *uic_cmd)
> *
> * Return: 0 only if success.
> */
> -static int
> -__ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command
> *uic_cmd,
> - bool completion)
> +static int __ufshcd_send_uic_cmd(struct ufs_hba *hba,
> + struct uic_command *uic_cmd)
> {
> lockdep_assert_held(&hba->uic_cmd_mutex);
>
> @@ -2553,8 +2552,7 @@ __ufshcd_send_uic_cmd(struct ufs_hba *hba,
> struct
> uic_command *uic_cmd,
> return -EIO;
> }
>
> -if (completion)
> -init_completion(&uic_cmd->done);
> +init_completion(&uic_cmd->done);
>
> uic_cmd->cmd_active = 1;
> ufshcd_dispatch_uic_cmd(hba, uic_cmd);
> @@ -2580,7 +2578,7 @@ int ufshcd_send_uic_cmd(struct ufs_hba *hba,
> struct uic_command *uic_cmd)
> mutex_lock(&hba->uic_cmd_mutex);
> ufshcd_add_delay_before_dme_cmd(hba);
>
> -ret = __ufshcd_send_uic_cmd(hba, uic_cmd, true);
> +ret = __ufshcd_send_uic_cmd(hba, uic_cmd);
> if (!ret)
> ret = ufshcd_wait_for_uic_cmd(hba, uic_cmd);
>
> @@ -4243,7 +4241,8 @@ EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr);
> *
> * Return: 0 on success, non-zero value on failure.
> */
> -static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct
> uic_command
> *cmd)
> +static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct
> uic_command
> *cmd,
> + bool disable_uic_compl_intr)
> {
> DECLARE_COMPLETION_ONSTACK(uic_async_done);
> unsigned long flags;
> @@ -4260,7 +4259,8 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba
> *hba, struct uic_command *cmd)
> goto out_unlock;
> }
> hba->uic_async_done = &uic_async_done;
> -if (ufshcd_readl(hba, REG_INTERRUPT_ENABLE) & UIC_COMMAND_COMPL) {
> +if (disable_uic_compl_intr &&
> + ufshcd_readl(hba, REG_INTERRUPT_ENABLE) & UIC_COMMAND_COMPL) {
> ufshcd_disable_intr(hba, UIC_COMMAND_COMPL);
> /*
> * Make sure UIC command completion interrupt is disabled before
> @@ -4270,7 +4270,7 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba
> *hba, struct uic_command *cmd)
> reenable_intr = true;
> }
> spin_unlock_irqrestore(hba->host->host_lock, flags);
> -ret = __ufshcd_send_uic_cmd(hba, cmd, false);
> +ret = __ufshcd_send_uic_cmd(hba, cmd);
> if (ret) {
> dev_err(hba->dev,
> "pwr ctrl cmd 0x%x with mode 0x%x uic error %d\n",
> @@ -4294,6 +4294,16 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba
> *hba, struct uic_command *cmd)
> goto out;
> }
>
> +if (!disable_uic_compl_intr &&
> + ufshcd_readl(hba, REG_INTERRUPT_ENABLE) & UIC_COMMAND_COMPL) {
> +ret = wait_for_completion_timeout(
> +&cmd->done, msecs_to_jiffies(uic_cmd_timeout));
> +if (ret == 0)
> +dev_err(hba->dev, "pwr ctrl cmd %#x timed out\n",
> +cmd->command);
> +ret = 0;
> +}
> +
> check_upmcrs:
> status = ufshcd_get_upmcrs(hba);
> if (status != PWR_LOCAL) {
> @@ -4353,7 +4363,7 @@ int ufshcd_uic_change_pwr_mode(struct ufs_hba
> *hba, u8 mode)
> }
>
> ufshcd_hold(hba);
> -ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd);
> +ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd, true);
> ufshcd_release(hba);
>
> out:
> @@ -4396,11 +4406,13 @@ int ufshcd_uic_hibern8_enter(struct ufs_hba
> *hba)
> .command = UIC_CMD_DME_HIBER_ENTER,
> };
> ktime_t start = ktime_get();
> +bool disable_uic_compl_intr = true;
> int ret;
>
> -ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_ENTER,
> PRE_CHANGE);
> +ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_ENTER, PRE_CHANGE,
> + &disable_uic_compl_intr);
>
> -ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd);
> +ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd, disable_uic_compl_intr);
> trace_ufshcd_profile_hibern8(dev_name(hba->dev), "enter",
> ktime_to_us(ktime_sub(ktime_get(), start)), ret);
>
> @@ -4409,7 +4421,7 @@ int ufshcd_uic_hibern8_enter(struct ufs_hba
> *hba)
> __func__, ret);
> else
> ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_ENTER,
> -POST_CHANGE);
> + POST_CHANGE, NULL);
>
> return ret;
> }
> @@ -4423,9 +4435,10 @@ int ufshcd_uic_hibern8_exit(struct ufs_hba
> *hba)
> int ret;
> ktime_t start = ktime_get();
>
> -ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_EXIT, PRE_CHANGE);
> +ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_EXIT, PRE_CHANGE,
> + NULL);
>
> -ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd);
> +ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd, true);
> trace_ufshcd_profile_hibern8(dev_name(hba->dev), "exit",
> ktime_to_us(ktime_sub(ktime_get(), start)), ret);
>
> @@ -4434,7 +4447,7 @@ int ufshcd_uic_hibern8_exit(struct ufs_hba
> *hba)
> __func__, ret);
> } else {
> ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_EXIT,
> -POST_CHANGE);
> + POST_CHANGE, NULL);
> hba->ufs_stats.last_hibern8_exit_tstamp = local_clock();
> hba->ufs_stats.hibern8_exit_cnt++;
> }
> diff --git a/drivers/ufs/host/cdns-pltfrm.c b/drivers/ufs/host/cdns-
> pltfrm.c
> index 66811d8d1929..24c05e5c455d 100644
> --- a/drivers/ufs/host/cdns-pltfrm.c
> +++ b/drivers/ufs/host/cdns-pltfrm.c
> @@ -164,7 +164,8 @@ static int cdns_ufs_hce_enable_notify(struct
> ufs_hba
> *hba,
> * @status: notify stage (pre, post change)
> */
> static void cdns_ufs_hibern8_notify(struct ufs_hba *hba, enum
> uic_cmd_dme cmd,
> - enum ufs_notify_change_status status)
> + enum ufs_notify_change_status status,
> + bool *disable_uic_compl_intr)
> {
> if (status == PRE_CHANGE && cmd == UIC_CMD_DME_HIBER_ENTER)
> cdns_ufs_get_l4_attr(hba);
> diff --git a/drivers/ufs/host/ufs-exynos.c b/drivers/ufs/host/ufs-
> exynos.c
> index 16ad3528d80b..e991d9e5e2e4 100644
> --- a/drivers/ufs/host/ufs-exynos.c
> +++ b/drivers/ufs/host/ufs-exynos.c
> @@ -1624,8 +1624,9 @@ static int exynos_ufs_pwr_change_notify(struct
> ufs_hba *hba,
> }
>
> static void exynos_ufs_hibern8_notify(struct ufs_hba *hba,
> - enum uic_cmd_dme enter,
> - enum ufs_notify_change_status notify)
> + enum uic_cmd_dme enter,
> + enum ufs_notify_change_status notify,
> + bool *disable_uic_compl_intr)
> {
> switch ((u8)notify) {
> case PRE_CHANGE:
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index a43b14276bc3..59b901d67400 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -355,8 +355,9 @@ struct ufs_hba_variant_ops {
> void(*setup_xfer_req)(struct ufs_hba *hba, int tag,
> bool is_scsi_cmd);
> void(*setup_task_mgmt)(struct ufs_hba *, int, u8);
> -void (*hibern8_notify)(struct ufs_hba *, enum uic_cmd_dme,
> -enum ufs_notify_change_status);
> +void(*hibern8_notify)(struct ufs_hba *, enum uic_cmd_dme,
> + enum ufs_notify_change_status,
> + bool *disable_uic_compl_intr);
> int(*apply_dev_quirks)(struct ufs_hba *hba);
> void(*fixup_dev_quirks)(struct ufs_hba *hba);
> int (*suspend)(struct ufs_hba *, enum ufs_pm_op,
>
