Re: [PATCH v3 2/2] phy: ti: ti-pipe3: allow configuring ACSPCIe TX/RX mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The driver improperly handles errors from `syscon_regmap_lookup_by_phandle_args()`, breaking probe deferral and masking failure states.
--

commit 222aba3021868be395b0b2808f4e70dc057987a7
Author: David Oberhollenzer <david.oberhollenzer@sigma-star.at>

phy: ti: ti-pipe3: allow configuring ACSPCIe TX/RX mode

For the PCIe PHYs, read the property from the device tree and
update the RX/TX mode selection bits in the specified syscon
register.

> diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
> index b5543b5c674c8..bc02010dadb74 100644
> --- a/drivers/phy/ti/phy-ti-pipe3.c
> +++ b/drivers/phy/ti/phy-ti-pipe3.c

[ ... ]

> @@ -785,6 +788,24 @@ static int ti_pipe3_get_pll_base(struct ti_pipe3 *phy)
>  	return PTR_ERR_OR_ZERO(phy->pll_ctrl_base);
>  }
>  
> +static int ti_pipe3_acspcie_tx_rx_mode(struct ti_pipe3 *phy)
> +{
> +	struct device_node *np = phy->dev->of_node;
> +	struct regmap *regmap;
> +	unsigned int args[2];
> +
> +	regmap = syscon_regmap_lookup_by_phandle_args(np,
> +						      "ti,syscon-acspcie-tx-rx",
> +						      2, args);
> +	if (IS_ERR(regmap)) {
> +		dev_warn(phy->dev, "can't get ti,syscon-acspcie-tx-rx\n");
> +		return -EINVAL;

[Severity: High]
Does this mask the original error code (such as -EPROBE_DEFER or -ENOENT)
from syscon_regmap_lookup_by_phandle_args() by returning a hardcoded
-EINVAL? Unconditionally logging a warning and returning -EINVAL could
break probe deferral if the syscon device probes later.

> +	}
> +
> +	return regmap_update_bits(regmap, args[0], PCIE_TX_RX_CTRL_MASK,
> +				  args[1] << PCIE_TX_RX_CTRL_SHIFT);
> +}
> +
>  static int ti_pipe3_probe(struct platform_device *pdev)
>  {
>  	struct ti_pipe3 *phy;
> @@ -807,6 +828,12 @@ static int ti_pipe3_probe(struct platform_device *pdev)
>  	phy->dpll_map = data->dpll_map;
>  	phy->settings = data->settings;
>  
> +	if (phy->mode == PIPE3_MODE_PCIE) {
> +		ret = ti_pipe3_acspcie_tx_rx_mode(phy);
> +		if (ret)
> +			dev_warn(dev, "failed to set ACSPCIe TX/RX mode\n");
> +	}
> +
>  	ret = ti_pipe3_get_pll_base(phy);

[Severity: High]
Does this overwrite the returned error code in ret with the result of
ti_pipe3_get_pll_base()? If ti_pipe3_acspcie_tx_rx_mode() failed or
should have been deferred, ignoring the error here causes the probe
function to falsely succeed without configuring the PHY.

>  	if (ret)
>  		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905211555.15944-1-david.oberhollenzer@sigma-star.at?part=2

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy