On Wed, 2024-08-21 at 11:29 -0700, Bart Van Assche wrote:
> drivers/ufs/core/ufshcd.c | 22 ++++++----------------
> include/ufs/ufshcd.h | 7 ++++---
> 2 files changed, 10 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index d0ae6e50becc..e12f30b8a83c 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -2585,6 +2585,7 @@ int ufshcd_send_uic_cmd(struct ufs_hba *hba,
> struct uic_command *uic_cmd)
> ufshcd_hold(hba);
> mutex_lock(&hba->uic_cmd_mutex);
> ufshcd_add_delay_before_dme_cmd(hba);
> + WARN_ON(hba->uic_async_done);
>
> ret = __ufshcd_send_uic_cmd(hba, uic_cmd, true);
> if (!ret)
> @@ -4255,7 +4256,6 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba
> *hba, struct uic_command *cmd)
> unsigned long flags;
> u8 status;
> int ret;
> - bool reenable_intr = false;
>
> mutex_lock(&hba->uic_cmd_mutex);
> ufshcd_add_delay_before_dme_cmd(hba);
> @@ -4266,15 +4266,6 @@ 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) {
> - ufshcd_disable_intr(hba, UIC_COMMAND_COMPL);
> - /*
> - * Make sure UIC command completion interrupt is
> disabled before
> - * issuing UIC command.
> - */
> - ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
> - reenable_intr = true;
> - }
> spin_unlock_irqrestore(hba->host->host_lock, flags);
> ret = __ufshcd_send_uic_cmd(hba, cmd, false);
> if (ret) {
> @@ -4318,8 +4309,6 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba
> *hba, struct uic_command *cmd)
> spin_lock_irqsave(hba->host->host_lock, flags);
> hba->active_uic_cmd = NULL;
> hba->uic_async_done = NULL;
> - if (reenable_intr)
> - ufshcd_enable_intr(hba, UIC_COMMAND_COMPL);
> if (ret) {
> ufshcd_set_link_broken(hba);
> ufshcd_schedule_eh_work(hba);
> @@ -5472,11 +5461,12 @@ static irqreturn_t
> ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
> hba->errors |= (UFSHCD_UIC_HIBERN8_MASK &
> intr_status);
>
> if (intr_status & UIC_COMMAND_COMPL && cmd) {
> - cmd->argument2 |= ufshcd_get_uic_cmd_result(hba);
> - cmd->argument3 = ufshcd_get_dme_attr_val(hba);
Bart,
My only concern is, removing disabling UIC completion IRQ, and keeping
is.uccs 1, then we don't read its status in case of ufshcd_uic_pwr_ctrl
path, whether this will affect the next UIC access result.
Other things look good to me.
Kind regards,
Bean
