diff mbox series

ufs: qcom: Add quirks for Samsung UFS devices

Message ID 20250325083857.23653-1-quic_mapa@quicinc.com (mailing list archive)
State New
Headers show
Series ufs: qcom: Add quirks for Samsung UFS devices | expand

Commit Message

MANISH PANDEY March 25, 2025, 8:38 a.m. UTC
Introduce quirks for Samsung UFS devices to override PA hibern8 time,
PA TX HSG1 sync length, and TX_HS_EQUALIZER for the Qualcomm UFS Host
controller. These adjustments are essential to maintain the proper
functionality of Samsung UFS devices for Qualcomm UFS Host controller.

Signed-off-by: Manish Pandey <quic_mapa@quicinc.com>
---
 drivers/ufs/host/ufs-qcom.c | 66 +++++++++++++++++++++++++++++++++++++
 drivers/ufs/host/ufs-qcom.h | 25 +++++++++++++-
 2 files changed, 90 insertions(+), 1 deletion(-)

Comments

Bart Van Assche March 25, 2025, 11:42 a.m. UTC | #1
On 3/25/25 4:38 AM, Manish Pandey wrote:
> Introduce quirks for Samsung UFS devices to override PA hibern8 time,
> PA TX HSG1 sync length, and TX_HS_EQUALIZER for the Qualcomm UFS Host
> controller. These adjustments are essential to maintain the proper
> functionality of Samsung UFS devices for Qualcomm UFS Host controller.

Which of these quirks are required for all host controllers and which of
these quirks are only required for Qualcomm host controllers?

> +	equalizer_val = (gear == 5) ? DEEMPHASIS_3_5_dB : NO_DEEMPHASIS;

I think that the parenthesis can be removed from the above statement
without reducing readability of the code.

Thanks,

Bart.
MANISH PANDEY March 26, 2025, 9:53 a.m. UTC | #2
On 3/25/2025 5:12 PM, Bart Van Assche wrote:
> On 3/25/25 4:38 AM, Manish Pandey wrote:
>> Introduce quirks for Samsung UFS devices to override PA hibern8 time,
>> PA TX HSG1 sync length, and TX_HS_EQUALIZER for the Qualcomm UFS Host
>> controller. These adjustments are essential to maintain the proper
>> functionality of Samsung UFS devices for Qualcomm UFS Host controller.
> 
> Which of these quirks are required for all host controllers and which of
> these quirks are only required for Qualcomm host controllers?
> 

PA_TX_HSG1_SYNC_LENGTH and PA_TX_DEEMPHASIS_TUNING are specific to the 
Qualcomm host controller.

The QUIRK_PA_HIBER8TIME may also be necessary for other SoC vendors host 
controllers. For instance, the ufs-exynos.c file implements a similar 
approach in the fsd_ufs_post_link() function:

ufshcd_dme_set(hba, UIC_ARG_MIB(0x15A7), max_rx_hibern8_time_cap + 1);

https://lore.kernel.org/lkml/001101d874c1$3d850eb0$b88f2c10$@samsung.com/

Should we consider moving the QUIRK_PA_HIBER8TIME quirk to the ufshcd 
driver? Please advise.

>> +    equalizer_val = (gear == 5) ? DEEMPHASIS_3_5_dB : NO_DEEMPHASIS;
> > I think that the parenthesis can be removed from the above statement
> without reducing readability of the code.
> 
> Thanks,
> 
> Bart.

equalizer_val = gear == 5 ? DEEMPHASIS_3_5_dB : NO_DEEMPHASIS;
equalizer_val = (gear == 5) ? DEEMPHASIS_3_5_dB : NO_DEEMPHASIS;

i have used parenthesis for better readability and to make the 
conditional expression more explicit.

Thanks
Manish
Bart Van Assche March 26, 2025, 10:39 a.m. UTC | #3
On 3/26/25 5:53 AM, MANISH PANDEY wrote:
> The QUIRK_PA_HIBER8TIME may also be necessary for other SoC vendors host 
> controllers. For instance, the ufs-exynos.c file implements a similar 
> approach in the fsd_ufs_post_link() function:
> 
> ufshcd_dme_set(hba, UIC_ARG_MIB(0x15A7), max_rx_hibern8_time_cap + 1);
> 
> https://lore.kernel.org/lkml/001101d874c1$3d850eb0$b88f2c10$@samsung.com/
> 
> Should we consider moving the QUIRK_PA_HIBER8TIME quirk to the ufshcd 
> driver? Please advise.

