Re: [PATCH v2 2/2] phy: airoha: add support for AN7583 Gen3 PCIe PHY driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing hardware state cleanup on error path in `an7583_pcie_phy_init()`.
--

commit 62d7cb2950076a9ff89670b2868f00bd58e5c43e
Author: Christian Marangi <ansuelsmth@gmail.com>

phy: airoha: add support for AN7583 Gen3 PCIe PHY driver
    
Add support for AN7583 PCIe PHY driver. This is required to make the
Gen3 PCIe port present on the Airoha AN7583 SoC correctly work.

> diff --git a/drivers/phy/airoha/phy-an7583-pcie.c b/drivers/phy/airoha/phy-an7583-pcie.c
> new file mode 100644
> index 0000000000000..61b5f1dd64c07
> --- /dev/null
> +++ b/drivers/phy/airoha/phy-an7583-pcie.c
[ ... ]
> +static int an7583_pcie_phy_init(struct phy *phy)
> +{
> +	struct an7583_pcie_phy *pcie_phy = phy_get_drvdata(phy);
> +	u32 val;
> +	int ret;
> +
> +	/* Setup Tx-Rx detection time */
> +	val = FIELD_PREP(PCIE_XTP_RXDET_VCM_OFF_STB_T_SEL, 0x33) |
> +	      FIELD_PREP(PCIE_XTP_RXDET_EN_STB_T_SEL, 0x1) |
> +	      FIELD_PREP(PCIE_XTP_RXDET_FINISH_STB_T_SEL, 0x2) |
> +	      FIELD_PREP(PCIE_XTP_TXPD_TX_DATA_EN_DLY, 0x3) |
> +	      FIELD_PREP(PCIE_XTP_RXDET_LATCH_STB_T_SEL, 0x1);
> +	regmap_write(pcie_phy->xr_dtime, REG_PCIE_PEXTP_DIG_GLB44, val);
[ ... ]
> +	regmap_set_bits(pcie_phy->g3_pma, REG_SS_DA_XPON_PWDB_0,
> +			G3_PMA_DA_XPON_CDR_PR_PWDB);
> +
> +	usleep_range(100, 200);
> +
> +	ret = phy_init(pcie_phy->qp_phy);
> +	if (ret) {
> +		dev_err(pcie_phy->dev, "failed to initialize QP phy\n");
> +		return ret;

[Severity: Medium]
Does this error path need to unwind the hardware state changes made earlier in
an7583_pcie_phy_init()?

If phy_init() fails, the PHY framework won't call the .exit callback because
init_count hasn't been incremented. This could leave hardware resources like
PLLs, LDOs, and clocks fully enabled and un-reset. Should there be a cleanup
sequence here or a call to a teardown helper?

> +	}
> +
> +	/* Wait for the PCIe PHY to complete initialization before returning */
> +	msleep(PHY_HW_INIT_TIME_MS);
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827122127.18874-1-ansuelsmth@gmail.com?part=2