[PATCH] PCI: tegra: Unwind partial PHY power-on failures

[PATCH] PCI: tegra: Unwind partial PHY power-on failures

[Pengpeng Hou]

The Tegra PCIe PHY setup has two nested acquisition loops.  Failure in a
per-port loop leaves earlier PHYs in that port powered, while failure in
the outer loop leaves all earlier ports powered.

Unwind the successfully powered PHYs in reverse order at both levels
before returning the original error.

The issue was identified via static analysis and manually reviewed.

Fixes: 6fe7c187e026 ("PCI: tegra: Support per-lane PHYs")

Assisted-by: LLM
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/pci/controller/pci-tegra.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
index 59b56446c507..443fa33b7f0e 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -1002,6 +1002,8 @@ static int tegra_pcie_port_phy_power_on(struct tegra_pcie_port *port)
 		err = phy_power_on(port->phys[i]);
 		if (err < 0) {
 			dev_err(dev, "failed to power on PHY#%u: %d\n", i, err);
+			while (i--)
+				phy_power_off(port->phys[i]);
 			return err;
 		}
 	}
@@ -1051,6 +1053,8 @@ static int tegra_pcie_phy_power_on(struct tegra_pcie *pcie)
 			dev_err(dev,
 				"failed to power on PCIe port %u PHY: %d\n",
 				port->index, err);
+			list_for_each_entry_continue_reverse(port, &pcie->ports, list)
+				tegra_pcie_port_phy_power_off(port);
 			return err;
 		}
 	}
-- 
2.43.0

Re: [PATCH] PCI: tegra: Unwind partial PHY power-on failures

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The new error unwinding path for `tegra_pcie_phy_power_on` uses `tegra_pcie_port_phy_power_off` to power off fully initialized previous ports. However, `tegra_pcie_port_phy_power_off` iterates over the port's lanes in forward order and returns early on the first `phy_power_off` failure. This violates the commit message's claim to unwind in reverse order at both levels, and introduces a resource leak if a `phy_power_off` call fails, because the remaining lanes for that port will incorrectly remain powered on.
--

commit 98ccb4105337a9e0fdf96c19a4beba14a45d67f0
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>

PCI: tegra: Unwind partial PHY power-on failures

This commit adds error handling to unwind successfully powered PHYs in the
Tegra PCIe PHY setup. It attempts to power off PHYs in reverse order for
both nested acquisition loops (lanes within a port, and ports within the
controller) when an error occurs.

> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> index 59b56446c507e..443fa33b7f0ec 100644
> --- a/drivers/pci/controller/pci-tegra.c
> +++ b/drivers/pci/controller/pci-tegra.c

[ ... ]

> @@ -1051,6 +1053,8 @@ static int tegra_pcie_phy_power_on(struct tegra_pcie *pcie)
>  			dev_err(dev,
>  				"failed to power on PCIe port %u PHY: %d\n",
>  				port->index, err);
> +			list_for_each_entry_continue_reverse(port, &pcie->ports, list)
> +				tegra_pcie_port_phy_power_off(port);

[Severity: Medium]
The commit message states the code will unwind successfully powered PHYs in
reverse order at both levels. However, does delegating to
tegra_pcie_port_phy_power_off() here contradict this design?

Looking at tegra_pcie_port_phy_power_off(), it iterates over the lanes in
forward order rather than reverse:

static int tegra_pcie_port_phy_power_off(struct tegra_pcie_port *port)
{
	...
	for (i = 0; i < port->lanes; i++) {
		err = phy_power_off(port->phys[i]);
	...
}

Additionally, if an error occurs during unwinding, does this cause a resource
leak of the remaining PHY power states?

If phy_power_off() fails for any lane, tegra_pcie_port_phy_power_off()
returns early:

		err = phy_power_off(port->phys[i]);
		if (err < 0) {
			dev_err(dev, "failed to power off PHY#%u: %d\n", i,
				err);
			return err;
		}

This early return abandons the power-off sequence for the remaining lanes
in the port, leaving those PHYs incorrectly powered on.

>  			return err;
>  		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828091747.7976-1-pengpeng@iscas.ac.cn?part=1

Re: [PATCH] PCI: tegra: Unwind partial PHY power-on failures

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 979 bytes --]

On Fri, Aug 28, 2026 at 05:17:47PM +0800, Pengpeng Hou wrote:
> The Tegra PCIe PHY setup has two nested acquisition loops.  Failure in a
> per-port loop leaves earlier PHYs in that port powered, while failure in
> the outer loop leaves all earlier ports powered.
> 
> Unwind the successfully powered PHYs in reverse order at both levels
> before returning the original error.
> 
> The issue was identified via static analysis and manually reviewed.
> 
> Fixes: 6fe7c187e026 ("PCI: tegra: Support per-lane PHYs")
> 
> Assisted-by: LLM
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
>  drivers/pci/controller/pci-tegra.c | 4 ++++
>  1 file changed, 4 insertions(+)

We technically could do something like this, but honestly I don't see
the point. If this ever happened, it'd mean that either the system is
improperly designed, badly configured in device tree or somehow broken.
In either case the unwind isn't really going to save you.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]