[RFC PATCH v1 0/2] permit vendor specific timeouts for PMC

[RFC PATCH v1 0/2] permit vendor specific timeouts for PMC

[Kiwoong Kim]

There are some attribute settings before power mode change in ufshcd.c
that should be variant per vendor.

Kiwoong Kim (2):
  ufs: add a quirk for vendor specifics before pmc
  ufs: ufs-exynos: apply vendor specifics for three timeouts

 drivers/scsi/ufs/ufs-exynos.c |  8 +++++++-
 drivers/scsi/ufs/ufshcd.c     | 40 +++++++++++++++++++++-------------------
 drivers/scsi/ufs/ufshcd.h     |  6 ++++++
 3 files changed, 34 insertions(+), 20 deletions(-)

-- 
2.7.4

[RFC PATCH v1 1/2] ufs: add a quirk for vendor specifics before pmc

[Kiwoong Kim]

Unipro specification says attribute IDs of the following
thing are vendor-specific values, so some SoCs could have
no regions at the defined address
- DME_LocalFC0ProtectionTimeOutVal
- DME_LocalTC0ReplayTimeOutVal
- DME_LocalAFC0ReqTimeOutVal

The following things should be set considering the compatibility
between host and device, so those values must not be fixed and
could be reset values or vendor specific values
- PA_PWRMODEUSERDATA0
- PA_PWRMODEUSERDATA1
- PA_PWRMODEUSERDATA2
- PA_PWRMODEUSERDATA3
- PA_PWRMODEUSERDATA4
- PA_PWRMODEUSERDATA5

Change-Id: Ifbb55e9ea221156804121c4dedb3a099ce02cb95
Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c | 40 +++++++++++++++++++++-------------------
 drivers/scsi/ufs/ufshcd.h |  6 ++++++
 2 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 92d433d..934d96f 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -4153,25 +4153,27 @@ 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);
+	if (!(hba->quirks & UFSHCD_QUIRK_SKIP_VENDOR_SPECIFIC_BEFORE_PMC)) {
+		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/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 61344c4..ab7af1d 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -549,6 +549,12 @@ enum ufshcd_quirks {
 	 */
 	UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL		= 1 << 12,
 
+	/*
+	 * This quirk needs to disable vendor specific setting
+	 * before power mode change
+	 */
+	UFSHCD_QUIRK_SKIP_VENDOR_SPECIFIC_BEFORE_PMC = 1 << 13,
+
 };
 
 enum ufshcd_caps {
-- 
2.7.4

[RFC PATCH v1 2/2] ufs: ufs-exynos: apply vendor specifics for three timeouts

[Kiwoong Kim]

Set optimized values for those timeouts
- FC0_PROTECTION_TIMER
- TC0_REPLAY_TIMER
- AFC0_REQUEST_TIMER

Exynos doesn't yet use traffic class #1.

Change-Id: Ib672b554ea109159cd75ff67a111f46ebff4a7dc
Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
---
 drivers/scsi/ufs/ufs-exynos.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufs-exynos.c b/drivers/scsi/ufs/ufs-exynos.c
index a8770ff..1ea8244 100644
--- a/drivers/scsi/ufs/ufs-exynos.c
+++ b/drivers/scsi/ufs/ufs-exynos.c
@@ -640,6 +640,11 @@ static int exynos_ufs_pre_pwr_mode(struct ufs_hba *hba,
 		}
 	}
 
+	/* setting for three timeout values for traffic class #0 */
+	ufshcd_dme_set(hba, UIC_ARG_MIB(PA_PWRMODEUSERDATA0), 8064);
+	ufshcd_dme_set(hba, UIC_ARG_MIB(PA_PWRMODEUSERDATA1), 28224);
+	ufshcd_dme_set(hba, UIC_ARG_MIB(PA_PWRMODEUSERDATA2), 20160);
+
 	return 0;
 out:
 	return ret;
@@ -1236,7 +1241,8 @@ struct exynos_ufs_drv_data exynos_ufs_drvs = {
 				  UFSHCI_QUIRK_BROKEN_HCE |
 				  UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR |
 				  UFSHCD_QUIRK_BROKEN_OCS_FATAL_ERROR |
-				  UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL,
+				  UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL |
+				  UFSHCD_QUIRK_SKIP_VENDOR_SPECIFIC_BEFORE_PMC,
 	.opts			= EXYNOS_UFS_OPT_HAS_APB_CLK_CTRL |
 				  EXYNOS_UFS_OPT_BROKEN_AUTO_CLK_CTRL |
 				  EXYNOS_UFS_OPT_BROKEN_RX_SEL_IDX |
-- 
2.7.4

RE: [RFC PATCH v1 0/2] permit vendor specific timeouts for PMC

[Avri Altman]

> 
> 
> There are some attribute settings before power mode change in ufshcd.c
> that should be variant per vendor.
But you've added a quirk, which is not what your commit log say.
If indeed unipro allows to skip/set values other than default - add the appropriate vop.
Otherwise, if it's just a non-standard behavior of exynos - then your patch is appropriate,
But you need to reword your commit log: both here and in your 1/2 patch.

Also, you forgot to remove the gerrit change-id.

Thanks,
Avri

> 
> Kiwoong Kim (2):
>   ufs: add a quirk for vendor specifics before pmc
>   ufs: ufs-exynos: apply vendor specifics for three timeouts
> 
>  drivers/scsi/ufs/ufs-exynos.c |  8 +++++++-
>  drivers/scsi/ufs/ufshcd.c     | 40 +++++++++++++++++++++-------------------
>  drivers/scsi/ufs/ufshcd.h     |  6 ++++++
>  3 files changed, 34 insertions(+), 20 deletions(-)
> 
> --
> 2.7.4

RE: [RFC PATCH v1 0/2] permit vendor specific timeouts for PMC

[Kiwoong Kim]

> > There are some attribute settings before power mode change in ufshcd.c
> > that should be variant per vendor.
> But you've added a quirk, which is not what your commit log say.
> If indeed unipro allows to skip/set values other than default - add the
> appropriate vop.
> Otherwise, if it's just a non-standard behavior of exynos - then your
> patch is appropriate, But you need to reword your commit log: both here
> and in your 1/2 patch.
> 
> Also, you forgot to remove the gerrit change-id.
> 
> Thanks,
> Avri

You're right and for change-id, there might be something wrong in my local branch.
Anyway thanks !


Thanks.
Kiwoong Kim

> >
> > Kiwoong Kim (2):
> >   ufs: add a quirk for vendor specifics before pmc
> >   ufs: ufs-exynos: apply vendor specifics for three timeouts
> >
> >  drivers/scsi/ufs/ufs-exynos.c |  8 +++++++-
> >  drivers/scsi/ufs/ufshcd.c     | 40 +++++++++++++++++++++----------------
> ---
> >  drivers/scsi/ufs/ufshcd.h     |  6 ++++++
> >  3 files changed, 34 insertions(+), 20 deletions(-)
> >
> > --
> > 2.7.4