Hi Bart,
Thanks for reviewing, I posted a v3.
Best,
Adrien
On Fri, Feb 17, 2023 at 09:13:59AM -0800, Bart Van Assche wrote:
> On 2/16/23 13:00, Adrien Thierry wrote:
> > During ufs initialization, devfreq initialization is asynchronous:
> > ufshcd_async_scan() calls ufshcd_add_lus(), which in turn initializes
> > devfreq for ufs. The simple ondemand governor is then loaded. If it is
> > build as a module, request_module() is called and throws a warning:
>
> build -> built?
>
> > @@ -9896,12 +9893,30 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
> > */
> > ufshcd_set_ufs_dev_active(hba);
> > + /* Initialize devfreq */
> > + if (ufshcd_is_clkscaling_supported(hba)) {
> > + memcpy(&hba->clk_scaling.saved_pwr_info.info,
> > + &hba->pwr_info,
> > + sizeof(struct ufs_pa_layer_attr));
> > + hba->clk_scaling.saved_pwr_info.is_valid = true;
> > + hba->clk_scaling.is_allowed = true;
> > +
> > + err = ufshcd_devfreq_init(hba);
> > + if (err)
> > + goto out_power_off;
> > +
> > + hba->clk_scaling.is_enabled = true;
> > + ufshcd_init_clk_scaling_sysfs(hba);
> > + }
> > +
> > async_schedule(ufshcd_async_scan, hba);
> > ufs_sysfs_add_nodes(hba->dev);
> > device_enable_async_suspend(dev);
> > return 0;
> > +out_power_off:
> > + pm_runtime_put_sync(dev);
> > free_tmf_queue:
> > blk_mq_destroy_queue(hba->tmf_queue);
> > blk_put_queue(hba->tmf_queue);
>
> Something I should have noticed while reviewing v1 of this patch: the label
> name "out_power_off" is misleading. pm_runtime_put_sync() does not power off
> a device but instead gives the power management core permission to apply the
> power management policy configured via sysfs. Runtime power management can
> be disabled via sysfs. How about renaming this label into "rpm_put_sync"
> (RPM = Runtime Power Management)?
>
> Thanks,
>
> Bart.
>