That would be appreciated.

Thanks,

Bart.
diff mbox series

Patch

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 23b9f6efa047..18ac687bc4ba 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -31,6 +31,10 @@ 
 	((((c) >> 16) & MCQ_QCFGPTR_MASK) * MCQ_QCFGPTR_UNIT)
 #define MCQ_QCFG_SIZE	0x40
 
+/* De-emphasis for gear-5 */
+#define DEEMPHASIS_3_5_dB	0x04
+#define NO_DEEMPHASIS		0x0
+
 enum {
 	TSTBUS_UAWM,
 	TSTBUS_UARM,
@@ -778,6 +782,24 @@  static int ufs_qcom_icc_update_bw(struct ufs_qcom_host *host)
 	return ufs_qcom_icc_set_bw(host, bw_table.mem_bw, bw_table.cfg_bw);
 }
 
+static void ufs_qcom_set_tx_hs_equalizer(struct ufs_hba *hba,
+				u32 gear, u32 tx_lanes)
+{
+	u32 equalizer_val;
+	int ret, i;
+
+	/* Determine the equalizer value based on the gear */
+	equalizer_val = (gear == 5) ? DEEMPHASIS_3_5_dB : NO_DEEMPHASIS;
+
+	for (i = 0; i < tx_lanes; i++) {
+		ret = ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(TX_HS_EQUALIZER, i),
+					equalizer_val);
+		if (ret)
+			dev_err(hba->dev, "%s: failed equalizer lane %d\n",
+					__func__, i);
+	}
+}
+
 static int ufs_qcom_pwr_change_notify(struct ufs_hba *hba,
 				enum ufs_notify_change_status status,
 				struct ufs_pa_layer_attr *dev_max_params,
@@ -829,6 +851,10 @@  static int ufs_qcom_pwr_change_notify(struct ufs_hba *hba,
 						dev_req_params->gear_tx,
 						PA_INITIAL_ADAPT);
 		}
+
+		if (hba->dev_quirks & UFS_DEVICE_QUIRK_PA_TX_DEEMPHASIS_TUNING)
+			ufs_qcom_set_tx_hs_equalizer(hba,
+				dev_req_params->gear_tx, dev_req_params->lane_tx);
 		break;
 	case POST_CHANGE:
 		if (ufs_qcom_cfg_timers(hba, dev_req_params->gear_rx,
@@ -878,6 +904,35 @@  static int ufs_qcom_quirk_host_pa_saveconfigtime(struct ufs_hba *hba)
 			    (pa_vs_config_reg1 | (1 << 12)));
 }
 
+static void ufs_qcom_override_pa_h8time(struct ufs_hba *hba)
+{
+	u32 pa_h8time = 0;
+	int ret;
+
+	ret = ufshcd_dme_get(hba, UIC_ARG_MIB(PA_HIBERN8TIME),
+				&pa_h8time);
+	if (ret) {
+		dev_err(hba->dev, "Failed to get PA_HIBERN8TIME: %d\n", ret);
+		return;
+	}
+
+	 /* Increment by 1 to increase hibernation time by 100 µs */
+	ret = ufshcd_dme_set(hba, UIC_ARG_MIB(PA_HIBERN8TIME),
+				pa_h8time + 1);
+	if (ret)
+		dev_err(hba->dev, "Failed updating PA_HIBERN8TIME: %d\n", ret);
+}
+
+static void ufs_qcom_override_pa_tx_hsg1_sync_len(struct ufs_hba *hba)
+{
+	int err;
+
+	err = ufshcd_dme_peer_set(hba, UIC_ARG_MIB(PA_TX_HSG1_SYNC_LENGTH),
+					PA_TX_HSG1_SYNC_LENGTH_VAL);
+	if (err)
+		dev_err(hba->dev, "Failed (%d) set PA_TX_HSG1_SYNC_LENGTH\n", err);
+}
+
 static int ufs_qcom_apply_dev_quirks(struct ufs_hba *hba)
 {
 	int err = 0;
@@ -885,6 +940,12 @@  static int ufs_qcom_apply_dev_quirks(struct ufs_hba *hba)
 	if (hba->dev_quirks & UFS_DEVICE_QUIRK_HOST_PA_SAVECONFIGTIME)
 		err = ufs_qcom_quirk_host_pa_saveconfigtime(hba);
 
+	if (hba->dev_quirks & UFS_DEVICE_QUIRK_PA_HIBER8TIME)
+		ufs_qcom_override_pa_h8time(hba);
+
+	if (hba->dev_quirks & UFS_DEVICE_QUIRK_PA_TX_HSG1_SYNC_LENGTH)
+		ufs_qcom_override_pa_tx_hsg1_sync_len(hba);
+
 	return err;
 }
 
