Re: [PATCH v5 1/2] phy: qcom-qmp-ufs: Add UFS PHY support on Hawi

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing UFS_HS_G4 PHY configuration fallback overlay for the Hawi SoC, leading to uninitialized critical registers.
--

--- Patch [1]: [PATCH v5 1/2] phy: qcom-qmp-ufs: Add UFS PHY support on Hawi ---
commit 4d0cc515a612b40d0f5a7fc9e44b9fbc6b1527fb
Author: Palash Kambar <[email protected]>
Subject: phy: qcom-qmp-ufs: Add UFS PHY support on Hawi

Add the init sequence tables and config 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 0f4ad24aa4058..d4aca22c181e4 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c

[ ... ]

> @@ -1845,6 +1868,119 @@ static const struct qmp_phy_cfg sm8750_ufsphy_cfg = {
>  
>  };
>  
> +static const struct qmp_phy_init_tbl hawi_ufsphy_serdes[] = {

[ ... ]

> +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 missing G4 overlay cause initialization to fail when a device
requests HS-G4?

If a UFS 3.1 device is attached (max gear G4) or link training drops to G4,
the host driver will call phy_set_mode() with UFS_HS_G4. Since the
tbls_hs_overlay array only defines an entry for UFS_HS_G5,
qmp_ufs_get_gear_overlay() will return -EINVAL.

This leaves critical PCS registers like QPHY_V7_PCS_UFS_PLL_CNTL,
TX_HSGEAR_CAPABILITY, and RX_HSGEAR_CAPABILITY at their hardware defaults,
which could lead to link failures. Should there be a fallback G4 overlay
defined here?

> +
> +	.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/[email protected]?part=1