Re: [PATCH v3 1/1] scsi: ufs: Add hibernation callbacks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/20/23 03:33, Anjana Hari wrote:
+		/* Make sure that UTRL and UTMRL base address registers
+		 * are updated with the latest queue addresses. Only after
+		 * updating these addresses, we can queue the new commands.
+		 */

Please follow the kernel coding style and start comment blocks with "/*" on a line by its own.

+	/* Resuming from hibernate, assume that link was OFF */
+	if (hba->restore)
+		ufshcd_set_link_off(hba);

Why two successive 'if (hba->restore)' statements? Can these two if-statements be combined into a single if-statement?

+int ufshcd_system_freeze(struct device *dev)
+{
+
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+	int ret = 0;
+
+	/*
+	 * Run time resume the controller to make sure
+	 * the PM work queue threads do not try to resume
+	 * the child (scsi host), which leads to errors as
+	 * the controller is not yet resumed.
+	 */
+	pm_runtime_get_sync(hba->dev);
+	ret = ufshcd_system_suspend(dev);
+	pm_runtime_put_sync(hba->dev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(ufshcd_system_freeze);

Why does the above function use 'hba->dev' instead of 'dev'?

I do not understand the comment in the above function. My understanding is that the power management core serializes hibernation and runtime power management. How could runtime resume be in progress when ufshcd_system_freeze() is called? Additionally, shouldn't ufshcd_system_freeze() skip the UFS controller if it is already runtime suspended instead of runtime resuming it? Some time ago I added the following code in sd_suspend_system() (commit 9131bff6a9f1 ("scsi: core: pm: Only runtime resume if necessary"):

	if (pm_runtime_suspended(dev))
		return 0;

Thanks,

Bart.



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux