On Thu, Jan 05, 2023 at 10:38:58AM -0800, Bart Van Assche wrote:
On 12/22/22 02:21, Johan Hovold wrote:
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index bda61be5f035..5c3821b2fcf8 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -1234,12 +1234,14 @@ static int ufshcd_clock_scaling_prepare(struct ufs_hba *hba) * clock scaling is in progress */ ufshcd_scsi_block_requests(hba);
- mutex_lock(&hba->wb_mutex); down_write(&hba->clk_scaling_lock);
if (!hba->clk_scaling.is_allowed || ufshcd_wait_for_doorbell_clr(hba, DOORBELL_CLR_TOUT_US)) { ret = -EBUSY; up_write(&hba->clk_scaling_lock);
mutex_unlock(&hba->wb_mutex);@@ -1251,12 +1253,16 @@ static int ufshcd_clock_scaling_prepare(struct ufs_hba *hba) return ret; }
Please add an __acquires(&hba->wb_mutex) annotation for sparse.
-static void ufshcd_clock_scaling_unprepare(struct ufs_hba *hba, bool writelock) +static void ufshcd_clock_scaling_unprepare(struct ufs_hba *hba, bool scale_up) {
- if (writelock)
up_write(&hba->clk_scaling_lock);- else
up_read(&hba->clk_scaling_lock);
- up_write(&hba->clk_scaling_lock);
- /* Enable Write Booster if we have scaled up else disable it */
- if (ufshcd_enable_wb_if_scaling_up(hba))
ufshcd_wb_toggle(hba, scale_up);- mutex_unlock(&hba->wb_mutex);
- ufshcd_scsi_unblock_requests(hba); ufshcd_release(hba); }
Please add a __releases(&hba->wb_mutex) annotation for sparse.
This would actually introduce new sparse warnings as mutex_lock/unlock() are not sparse annotated:
drivers/ufs/core/ufshcd.c:1254:9: warning: context imbalance in 'ufshcd_clock_scaling_prepare' - wrong count at exit drivers/ufs/core/ufshcd.c:1266:9: warning: context imbalance in 'ufshcd_clock_scaling_unprepare' - wrong count at exit drivers/ufs/core/ufshcd.c:1281:12: warning: context imbalance in 'ufshcd_devfreq_scale' - wrong count at exit
I guess it's not worth adding explicit __acquire()/__release() to these helpers either (e.g. as they are only used in one function).
@@ -1273,7 +1279,6 @@ static void ufshcd_clock_scaling_unprepare(struct ufs_hba *hba, bool writelock) static int ufshcd_devfreq_scale(struct ufs_hba *hba, bool scale_up) { int ret = 0;
- bool is_writelock = true;
ret = ufshcd_clock_scaling_prepare(hba); if (ret) @@ -1302,15 +1307,8 @@ static int ufshcd_devfreq_scale(struct ufs_hba *hba, bool scale_up) } }
- /* Enable Write Booster if we have scaled up else disable it */
- if (ufshcd_enable_wb_if_scaling_up(hba)) {
downgrade_write(&hba->clk_scaling_lock);is_writelock = false;ufshcd_wb_toggle(hba, scale_up);- }
- out_unprepare:
- ufshcd_clock_scaling_unprepare(hba, is_writelock);
- ufshcd_clock_scaling_unprepare(hba, scale_up); return ret; }
This patch moves the ufshcd_wb_toggle() from before the out_unprepare label to after the out_unprepare label (into ufshcd_clock_scaling_unprepare()). Does this change perhaps introduce a new call to ufshcd_wb_toggle() in error paths?
Thanks for spotting that. I'll leave the setting unchanged on errors in v2.
Johan