Re: [PATCH v1 1/2] scsi: ufs: set device as default active power mode during initialization only

Can Guo
2019-12-31T08:35:31Z

On 2019-12-31 15:44, Stanley Chu wrote:
> Hi Can,
>
> On Tue, 2019-12-31 at 12:22 +0800, Stanley Chu wrote:
>> Hi Can,
>>
>>
>> > Hi Stanley,
>> >
>> > I see skipping ufshcd_set_ufs_dev_active() in ufshcd_probe_hba()
>> > if it is called from ufshcd_resume() path is the purpose here.
>> >
>> > If so, then ufshcd_set_dev_pwr_mode() would be called, meaning
>> > SSU command will be sent. Why is this SSU command needed to be
>> > sent after a full host reset and restore? Is ufshcd_probe_hba()
>> > not enough to make UFS device fully functional?
>>
>> After resume (for both runtime resume and system resume), device power
>> mode shall be back to "Active" to service incoming requests.
>>
>> I see two cases need device power mode transition during resume flow
>> 1. Device Power Mode = Sleep
>> 2. Device Power Mode = PowerDown
>>
>> For 1, ufshcd_probe_hba() is not invoked during resume flow,
>> hba->curr_dev_pwr_mode = SLEEP, thus ufshcd_resume() can invoke
>> ufshcd_set_dev_pwr_mode() to change device power mode.
>>
>> For 2, ufshcd_probe_hba() is invoked during resume flow, before this
>> fix, hba->curr_dev_pwr_mode will be set to ACTIVE (note that only this
>> flag is set as ACTIVE, but device may be still in PowerDown state if
>> device power is not fully shutdown or device HW reset is not executed
>> before resume), in the end, ufshcd_resume() will not invoke
>> ufshcd_set_dev_pwr_mode() to send SSU command to make device change
>> back
>> to Active power mode.
>>
>> But if device is fully reset before resume (not by current mainstream
>> driver), device can be already in "Active" power mode after power on
>> again during resume flow. In this case, it is OK to set
>> hba->curr_dev_pwr_mode as ACTIVE in ufshcd_probe_hba() and SSU command
>> is not necessary.
>>
>> Thanks,
>> Stanley
>
> I think currently the assumption in ufshcd_probe_hba() that "device
> shall be already in Active power mode" makes sense because many device
> commands will be sent to device in ufshcd_probe_hba() but device is not
> promised to handle those commands in PowerDown power mode according to
> UFS spec.
>
> So, maybe always ensuring device being Active power mode before leaving
> ufshcd_probe_hba() is more reasonable. If so, I will drop this patch
> first.
>
> Thanks so much for your reviews.
>
> Happy new year!
>
> Thanks,
> Stanley

Hi Stanley,

I missed this mail before I hit send. In current code, as per my
understanding,
UFS device's power state should be Active after ufshcd_link_startup()
returns.
If I am wrong, please feel free to correct me.

Due to you are almost trying to revert commit 7caf489b99a42a, I am just
wondering
if you encounter failure/error caused by it.

Happy new year to you too!

Thanks,

Can Guo