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

[Can Guo]

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