diff mbox series

[v4,4/5] scsi: ufs: Do not clear the DL layer timers

Message ID 1573624824-671-5-git-send-email-cang@codeaurora.org (mailing list archive)
State Superseded
Headers show
Series UFS driver general fixes bundle 1 | expand

Commit Message

Can Guo Nov. 13, 2019, 6 a.m. UTC
During power mode change, PACP_PWR_Req frame sends
PAPowerModeUserData parameters (and they are considered valid by device if
Flags[4] - UserDataValid bit is set in the same frame).
Currently we don't set these PAPowerModeUserData parameters and hardware
always sets UserDataValid bit which would clear all the DL layer timeout
values of the peer device after the power mode change.

This change sets the PAPowerModeUserData[0..5] to UniPro specification
recommended default values, in addition we are also setting the relevant
DME_LOCAL_* timer attributes as required by UFS HCI specification.

Signed-off-by: Can Guo <cang@codeaurora.org>
---
 drivers/scsi/ufs/ufshcd.c | 20 ++++++++++++++++++++
 drivers/scsi/ufs/unipro.h | 11 +++++++++++
 2 files changed, 31 insertions(+)

Comments

Avri Altman Nov. 20, 2019, 3:03 p.m. UTC | #1
> 
> During power mode change, PACP_PWR_Req frame sends
> PAPowerModeUserData parameters (and they are considered valid by device if
> Flags[4] - UserDataValid bit is set in the same frame).
> Currently we don't set these PAPowerModeUserData parameters and hardware
> always sets UserDataValid bit which would clear all the DL layer timeout values
> of the peer device after the power mode change.
> 
> This change sets the PAPowerModeUserData[0..5] to UniPro specification
> recommended default values, in addition we are also setting the relevant
> DME_LOCAL_* timer attributes as required by UFS HCI specification.
> 
> Signed-off-by: Can Guo <cang@codeaurora.org>
Reviewed-by Avri Altman <avri.altman@wdc.com>
Avri Altman Nov. 25, 2019, 7:22 a.m. UTC | #2
> >
> > During power mode change, PACP_PWR_Req frame sends
> PAPowerModeUserData
> > parameters (and they are considered valid by device if Flags[4] -
> > UserDataValid bit is set in the same frame).
> > Currently we don't set these PAPowerModeUserData parameters and
> > hardware always sets UserDataValid bit which would clear all the DL
> > layer timeout values of the peer device after the power mode change.
> >
> > This change sets the PAPowerModeUserData[0..5] to UniPro specification
> > recommended default values, in addition we are also setting the
> > relevant
> > DME_LOCAL_* timer attributes as required by UFS HCI specification.
> >
> > Signed-off-by: Can Guo <cang@codeaurora.org>
> Reviewed-by Avri Altman <avri.altman@wdc.com>
BTW, I noticed that you are only updating the TC0 registers.
Why not setting the TC1 registers as well?

Thanks,
Avri
Can Guo Nov. 25, 2019, 7:39 a.m. UTC | #3
On 2019-11-25 15:22, Avri Altman wrote:
>> >
>> > During power mode change, PACP_PWR_Req frame sends
>> PAPowerModeUserData
>> > parameters (and they are considered valid by device if Flags[4] -
>> > UserDataValid bit is set in the same frame).
>> > Currently we don't set these PAPowerModeUserData parameters and
>> > hardware always sets UserDataValid bit which would clear all the DL
>> > layer timeout values of the peer device after the power mode change.
>> >
>> > This change sets the PAPowerModeUserData[0..5] to UniPro specification
>> > recommended default values, in addition we are also setting the
>> > relevant
>> > DME_LOCAL_* timer attributes as required by UFS HCI specification.
>> >
>> > Signed-off-by: Can Guo <cang@codeaurora.org>
>> Reviewed-by Avri Altman <avri.altman@wdc.com>
> BTW, I noticed that you are only updating the TC0 registers.
> Why not setting the TC1 registers as well?
> 
> Thanks,
> Avri

Hi Avri,

In the HCI spec, it goes

Currently, UFS uses TC0 only. Therefore, setting the following values 
are not needed:
 DME_ Local_FC1ProtectionTimeOutVal
 DME_ Local_TC1ReplayTimeOutVal
 DME_ Local_ AFC1ReqTimeOutVal

