* [bug report] scsi: ufs: Try to save power mode change and UIC cmd completion timeout
@ 2022-05-23 7:13 Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2022-05-23 7:13 UTC (permalink / raw)
To: cang; +Cc: linux-scsi
[ Renaming the files means that all these old warnings show up as new
warnings. I reported this two years ago but never heard back. -dan ]
Hello Can Guo,
This is a semi-automatic email about new static checker warnings.
The patch 0f52fcb99ea2: "scsi: ufs: Try to save power mode change and
UIC cmd completion timeout" from Nov 2, 2020, leads to the following
Smatch complaint:
drivers/ufs/core/ufshcd.c:5283 ufshcd_uic_cmd_compl()
error: we previously assumed 'hba->active_uic_cmd' could be null (see line 5271)
drivers/ufs/core/ufshcd.c
5263 static irqreturn_t ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
5264 {
5265 irqreturn_t retval = IRQ_NONE;
5266
5267 spin_lock(hba->host->host_lock);
5268 if (ufshcd_is_auto_hibern8_error(hba, intr_status))
5269 hba->errors |= (UFSHCD_UIC_HIBERN8_MASK & intr_status);
5270
5271 if ((intr_status & UIC_COMMAND_COMPL) && hba->active_uic_cmd) {
^^^^^^^^^^^^^^^^^^^
This code checks for NULL
5272 hba->active_uic_cmd->argument2 |=
5273 ufshcd_get_uic_cmd_result(hba);
5274 hba->active_uic_cmd->argument3 =
5275 ufshcd_get_dme_attr_val(hba);
5276 if (!hba->uic_async_done)
5277 hba->active_uic_cmd->cmd_active = 0;
5278 complete(&hba->active_uic_cmd->done);
5279 retval = IRQ_HANDLED;
5280 }
5281
5282 if ((intr_status & UFSHCD_UIC_PWR_MASK) && hba->uic_async_done) {
5283 hba->active_uic_cmd->cmd_active = 0;
^^^^^^^^^^^^^^^^^^^^^
Unchecked dereference
5284 complete(hba->uic_async_done);
5285 retval = IRQ_HANDLED;
5286 }
5287
5288 if (retval == IRQ_HANDLED)
5289 ufshcd_add_uic_command_trace(hba, hba->active_uic_cmd,
^^^^^^^^^^^^^^^^^^^
Unchecked dereference
5290 UFS_CMD_COMP);
5291 spin_unlock(hba->host->host_lock);
5292 return retval;
5293 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* [bug report] scsi: ufs: Try to save power mode change and UIC cmd completion timeout
@ 2022-05-23 7:06 Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2022-05-23 7:06 UTC (permalink / raw)
To: cang; +Cc: linux-scsi
Hello Can Guo,
The patch 0f52fcb99ea2: "scsi: ufs: Try to save power mode change and
UIC cmd completion timeout" from Nov 2, 2020, leads to the following
Smatch static checker warning:
drivers/ufs/core/ufshcd.c:4048 ufshcd_uic_pwr_ctrl()
warn: missing error code here? '_dev_err()' failed. 'ret' = '0'
drivers/ufs/core/ufshcd.c
4004 static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct uic_command *cmd)
4005 {
4006 DECLARE_COMPLETION_ONSTACK(uic_async_done);
4007 unsigned long flags;
4008 u8 status;
4009 int ret;
4010 bool reenable_intr = false;
4011
4012 mutex_lock(&hba->uic_cmd_mutex);
4013 ufshcd_add_delay_before_dme_cmd(hba);
4014
4015 spin_lock_irqsave(hba->host->host_lock, flags);
4016 if (ufshcd_is_link_broken(hba)) {
4017 ret = -ENOLINK;
4018 goto out_unlock;
4019 }
4020 hba->uic_async_done = &uic_async_done;
4021 if (ufshcd_readl(hba, REG_INTERRUPT_ENABLE) & UIC_COMMAND_COMPL) {
4022 ufshcd_disable_intr(hba, UIC_COMMAND_COMPL);
4023 /*
4024 * Make sure UIC command completion interrupt is disabled before
4025 * issuing UIC command.
4026 */
4027 wmb();
4028 reenable_intr = true;
4029 }
4030 ret = __ufshcd_send_uic_cmd(hba, cmd, false);
4031 spin_unlock_irqrestore(hba->host->host_lock, flags);
4032 if (ret) {
4033 dev_err(hba->dev,
4034 "pwr ctrl cmd 0x%x with mode 0x%x uic error %d\n",
4035 cmd->command, cmd->argument3, ret);
4036 goto out;
4037 }
4038
4039 if (!wait_for_completion_timeout(hba->uic_async_done,
4040 msecs_to_jiffies(UIC_CMD_TIMEOUT))) {
4041 dev_err(hba->dev,
4042 "pwr ctrl cmd 0x%x with mode 0x%x completion timeout\n",
4043 cmd->command, cmd->argument3);
4044
4045 if (!cmd->cmd_active) {
4046 dev_err(hba->dev, "%s: Power Mode Change operation has been completed, go check UPMCRS\n",
4047 __func__);
--> 4048 goto check_upmcrs;
Why is this a dev_err()? It looks like a success path. I suspect this
is debugging code which escaped into production. Or alternatively, if
this is an error path the error code needs to be set and the error
message needs to be re-written to be more clear.
4049 }
4050
4051 ret = -ETIMEDOUT;
4052 goto out;
4053 }
4054
4055 check_upmcrs:
4056 status = ufshcd_get_upmcrs(hba);
4057 if (status != PWR_LOCAL) {
4058 dev_err(hba->dev,
4059 "pwr ctrl cmd 0x%x failed, host upmcrs:0x%x\n",
4060 cmd->command, status);
4061 ret = (status != PWR_OK) ? status : -1;
4062 }
4063 out:
4064 if (ret) {
4065 ufshcd_print_host_state(hba);
4066 ufshcd_print_pwr_info(hba);
4067 ufshcd_print_evt_hist(hba);
4068 }
4069
4070 spin_lock_irqsave(hba->host->host_lock, flags);
4071 hba->active_uic_cmd = NULL;
4072 hba->uic_async_done = NULL;
4073 if (reenable_intr)
4074 ufshcd_enable_intr(hba, UIC_COMMAND_COMPL);
4075 if (ret) {
4076 ufshcd_set_link_broken(hba);
4077 ufshcd_schedule_eh_work(hba);
4078 }
4079 out_unlock:
4080 spin_unlock_irqrestore(hba->host->host_lock, flags);
4081 mutex_unlock(&hba->uic_cmd_mutex);
4082
4083 return ret;
4084 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* [bug report] scsi: ufs: Try to save power mode change and UIC cmd completion timeout
@ 2020-11-09 9:57 Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2020-11-09 9:57 UTC (permalink / raw)
To: cang; +Cc: linux-scsi
Hello Can Guo,
This is a semi-automatic email about new static checker warnings.
The patch 0f52fcb99ea2: "scsi: ufs: Try to save power mode change and
UIC cmd completion timeout" from Nov 2, 2020, leads to the following
Smatch complaint:
drivers/scsi/ufs/ufshcd.c:4941 ufshcd_uic_cmd_compl()
error: we previously assumed 'hba->active_uic_cmd' could be null (see line 4929)
drivers/scsi/ufs/ufshcd.c
4928
4929 if ((intr_status & UIC_COMMAND_COMPL) && hba->active_uic_cmd) {
^^^^^^^^^^^^^^^^^^^
Here is the NULL check
4930 hba->active_uic_cmd->argument2 |=
4931 ufshcd_get_uic_cmd_result(hba);
4932 hba->active_uic_cmd->argument3 =
4933 ufshcd_get_dme_attr_val(hba);
4934 if (!hba->uic_async_done)
4935 hba->active_uic_cmd->cmd_active = 0;
4936 complete(&hba->active_uic_cmd->done);
4937 retval = IRQ_HANDLED;
4938 }
4939
4940 if ((intr_status & UFSHCD_UIC_PWR_MASK) && hba->uic_async_done) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Smatch isn't clever enough to tie this to "hba->active_uic_cmd" and I
looked at it briefly and wasn't able to right away either.
4941 hba->active_uic_cmd->cmd_active = 0;
^^^^^^^^^^^^^^^^^^^^^
Unchecked NULL dereference.
4942 complete(hba->uic_async_done);
4943 retval = IRQ_HANDLED;
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-05-23 7:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-23 7:13 [bug report] scsi: ufs: Try to save power mode change and UIC cmd completion timeout Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2022-05-23 7:06 Dan Carpenter
2020-11-09 9:57 Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.