Messages in this thread | | | Date | Mon, 14 Feb 2022 11:29:08 -0800 | Subject | Re: [PATCH v1] scsi: ufs: remove clk_scaling_lock when clkscaling isn't supported. | From | Bart Van Assche <> |
| |
On 2/4/22 23:39, Kiwoong Kim wrote: > clk_scaling_lock is to prevent from running clkscaling related operations > with others which might be affected by the operations concurrently. > I think it looks hardware specific. > If the feature isn't supported, I think there is no reasonto prevent from > running other functions, such as ufshcd_queuecommand and > ufshcd_exec_dev_cmd, concurrently. > > So I add a condition at some points protecting with clk_scaling_lock. > > Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com> > --- > drivers/scsi/ufs/ufshcd.c | 21 ++++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index 460d2b4..8471c90 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -2980,7 +2980,8 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba, > /* Protects use of hba->reserved_slot. */ > lockdep_assert_held(&hba->dev_cmd.lock); > > - down_read(&hba->clk_scaling_lock); > + if (ufshcd_is_clkscaling_supported(hba)) > + down_read(&hba->clk_scaling_lock); > > lrbp = &hba->lrb[tag]; > WARN_ON(lrbp->cmd);
I don't like this patch at all. This patch makes testing the UFS driver more complicated without having any clear benefit. Additionally, adding if-statements in front of locking makes static source code analysis harder and is an anti-pattern. Please don't do this.
Bart.
| |