Messages in this thread | | | Date | Thu, 24 Jun 2021 10:02:46 +0800 | From | Can Guo <> | Subject | Re: [PATCH v4 01/10] scsi: ufs: Rename flags pm_op_in_progress and is_sys_suspended |
| |
Hi Bart,
On 2021-06-24 04:57, Bart Van Assche wrote: > On 6/23/21 1:05 PM, Bart Van Assche wrote: >> On 6/23/21 12:35 AM, Can Guo wrote: >>> Rename pm_op_in_progress and is_sys_suspended to >>> wlu_pm_op_in_progress and >>> is_wlu_sys_suspended accordingly. >> >> My understanding is that power management operations must be submitted >> to one particular UFS WLUN (hba->sdev_ufs_device). That makes the >> "wlu_" >> part of the new names redundant. In other words, I like the current >> names better than the new names. Unless if I missed something, >> consider >> dropping this patch. > > Hi Can, > > Reviewing later patches in this series made me realize that there are > two families of suspend/resume functions. One family of functions > operates at the platform level while the other family operates at the > SCSI LUN level. My comments about the suspend/resume functions are as > follows: > - It seems redundant to me to have system suspend support at the SCSI > LUN level (__ufshcd_wl_suspend(hba, UFS_SYSTEM_PM)) and also at the > platform level. Since the platform device is a parent of the SCSI > WLUN, can system suspend/resume support be left out from > ufshcd_wl_pm_ops (or in other words, remove the .freeze and .thaw > callbacks)? Do we really need two calls from the power management > subsystem into the UFS driver for every system suspend and every > system resume?
Asutosh and Adrian should be the right persons to answer this, since they've been working together on that huge change for 4 months -
https://lore.kernel.org/patchwork/patch/1417696/
In short, we need a dedicated suspend/resume ops for the UFS device W-LU because one cannot send requests (not even PM requests) after the device is runtime suspended - sending SSU cmds in hba suspend/resume cannot pass through blk_queue_enter() as SSU cmd is sent to the UFS device W-LU scsi device (by now it is runtime suspended) but not the hba device.
Of course we can keep the old way and send the SSU cmd through a request queue without block layer PM initialized (hba->cmd_queue for example, by pointing cmd_queue->dev to the UFS device W-LU scsi device), but that would look like a hack.
> - Because of the device links (device_link_add()), the ufschd_wl_*() > RPM callbacks are invoked after all LUNs have been suspended. I would > appreciate it if the "ufshcd_wl_" prefix would be changed into > "ufshcd_lun_" since that would make it more clear that these > callbacks > are associated with all LUNs and not only with the WLUN through which > power management commands are submitted. >
Sure, we will do that later.
Thanks,
Can Guo.
> Thanks, > > Bart.
| |