Re: [PATCH v3 3/3] phy: qcom-qmp-ufs: Add UFS PHY support on Hawi - sashiko-bot

From: sashiko-bot@kernel.org
To: palash.kambar@oss.qualcomm.com
Cc: olteanv@gmail.com, linux-phy@lists.infradead.org,
	conor+dt@kernel.org, robh@kernel.org, devicetree@vger.kernel.org,
	vkoul@kernel.org, neil.armstrong@linaro.org
Subject: Re: [PATCH v3 3/3] phy: qcom-qmp-ufs: Add UFS PHY support on Hawi
Date: Tue, 26 May 2026 10:02:54 +0000	[thread overview]
Message-ID: <20260526100255.0D1BC1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260526090956.2340262-4-palash.kambar@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The Hawi UFS PHY configuration (`hawi_ufsphy_cfg`) lacks a fallback overlay for `UFS_HS_G4`, preventing proper initialization for UFS 3.1 or lower devices.
- [High] Incorrect HSGEAR_CAPABILITY value configured for the UFS HS Gear 5 overlay.
--

commit 1174a7d9d404cf84ecf6ba8228fcab24e26ef4df
Author: Palash Kambar <palash.kambar@oss.qualcomm.com>

phy: qcom-qmp-ufs: Add UFS PHY support on Hawi

This commit adds the initialization sequence tables and configuration
for the UFS QMP PHY found in the Hawi SoC.

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> index 771bc7c2ab505..2fac3a7eb8201 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c

[ ... ]

> @@ -1844,6 +1867,119 @@ static const struct qmp_phy_cfg sm8750_ufsphy_cfg = {
>  
>  };
>  

[ ... ]

> +static const struct qmp_phy_init_tbl hawi_ufsphy_g5_pcs[] = {
> +	QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_PLL_CNTL, 0x3b),
> +	QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_TX_HSGEAR_CAPABILITY, 0x06),
> +	QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_RX_HSGEAR_CAPABILITY, 0x06),

[Severity: High]
Does this code advertise an incorrect gear capability?

The overlay is named g5_pcs and the maximum supported gear is UFS_HS_G5, but
these capability registers are being programmed with 0x06. This typically maps
to a Gear 6 capability, which could cause link training failures or mismatches
during UFS negotiation if the value should actually be 0x05.

> +};
> +
> +static const struct qmp_phy_cfg hawi_ufsphy_cfg = {
> +	.lanes			= 2,
> +
> +	.offsets		= &qmp_ufs_offsets_v7,
> +	.max_supported_gear	= UFS_HS_G5,
> +
> +	.tbls = {
> +		.serdes		= hawi_ufsphy_serdes,
> +		.serdes_num	= ARRAY_SIZE(hawi_ufsphy_serdes),
> +		.tx		= hawi_ufsphy_tx,
> +		.tx_num		= ARRAY_SIZE(hawi_ufsphy_tx),
> +		.rx		= hawi_ufsphy_rx,
> +		.rx_num		= ARRAY_SIZE(hawi_ufsphy_rx),
> +		.pcs		= hawi_ufsphy_pcs,
> +		.pcs_num	= ARRAY_SIZE(hawi_ufsphy_pcs),
> +	},
> +
> +	.tbls_hs_overlay[0] = {
> +		.pcs		= hawi_ufsphy_g5_pcs,
> +		.pcs_num	= ARRAY_SIZE(hawi_ufsphy_g5_pcs),
> +		.max_gear	= UFS_HS_G5,
> +	},

[Severity: High]
Does this configuration lack a fallback overlay for UFS_HS_G4?

If the UFS host controller negotiates Gear 4 (for a UFS 3.1 device) or lower,
qmp_ufs_get_gear_overlay() will attempt to find the closest higher supported
gear overlay. Because floor_max_gear is initialized to cfg->max_supported_gear
(5), the fallback condition max_gear < floor_max_gear evaluates to 5 < 5 (false)
for this single available overlay.

This results in qmp_ufs_get_gear_overlay() returning -EINVAL with no overlay
applied, leaving critical PCS registers uninitialized and causing link
establishment to fail. Should a G4 overlay be added here similar to other
G5-capable PHYs like sm8750_ufsphy_cfg?

> +
> +	.vreg_list		= hawi_ufsphy_vreg_l,
> +	.num_vregs		= ARRAY_SIZE(hawi_ufsphy_vreg_l),
> +	.regs			= ufsphy_v7_regs_layout,
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526090956.2340262-1-palash.kambar@oss.qualcomm.com?part=3