* [PATCH 1/2] dt-bindings: ufs: Document static TX Equalization settings properties
[not found] <20260527144055.2758170-1-can.guo@oss.qualcomm.com>
@ 2026-05-27 14:40 ` Can Guo
2026-05-28 6:18 ` Manivannan Sadhasivam
2026-05-30 11:36 ` Krzysztof Kozlowski
2026-05-27 14:40 ` [PATCH 2/2] scsi: ufs: core: Add support for static TX Equalization settings Can Guo
1 sibling, 2 replies; 15+ messages in thread
From: Can Guo @ 2026-05-27 14:40 UTC (permalink / raw)
To: bvanassche, beanhuo, peter.wang, martin.petersen, mani
Cc: linux-scsi, Can Guo, Alim Akhtar, Avri Altman, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Zhaoming Luo,
Ram Kumar Dwivedi,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
UFS v5.0/UFSHCI v5.0 add HS-G6 support (46.6 Gbps/lane) via UniPro v3.0
and M-PHY v6.0. In these specs, TX Equalization is defined for all High
Speed Gears (not only HS-G6) to compensate channel loss and improve signal
integrity at high speed operation.
For HS-G6, M-PHY uses PAM4 1b1b line coding, Pre-Coding may also be
required depending on channel characteristics.
Add vendor-neutral DT properties:
- patternProperties for txeq-preshoot-g[1-6] and txeq-deemphasis-g[1-6]
- fixed property tx-precode-enable-g6
Each property is a uint32 array of per-lane tuples:
<Host_Lane0 Device_Lane0>, [<Host_Lane1 Device_Lane1>]
Accept 2 or 4 values (x1/x2 lane configs). PreShoot and DeEmphasis values
are 0..7. Precode enable values are 0/1 and only applicable to HS-G6.
Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
---
.../devicetree/bindings/ufs/ufs-common.yaml | 45 +++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/Documentation/devicetree/bindings/ufs/ufs-common.yaml b/Documentation/devicetree/bindings/ufs/ufs-common.yaml
index ed97f5682509..d90cf25adfa5 100644
--- a/Documentation/devicetree/bindings/ufs/ufs-common.yaml
+++ b/Documentation/devicetree/bindings/ufs/ufs-common.yaml
@@ -105,6 +105,51 @@ properties:
Restricts the UFS controller to rate-a or rate-b for both TX and
RX directions.
+ tx-precode-enable-g6:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ oneOf:
+ - minItems: 2
+ maxItems: 2
+ - minItems: 4
+ maxItems: 4
+ items:
+ enum: [0, 1]
+ description: |
+ Static TX Precode enable values for HS-G6 only.
+ Values are specified as per-lane tuples:
+ <Host_Lane0 Device_Lane0>, [<Host_Lane1 Device_Lane1>].
+
+patternProperties:
+ "^txeq-preshoot-g[1-6]$":
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ oneOf:
+ - minItems: 2
+ maxItems: 2
+ - minItems: 4
+ maxItems: 4
+ items:
+ minimum: 0
+ maximum: 7
+ description: |
+ Static TX Equalization PreShoot values for High Speed Gears.
+ Values are specified as per-lane tuples:
+ <Host_Lane0 Device_Lane0>, [<Host_Lane1 Device_Lane1>].
+
+ "^txeq-deemphasis-g[1-6]$":
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ oneOf:
+ - minItems: 2
+ maxItems: 2
+ - minItems: 4
+ maxItems: 4
+ items:
+ minimum: 0
+ maximum: 7
+ description: |
+ Static TX Equalization DeEmphasis values for High Speed Gears.
+ Values are specified as per-lane tuples:
+ <Host_Lane0 Device_Lane0>, [<Host_Lane1 Device_Lane1>].
+
dependencies:
freq-table-hz: [ clocks ]
operating-points-v2: [ clocks, clock-names ]
--
2.34.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/2] scsi: ufs: core: Add support for static TX Equalization settings
[not found] <20260527144055.2758170-1-can.guo@oss.qualcomm.com>
2026-05-27 14:40 ` [PATCH 1/2] dt-bindings: ufs: Document static TX Equalization settings properties Can Guo
@ 2026-05-27 14:40 ` Can Guo
2026-05-28 6:13 ` Manivannan Sadhasivam
2026-05-28 6:18 ` Manivannan Sadhasivam
1 sibling, 2 replies; 15+ messages in thread
From: Can Guo @ 2026-05-27 14:40 UTC (permalink / raw)
To: bvanassche, beanhuo, peter.wang, martin.petersen, mani
Cc: linux-scsi, Can Guo, Alim Akhtar, Avri Altman,
James E.J. Bottomley, Ram Kumar Dwivedi, Nitin Rawat, open list
Static TX Equalization settings and TX Precode enable indication from DT
properties txeq-preshoot-g[1-6], txeq-deemphasis-g[1-6], and
tx-precode-enable-g6 are board-specific baseline values. Values are
provided as per-lane tuples:
<Host_Lane0 Device_Lane0>, [<Host_Lane1 Device_Lane1>]
Parse DT u32 properties with explicit range checks by using
of_property_count_u32_elems()/of_property_read_u32_array().
When adaptive TX Equalization is used, these static settings are not final:
- If valid settings are retrieved from qTxEQGnSettings/wTxEQGnSettingsExt,
those retrieved settings override static DT settings.
- If retrieval is not available/valid, TX EQTR runs and trained settings
override static DT settings.
So static DT settings are a fallback and are intended for cases where
adaptive TX Equalization is not enabled/used. Adaptive TX Equalization
remains the primary path when enabled.
No behavior changes for platforms that do not provide these properties.
Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
---
drivers/ufs/core/ufs-txeq.c | 4 +-
drivers/ufs/host/ufshcd-pltfrm.c | 128 +++++++++++++++++++++++++++++++
include/ufs/ufshcd.h | 2 +
3 files changed, 133 insertions(+), 1 deletion(-)
diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
index 4b264adfdf49..634ec039e129 100644
--- a/drivers/ufs/core/ufs-txeq.c
+++ b/drivers/ufs/core/ufs-txeq.c
@@ -1297,7 +1297,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
}
params = &hba->tx_eq_params[gear - 1];
- if (!params->is_valid || force_tx_eqtr) {
+ if (!params->is_valid || params->is_static || force_tx_eqtr) {
int ret;
ret = ufshcd_tx_eqtr(hba, params, pwr_mode);
@@ -1310,6 +1310,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
/* Mark TX Equalization settings as valid */
params->is_valid = true;
params->is_trained = true;
+ params->is_static = false;
params->is_applied = false;
}
@@ -1495,6 +1496,7 @@ static void ufshcd_extract_tx_eq_settings_attrs(struct ufs_hba *hba, u8 gear)
}
params->is_valid = true;
+ params->is_static = false;
}
void ufshcd_retrieve_tx_eq_settings(struct ufs_hba *hba)
diff --git a/drivers/ufs/host/ufshcd-pltfrm.c b/drivers/ufs/host/ufshcd-pltfrm.c
index c2dafb583cf5..2db2103a6ac0 100644
--- a/drivers/ufs/host/ufshcd-pltfrm.c
+++ b/drivers/ufs/host/ufshcd-pltfrm.c
@@ -210,6 +210,132 @@ static void ufshcd_init_lanes_per_dir(struct ufs_hba *hba)
}
}
+static void ufshcd_parse_static_tx_eq_settings(struct ufs_hba *hba)
+{
+ size_t sz = hba->lanes_per_direction * 2;
+ u32 lpd = hba->lanes_per_direction;
+ struct ufshcd_tx_eq_params *params;
+ u32 deemphasis[UFS_MAX_LANES * 2];
+ u32 precode_en[UFS_MAX_LANES * 2];
+ u32 preshoot[UFS_MAX_LANES * 2];
+ struct device *dev = hba->dev;
+ char prop_name[MAX_PROP_SIZE];
+ int i, err, count, gear, lane;
+
+ if (!lpd || lpd > UFS_MAX_LANES) {
+ dev_err(dev, "Invalid lanes-per-direction value (%u) provided\n", lpd);
+ return;
+ }
+
+ for (gear = UFS_HS_G1; gear <= UFS_HS_GEAR_MAX; gear++) {
+ snprintf(prop_name, MAX_PROP_SIZE, "txeq-preshoot-g%d", gear);
+ count = of_property_count_u32_elems(dev->of_node, prop_name);
+ if (count <= 0)
+ continue;
+
+ if (count != sz) {
+ dev_err(dev, "Property %s has invalid count (%d), expecting %zu\n",
+ prop_name, count, sz);
+ continue;
+ }
+
+ err = of_property_read_u32_array(dev->of_node, prop_name, preshoot, sz);
+ if (err) {
+ dev_err(dev, "Failed to read %s property, %d\n",
+ prop_name, err);
+ continue;
+ }
+
+ for (i = 0; i < count; i++) {
+ if (preshoot[i] >= TX_HS_NUM_PRESHOOT) {
+ dev_err(dev, "An invalid TX EQ PreShoot (%d) provided in %s property\n",
+ preshoot[i], prop_name);
+ break;
+ }
+ }
+
+ if (i != count)
+ continue;
+
+ snprintf(prop_name, MAX_PROP_SIZE, "txeq-deemphasis-g%d", gear);
+ count = of_property_count_u32_elems(dev->of_node, prop_name);
+ if (count <= 0) {
+ dev_err(dev, "Missing required %s property\n", prop_name);
+ continue;
+ }
+
+ if (count != sz) {
+ dev_err(dev, "Property %s has invalid count (%d), expecting %zu\n",
+ prop_name, count, sz);
+ continue;
+ }
+
+ err = of_property_read_u32_array(dev->of_node, prop_name, deemphasis, sz);
+ if (err) {
+ dev_err(dev, "Failed to read %s property, %d\n",
+ prop_name, err);
+ continue;
+ }
+
+ for (i = 0; i < count; i++) {
+ if (deemphasis[i] >= TX_HS_NUM_DEEMPHASIS) {
+ dev_err(dev, "An invalid TX EQ DeEmphasis (%d) provided in %s property\n",
+ deemphasis[i], prop_name);
+ break;
+ }
+ }
+
+ if (i != count)
+ continue;
+
+ memset(precode_en, 0, sizeof(precode_en));
+ if (gear == UFS_HS_G6) {
+ snprintf(prop_name, MAX_PROP_SIZE, "tx-precode-enable-g%d", gear);
+ count = of_property_count_u32_elems(dev->of_node, prop_name);
+ if (count > 0) {
+ if (count != sz) {
+ dev_err(dev, "Property %s has invalid count (%d), expecting %zu\n",
+ prop_name, count, sz);
+ continue;
+ }
+
+ err = of_property_read_u32_array(dev->of_node, prop_name,
+ precode_en, sz);
+ if (err) {
+ dev_err(dev, "Failed to read %s property, %d\n",
+ prop_name, err);
+ continue;
+ }
+
+ for (i = 0; i < count; i++) {
+ if (precode_en[i] > 1) {
+ dev_err(dev, "An invalid PrecodeEn (%d) provided in %s property\n",
+ precode_en[i], prop_name);
+ break;
+ }
+ }
+
+ if (i != count)
+ continue;
+ }
+ }
+
+ params = &hba->tx_eq_params[gear - 1];
+ for (lane = 0; lane < lpd; lane++) {
+ params->host[lane].preshoot = preshoot[lane * 2];
+ params->host[lane].deemphasis = deemphasis[lane * 2];
+ params->host[lane].precode_en = precode_en[lane * 2];
+
+ params->device[lane].preshoot = preshoot[lane * 2 + 1];
+ params->device[lane].deemphasis = deemphasis[lane * 2 + 1];
+ params->device[lane].precode_en = precode_en[lane * 2 + 1];
+ }
+
+ params->is_valid = true;
+ params->is_static = true;
+ }
+}
+
/**
* ufshcd_parse_clock_min_max_freq - Parse MIN and MAX clocks freq
* @hba: per adapter instance
@@ -528,6 +654,8 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
ufshcd_init_lanes_per_dir(hba);
+ ufshcd_parse_static_tx_eq_settings(hba);
+
err = ufshcd_parse_operating_points(hba);
if (err) {
dev_err(dev, "%s: OPP parse failed %d\n", __func__, err);
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index f48d6416e299..c01824576472 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -359,6 +359,7 @@ struct ufshcd_tx_eqtr_record {
* @is_valid: True if parameter contains valid TX Equalization settings
* @is_applied: True if settings have been applied to UniPro of both sides
* @is_trained: True if parameters obtained from TX EQTR procedure
+ * @is_static: True if settings are static
*/
struct ufshcd_tx_eq_params {
struct ufshcd_tx_eq_settings host[UFS_MAX_LANES];
@@ -367,6 +368,7 @@ struct ufshcd_tx_eq_params {
bool is_valid;
bool is_applied;
bool is_trained;
+ bool is_static;
};
/**
--
2.34.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] scsi: ufs: core: Add support for static TX Equalization settings
2026-05-27 14:40 ` [PATCH 2/2] scsi: ufs: core: Add support for static TX Equalization settings Can Guo
@ 2026-05-28 6:13 ` Manivannan Sadhasivam
2026-05-28 7:24 ` Can Guo
2026-05-28 6:18 ` Manivannan Sadhasivam
1 sibling, 1 reply; 15+ messages in thread
From: Manivannan Sadhasivam @ 2026-05-28 6:13 UTC (permalink / raw)
To: Can Guo
Cc: bvanassche, beanhuo, peter.wang, martin.petersen, linux-scsi,
Alim Akhtar, Avri Altman, James E.J. Bottomley,
Ram Kumar Dwivedi, Nitin Rawat, open list
On Wed, May 27, 2026 at 07:40:55AM -0700, Can Guo wrote:
> Static TX Equalization settings and TX Precode enable indication from DT
> properties txeq-preshoot-g[1-6], txeq-deemphasis-g[1-6], and
> tx-precode-enable-g6 are board-specific baseline values. Values are
> provided as per-lane tuples:
>
> <Host_Lane0 Device_Lane0>, [<Host_Lane1 Device_Lane1>]
>
> Parse DT u32 properties with explicit range checks by using
> of_property_count_u32_elems()/of_property_read_u32_array().
>
> When adaptive TX Equalization is used, these static settings are not final:
>
> - If valid settings are retrieved from qTxEQGnSettings/wTxEQGnSettingsExt,
> those retrieved settings override static DT settings.
> - If retrieval is not available/valid, TX EQTR runs and trained settings
> override static DT settings.
>
> So static DT settings are a fallback and are intended for cases where
> adaptive TX Equalization is not enabled/used. Adaptive TX Equalization
> remains the primary path when enabled.
>
> No behavior changes for platforms that do not provide these properties.
>
> Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
> ---
> drivers/ufs/core/ufs-txeq.c | 4 +-
> drivers/ufs/host/ufshcd-pltfrm.c | 128 +++++++++++++++++++++++++++++++
> include/ufs/ufshcd.h | 2 +
> 3 files changed, 133 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
> index 4b264adfdf49..634ec039e129 100644
> --- a/drivers/ufs/core/ufs-txeq.c
> +++ b/drivers/ufs/core/ufs-txeq.c
> @@ -1297,7 +1297,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
> }
>
> params = &hba->tx_eq_params[gear - 1];
> - if (!params->is_valid || force_tx_eqtr) {
> + if (!params->is_valid || params->is_static || force_tx_eqtr) {
> int ret;
>
> ret = ufshcd_tx_eqtr(hba, params, pwr_mode);
> @@ -1310,6 +1310,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
> /* Mark TX Equalization settings as valid */
> params->is_valid = true;
> params->is_trained = true;
> + params->is_static = false;
> params->is_applied = false;
> }
>
> @@ -1495,6 +1496,7 @@ static void ufshcd_extract_tx_eq_settings_attrs(struct ufs_hba *hba, u8 gear)
> }
>
> params->is_valid = true;
> + params->is_static = false;
Maybe it's me, but I'm not able to understand how you want to apply these static
EQ settings. In commit message you said, the static values should be used as a
fallback, but you just check for 'params->is_static' while triggering
ufshcd_tx_eqtr() which is supposed to perform adaptive TX EQ training. IMO, you
don't need any check at all for applying static setting. If '(!params->is_valid
|| force_tx_eqtr)' condition is not satisfied, then the static setting should be
used.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] scsi: ufs: core: Add support for static TX Equalization settings
2026-05-27 14:40 ` [PATCH 2/2] scsi: ufs: core: Add support for static TX Equalization settings Can Guo
2026-05-28 6:13 ` Manivannan Sadhasivam
@ 2026-05-28 6:18 ` Manivannan Sadhasivam
2026-05-28 7:27 ` Can Guo
1 sibling, 1 reply; 15+ messages in thread
From: Manivannan Sadhasivam @ 2026-05-28 6:18 UTC (permalink / raw)
To: Can Guo
Cc: bvanassche, beanhuo, peter.wang, martin.petersen, linux-scsi,
Alim Akhtar, Avri Altman, James E.J. Bottomley,
Ram Kumar Dwivedi, Nitin Rawat, open list
On Wed, May 27, 2026 at 07:40:55AM -0700, Can Guo wrote:
> Static TX Equalization settings and TX Precode enable indication from DT
> properties txeq-preshoot-g[1-6], txeq-deemphasis-g[1-6], and
> tx-precode-enable-g6 are board-specific baseline values. Values are
> provided as per-lane tuples:
>
> <Host_Lane0 Device_Lane0>, [<Host_Lane1 Device_Lane1>]
>
> Parse DT u32 properties with explicit range checks by using
> of_property_count_u32_elems()/of_property_read_u32_array().
>
> When adaptive TX Equalization is used, these static settings are not final:
>
> - If valid settings are retrieved from qTxEQGnSettings/wTxEQGnSettingsExt,
> those retrieved settings override static DT settings.
> - If retrieval is not available/valid, TX EQTR runs and trained settings
> override static DT settings.
>
> So static DT settings are a fallback and are intended for cases where
> adaptive TX Equalization is not enabled/used. Adaptive TX Equalization
> remains the primary path when enabled.
>
> No behavior changes for platforms that do not provide these properties.
>
> Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
> ---
> drivers/ufs/core/ufs-txeq.c | 4 +-
> drivers/ufs/host/ufshcd-pltfrm.c | 128 +++++++++++++++++++++++++++++++
> include/ufs/ufshcd.h | 2 +
> 3 files changed, 133 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
> index 4b264adfdf49..634ec039e129 100644
> --- a/drivers/ufs/core/ufs-txeq.c
> +++ b/drivers/ufs/core/ufs-txeq.c
> @@ -1297,7 +1297,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
> }
>
> params = &hba->tx_eq_params[gear - 1];
> - if (!params->is_valid || force_tx_eqtr) {
> + if (!params->is_valid || params->is_static || force_tx_eqtr) {
> int ret;
>
> ret = ufshcd_tx_eqtr(hba, params, pwr_mode);
> @@ -1310,6 +1310,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
> /* Mark TX Equalization settings as valid */
> params->is_valid = true;
> params->is_trained = true;
> + params->is_static = false;
> params->is_applied = false;
> }
>
> @@ -1495,6 +1496,7 @@ static void ufshcd_extract_tx_eq_settings_attrs(struct ufs_hba *hba, u8 gear)
> }
>
> params->is_valid = true;
> + params->is_static = false;
> }
>
> void ufshcd_retrieve_tx_eq_settings(struct ufs_hba *hba)
> diff --git a/drivers/ufs/host/ufshcd-pltfrm.c b/drivers/ufs/host/ufshcd-pltfrm.c
> index c2dafb583cf5..2db2103a6ac0 100644
> --- a/drivers/ufs/host/ufshcd-pltfrm.c
> +++ b/drivers/ufs/host/ufshcd-pltfrm.c
> @@ -210,6 +210,132 @@ static void ufshcd_init_lanes_per_dir(struct ufs_hba *hba)
> }
> }
>
> +static void ufshcd_parse_static_tx_eq_settings(struct ufs_hba *hba)
> +{
> + size_t sz = hba->lanes_per_direction * 2;
> + u32 lpd = hba->lanes_per_direction;
> + struct ufshcd_tx_eq_params *params;
> + u32 deemphasis[UFS_MAX_LANES * 2];
> + u32 precode_en[UFS_MAX_LANES * 2];
> + u32 preshoot[UFS_MAX_LANES * 2];
> + struct device *dev = hba->dev;
> + char prop_name[MAX_PROP_SIZE];
> + int i, err, count, gear, lane;
> +
> + if (!lpd || lpd > UFS_MAX_LANES) {
> + dev_err(dev, "Invalid lanes-per-direction value (%u) provided\n", lpd);
> + return;
> + }
> +
'lanes_per_direction' can be 0 for platforms that do not support deriving the
lanes count from DT:
https://lore.kernel.org/linux-scsi/20260520070009epcms2p6542f3abb7660839e9d8140b3f2f145c3@epcms2p6/
You should just drop the err message to avoid spamming those platforms.
- Mani
> + for (gear = UFS_HS_G1; gear <= UFS_HS_GEAR_MAX; gear++) {
> + snprintf(prop_name, MAX_PROP_SIZE, "txeq-preshoot-g%d", gear);
> + count = of_property_count_u32_elems(dev->of_node, prop_name);
> + if (count <= 0)
> + continue;
> +
> + if (count != sz) {
> + dev_err(dev, "Property %s has invalid count (%d), expecting %zu\n",
> + prop_name, count, sz);
> + continue;
> + }
> +
> + err = of_property_read_u32_array(dev->of_node, prop_name, preshoot, sz);
> + if (err) {
> + dev_err(dev, "Failed to read %s property, %d\n",
> + prop_name, err);
> + continue;
> + }
> +
> + for (i = 0; i < count; i++) {
> + if (preshoot[i] >= TX_HS_NUM_PRESHOOT) {
> + dev_err(dev, "An invalid TX EQ PreShoot (%d) provided in %s property\n",
> + preshoot[i], prop_name);
> + break;
> + }
> + }
> +
> + if (i != count)
> + continue;
> +
> + snprintf(prop_name, MAX_PROP_SIZE, "txeq-deemphasis-g%d", gear);
> + count = of_property_count_u32_elems(dev->of_node, prop_name);
> + if (count <= 0) {
> + dev_err(dev, "Missing required %s property\n", prop_name);
> + continue;
> + }
> +
> + if (count != sz) {
> + dev_err(dev, "Property %s has invalid count (%d), expecting %zu\n",
> + prop_name, count, sz);
> + continue;
> + }
> +
> + err = of_property_read_u32_array(dev->of_node, prop_name, deemphasis, sz);
> + if (err) {
> + dev_err(dev, "Failed to read %s property, %d\n",
> + prop_name, err);
> + continue;
> + }
> +
> + for (i = 0; i < count; i++) {
> + if (deemphasis[i] >= TX_HS_NUM_DEEMPHASIS) {
> + dev_err(dev, "An invalid TX EQ DeEmphasis (%d) provided in %s property\n",
> + deemphasis[i], prop_name);
> + break;
> + }
> + }
> +
> + if (i != count)
> + continue;
> +
> + memset(precode_en, 0, sizeof(precode_en));
> + if (gear == UFS_HS_G6) {
> + snprintf(prop_name, MAX_PROP_SIZE, "tx-precode-enable-g%d", gear);
> + count = of_property_count_u32_elems(dev->of_node, prop_name);
> + if (count > 0) {
> + if (count != sz) {
> + dev_err(dev, "Property %s has invalid count (%d), expecting %zu\n",
> + prop_name, count, sz);
> + continue;
> + }
> +
> + err = of_property_read_u32_array(dev->of_node, prop_name,
> + precode_en, sz);
> + if (err) {
> + dev_err(dev, "Failed to read %s property, %d\n",
> + prop_name, err);
> + continue;
> + }
> +
> + for (i = 0; i < count; i++) {
> + if (precode_en[i] > 1) {
> + dev_err(dev, "An invalid PrecodeEn (%d) provided in %s property\n",
> + precode_en[i], prop_name);
> + break;
> + }
> + }
> +
> + if (i != count)
> + continue;
> + }
> + }
> +
> + params = &hba->tx_eq_params[gear - 1];
> + for (lane = 0; lane < lpd; lane++) {
> + params->host[lane].preshoot = preshoot[lane * 2];
> + params->host[lane].deemphasis = deemphasis[lane * 2];
> + params->host[lane].precode_en = precode_en[lane * 2];
> +
> + params->device[lane].preshoot = preshoot[lane * 2 + 1];
> + params->device[lane].deemphasis = deemphasis[lane * 2 + 1];
> + params->device[lane].precode_en = precode_en[lane * 2 + 1];
> + }
> +
> + params->is_valid = true;
> + params->is_static = true;
> + }
> +}
> +
> /**
> * ufshcd_parse_clock_min_max_freq - Parse MIN and MAX clocks freq
> * @hba: per adapter instance
> @@ -528,6 +654,8 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
>
> ufshcd_init_lanes_per_dir(hba);
>
> + ufshcd_parse_static_tx_eq_settings(hba);
> +
> err = ufshcd_parse_operating_points(hba);
> if (err) {
> dev_err(dev, "%s: OPP parse failed %d\n", __func__, err);
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index f48d6416e299..c01824576472 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -359,6 +359,7 @@ struct ufshcd_tx_eqtr_record {
> * @is_valid: True if parameter contains valid TX Equalization settings
> * @is_applied: True if settings have been applied to UniPro of both sides
> * @is_trained: True if parameters obtained from TX EQTR procedure
> + * @is_static: True if settings are static
> */
> struct ufshcd_tx_eq_params {
> struct ufshcd_tx_eq_settings host[UFS_MAX_LANES];
> @@ -367,6 +368,7 @@ struct ufshcd_tx_eq_params {
> bool is_valid;
> bool is_applied;
> bool is_trained;
> + bool is_static;
> };
>
> /**
> --
> 2.34.1
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] dt-bindings: ufs: Document static TX Equalization settings properties
2026-05-27 14:40 ` [PATCH 1/2] dt-bindings: ufs: Document static TX Equalization settings properties Can Guo
@ 2026-05-28 6:18 ` Manivannan Sadhasivam
2026-05-30 11:36 ` Krzysztof Kozlowski
1 sibling, 0 replies; 15+ messages in thread
From: Manivannan Sadhasivam @ 2026-05-28 6:18 UTC (permalink / raw)
To: Can Guo
Cc: bvanassche, beanhuo, peter.wang, martin.petersen, linux-scsi,
Alim Akhtar, Avri Altman, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Zhaoming Luo, Ram Kumar Dwivedi,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On Wed, May 27, 2026 at 07:40:54AM -0700, Can Guo wrote:
> UFS v5.0/UFSHCI v5.0 add HS-G6 support (46.6 Gbps/lane) via UniPro v3.0
> and M-PHY v6.0. In these specs, TX Equalization is defined for all High
> Speed Gears (not only HS-G6) to compensate channel loss and improve signal
> integrity at high speed operation.
>
> For HS-G6, M-PHY uses PAM4 1b1b line coding, Pre-Coding may also be
> required depending on channel characteristics.
>
> Add vendor-neutral DT properties:
>
> - patternProperties for txeq-preshoot-g[1-6] and txeq-deemphasis-g[1-6]
> - fixed property tx-precode-enable-g6
>
> Each property is a uint32 array of per-lane tuples:
> <Host_Lane0 Device_Lane0>, [<Host_Lane1 Device_Lane1>]
>
> Accept 2 or 4 values (x1/x2 lane configs). PreShoot and DeEmphasis values
> are 0..7. Precode enable values are 0/1 and only applicable to HS-G6.
>
> Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
Acked-by: Manivannan Sadhasivam <mani@kernel.org>
- Mani
> ---
> .../devicetree/bindings/ufs/ufs-common.yaml | 45 +++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/ufs/ufs-common.yaml b/Documentation/devicetree/bindings/ufs/ufs-common.yaml
> index ed97f5682509..d90cf25adfa5 100644
> --- a/Documentation/devicetree/bindings/ufs/ufs-common.yaml
> +++ b/Documentation/devicetree/bindings/ufs/ufs-common.yaml
> @@ -105,6 +105,51 @@ properties:
> Restricts the UFS controller to rate-a or rate-b for both TX and
> RX directions.
>
> + tx-precode-enable-g6:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + oneOf:
> + - minItems: 2
> + maxItems: 2
> + - minItems: 4
> + maxItems: 4
> + items:
> + enum: [0, 1]
> + description: |
> + Static TX Precode enable values for HS-G6 only.
> + Values are specified as per-lane tuples:
> + <Host_Lane0 Device_Lane0>, [<Host_Lane1 Device_Lane1>].
> +
> +patternProperties:
> + "^txeq-preshoot-g[1-6]$":
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + oneOf:
> + - minItems: 2
> + maxItems: 2
> + - minItems: 4
> + maxItems: 4
> + items:
> + minimum: 0
> + maximum: 7
> + description: |
> + Static TX Equalization PreShoot values for High Speed Gears.
> + Values are specified as per-lane tuples:
> + <Host_Lane0 Device_Lane0>, [<Host_Lane1 Device_Lane1>].
> +
> + "^txeq-deemphasis-g[1-6]$":
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + oneOf:
> + - minItems: 2
> + maxItems: 2
> + - minItems: 4
> + maxItems: 4
> + items:
> + minimum: 0
> + maximum: 7
> + description: |
> + Static TX Equalization DeEmphasis values for High Speed Gears.
> + Values are specified as per-lane tuples:
> + <Host_Lane0 Device_Lane0>, [<Host_Lane1 Device_Lane1>].
> +
> dependencies:
> freq-table-hz: [ clocks ]
> operating-points-v2: [ clocks, clock-names ]
> --
> 2.34.1
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] scsi: ufs: core: Add support for static TX Equalization settings
2026-05-28 6:13 ` Manivannan Sadhasivam
@ 2026-05-28 7:24 ` Can Guo
2026-05-28 8:16 ` Manivannan Sadhasivam
0 siblings, 1 reply; 15+ messages in thread
From: Can Guo @ 2026-05-28 7:24 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: bvanassche, beanhuo, peter.wang, martin.petersen, linux-scsi,
Alim Akhtar, Avri Altman, James E.J. Bottomley,
Ram Kumar Dwivedi, Nitin Rawat, open list
On 5/28/2026 2:13 PM, Manivannan Sadhasivam wrote:
> On Wed, May 27, 2026 at 07:40:55AM -0700, Can Guo wrote:
>> Static TX Equalization settings and TX Precode enable indication from DT
>> properties txeq-preshoot-g[1-6], txeq-deemphasis-g[1-6], and
>> tx-precode-enable-g6 are board-specific baseline values. Values are
>> provided as per-lane tuples:
>>
>> <Host_Lane0 Device_Lane0>, [<Host_Lane1 Device_Lane1>]
>>
>> Parse DT u32 properties with explicit range checks by using
>> of_property_count_u32_elems()/of_property_read_u32_array().
>>
>> When adaptive TX Equalization is used, these static settings are not final:
>>
>> - If valid settings are retrieved from qTxEQGnSettings/wTxEQGnSettingsExt,
>> those retrieved settings override static DT settings.
>> - If retrieval is not available/valid, TX EQTR runs and trained settings
>> override static DT settings.
>>
>> So static DT settings are a fallback and are intended for cases where
>> adaptive TX Equalization is not enabled/used. Adaptive TX Equalization
>> remains the primary path when enabled.
>>
>> No behavior changes for platforms that do not provide these properties.
>>
>> Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
>> ---
>> drivers/ufs/core/ufs-txeq.c | 4 +-
>> drivers/ufs/host/ufshcd-pltfrm.c | 128 +++++++++++++++++++++++++++++++
>> include/ufs/ufshcd.h | 2 +
>> 3 files changed, 133 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
>> index 4b264adfdf49..634ec039e129 100644
>> --- a/drivers/ufs/core/ufs-txeq.c
>> +++ b/drivers/ufs/core/ufs-txeq.c
>> @@ -1297,7 +1297,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
>> }
>>
>> params = &hba->tx_eq_params[gear - 1];
>> - if (!params->is_valid || force_tx_eqtr) {
>> + if (!params->is_valid || params->is_static || force_tx_eqtr) {
>> int ret;
>>
>> ret = ufshcd_tx_eqtr(hba, params, pwr_mode);
>> @@ -1310,6 +1310,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
>> /* Mark TX Equalization settings as valid */
>> params->is_valid = true;
>> params->is_trained = true;
>> + params->is_static = false;
>> params->is_applied = false;
>> }
>>
>> @@ -1495,6 +1496,7 @@ static void ufshcd_extract_tx_eq_settings_attrs(struct ufs_hba *hba, u8 gear)
>> }
>>
>> params->is_valid = true;
>> + params->is_static = false;
> Maybe it's me, but I'm not able to understand how you want to apply these static
> EQ settings. In commit message you said, the static values should be used as a
> fallback, but you just check for 'params->is_static' while triggering
> ufshcd_tx_eqtr() which is supposed to perform adaptive TX EQ training. IMO, you
> don't need any check at all for applying static setting. If '(!params->is_valid
> || force_tx_eqtr)' condition is not satisfied, then the static setting should be
> used.
Thanks for the review.
The distinction is between two different sources that can pre-populate
txeq_params with
is_valid set to true before ufshcd_config_tx_eq_settings() is called:
1. DT properties — parsed by ufshcd_pltfrm_parse_tx_eq_settings(),
sets is_valid = true, is_static = true.
2. UFS Attributes (qTxEQGnSettings/wTxEQGnSettingsExt) — retrieved by
ufshcd_retrieve_tx_eq_settings() (introduced in the 2nd series),
sets is_valid = true, is_static = false.
Since both sources set is_valid = true, the is_valid flag alone cannot
tell them apart.
The is_static flag is the discriminator:
- is_valid && is_static -> settings came from DT; they are a board-level
baseline.
TX EQTR should still run to find optimal settings, which will then
overwrite the static ones.
- is_valid && !is_static -> settings came from UFS Attributes; they are
previously trained
known-good values. TX EQTR can be skipped.
- !is_valid -> no settings available yet; TX EQTR must run.
So the condition check on (!params->is_valid || params->is_static ||
force_tx_eqtr) handles
all three cases.
Thanks,
Can Guo.
>
> - Mani
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] scsi: ufs: core: Add support for static TX Equalization settings
2026-05-28 6:18 ` Manivannan Sadhasivam
@ 2026-05-28 7:27 ` Can Guo
0 siblings, 0 replies; 15+ messages in thread
From: Can Guo @ 2026-05-28 7:27 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: bvanassche, beanhuo, peter.wang, martin.petersen, linux-scsi,
Alim Akhtar, Avri Altman, James E.J. Bottomley,
Ram Kumar Dwivedi, Nitin Rawat, open list
On 5/28/2026 2:18 PM, Manivannan Sadhasivam wrote:
> On Wed, May 27, 2026 at 07:40:55AM -0700, Can Guo wrote:
>> Static TX Equalization settings and TX Precode enable indication from DT
>> properties txeq-preshoot-g[1-6], txeq-deemphasis-g[1-6], and
>> tx-precode-enable-g6 are board-specific baseline values. Values are
>> provided as per-lane tuples:
>>
>> <Host_Lane0 Device_Lane0>, [<Host_Lane1 Device_Lane1>]
>>
>> Parse DT u32 properties with explicit range checks by using
>> of_property_count_u32_elems()/of_property_read_u32_array().
>>
>> When adaptive TX Equalization is used, these static settings are not final:
>>
>> - If valid settings are retrieved from qTxEQGnSettings/wTxEQGnSettingsExt,
>> those retrieved settings override static DT settings.
>> - If retrieval is not available/valid, TX EQTR runs and trained settings
>> override static DT settings.
>>
>> So static DT settings are a fallback and are intended for cases where
>> adaptive TX Equalization is not enabled/used. Adaptive TX Equalization
>> remains the primary path when enabled.
>>
>> No behavior changes for platforms that do not provide these properties.
>>
>> Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
>> ---
>> drivers/ufs/core/ufs-txeq.c | 4 +-
>> drivers/ufs/host/ufshcd-pltfrm.c | 128 +++++++++++++++++++++++++++++++
>> include/ufs/ufshcd.h | 2 +
>> 3 files changed, 133 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
>> index 4b264adfdf49..634ec039e129 100644
>> --- a/drivers/ufs/core/ufs-txeq.c
>> +++ b/drivers/ufs/core/ufs-txeq.c
>> @@ -1297,7 +1297,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
>> }
>>
>> params = &hba->tx_eq_params[gear - 1];
>> - if (!params->is_valid || force_tx_eqtr) {
>> + if (!params->is_valid || params->is_static || force_tx_eqtr) {
>> int ret;
>>
>> ret = ufshcd_tx_eqtr(hba, params, pwr_mode);
>> @@ -1310,6 +1310,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
>> /* Mark TX Equalization settings as valid */
>> params->is_valid = true;
>> params->is_trained = true;
>> + params->is_static = false;
>> params->is_applied = false;
>> }
>>
>> @@ -1495,6 +1496,7 @@ static void ufshcd_extract_tx_eq_settings_attrs(struct ufs_hba *hba, u8 gear)
>> }
>>
>> params->is_valid = true;
>> + params->is_static = false;
>> }
>>
>> void ufshcd_retrieve_tx_eq_settings(struct ufs_hba *hba)
>> diff --git a/drivers/ufs/host/ufshcd-pltfrm.c b/drivers/ufs/host/ufshcd-pltfrm.c
>> index c2dafb583cf5..2db2103a6ac0 100644
>> --- a/drivers/ufs/host/ufshcd-pltfrm.c
>> +++ b/drivers/ufs/host/ufshcd-pltfrm.c
>> @@ -210,6 +210,132 @@ static void ufshcd_init_lanes_per_dir(struct ufs_hba *hba)
>> }
>> }
>>
>> +static void ufshcd_parse_static_tx_eq_settings(struct ufs_hba *hba)
>> +{
>> + size_t sz = hba->lanes_per_direction * 2;
>> + u32 lpd = hba->lanes_per_direction;
>> + struct ufshcd_tx_eq_params *params;
>> + u32 deemphasis[UFS_MAX_LANES * 2];
>> + u32 precode_en[UFS_MAX_LANES * 2];
>> + u32 preshoot[UFS_MAX_LANES * 2];
>> + struct device *dev = hba->dev;
>> + char prop_name[MAX_PROP_SIZE];
>> + int i, err, count, gear, lane;
>> +
>> + if (!lpd || lpd > UFS_MAX_LANES) {
>> + dev_err(dev, "Invalid lanes-per-direction value (%u) provided\n", lpd);
>> + return;
>> + }
>> +
> 'lanes_per_direction' can be 0 for platforms that do not support deriving the
> lanes count from DT:
>
> https://lore.kernel.org/linux-scsi/20260520070009epcms2p6542f3abb7660839e9d8140b3f2f145c3@epcms2p6/
>
> You should just drop the err message to avoid spamming those platforms.
Good catch! Will do in next version.
Thanks,
Can Guo.
>
> - Mani
>
>> + for (gear = UFS_HS_G1; gear <= UFS_HS_GEAR_MAX; gear++) {
>> + snprintf(prop_name, MAX_PROP_SIZE, "txeq-preshoot-g%d", gear);
>> + count = of_property_count_u32_elems(dev->of_node, prop_name);
>> + if (count <= 0)
>> + continue;
>> +
>> + if (count != sz) {
>> + dev_err(dev, "Property %s has invalid count (%d), expecting %zu\n",
>> + prop_name, count, sz);
>> + continue;
>> + }
>> +
>> + err = of_property_read_u32_array(dev->of_node, prop_name, preshoot, sz);
>> + if (err) {
>> + dev_err(dev, "Failed to read %s property, %d\n",
>> + prop_name, err);
>> + continue;
>> + }
>> +
>> + for (i = 0; i < count; i++) {
>> + if (preshoot[i] >= TX_HS_NUM_PRESHOOT) {
>> + dev_err(dev, "An invalid TX EQ PreShoot (%d) provided in %s property\n",
>> + preshoot[i], prop_name);
>> + break;
>> + }
>> + }
>> +
>> + if (i != count)
>> + continue;
>> +
>> + snprintf(prop_name, MAX_PROP_SIZE, "txeq-deemphasis-g%d", gear);
>> + count = of_property_count_u32_elems(dev->of_node, prop_name);
>> + if (count <= 0) {
>> + dev_err(dev, "Missing required %s property\n", prop_name);
>> + continue;
>> + }
>> +
>> + if (count != sz) {
>> + dev_err(dev, "Property %s has invalid count (%d), expecting %zu\n",
>> + prop_name, count, sz);
>> + continue;
>> + }
>> +
>> + err = of_property_read_u32_array(dev->of_node, prop_name, deemphasis, sz);
>> + if (err) {
>> + dev_err(dev, "Failed to read %s property, %d\n",
>> + prop_name, err);
>> + continue;
>> + }
>> +
>> + for (i = 0; i < count; i++) {
>> + if (deemphasis[i] >= TX_HS_NUM_DEEMPHASIS) {
>> + dev_err(dev, "An invalid TX EQ DeEmphasis (%d) provided in %s property\n",
>> + deemphasis[i], prop_name);
>> + break;
>> + }
>> + }
>> +
>> + if (i != count)
>> + continue;
>> +
>> + memset(precode_en, 0, sizeof(precode_en));
>> + if (gear == UFS_HS_G6) {
>> + snprintf(prop_name, MAX_PROP_SIZE, "tx-precode-enable-g%d", gear);
>> + count = of_property_count_u32_elems(dev->of_node, prop_name);
>> + if (count > 0) {
>> + if (count != sz) {
>> + dev_err(dev, "Property %s has invalid count (%d), expecting %zu\n",
>> + prop_name, count, sz);
>> + continue;
>> + }
>> +
>> + err = of_property_read_u32_array(dev->of_node, prop_name,
>> + precode_en, sz);
>> + if (err) {
>> + dev_err(dev, "Failed to read %s property, %d\n",
>> + prop_name, err);
>> + continue;
>> + }
>> +
>> + for (i = 0; i < count; i++) {
>> + if (precode_en[i] > 1) {
>> + dev_err(dev, "An invalid PrecodeEn (%d) provided in %s property\n",
>> + precode_en[i], prop_name);
>> + break;
>> + }
>> + }
>> +
>> + if (i != count)
>> + continue;
>> + }
>> + }
>> +
>> + params = &hba->tx_eq_params[gear - 1];
>> + for (lane = 0; lane < lpd; lane++) {
>> + params->host[lane].preshoot = preshoot[lane * 2];
>> + params->host[lane].deemphasis = deemphasis[lane * 2];
>> + params->host[lane].precode_en = precode_en[lane * 2];
>> +
>> + params->device[lane].preshoot = preshoot[lane * 2 + 1];
>> + params->device[lane].deemphasis = deemphasis[lane * 2 + 1];
>> + params->device[lane].precode_en = precode_en[lane * 2 + 1];
>> + }
>> +
>> + params->is_valid = true;
>> + params->is_static = true;
>> + }
>> +}
>> +
>> /**
>> * ufshcd_parse_clock_min_max_freq - Parse MIN and MAX clocks freq
>> * @hba: per adapter instance
>> @@ -528,6 +654,8 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
>>
>> ufshcd_init_lanes_per_dir(hba);
>>
>> + ufshcd_parse_static_tx_eq_settings(hba);
>> +
>> err = ufshcd_parse_operating_points(hba);
>> if (err) {
>> dev_err(dev, "%s: OPP parse failed %d\n", __func__, err);
>> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
>> index f48d6416e299..c01824576472 100644
>> --- a/include/ufs/ufshcd.h
>> +++ b/include/ufs/ufshcd.h
>> @@ -359,6 +359,7 @@ struct ufshcd_tx_eqtr_record {
>> * @is_valid: True if parameter contains valid TX Equalization settings
>> * @is_applied: True if settings have been applied to UniPro of both sides
>> * @is_trained: True if parameters obtained from TX EQTR procedure
>> + * @is_static: True if settings are static
>> */
>> struct ufshcd_tx_eq_params {
>> struct ufshcd_tx_eq_settings host[UFS_MAX_LANES];
>> @@ -367,6 +368,7 @@ struct ufshcd_tx_eq_params {
>> bool is_valid;
>> bool is_applied;
>> bool is_trained;
>> + bool is_static;
>> };
>>
>> /**
>> --
>> 2.34.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] scsi: ufs: core: Add support for static TX Equalization settings
2026-05-28 7:24 ` Can Guo
@ 2026-05-28 8:16 ` Manivannan Sadhasivam
2026-05-28 8:40 ` Can Guo
0 siblings, 1 reply; 15+ messages in thread
From: Manivannan Sadhasivam @ 2026-05-28 8:16 UTC (permalink / raw)
To: Can Guo
Cc: bvanassche, beanhuo, peter.wang, martin.petersen, linux-scsi,
Alim Akhtar, Avri Altman, James E.J. Bottomley,
Ram Kumar Dwivedi, Nitin Rawat, open list
On Thu, May 28, 2026 at 03:24:37PM +0800, Can Guo wrote:
>
>
> On 5/28/2026 2:13 PM, Manivannan Sadhasivam wrote:
> > On Wed, May 27, 2026 at 07:40:55AM -0700, Can Guo wrote:
> > > Static TX Equalization settings and TX Precode enable indication from DT
> > > properties txeq-preshoot-g[1-6], txeq-deemphasis-g[1-6], and
> > > tx-precode-enable-g6 are board-specific baseline values. Values are
> > > provided as per-lane tuples:
> > >
> > > <Host_Lane0 Device_Lane0>, [<Host_Lane1 Device_Lane1>]
> > >
> > > Parse DT u32 properties with explicit range checks by using
> > > of_property_count_u32_elems()/of_property_read_u32_array().
> > >
> > > When adaptive TX Equalization is used, these static settings are not final:
> > >
> > > - If valid settings are retrieved from qTxEQGnSettings/wTxEQGnSettingsExt,
> > > those retrieved settings override static DT settings.
> > > - If retrieval is not available/valid, TX EQTR runs and trained settings
> > > override static DT settings.
> > >
> > > So static DT settings are a fallback and are intended for cases where
> > > adaptive TX Equalization is not enabled/used. Adaptive TX Equalization
> > > remains the primary path when enabled.
> > >
> > > No behavior changes for platforms that do not provide these properties.
> > >
> > > Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
> > > ---
> > > drivers/ufs/core/ufs-txeq.c | 4 +-
> > > drivers/ufs/host/ufshcd-pltfrm.c | 128 +++++++++++++++++++++++++++++++
> > > include/ufs/ufshcd.h | 2 +
> > > 3 files changed, 133 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
> > > index 4b264adfdf49..634ec039e129 100644
> > > --- a/drivers/ufs/core/ufs-txeq.c
> > > +++ b/drivers/ufs/core/ufs-txeq.c
> > > @@ -1297,7 +1297,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
> > > }
> > > params = &hba->tx_eq_params[gear - 1];
> > > - if (!params->is_valid || force_tx_eqtr) {
> > > + if (!params->is_valid || params->is_static || force_tx_eqtr) {
> > > int ret;
> > > ret = ufshcd_tx_eqtr(hba, params, pwr_mode);
> > > @@ -1310,6 +1310,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
> > > /* Mark TX Equalization settings as valid */
> > > params->is_valid = true;
> > > params->is_trained = true;
> > > + params->is_static = false;
> > > params->is_applied = false;
> > > }
> > > @@ -1495,6 +1496,7 @@ static void ufshcd_extract_tx_eq_settings_attrs(struct ufs_hba *hba, u8 gear)
> > > }
> > > params->is_valid = true;
> > > + params->is_static = false;
> > Maybe it's me, but I'm not able to understand how you want to apply these static
> > EQ settings. In commit message you said, the static values should be used as a
> > fallback, but you just check for 'params->is_static' while triggering
> > ufshcd_tx_eqtr() which is supposed to perform adaptive TX EQ training. IMO, you
> > don't need any check at all for applying static setting. If '(!params->is_valid
> > || force_tx_eqtr)' condition is not satisfied, then the static setting should be
> > used.
> Thanks for the review.
>
> The distinction is between two different sources that can pre-populate
> txeq_params with
> is_valid set to true before ufshcd_config_tx_eq_settings() is called:
>
> 1. DT properties — parsed by ufshcd_pltfrm_parse_tx_eq_settings(),
> sets is_valid = true, is_static = true.
> 2. UFS Attributes (qTxEQGnSettings/wTxEQGnSettingsExt) — retrieved by
> ufshcd_retrieve_tx_eq_settings() (introduced in the 2nd series),
> sets is_valid = true, is_static = false.
>
> Since both sources set is_valid = true, the is_valid flag alone cannot tell
> them apart.
> The is_static flag is the discriminator:
>
> - is_valid && is_static -> settings came from DT; they are a board-level
> baseline.
> TX EQTR should still run to find optimal settings, which will then
> overwrite the static ones.
> - is_valid && !is_static -> settings came from UFS Attributes; they are
> previously trained
You use '&&' here, but '||' in the code. When you use '||', then I see no point
for 'is_static' check.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] scsi: ufs: core: Add support for static TX Equalization settings
2026-05-28 8:16 ` Manivannan Sadhasivam
@ 2026-05-28 8:40 ` Can Guo
2026-05-28 9:29 ` Manivannan Sadhasivam
0 siblings, 1 reply; 15+ messages in thread
From: Can Guo @ 2026-05-28 8:40 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: bvanassche, beanhuo, peter.wang, martin.petersen, linux-scsi,
Alim Akhtar, Avri Altman, James E.J. Bottomley,
Ram Kumar Dwivedi, Nitin Rawat, open list
On 5/28/2026 4:16 PM, Manivannan Sadhasivam wrote:
> On Thu, May 28, 2026 at 03:24:37PM +0800, Can Guo wrote:
>>
>> On 5/28/2026 2:13 PM, Manivannan Sadhasivam wrote:
>>> On Wed, May 27, 2026 at 07:40:55AM -0700, Can Guo wrote:
>>>> Static TX Equalization settings and TX Precode enable indication from DT
>>>> properties txeq-preshoot-g[1-6], txeq-deemphasis-g[1-6], and
>>>> tx-precode-enable-g6 are board-specific baseline values. Values are
>>>> provided as per-lane tuples:
>>>>
>>>> <Host_Lane0 Device_Lane0>, [<Host_Lane1 Device_Lane1>]
>>>>
>>>> Parse DT u32 properties with explicit range checks by using
>>>> of_property_count_u32_elems()/of_property_read_u32_array().
>>>>
>>>> When adaptive TX Equalization is used, these static settings are not final:
>>>>
>>>> - If valid settings are retrieved from qTxEQGnSettings/wTxEQGnSettingsExt,
>>>> those retrieved settings override static DT settings.
>>>> - If retrieval is not available/valid, TX EQTR runs and trained settings
>>>> override static DT settings.
>>>>
>>>> So static DT settings are a fallback and are intended for cases where
>>>> adaptive TX Equalization is not enabled/used. Adaptive TX Equalization
>>>> remains the primary path when enabled.
>>>>
>>>> No behavior changes for platforms that do not provide these properties.
>>>>
>>>> Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
>>>> ---
>>>> drivers/ufs/core/ufs-txeq.c | 4 +-
>>>> drivers/ufs/host/ufshcd-pltfrm.c | 128 +++++++++++++++++++++++++++++++
>>>> include/ufs/ufshcd.h | 2 +
>>>> 3 files changed, 133 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
>>>> index 4b264adfdf49..634ec039e129 100644
>>>> --- a/drivers/ufs/core/ufs-txeq.c
>>>> +++ b/drivers/ufs/core/ufs-txeq.c
>>>> @@ -1297,7 +1297,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
>>>> }
>>>> params = &hba->tx_eq_params[gear - 1];
>>>> - if (!params->is_valid || force_tx_eqtr) {
>>>> + if (!params->is_valid || params->is_static || force_tx_eqtr) {
>>>> int ret;
>>>> ret = ufshcd_tx_eqtr(hba, params, pwr_mode);
>>>> @@ -1310,6 +1310,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
>>>> /* Mark TX Equalization settings as valid */
>>>> params->is_valid = true;
>>>> params->is_trained = true;
>>>> + params->is_static = false;
>>>> params->is_applied = false;
>>>> }
>>>> @@ -1495,6 +1496,7 @@ static void ufshcd_extract_tx_eq_settings_attrs(struct ufs_hba *hba, u8 gear)
>>>> }
>>>> params->is_valid = true;
>>>> + params->is_static = false;
>>> Maybe it's me, but I'm not able to understand how you want to apply these static
>>> EQ settings. In commit message you said, the static values should be used as a
>>> fallback, but you just check for 'params->is_static' while triggering
>>> ufshcd_tx_eqtr() which is supposed to perform adaptive TX EQ training. IMO, you
>>> don't need any check at all for applying static setting. If '(!params->is_valid
>>> || force_tx_eqtr)' condition is not satisfied, then the static setting should be
>>> used.
>> Thanks for the review.
>>
>> The distinction is between two different sources that can pre-populate
>> txeq_params with
>> is_valid set to true before ufshcd_config_tx_eq_settings() is called:
>>
>> 1. DT properties — parsed by ufshcd_pltfrm_parse_tx_eq_settings(),
>> sets is_valid = true, is_static = true.
>> 2. UFS Attributes (qTxEQGnSettings/wTxEQGnSettingsExt) — retrieved by
>> ufshcd_retrieve_tx_eq_settings() (introduced in the 2nd series),
>> sets is_valid = true, is_static = false.
>>
>> Since both sources set is_valid = true, the is_valid flag alone cannot tell
>> them apart.
>> The is_static flag is the discriminator:
>>
>> - is_valid && is_static -> settings came from DT; they are a board-level
>> baseline.
>> TX EQTR should still run to find optimal settings, which will then
>> overwrite the static ones.
>> - is_valid && !is_static -> settings came from UFS Attributes; they are
>> previously trained
> You use '&&' here, but '||' in the code. When you use '||', then I see no point
> for 'is_static' check.
The code is correct. My reply was explaining why the check is there, but not
explaining the check itself.
Original check in the code is (!params->is_valid || force_tx_eqtr).
Static TX EQ settings are valid, so '!params->is_valid' is false, TX
EQTR would be skipped.
Update the check as (!params->is_valid || params->is_static ||
force_tx_eqtr) so TX EQTR
must run when static settings are provided.
Thanks,
Can Guo.
>
> - Mani
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] scsi: ufs: core: Add support for static TX Equalization settings
2026-05-28 8:40 ` Can Guo
@ 2026-05-28 9:29 ` Manivannan Sadhasivam
2026-05-28 9:31 ` Can Guo
0 siblings, 1 reply; 15+ messages in thread
From: Manivannan Sadhasivam @ 2026-05-28 9:29 UTC (permalink / raw)
To: Can Guo
Cc: bvanassche, beanhuo, peter.wang, martin.petersen, linux-scsi,
Alim Akhtar, Avri Altman, James E.J. Bottomley,
Ram Kumar Dwivedi, Nitin Rawat, open list
On Thu, May 28, 2026 at 04:40:59PM +0800, Can Guo wrote:
>
>
> On 5/28/2026 4:16 PM, Manivannan Sadhasivam wrote:
> > On Thu, May 28, 2026 at 03:24:37PM +0800, Can Guo wrote:
> > >
> > > On 5/28/2026 2:13 PM, Manivannan Sadhasivam wrote:
> > > > On Wed, May 27, 2026 at 07:40:55AM -0700, Can Guo wrote:
> > > > > Static TX Equalization settings and TX Precode enable indication from DT
> > > > > properties txeq-preshoot-g[1-6], txeq-deemphasis-g[1-6], and
> > > > > tx-precode-enable-g6 are board-specific baseline values. Values are
> > > > > provided as per-lane tuples:
> > > > >
> > > > > <Host_Lane0 Device_Lane0>, [<Host_Lane1 Device_Lane1>]
> > > > >
> > > > > Parse DT u32 properties with explicit range checks by using
> > > > > of_property_count_u32_elems()/of_property_read_u32_array().
> > > > >
> > > > > When adaptive TX Equalization is used, these static settings are not final:
> > > > >
> > > > > - If valid settings are retrieved from qTxEQGnSettings/wTxEQGnSettingsExt,
> > > > > those retrieved settings override static DT settings.
> > > > > - If retrieval is not available/valid, TX EQTR runs and trained settings
> > > > > override static DT settings.
> > > > >
> > > > > So static DT settings are a fallback and are intended for cases where
> > > > > adaptive TX Equalization is not enabled/used. Adaptive TX Equalization
> > > > > remains the primary path when enabled.
> > > > >
> > > > > No behavior changes for platforms that do not provide these properties.
> > > > >
> > > > > Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
> > > > > ---
> > > > > drivers/ufs/core/ufs-txeq.c | 4 +-
> > > > > drivers/ufs/host/ufshcd-pltfrm.c | 128 +++++++++++++++++++++++++++++++
> > > > > include/ufs/ufshcd.h | 2 +
> > > > > 3 files changed, 133 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
> > > > > index 4b264adfdf49..634ec039e129 100644
> > > > > --- a/drivers/ufs/core/ufs-txeq.c
> > > > > +++ b/drivers/ufs/core/ufs-txeq.c
> > > > > @@ -1297,7 +1297,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
> > > > > }
> > > > > params = &hba->tx_eq_params[gear - 1];
> > > > > - if (!params->is_valid || force_tx_eqtr) {
> > > > > + if (!params->is_valid || params->is_static || force_tx_eqtr) {
> > > > > int ret;
> > > > > ret = ufshcd_tx_eqtr(hba, params, pwr_mode);
> > > > > @@ -1310,6 +1310,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
> > > > > /* Mark TX Equalization settings as valid */
> > > > > params->is_valid = true;
> > > > > params->is_trained = true;
> > > > > + params->is_static = false;
> > > > > params->is_applied = false;
> > > > > }
> > > > > @@ -1495,6 +1496,7 @@ static void ufshcd_extract_tx_eq_settings_attrs(struct ufs_hba *hba, u8 gear)
> > > > > }
> > > > > params->is_valid = true;
> > > > > + params->is_static = false;
> > > > Maybe it's me, but I'm not able to understand how you want to apply these static
> > > > EQ settings. In commit message you said, the static values should be used as a
> > > > fallback, but you just check for 'params->is_static' while triggering
> > > > ufshcd_tx_eqtr() which is supposed to perform adaptive TX EQ training. IMO, you
> > > > don't need any check at all for applying static setting. If '(!params->is_valid
> > > > || force_tx_eqtr)' condition is not satisfied, then the static setting should be
> > > > used.
> > > Thanks for the review.
> > >
> > > The distinction is between two different sources that can pre-populate
> > > txeq_params with
> > > is_valid set to true before ufshcd_config_tx_eq_settings() is called:
> > >
> > > 1. DT properties — parsed by ufshcd_pltfrm_parse_tx_eq_settings(),
> > > sets is_valid = true, is_static = true.
> > > 2. UFS Attributes (qTxEQGnSettings/wTxEQGnSettingsExt) — retrieved by
> > > ufshcd_retrieve_tx_eq_settings() (introduced in the 2nd series),
> > > sets is_valid = true, is_static = false.
> > >
> > > Since both sources set is_valid = true, the is_valid flag alone cannot tell
> > > them apart.
> > > The is_static flag is the discriminator:
> > >
> > > - is_valid && is_static -> settings came from DT; they are a board-level
> > > baseline.
> > > TX EQTR should still run to find optimal settings, which will then
> > > overwrite the static ones.
> > > - is_valid && !is_static -> settings came from UFS Attributes; they are
> > > previously trained
> > You use '&&' here, but '||' in the code. When you use '||', then I see no point
> > for 'is_static' check.
> The code is correct. My reply was explaining why the check is there, but not
> explaining the check itself.
>
> Original check in the code is (!params->is_valid || force_tx_eqtr).
>
> Static TX EQ settings are valid, so '!params->is_valid' is false, TX EQTR
> would be skipped.
>
> Update the check as (!params->is_valid || params->is_static ||
> force_tx_eqtr) so TX EQTR
> must run when static settings are provided.
>
Ok. I still see having 'is_static' is a bit of overkill, but I don't see a
sensible way to check whether the static EQ settings are available or not.
But please add a comment above the check to make it clear.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] scsi: ufs: core: Add support for static TX Equalization settings
2026-05-28 9:29 ` Manivannan Sadhasivam
@ 2026-05-28 9:31 ` Can Guo
0 siblings, 0 replies; 15+ messages in thread
From: Can Guo @ 2026-05-28 9:31 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: bvanassche, beanhuo, peter.wang, martin.petersen, linux-scsi,
Alim Akhtar, Avri Altman, James E.J. Bottomley,
Ram Kumar Dwivedi, Nitin Rawat, open list
On 5/28/2026 5:29 PM, Manivannan Sadhasivam wrote:
> On Thu, May 28, 2026 at 04:40:59PM +0800, Can Guo wrote:
>>
>> On 5/28/2026 4:16 PM, Manivannan Sadhasivam wrote:
>>> On Thu, May 28, 2026 at 03:24:37PM +0800, Can Guo wrote:
>>>> On 5/28/2026 2:13 PM, Manivannan Sadhasivam wrote:
>>>>> On Wed, May 27, 2026 at 07:40:55AM -0700, Can Guo wrote:
>>>>>> Static TX Equalization settings and TX Precode enable indication from DT
>>>>>> properties txeq-preshoot-g[1-6], txeq-deemphasis-g[1-6], and
>>>>>> tx-precode-enable-g6 are board-specific baseline values. Values are
>>>>>> provided as per-lane tuples:
>>>>>>
>>>>>> <Host_Lane0 Device_Lane0>, [<Host_Lane1 Device_Lane1>]
>>>>>>
>>>>>> Parse DT u32 properties with explicit range checks by using
>>>>>> of_property_count_u32_elems()/of_property_read_u32_array().
>>>>>>
>>>>>> When adaptive TX Equalization is used, these static settings are not final:
>>>>>>
>>>>>> - If valid settings are retrieved from qTxEQGnSettings/wTxEQGnSettingsExt,
>>>>>> those retrieved settings override static DT settings.
>>>>>> - If retrieval is not available/valid, TX EQTR runs and trained settings
>>>>>> override static DT settings.
>>>>>>
>>>>>> So static DT settings are a fallback and are intended for cases where
>>>>>> adaptive TX Equalization is not enabled/used. Adaptive TX Equalization
>>>>>> remains the primary path when enabled.
>>>>>>
>>>>>> No behavior changes for platforms that do not provide these properties.
>>>>>>
>>>>>> Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
>>>>>> ---
>>>>>> drivers/ufs/core/ufs-txeq.c | 4 +-
>>>>>> drivers/ufs/host/ufshcd-pltfrm.c | 128 +++++++++++++++++++++++++++++++
>>>>>> include/ufs/ufshcd.h | 2 +
>>>>>> 3 files changed, 133 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
>>>>>> index 4b264adfdf49..634ec039e129 100644
>>>>>> --- a/drivers/ufs/core/ufs-txeq.c
>>>>>> +++ b/drivers/ufs/core/ufs-txeq.c
>>>>>> @@ -1297,7 +1297,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
>>>>>> }
>>>>>> params = &hba->tx_eq_params[gear - 1];
>>>>>> - if (!params->is_valid || force_tx_eqtr) {
>>>>>> + if (!params->is_valid || params->is_static || force_tx_eqtr) {
>>>>>> int ret;
>>>>>> ret = ufshcd_tx_eqtr(hba, params, pwr_mode);
>>>>>> @@ -1310,6 +1310,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
>>>>>> /* Mark TX Equalization settings as valid */
>>>>>> params->is_valid = true;
>>>>>> params->is_trained = true;
>>>>>> + params->is_static = false;
>>>>>> params->is_applied = false;
>>>>>> }
>>>>>> @@ -1495,6 +1496,7 @@ static void ufshcd_extract_tx_eq_settings_attrs(struct ufs_hba *hba, u8 gear)
>>>>>> }
>>>>>> params->is_valid = true;
>>>>>> + params->is_static = false;
>>>>> Maybe it's me, but I'm not able to understand how you want to apply these static
>>>>> EQ settings. In commit message you said, the static values should be used as a
>>>>> fallback, but you just check for 'params->is_static' while triggering
>>>>> ufshcd_tx_eqtr() which is supposed to perform adaptive TX EQ training. IMO, you
>>>>> don't need any check at all for applying static setting. If '(!params->is_valid
>>>>> || force_tx_eqtr)' condition is not satisfied, then the static setting should be
>>>>> used.
>>>> Thanks for the review.
>>>>
>>>> The distinction is between two different sources that can pre-populate
>>>> txeq_params with
>>>> is_valid set to true before ufshcd_config_tx_eq_settings() is called:
>>>>
>>>> 1. DT properties — parsed by ufshcd_pltfrm_parse_tx_eq_settings(),
>>>> sets is_valid = true, is_static = true.
>>>> 2. UFS Attributes (qTxEQGnSettings/wTxEQGnSettingsExt) — retrieved by
>>>> ufshcd_retrieve_tx_eq_settings() (introduced in the 2nd series),
>>>> sets is_valid = true, is_static = false.
>>>>
>>>> Since both sources set is_valid = true, the is_valid flag alone cannot tell
>>>> them apart.
>>>> The is_static flag is the discriminator:
>>>>
>>>> - is_valid && is_static -> settings came from DT; they are a board-level
>>>> baseline.
>>>> TX EQTR should still run to find optimal settings, which will then
>>>> overwrite the static ones.
>>>> - is_valid && !is_static -> settings came from UFS Attributes; they are
>>>> previously trained
>>> You use '&&' here, but '||' in the code. When you use '||', then I see no point
>>> for 'is_static' check.
>> The code is correct. My reply was explaining why the check is there, but not
>> explaining the check itself.
>>
>> Original check in the code is (!params->is_valid || force_tx_eqtr).
>>
>> Static TX EQ settings are valid, so '!params->is_valid' is false, TX EQTR
>> would be skipped.
>>
>> Update the check as (!params->is_valid || params->is_static ||
>> force_tx_eqtr) so TX EQTR
>> must run when static settings are provided.
>>
> Ok. I still see having 'is_static' is a bit of overkill, but I don't see a
> sensible way to check whether the static EQ settings are available or not.
>
> But please add a comment above the check to make it clear.
OK sir.
Thanks,
Can Guo.
>
> - Mani
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] dt-bindings: ufs: Document static TX Equalization settings properties
2026-05-27 14:40 ` [PATCH 1/2] dt-bindings: ufs: Document static TX Equalization settings properties Can Guo
2026-05-28 6:18 ` Manivannan Sadhasivam
@ 2026-05-30 11:36 ` Krzysztof Kozlowski
1 sibling, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2026-05-30 11:36 UTC (permalink / raw)
To: Can Guo
Cc: bvanassche, beanhuo, peter.wang, martin.petersen, mani,
linux-scsi, Alim Akhtar, Avri Altman, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Zhaoming Luo,
Ram Kumar Dwivedi,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On Wed, May 27, 2026 at 07:40:54AM -0700, Can Guo wrote:
> UFS v5.0/UFSHCI v5.0 add HS-G6 support (46.6 Gbps/lane) via UniPro v3.0
> and M-PHY v6.0. In these specs, TX Equalization is defined for all High
> Speed Gears (not only HS-G6) to compensate channel loss and improve signal
> integrity at high speed operation.
>
> For HS-G6, M-PHY uses PAM4 1b1b line coding, Pre-Coding may also be
> required depending on channel characteristics.
>
> Add vendor-neutral DT properties:
>
> - patternProperties for txeq-preshoot-g[1-6] and txeq-deemphasis-g[1-6]
> - fixed property tx-precode-enable-g6
>
> Each property is a uint32 array of per-lane tuples:
> <Host_Lane0 Device_Lane0>, [<Host_Lane1 Device_Lane1>]
>
> Accept 2 or 4 values (x1/x2 lane configs). PreShoot and DeEmphasis values
> are 0..7. Precode enable values are 0/1 and only applicable to HS-G6.
>
Why are they SIX versions within three days? I see this for the third
time in Patchwork already.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] scsi: ufs: core: Add support for static TX Equalization settings
2026-05-14 13:57 ` Bean Huo
@ 2026-05-15 8:12 ` Can Guo
0 siblings, 0 replies; 15+ messages in thread
From: Can Guo @ 2026-05-15 8:12 UTC (permalink / raw)
To: Bean Huo, bvanassche, beanhuo, peter.wang, martin.petersen, mani,
powenkao
Cc: linux-scsi, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Ram Kumar Dwivedi, Nitin Rawat, open list
Hi Bean,
On 5/14/2026 9:57 PM, Bean Huo wrote:
> Can,
>
>
> Sorry for the late review of this patch. I have several questions:
Not at all sir.
>
>
> On Fri, 2026-05-01 at 06:44 -0700, Can Guo wrote:
>> @@ -1297,7 +1297,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
>> }
>>
>> params = &hba->tx_eq_params[gear - 1];
>> - if (!params->is_valid || force_tx_eqtr) {
>> + if (!params->is_valid || params->is_static || force_tx_eqtr) {
> When use_adaptive_txeq is on and params->is_static is true, EQTR will overwrite
> the static values. That is reasonable since EQTR is more accurate. Is the
> is_static check really needed here?
The check of flag is_static is required because we need it to
differentiate the static
(DTS) settings from the settings retrieved from persistent storage:
- If settings are retrieved from persistent storage, TX EQTR should be
skipped.
- If settings are from DTS, TX EQTR should execute anyways to override
static settings.
>
>> int ret;
>>
>>
> ...
>
>>
>> void ufshcd_retrieve_tx_eq_settings(struct ufs_hba *hba)
>> diff --git a/drivers/ufs/host/ufshcd-pltfrm.c b/drivers/ufs/host/ufshcd-
>> pltfrm.c
>> index c2dafb583cf5..de6302e8c067 100644
>> --- a/drivers/ufs/host/ufshcd-pltfrm.c
>> +++ b/drivers/ufs/host/ufshcd-pltfrm.c
>> @@ -210,6 +210,86 @@ static void ufshcd_init_lanes_per_dir(struct ufs_hba
>> *hba)
>> }
>> }
>>
>> +static void ufshcd_parse_static_tx_eq_settings(struct ufs_hba *hba)
>> +{
>> + size_t sz = hba->lanes_per_direction * 2 * TX_EQ_SETTINGS_TUPLE_SZ;
>> + u32 settings[UFS_MAX_LANES * 2 * TX_EQ_SETTINGS_TUPLE_SZ];
>> + u32 *host_settings, *device_settings;
>> + u32 lpd = hba->lanes_per_direction;
>> + struct ufshcd_tx_eq_params *params;
>>
> ....
>> +
>> + params = &hba->tx_eq_params[gear - 1];
>> + host_settings = settings;
>> + device_settings = settings + lpd * TX_EQ_SETTINGS_TUPLE_SZ;
>> +
>> + for (lane = 0; lane < lpd; lane++) {
>> + params->host[lane].preshoot = host_settings[0];
>> + params->host[lane].deemphasis = host_settings[1];
>> + params->host[lane].precode_en = host_settings[2];
>> + host_settings += TX_EQ_SETTINGS_TUPLE_SZ;
>> +
>> + params->device[lane].preshoot = device_settings[0];
>> + params->device[lane].deemphasis = device_settings[1];
>> + params->device[lane].precode_en = device_settings[2];
>> + device_settings += TX_EQ_SETTINGS_TUPLE_SZ;
>> + }
>> +
>> + params->is_valid = true;
>> + params->is_static = true;
> I want to confirm I understand the code correctly. Please tell me if I am wrong:
>
> 1, When use_adaptive_txeq = 0: static values are used directly as TX EQ for HS-
> G4 to G6. But ufshcd_config_tx_eq_settings() returns early when
> use_adaptive_txeq = 0. So which function applies the static values in this
> case?
ufshcd_post_device_init()->
ufshcd_tune_unipro_params()->
ufshcd_apply_valid_tx_eq_settings()
>
> 2, when use_adaptive_txeq = 1: static host values are used as the fixed host TX
> EQ during EQTR. This is because ufs_qcom_get_rx_fom() only sweeps the device
> side. It reads host values from hba->tx_eq_params[gear-1]->host[]. The static
> values also work as the per-lane fallback in ufshcd_update_tx_eq_params() when
> FOM is 0.
Correct.
>
>> + }
>> +}
>> +
>> /**
>> * ufshcd_parse_clock_min_max_freq - Parse MIN and MAX clocks freq
>> * @hba: per adapter instance
>> @@ -528,6 +608,8 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
>>
>> ufshcd_init_lanes_per_dir(hba);
>>
>> + ufshcd_parse_static_tx_eq_settings(hba);
>> +
>> err = ufshcd_parse_operating_points(hba);
>> if (err) {
>> dev_err(dev, "%s: OPP parse failed %d\n", __func__, err);
>> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
>> index f48d6416e299..2d385d42fcff 100644
>> --- a/include/ufs/ufshcd.h
>> +++ b/include/ufs/ufshcd.h
>> @@ -359,6 +359,7 @@ struct ufshcd_tx_eqtr_record {
>> * @is_valid: True if parameter contains valid TX Equalization settings
>> * @is_applied: True if settings have been applied to UniPro of both sides
>> * @is_trained: True if parameters obtained from TX EQTR procedure
>> + * @is_static: True if settings are static
>> */
>> struct ufshcd_tx_eq_params {
>> struct ufshcd_tx_eq_settings host[UFS_MAX_LANES];
>> @@ -367,8 +368,12 @@ struct ufshcd_tx_eq_params {
>> bool is_valid;
>> bool is_applied;
>> bool is_trained;
>> + bool is_static;
> is_static is added next to is_trained, which was added in your "Add persistent
> TX Equalization settings support" series, That series still has Brian's open
> question about wTxEQGnSettingsExt Bit[15] being RFU per JESD220H:
>
> https://patchwork.kernel.org/project/linux-scsi/cover/20260424151420.111675-1-can.guo@oss.qualcomm.co
>
> Is this the reason why "Add persistent..." has not been merged?
>
> I'd prefer to wait until that discussion concludes before tagging this one. I
> hope this is ok for you.
The persistent patch series has been merged by Martin, we just need to
wait for Martin
to push the branch, then I will upload Patch V2 to address the comment
from Conor.
Thanks,
Can Guo.
>
> Kind regards,
> Bean
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] scsi: ufs: core: Add support for static TX Equalization settings
2026-05-01 13:44 ` Can Guo
@ 2026-05-14 13:57 ` Bean Huo
2026-05-15 8:12 ` Can Guo
0 siblings, 1 reply; 15+ messages in thread
From: Bean Huo @ 2026-05-14 13:57 UTC (permalink / raw)
To: Can Guo, bvanassche, beanhuo, peter.wang, martin.petersen, mani,
powenkao
Cc: linux-scsi, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Ram Kumar Dwivedi, Nitin Rawat, open list
Can,
Sorry for the late review of this patch. I have several questions:
On Fri, 2026-05-01 at 06:44 -0700, Can Guo wrote:
> @@ -1297,7 +1297,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
> }
>
> params = &hba->tx_eq_params[gear - 1];
> - if (!params->is_valid || force_tx_eqtr) {
> + if (!params->is_valid || params->is_static || force_tx_eqtr) {
When use_adaptive_txeq is on and params->is_static is true, EQTR will overwrite
the static values. That is reasonable since EQTR is more accurate. Is the
is_static check really needed here?
> int ret;
>
>
...
>
> void ufshcd_retrieve_tx_eq_settings(struct ufs_hba *hba)
> diff --git a/drivers/ufs/host/ufshcd-pltfrm.c b/drivers/ufs/host/ufshcd-
> pltfrm.c
> index c2dafb583cf5..de6302e8c067 100644
> --- a/drivers/ufs/host/ufshcd-pltfrm.c
> +++ b/drivers/ufs/host/ufshcd-pltfrm.c
> @@ -210,6 +210,86 @@ static void ufshcd_init_lanes_per_dir(struct ufs_hba
> *hba)
> }
> }
>
> +static void ufshcd_parse_static_tx_eq_settings(struct ufs_hba *hba)
> +{
> + size_t sz = hba->lanes_per_direction * 2 * TX_EQ_SETTINGS_TUPLE_SZ;
> + u32 settings[UFS_MAX_LANES * 2 * TX_EQ_SETTINGS_TUPLE_SZ];
> + u32 *host_settings, *device_settings;
> + u32 lpd = hba->lanes_per_direction;
> + struct ufshcd_tx_eq_params *params;
>
....
> +
> + params = &hba->tx_eq_params[gear - 1];
> + host_settings = settings;
> + device_settings = settings + lpd * TX_EQ_SETTINGS_TUPLE_SZ;
> +
> + for (lane = 0; lane < lpd; lane++) {
> + params->host[lane].preshoot = host_settings[0];
> + params->host[lane].deemphasis = host_settings[1];
> + params->host[lane].precode_en = host_settings[2];
> + host_settings += TX_EQ_SETTINGS_TUPLE_SZ;
> +
> + params->device[lane].preshoot = device_settings[0];
> + params->device[lane].deemphasis = device_settings[1];
> + params->device[lane].precode_en = device_settings[2];
> + device_settings += TX_EQ_SETTINGS_TUPLE_SZ;
> + }
> +
> + params->is_valid = true;
> + params->is_static = true;
I want to confirm I understand the code correctly. Please tell me if I am wrong:
1, When use_adaptive_txeq = 0: static values are used directly as TX EQ for HS-
G4 to G6. But ufshcd_config_tx_eq_settings() returns early when
use_adaptive_txeq = 0. So which function applies the static values in this
case?
2, when use_adaptive_txeq = 1: static host values are used as the fixed host TX
EQ during EQTR. This is because ufs_qcom_get_rx_fom() only sweeps the device
side. It reads host values from hba->tx_eq_params[gear-1]->host[]. The static
values also work as the per-lane fallback in ufshcd_update_tx_eq_params() when
FOM is 0.
> + }
> +}
> +
> /**
> * ufshcd_parse_clock_min_max_freq - Parse MIN and MAX clocks freq
> * @hba: per adapter instance
> @@ -528,6 +608,8 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
>
> ufshcd_init_lanes_per_dir(hba);
>
> + ufshcd_parse_static_tx_eq_settings(hba);
> +
> err = ufshcd_parse_operating_points(hba);
> if (err) {
> dev_err(dev, "%s: OPP parse failed %d\n", __func__, err);
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index f48d6416e299..2d385d42fcff 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -359,6 +359,7 @@ struct ufshcd_tx_eqtr_record {
> * @is_valid: True if parameter contains valid TX Equalization settings
> * @is_applied: True if settings have been applied to UniPro of both sides
> * @is_trained: True if parameters obtained from TX EQTR procedure
> + * @is_static: True if settings are static
> */
> struct ufshcd_tx_eq_params {
> struct ufshcd_tx_eq_settings host[UFS_MAX_LANES];
> @@ -367,8 +368,12 @@ struct ufshcd_tx_eq_params {
> bool is_valid;
> bool is_applied;
> bool is_trained;
> + bool is_static;
is_static is added next to is_trained, which was added in your "Add persistent
TX Equalization settings support" series, That series still has Brian's open
question about wTxEQGnSettingsExt Bit[15] being RFU per JESD220H:
https://patchwork.kernel.org/project/linux-scsi/cover/20260424151420.111675-1-can.guo@oss.qualcomm.co
Is this the reason why "Add persistent..." has not been merged?
I'd prefer to wait until that discussion concludes before tagging this one. I
hope this is ok for you.
Kind regards,
Bean
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/2] scsi: ufs: core: Add support for static TX Equalization settings
[not found] <20260501134418.863432-1-can.guo@oss.qualcomm.com>
@ 2026-05-01 13:44 ` Can Guo
2026-05-14 13:57 ` Bean Huo
0 siblings, 1 reply; 15+ messages in thread
From: Can Guo @ 2026-05-01 13:44 UTC (permalink / raw)
To: bvanassche, beanhuo, peter.wang, martin.petersen, mani
Cc: linux-scsi, Can Guo, Alim Akhtar, Avri Altman,
James E.J. Bottomley, Ram Kumar Dwivedi, Nitin Rawat, open list
HW design team usually provide static TX Equalization settings based on
PCB board characteristics. The static TX Equalization settings may not be
optimal for all PCBs, but it is better than having nothing when adaptive
TX EQTR is not used. Since the static TX Equalization settings are PCB
board specific, pass the static TX Equalization settings from device tree.
Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
---
drivers/ufs/core/ufs-txeq.c | 4 +-
drivers/ufs/host/ufshcd-pltfrm.c | 82 ++++++++++++++++++++++++++++++++
include/ufs/ufshcd.h | 5 ++
3 files changed, 90 insertions(+), 1 deletion(-)
diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
index 4b264adfdf49..634ec039e129 100644
--- a/drivers/ufs/core/ufs-txeq.c
+++ b/drivers/ufs/core/ufs-txeq.c
@@ -1297,7 +1297,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
}
params = &hba->tx_eq_params[gear - 1];
- if (!params->is_valid || force_tx_eqtr) {
+ if (!params->is_valid || params->is_static || force_tx_eqtr) {
int ret;
ret = ufshcd_tx_eqtr(hba, params, pwr_mode);
@@ -1310,6 +1310,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
/* Mark TX Equalization settings as valid */
params->is_valid = true;
params->is_trained = true;
+ params->is_static = false;
params->is_applied = false;
}
@@ -1495,6 +1496,7 @@ static void ufshcd_extract_tx_eq_settings_attrs(struct ufs_hba *hba, u8 gear)
}
params->is_valid = true;
+ params->is_static = false;
}
void ufshcd_retrieve_tx_eq_settings(struct ufs_hba *hba)
diff --git a/drivers/ufs/host/ufshcd-pltfrm.c b/drivers/ufs/host/ufshcd-pltfrm.c
index c2dafb583cf5..de6302e8c067 100644
--- a/drivers/ufs/host/ufshcd-pltfrm.c
+++ b/drivers/ufs/host/ufshcd-pltfrm.c
@@ -210,6 +210,86 @@ static void ufshcd_init_lanes_per_dir(struct ufs_hba *hba)
}
}
+static void ufshcd_parse_static_tx_eq_settings(struct ufs_hba *hba)
+{
+ size_t sz = hba->lanes_per_direction * 2 * TX_EQ_SETTINGS_TUPLE_SZ;
+ u32 settings[UFS_MAX_LANES * 2 * TX_EQ_SETTINGS_TUPLE_SZ];
+ u32 *host_settings, *device_settings;
+ u32 lpd = hba->lanes_per_direction;
+ struct ufshcd_tx_eq_params *params;
+ struct device *dev = hba->dev;
+ int i, err, count, gear, lane;
+ char prop_name[MAX_PROP_SIZE];
+
+ if (!lpd || lpd > UFS_MAX_LANES) {
+ dev_err(dev, "Invalid lanes-per-direction value (%u) provided\n", lpd);
+ return;
+ }
+
+ for (gear = UFS_HS_G1; gear <= UFS_HS_GEAR_MAX; gear++) {
+ snprintf(prop_name, MAX_PROP_SIZE, "txeq-settings-g%d", gear);
+ count = of_property_count_u32_elems(dev->of_node, prop_name);
+ if (count <= 0)
+ continue;
+
+ if (count != sz) {
+ dev_err(dev, "Property %s has invalid count (%d), expecting %zu \n",
+ prop_name, count, sz);
+ continue;
+ }
+
+ err = of_property_read_u32_array(dev->of_node, prop_name,
+ settings, sz);
+ if (err) {
+ dev_err(dev, "Failed to read %s property, %d\n",
+ prop_name, err);
+ continue;
+ }
+
+ for (i = 0; i < count; i += TX_EQ_SETTINGS_TUPLE_SZ) {
+ if (settings[i] >= TX_HS_NUM_PRESHOOT) {
+ dev_err(dev, "An invalid TX EQ PreShoot (%d) provided in %s property\n",
+ settings[i], prop_name);
+ break;
+ }
+
+ if (settings[i + 1] >= TX_HS_NUM_DEEMPHASIS) {
+ dev_err(dev, "An invalid TX EQ DeEmphasis (%d) provided in %s property\n",
+ settings[i + 1], prop_name);
+ break;
+ }
+
+ if (settings[i + 2] > 1) {
+ dev_err(dev, "An invalid PrecodeEn (%d) provided in %s property\n",
+ settings[i + 2], prop_name);
+ break;
+ }
+ }
+
+ if (i != count)
+ continue;
+
+ params = &hba->tx_eq_params[gear - 1];
+ host_settings = settings;
+ device_settings = settings + lpd * TX_EQ_SETTINGS_TUPLE_SZ;
+
+ for (lane = 0; lane < lpd; lane++) {
+ params->host[lane].preshoot = host_settings[0];
+ params->host[lane].deemphasis = host_settings[1];
+ params->host[lane].precode_en = host_settings[2];
+ host_settings += TX_EQ_SETTINGS_TUPLE_SZ;
+
+ params->device[lane].preshoot = device_settings[0];
+ params->device[lane].deemphasis = device_settings[1];
+ params->device[lane].precode_en = device_settings[2];
+ device_settings += TX_EQ_SETTINGS_TUPLE_SZ;
+ }
+
+ params->is_valid = true;
+ params->is_static = true;
+ }
+}
+
/**
* ufshcd_parse_clock_min_max_freq - Parse MIN and MAX clocks freq
* @hba: per adapter instance
@@ -528,6 +608,8 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
ufshcd_init_lanes_per_dir(hba);
+ ufshcd_parse_static_tx_eq_settings(hba);
+
err = ufshcd_parse_operating_points(hba);
if (err) {
dev_err(dev, "%s: OPP parse failed %d\n", __func__, err);
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index f48d6416e299..2d385d42fcff 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -359,6 +359,7 @@ struct ufshcd_tx_eqtr_record {
* @is_valid: True if parameter contains valid TX Equalization settings
* @is_applied: True if settings have been applied to UniPro of both sides
* @is_trained: True if parameters obtained from TX EQTR procedure
+ * @is_static: True if settings are static
*/
struct ufshcd_tx_eq_params {
struct ufshcd_tx_eq_settings host[UFS_MAX_LANES];
@@ -367,8 +368,12 @@ struct ufshcd_tx_eq_params {
bool is_valid;
bool is_applied;
bool is_trained;
+ bool is_static;
};
+/* TX EQ Settings Tuple has 3 elements - PreShoot, DeEmphasis and PrecodeEn. */
+#define TX_EQ_SETTINGS_TUPLE_SZ 3
+
/**
* struct ufs_hba_variant_ops - variant specific callbacks
* @name: variant name
--
2.34.1
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-05-30 11:36 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20260527144055.2758170-1-can.guo@oss.qualcomm.com>
2026-05-27 14:40 ` [PATCH 1/2] dt-bindings: ufs: Document static TX Equalization settings properties Can Guo
2026-05-28 6:18 ` Manivannan Sadhasivam
2026-05-30 11:36 ` Krzysztof Kozlowski
2026-05-27 14:40 ` [PATCH 2/2] scsi: ufs: core: Add support for static TX Equalization settings Can Guo
2026-05-28 6:13 ` Manivannan Sadhasivam
2026-05-28 7:24 ` Can Guo
2026-05-28 8:16 ` Manivannan Sadhasivam
2026-05-28 8:40 ` Can Guo
2026-05-28 9:29 ` Manivannan Sadhasivam
2026-05-28 9:31 ` Can Guo
2026-05-28 6:18 ` Manivannan Sadhasivam
2026-05-28 7:27 ` Can Guo
[not found] <20260501134418.863432-1-can.guo@oss.qualcomm.com>
2026-05-01 13:44 ` Can Guo
2026-05-14 13:57 ` Bean Huo
2026-05-15 8:12 ` Can Guo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®