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.