Best Regards,
Can Guo.
Avri Altman Dec. 2, 2019, 7:42 a.m. UTC | #4
> During power mode change, PACP_PWR_Req frame sends
> PAPowerModeUserData parameters (and they are considered valid by device if
> Flags[4] - UserDataValid bit is set in the same frame).
> Currently we don't set these PAPowerModeUserData parameters and hardware
> always sets UserDataValid bit which would clear all the DL layer timeout values
> of the peer device after the power mode change.
> 
> This change sets the PAPowerModeUserData[0..5] to UniPro specification
> recommended default values, in addition we are also setting the relevant
> DME_LOCAL_* timer attributes as required by UFS HCI specification.
> 
> Signed-off-by: Can Guo <cang@codeaurora.org>
Reviewed-by: Avri Altman <avri.altman@wdc.com>
Bean Huo Dec. 4, 2019, 1:45 a.m. UTC | #5
> >
> > Signed-off-by: Can Guo <cang@codeaurora.org>
> Reviewed-by: Avri Altman <avri.altman@wdc.com>
Reviewed-by: Bean Huo <beanhuo@micron.com>
diff mbox series

Patch

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 9e44506..086d359 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -4084,6 +4084,26 @@  static int ufshcd_change_power_mode(struct ufs_hba *hba,
 		ufshcd_dme_set(hba, UIC_ARG_MIB(PA_HSSERIES),
 						pwr_mode->hs_rate);
 
+	ufshcd_dme_set(hba, UIC_ARG_MIB(PA_PWRMODEUSERDATA0),
+			DL_FC0ProtectionTimeOutVal_Default);
+	ufshcd_dme_set(hba, UIC_ARG_MIB(PA_PWRMODEUSERDATA1),
+			DL_TC0ReplayTimeOutVal_Default);
+	ufshcd_dme_set(hba, UIC_ARG_MIB(PA_PWRMODEUSERDATA2),
+			DL_AFC0ReqTimeOutVal_Default);
+	ufshcd_dme_set(hba, UIC_ARG_MIB(PA_PWRMODEUSERDATA3),
+			DL_FC1ProtectionTimeOutVal_Default);
+	ufshcd_dme_set(hba, UIC_ARG_MIB(PA_PWRMODEUSERDATA4),
+			DL_TC1ReplayTimeOutVal_Default);
+	ufshcd_dme_set(hba, UIC_ARG_MIB(PA_PWRMODEUSERDATA5),
+			DL_AFC1ReqTimeOutVal_Default);
+
+	ufshcd_dme_set(hba, UIC_ARG_MIB(DME_LocalFC0ProtectionTimeOutVal),
+			DL_FC0ProtectionTimeOutVal_Default);
+	ufshcd_dme_set(hba, UIC_ARG_MIB(DME_LocalTC0ReplayTimeOutVal),
+			DL_TC0ReplayTimeOutVal_Default);
+	ufshcd_dme_set(hba, UIC_ARG_MIB(DME_LocalAFC0ReqTimeOutVal),
+			DL_AFC0ReqTimeOutVal_Default);
+
 	ret = ufshcd_uic_change_pwr_mode(hba, pwr_mode->pwr_rx << 4
 			| pwr_mode->pwr_tx);
 
diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h
index f539f87..3dc4d8b 100644
--- a/drivers/scsi/ufs/unipro.h
+++ b/drivers/scsi/ufs/unipro.h
@@ -161,6 +161,17 @@ 
 /* PHY Adapter Protocol Constants */
 #define PA_MAXDATALANES	4
 
+#define DL_FC0ProtectionTimeOutVal_Default	8191
+#define DL_TC0ReplayTimeOutVal_Default		65535
+#define DL_AFC0ReqTimeOutVal_Default		32767
+#define DL_FC1ProtectionTimeOutVal_Default	8191
+#define DL_TC1ReplayTimeOutVal_Default		65535
+#define DL_AFC1ReqTimeOutVal_Default		32767
+
+#define DME_LocalFC0ProtectionTimeOutVal	0xD041
+#define DME_LocalTC0ReplayTimeOutVal		0xD042
+#define DME_LocalAFC0ReqTimeOutVal		0xD043
+
 /* PA power modes */
 enum {
 	FAST_MODE	= 1,