Re: [PATCH v2] scsi: ufs: initialize devfreq synchronously

[Adrien Thierry <athierry@redhat.com>]

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.
>