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

On Wed, Jul 22, 2026 at 12:40:10PM -0500, Bjorn Helgaas wrote:
> On Tue, Jul 14, 2026 at 01:58:46PM +0200, Christian Marangi wrote:
> > The Airoha AN7581 SoC supports configuring the first PCIe0 lane to 2-lanes
> > mode (2x link) by bonding it with the second PCIe lane (PCIe1). This is
> > done by configuring the PCIe MUX in the SCU register.
> 
> s/2x/x2/ to match usual convention, e.g., PCIe r7.0 sec 1.2
> 
> I mentioned this before at
> https://lore.kernel.org/all/20260626162508.GA27685@bhelgaas
> 
> > To correctly configure PCIe0 in 2x link, define in DT the following
> > additional property:
> 
> s/property/properties/
> 
> Add a blank line here and indent the following list a couple spaces:
> 
> > - 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 2x link
> 
> Add '' around 'airoha,scu' to match the others
> s/2x/x2/
> 
> > In such configuration the EQ preset are configured to the same values.
> > 
> > To permit correct configuration of the PCIe line, additional logic is added
> > to assert and deassert the PERSTOUT resets.
> 
> s/line/link/
> 
> In the PCIe context, "line" doesn't mean anything.
> 
> > Signed-off-by: Christian Marangi <[email protected]>
> > ---
> >  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..764974045340 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");
> > +
> >  	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");
> > @@ -949,18 +965,38 @@ static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie)
> >  			pcie->num_lanes = num_lanes;
> >  	}
> >  
> > +	/* Map secondary PCIe for 2-lanes mode for EN7581 */
> > +	if (pcie->num_lanes == 2 && device_is_compatible(dev, "airoha,en7581-pcie")) {
> > +		regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "sec-pcie-mac");
> > +		if (!regs)
> > +			return -EINVAL;
> > +		pcie->sec_base = devm_ioremap_resource(dev, regs);
> > +		if (IS_ERR(pcie->sec_base))
> > +			return dev_err_probe(dev, PTR_ERR(pcie->sec_base), "failed to map secondary register base\n");
> > +	}
> > +
> >  	return 0;
> >  }
> >  
> >  static int mtk_pcie_en7581_power_up(struct mtk_gen3_pcie *pcie)
> >  {
> >  	struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie);
> > +	unsigned int num_lanes = max(1, pcie->num_lanes);
> > +	struct regmap *pbus_regmap, *scu;
> >  	struct device *dev = pcie->dev;
> >  	struct resource_entry *entry;
> > -	struct regmap *pbus_regmap;
> >  	u32 val, args[2], size;
> >  	resource_size_t addr;
> > -	int err;
> > +	int i, err;
> > +
> > +	if (num_lanes > 2)
> > +		return dev_err_probe(dev, -EINVAL, "unsupported num-lanes, maximum 2 lanes supported\n");
> > +
> > +	if (num_lanes == 2) {
> > +		scu = syscon_regmap_lookup_by_phandle(dev->of_node, "airoha,scu");
> > +		if (IS_ERR(scu))
> > +			return dev_err_probe(dev, PTR_ERR(scu), "failed to map SCU regmap\n");
> > +	}
> >  
> >  	/*
> >  	 * The controller may have been left out of reset by the bootloader
> > @@ -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 lines */
> 
> s/lines/lanes/ (also several more places below)
> 
> > +	err = reset_control_bulk_assert(MAX_NUM_PERSTOUT_RESETS,
> > +					pcie->perstout_resets);
> > +	if (err) {
> > +		dev_err(dev, "failed to assert PERSTOUTs\n");
> > +		return err;
> > +	}
> > +
> > +	/* Configure SCU MUX to disable PCIE1 for 2 lines mode */
> > +	if (num_lanes == 2)
> > +		regmap_update_bits(scu, SCU_PCIC, SCU_PCIC_PCIE_CTRL,
> > +				   FIELD_PREP(SCU_PCIC_PCIE_CTRL, BIT(1)));
> > +
> >  	/*
> >  	 * Unlike the other MediaTek Gen3 controllers, the Airoha EN7581
> >  	 * requires PHY initialization and power-on before PHY reset deassert.
> > @@ -1024,18 +1073,6 @@ static int mtk_pcie_en7581_power_up(struct mtk_gen3_pcie *pcie)
> >  	pm_runtime_enable(dev);
> >  	pm_runtime_get_sync(dev);
> >  
> > -	val = FIELD_PREP(PCIE_VAL_LN0_DOWNSTREAM, 0x47) |
> > -	      FIELD_PREP(PCIE_VAL_LN1_DOWNSTREAM, 0x47) |
> > -	      FIELD_PREP(PCIE_VAL_LN0_UPSTREAM, 0x41) |
> > -	      FIELD_PREP(PCIE_VAL_LN1_UPSTREAM, 0x41);
> > -	writel_relaxed(val, pcie->base + PCIE_EQ_PRESET_01_REG);
> > -
> > -	val = PCIE_K_PHYPARAM_QUERY | PCIE_K_QUERY_TIMEOUT |
> > -	      FIELD_PREP(PCIE_K_PRESET_TO_USE_16G, 0x80) |
> > -	      FIELD_PREP(PCIE_K_PRESET_TO_USE, 0x2) |
> > -	      FIELD_PREP(PCIE_K_FINETUNE_MAX, 0xf);
> > -	writel_relaxed(val, pcie->base + PCIE_PIPE4_PIE8_REG);
> > -
> >  	err = clk_bulk_prepare_enable(pcie->num_clks, pcie->clks);
> >  	if (err) {
> >  		dev_err(dev, "failed to prepare clock\n");
> > @@ -1043,14 +1080,47 @@ static int mtk_pcie_en7581_power_up(struct mtk_gen3_pcie *pcie)
> >  	}
> >  
> >  	/*
> > -	 * Airoha EN7581 performs PCIe reset via clk callbacks since it has a
> > -	 * hw issue with PCIE_PE_RSTB signal. Add wait for the time needed to
> > -	 * complete the PCIe reset.
> > +	 * Old Airoha EN7581 clock driver performed PCIe reset via
> > +	 * clk callbacks since it has a hw issue with PCIE_PE_RSTB signal.
> > +	 * This is now handled by dedicated PERSTOUT resets with clk
> > +	 * driver only enabling the refclk.
> 
> This reference to the "old Airoha EN7581 clock driver" makes me think
> there's a dependency between a EN7581 clock driver change and this
> PERSTOUT change.  If so, please mention the commit SHA1 for the EN7581
> clock driver change so they're connected and people backporting to
> older kernels know to be careful to get both of them.
> 
> It sounds like this PERSTOUT change might be independent of the x2
> configuration support.  If so, it should be done in a separate patch.
>

Hi,

thanks for the review. The clock change can go on a separate patch but
since the hash will change when it will get merged what should I use as
reference? Also any hint on the tag to use or the format?

Or just describe this in the commit description following the usual
hash + commit title format?
 
> > +	 * Wait is still needed for refclk to stabilize
> >  	 */
> >  	msleep(PCIE_T_PVPERL_MS);
> >  
> > +	/* Configure all the lines to the same EQ config */
> > +	for (i = 0; i < num_lanes; i++) {
> > +		void __iomem *base = pcie->base;
> > +
> > +		if (i == 1)
> > +			base = pcie->sec_base;
> > +
> > +		val = FIELD_PREP(PCIE_VAL_LN0_DOWNSTREAM, 0x47) |
> > +			FIELD_PREP(PCIE_VAL_LN1_DOWNSTREAM, 0x47) |
> > +			FIELD_PREP(PCIE_VAL_LN0_UPSTREAM, 0x41) |
> > +			FIELD_PREP(PCIE_VAL_LN1_UPSTREAM, 0x41);
> > +		writel_relaxed(val, base + PCIE_EQ_PRESET_01_REG);
> > +
> > +		val = PCIE_K_PHYPARAM_QUERY | PCIE_K_QUERY_TIMEOUT |
> > +			FIELD_PREP(PCIE_K_PRESET_TO_USE_16G, 0x80) |
> > +			FIELD_PREP(PCIE_K_PRESET_TO_USE, 0x2) |
> > +			FIELD_PREP(PCIE_K_FINETUNE_MAX, 0xf);
> > +		writel_relaxed(val, base + PCIE_PIPE4_PIE8_REG);
> > +	}
> > +
> > +	/* Deassert PERSTOUT for all relevant lines */
> > +	err = reset_control_bulk_deassert(MAX_NUM_PERSTOUT_RESETS,
> > +					  pcie->perstout_resets);
> > +	if (err) {
> > +		dev_err(dev, "failed to deassert PERSTOUTs\n");
> > +		goto err_perstout_deassert;
> > +	}
> > +
> >  	return 0;
> >  
> > +err_perstout_deassert:
> > +	clk_bulk_disable_unprepare(pcie->num_clks, pcie->clks);
> >  err_clk_prepare_enable:
> >  	pm_runtime_put_sync(dev);
> >  	pm_runtime_disable(dev);
> > @@ -1136,6 +1206,8 @@ static void mtk_pcie_power_down(struct mtk_gen3_pcie *pcie)
> >  
> >  	phy_power_off(pcie->phy);
> >  	phy_exit(pcie->phy);
> > +	reset_control_bulk_assert(MAX_NUM_PERSTOUT_RESETS,
> > +				  pcie->perstout_resets);
> >  	reset_control_bulk_assert(pcie->soc->phy_resets.num_resets,
> >  				  pcie->phy_resets);
> >  }
> > -- 
> > 2.53.0
> > 

-- 
	Ansuel