On 11/02/2022 04:15, Kiwoong Kim wrote: >>> I think it looks hardware specific. >>> If the feature isn't supported, I think there is no reasonto prevent >>> from >> ^^^ >> reason to >> >>> running other functions, such as ufshcd_queuecommand and >> It is no longer used in queuecommand since 5675c381ea51 and 8d077ede48c1 > > Yeah, you're right. It's just an example. I just want to tell that the lock also protects things that are not related with clk scaling directly. > >> >>> ufshcd_exec_dev_cmd, concurrently. >>> >>> So I add a condition at some points protecting with clk_scaling_lock. >> But you still need a way to serialize device management commands. >> >> Thanks, >> Avri > > The dev cmd execution period is protected by mutex. > And actual ringing a doorbell is protected by spin lock. > > Is there another reason to need clk_scaling_lock even with it? > The error handler really should have exclusive access. One of the places you change does explain that: * Hold the scaling lock just in case dev cmds * are sent via bsg and/or sysfs. */ - down_write(&hba->clk_scaling_lock); + if (ufshcd_is_clkscaling_supported(hba)) + down_write(&hba->clk_scaling_lock);
