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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing UFS HS-G4 gear configuration overlay for the Hawi PHY will leave critical hardware capabilities uninitialized during G4 negotiation.
--

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

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

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
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
[ ... ]
> @@ -1844,6 +1868,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, 0x05),
> +	QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_RX_HSGEAR_CAPABILITY, 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,
> +	},
> +

Is there a missing UFS HS-G4 gear configuration overlay for the Hawi PHY here?

When the UFS controller negotiates HS-G4 (e.g., connected to a G4-only
device or during fallback) and requests the PHY to operate at G4,
qmp_ufs_get_gear_overlay() will fail to find a matching configuration.

In qmp_ufs_get_gear_overlay(), it initializes floor_max_gear to 5.
It then checks the only available table where max_gear is 5.
Since submode (4) == max_gear (5) is false, and max_gear (5) <
floor_max_gear (5) is also false, it returns -EINVAL.

Because qmp_ufs_get_gear_overlay() returns -EINVAL,
qmp_ufs_init_registers() will skip applying any overlay. This means
capabilities like QPHY_V7_PCS_UFS_TX_HSGEAR_CAPABILITY are never
programmed during G4 operation.

Other G5-capable PHYs (like sm8750) seem to provide both G4 and G5 overlays:

static const struct qmp_phy_cfg sm8750_ufsphy_cfg = {
[ ... ]
	.tbls_hs_overlay[0] = {
		.pcs		= sm8750_ufsphy_g4_pcs,
		.pcs_num	= ARRAY_SIZE(sm8750_ufsphy_g4_pcs),
		.max_gear	= UFS_HS_G4,
	},
	.tbls_hs_overlay[1] = {
		.pcs		= sm8650_ufsphy_g5_pcs,
		.pcs_num	= ARRAY_SIZE(sm8650_ufsphy_g5_pcs),
		.max_gear	= UFS_HS_G5,
	},
[ ... ]
};

Will leaving these uninitialized cause link instability or initialization
failure on devices falling back to HS-G4?

> +	.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/20260518165346.1732548-1-palash.kambar@oss.qualcomm.com?part=3