Re: [PATCH 2/2] PCI: mediatek: handle optional reset for perstout for AN7583

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing T_PERST delay between PCIe PERST# assert and deassert.
- [Medium] Fails to handle non-EPROBE_DEFER error pointers from devm_reset_control_get_optional_exclusive().
--

--- Patch [2]: [PATCH 2/2] PCI: mediatek: handle optional reset for perstout for AN7583 ---
commit 78554414cf365c230ecb9c4411ea14d8c699dd5c
Author: Christian Marangi <ansuelsmth@gmail.com>

PCI: mediatek: handle optional reset for perstout for AN7583

The commit adds support for a dedicated perstout reset line for the Airoha
AN7583 SoC to avoid glitches. It introduces the pcie-perstout[slot] reset
name handling and asserts/deasserts it during AN7583 specific startup.

> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index 8b57c3d75b942..3d4de9d694071 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
[ ... ]
> @@ -865,6 +867,9 @@ static int mtk_pcie_startup_port_an7583(struct mtk_pcie_port *port)
>  	size = lower_32_bits(resource_size(entry->res));
>  	regmap_write(pbus_regmap, args[1], GENMASK(31, __fls(size)));
>  
> +	reset_control_assert(port->perstout_reset);
> +	reset_control_deassert(port->perstout_reset);

[Severity: High]
Is it intentional to omit the 100ms T_PERST delay here?

Without a delay between assert and deassert, the endpoint device will receive
a zero-width reset pulse which violates the PCIe CEM specification. Because
the AN7583 quirks likely include MTK_PCIE_SKIP_RSTB, the standard 100ms
delay in mtk_pcie_startup_port_v2() will be skipped, leaving no delay at all
before link training begins.

> +
>  	return mtk_pcie_startup_port_v2(port);
>  }
>  
> @@ -1020,6 +1025,11 @@ static int mtk_pcie_parse_port(struct mtk_pcie *pcie,
>  	if (PTR_ERR(port->reset) == -EPROBE_DEFER)
>  		return PTR_ERR(port->reset);
>  
> +	snprintf(name, sizeof(name), "pcie-perstout%d", slot);
> +	port->perstout_reset = devm_reset_control_get_optional_exclusive(dev, name);
> +	if (PTR_ERR(port->perstout_reset) == -EPROBE_DEFER)
> +		return PTR_ERR(port->perstout_reset);

[Severity: Medium]
Will this silently leak non-EPROBE_DEFER error pointers (like -EINVAL or
-ENOMEM) into port->perstout_reset?

If devm_reset_control_get_optional_exclusive() fails with something other
than -EPROBE_DEFER, the error pointer will be stored in port->perstout_reset
since IS_ERR() isn't checked first. 

When this pointer is later passed to reset_control_assert() in
mtk_pcie_startup_port_an7583(), it will trigger a WARN_ON(IS_ERR(rstc)) in
the reset core framework.

> +
>  	/* some platforms may use default PHY setting */
>  	snprintf(name, sizeof(name), "pcie-phy%d", slot);
>  	port->phy = devm_phy_optional_get(dev, name);

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