> On Tue, 2024-11-05 at 13:25 +0200, Avri Altman wrote:
> > - spin_lock_irqsave(hba->host->host_lock, flags);
> > - /*
> > - * In case you are here to cancel this work the gating state
> > - * would be marked as REQ_CLKS_ON. In this case save time by
> > - * skipping the gating work and exit after changing the clock
> > - * state to CLKS_ON.
> > - */
> > - if (hba->clk_gating.is_suspended ||
> > - (hba->clk_gating.state != REQ_CLKS_OFF)) {
> > - hba->clk_gating.state = CLKS_ON;
> > - trace_ufshcd_clk_gating(dev_name(hba->dev),
> > - hba->clk_gating.state);
> > - goto rel_lock;
> > + scoped_guard(spinlock_irqsave, &hba->clk_gating.lock)
> > + {
> > + /*
> > + * In case you are here to cancel this work the
> > gating state
> > + * would be marked as REQ_CLKS_ON. In this case save
> > time by
> > + * skipping the gating work and exit after changing
> > the clock
> > + * state to CLKS_ON.
> > + */
> > + if (hba->clk_gating.is_suspended ||
> > + hba->clk_gating.state != REQ_CLKS_OFF) {
> > + hba->clk_gating.state = CLKS_ON;
> > + trace_ufshcd_clk_gating(dev_name(hba->dev),
> > + hba-
> > >clk_gating.state);
> > + return;
> > + }
> > + if (ufshcd_is_ufs_dev_busy(hba) ||
> > + hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL)
> > + return;
> > }
>
> I'm wondering if it would be safe to replace host_lock with gating.lock or
> scaling.lock. For instance, in above context, ufshcd_state needs to be checked,
> but it's currently serialized by host_lock.
Hi, thank you for your feedback.
Yeah - I think you have a valid point.
I will remove the state check out of the scope of the clk_gating.lock,
and restore it under the host lock.
Thanks,
Avri
>
> King regards,
> Bean