@@ -899,6 +960,11 @@  static struct ufs_dev_quirk ufs_qcom_dev_fixups[] = {
 	{ .wmanufacturerid = UFS_VENDOR_WDC,
 	  .model = UFS_ANY_MODEL,
 	  .quirk = UFS_DEVICE_QUIRK_HOST_PA_TACTIVATE },
+	{ .wmanufacturerid = UFS_VENDOR_SAMSUNG,
+	  .model = UFS_ANY_MODEL,
+	  .quirk = UFS_DEVICE_QUIRK_PA_HIBER8TIME |
+		   UFS_DEVICE_QUIRK_PA_TX_HSG1_SYNC_LENGTH |
+		   UFS_DEVICE_QUIRK_PA_TX_DEEMPHASIS_TUNING },
 	{}
 };
 
diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
index 919f53682beb..43274de24706 100644
--- a/drivers/ufs/host/ufs-qcom.h
+++ b/drivers/ufs/host/ufs-qcom.h
@@ -116,8 +116,11 @@  enum {
 				 TMRLUT_HW_CGC_EN | OCSC_HW_CGC_EN)
 
 /* QUniPro Vendor specific attributes */
+#define PA_TX_HSG1_SYNC_LENGTH	0x1552
 #define PA_VS_CONFIG_REG1	0x9000
 #define DME_VS_CORE_CLK_CTRL	0xD002
+#define TX_HS_EQUALIZER		0x0037
+
 /* bit and mask definitions for DME_VS_CORE_CLK_CTRL attribute */
 #define CLK_1US_CYCLES_MASK_V4				GENMASK(27, 16)
 #define CLK_1US_CYCLES_MASK				GENMASK(7, 0)
@@ -125,7 +128,6 @@  enum {
 #define PA_VS_CORE_CLK_40NS_CYCLES			0x9007
 #define PA_VS_CORE_CLK_40NS_CYCLES_MASK			GENMASK(6, 0)
 
-
 /* QCOM UFS host controller core clk frequencies */
 #define UNIPRO_CORE_CLK_FREQ_37_5_MHZ          38
 #define UNIPRO_CORE_CLK_FREQ_75_MHZ            75
@@ -135,6 +137,27 @@  enum {
 #define UNIPRO_CORE_CLK_FREQ_201_5_MHZ         202
 #define UNIPRO_CORE_CLK_FREQ_403_MHZ           403
 
+/* TX_HSG1_SYNC_LENGTH attr value */
+#define PA_TX_HSG1_SYNC_LENGTH_VAL	0x4A
+
+/*
+ * Some ufs devices may need more time to be in hibern8 before exiting.
+ * Enable this quirk to give it an additional 100us.
+ */
+#define UFS_DEVICE_QUIRK_PA_HIBER8TIME          (1 << 15)
+
+/*
+ * Some ufs device vendors need a different TSync length.
+ * Enable this quirk to give an additional TX_HS_SYNC_LENGTH.
+ */
+#define UFS_DEVICE_QUIRK_PA_TX_HSG1_SYNC_LENGTH (1 << 16)
+
+/*
+ * Some ufs device vendors need a different Deemphasis setting.
+ * Enable this quirk to tune TX Deemphasis parameters.
+ */
+#define UFS_DEVICE_QUIRK_PA_TX_DEEMPHASIS_TUNING (1 << 17)
+
 static inline void
 ufs_qcom_get_controller_revision(struct ufs_hba *hba,
 				 u8 *major, u16 *minor, u16 *step)