Re: [PATCH v5 2/2] PCI: mediatek-gen3: Add 2-lanes mode support for Airoha AN7581

[Philipp Zabel <p.zabel@pengutronix.de>]

On Do, 2026-08-06 at 18:53 +0200, Christian Marangi wrote:
> The Airoha AN7581 SoC supports configuring the first PCIe0 lane to 2-lanes
> mode (x2 link) by bonding it with the second PCIe lane (PCIe1). This is
> done by configuring the PCIe MUX in the SCU register.
> 
> To correctly configure PCIe0 in x2 link, define in DT the following
> additional properties:
> 
>   - additional reg, 'sec-pcie-mac' for the secondary PCIe.
>   - PERSTOUT reset for both main and secondary PCIE0, called 'perstout' and
>     'sec-perstout'
>   - 'airoha,scu' property to correctly configure the SCU register for the
>     PCIe MUX
>   - 'num-lanes' set to '2' to enable PCIe0 in x2 link
> 
> In such configuration the EQ preset are configured to the same values.
> 
> To permit correct configuration of the PCIe link, additional logic is added
> to assert and deassert the PERSTOUT resets. Support of these additional
> reset was introduced in Airoha clk driver with commit
> 6712f48eb3a1 ("clk: en7523: add support for dedicated PCIe PERSTOUT reset")
> and on backporting of this commit also the clk driver change will be
> needed.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  drivers/pci/controller/pcie-mediatek-gen3.c | 106 ++++++++++++++++----
>  1 file changed, 89 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
> index b0accd828589..c8300c47374b 100644
> --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> @@ -32,6 +32,11 @@
>  
>  #include "../pci.h"
>  
> +/* AN7581 SCU register */
> +#define SCU_PCIC			0x88
> +#define SCU_PCIC_PCIE_CTRL		GENMASK(7, 0)
> +
> +/* PCIe register */
>  #define PCIE_BASE_CFG_REG		0x14
>  #define PCIE_BASE_CFG_SPEED		GENMASK(15, 8)
>  
> @@ -131,6 +136,7 @@
>  #define PCIE_ATR_TLP_TYPE_IO		PCIE_ATR_TLP_TYPE(2)
>  
>  #define MAX_NUM_PHY_RESETS		3
> +#define MAX_NUM_PERSTOUT_RESETS		2
>  
>  #define PCIE_MTK_RESET_TIME_US		10
>  
> @@ -203,9 +209,11 @@ struct mtk_msi_set {
>  struct mtk_gen3_pcie {
>  	struct device *dev;
>  	void __iomem *base;
> +	void __iomem *sec_base;
>  	phys_addr_t reg_base;
>  	struct reset_control *mac_reset;
>  	struct reset_control_bulk_data phy_resets[MAX_NUM_PHY_RESETS];
> +	struct reset_control_bulk_data perstout_resets[MAX_NUM_PERSTOUT_RESETS];
>  	struct phy *phy;
>  	struct clk_bulk_data *clks;
>  	int num_clks;
> @@ -928,6 +936,14 @@ static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie)
>  	if (ret)
>  		return dev_err_probe(dev, ret, "failed to get PHY bulk reset\n");
>  
> +	pcie->perstout_resets[0].id = "perstout";
> +	pcie->perstout_resets[1].id = "sec-perstout";
> +
> +	ret = devm_reset_control_bulk_get_optional_exclusive(dev, MAX_NUM_PERSTOUT_RESETS,
> +							     pcie->perstout_resets);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to get PERSTOUT bulk reset\n");

"bulk" is a property of the API, not the reset controls. Maybe just
"failed to get PERSTOUT resets"?

> +
>  	pcie->mac_reset = devm_reset_control_get_optional_exclusive(dev, "mac");
>  	if (IS_ERR(pcie->mac_reset))
>  		return dev_err_probe(dev, PTR_ERR(pcie->mac_reset), "failed to get MAC reset\n");
[...]
> @@ -992,6 +1028,19 @@ static int mtk_pcie_en7581_power_up(struct mtk_gen3_pcie *pcie)
>  	size = lower_32_bits(resource_size(entry->res));
>  	regmap_write(pbus_regmap, args[1], GENMASK(31, __fls(size)));
>  
> +	/* Assert PERSTOUT for all relevant lanes */
> +	err = reset_control_bulk_assert(MAX_NUM_PERSTOUT_RESETS,
> +					pcie->perstout_resets);

Isn't there a possible error return before this? Should the PERSTOUT
resets be asserted in those error cases as well?


regards
Philipp