Re: [PATCH v3 4/4] PCI: mediatek: add support for Airoha AN7583 SoC

[Bjorn Helgaas <helgaas@kernel.org>]

In subject:

  PCI: mediatek: Add support for Airoha AN7583 SoC

On Thu, Sep 25, 2025 at 06:23:18PM +0200, Christian Marangi wrote:
> Add support for the second PCIe line present on Airoha AN7583 SoC.

What is a "second PCIe line"?  Does this mean a second Root Complex?
Or a second Root Port in a Root Complex?

I guess maybe it just means this adds support for another variant of
the Mediatek IP that is used in Airoha AN7583?

> This is based on the Mediatek Gen1/2 PCIe driver and similar to Gen3
> also require workaround for the reset signals.
> 
> Introduce a new bool to skip having to reset signals and also introduce
> some additional logic to configure the PBUS registers required for
> Airoha SoC.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  drivers/pci/controller/pcie-mediatek.c | 85 +++++++++++++++++++-------
>  1 file changed, 63 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index 24cc30a2ab6c..640d1f1a6478 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -147,6 +147,7 @@ struct mtk_pcie_port;
>   * @need_fix_class_id: whether this host's class ID needed to be fixed or not
>   * @need_fix_device_id: whether this host's device ID needed to be fixed or not
>   * @no_msi: Bridge has no MSI support, and relies on an external block
> + * @skip_pcie_rstb: Skip calling RSTB bits on PCIe probe
>   * @device_id: device ID which this host need to be fixed
>   * @ops: pointer to configuration access functions
>   * @startup: pointer to controller setting functions
> @@ -156,6 +157,7 @@ struct mtk_pcie_soc {
>  	bool need_fix_class_id;
>  	bool need_fix_device_id;
>  	bool no_msi;
> +	bool skip_pcie_rstb;
>  	unsigned int device_id;
>  	struct pci_ops *ops;
>  	int (*startup)(struct mtk_pcie_port *port);
> @@ -679,28 +681,30 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
>  		regmap_update_bits(pcie->cfg, PCIE_SYS_CFG_V2, val, val);
>  	}
>  
> -	/* Assert all reset signals */
> -	writel(0, port->base + PCIE_RST_CTRL);
> -
> -	/*
> -	 * Enable PCIe link down reset, if link status changed from link up to
> -	 * link down, this will reset MAC control registers and configuration
> -	 * space.
> -	 */
> -	writel(PCIE_LINKDOWN_RST_EN, port->base + PCIE_RST_CTRL);
> -
> -	/*
> -	 * Described in PCIe CEM specification sections 2.2 (PERST# Signal) and
> -	 * 2.2.1 (Initial Power-Up (G3 to S0)). The deassertion of PERST# should
> -	 * be delayed 100ms (TPVPERL) for the power and clock to become stable.
> -	 */
> -	msleep(100);
> -
> -	/* De-assert PHY, PE, PIPE, MAC and configuration reset	*/
> -	val = readl(port->base + PCIE_RST_CTRL);
> -	val |= PCIE_PHY_RSTB | PCIE_PERSTB | PCIE_PIPE_SRSTB |
> -	       PCIE_MAC_SRSTB | PCIE_CRSTB;
> -	writel(val, port->base + PCIE_RST_CTRL);
> +	if (!soc->skip_pcie_rstb) {
> +		/* Assert all reset signals */
> +		writel(0, port->base + PCIE_RST_CTRL);
> +
> +		/*
> +		 * Enable PCIe link down reset, if link status changed from link up to
> +		 * link down, this will reset MAC control registers and configuration
> +		 * space.
> +		 */
> +		writel(PCIE_LINKDOWN_RST_EN, port->base + PCIE_RST_CTRL);
> +
> +		/*
> +		 * Described in PCIe CEM specification sections 2.2 (PERST# Signal) and
> +		 * 2.2.1 (Initial Power-Up (G3 to S0)). The deassertion of PERST# should
> +		 * be delayed 100ms (TPVPERL) for the power and clock to become stable.

Wrap these comments to fit in 80 columns like the rest of the file.

You only moved the comment, without updating it, but since you're
touching it anyway, it's a chance to add the CEM spec revision, e.g.,
"PCIe CEM r5.0, sec 2.2"

> +		 */
> +		msleep(100);

Use PCIE_T_PVPERL_MS if that's what this is.

> +
> +		/* De-assert PHY, PE, PIPE, MAC and configuration reset	*/
> +		val = readl(port->base + PCIE_RST_CTRL);
> +		val |= PCIE_PHY_RSTB | PCIE_PERSTB | PCIE_PIPE_SRSTB |
> +		       PCIE_MAC_SRSTB | PCIE_CRSTB;
> +		writel(val, port->base + PCIE_RST_CTRL);
> +	}
>  
>  	/* Set up vendor ID and class code */
>  	if (soc->need_fix_class_id) {
> @@ -1105,6 +1109,33 @@ static int mtk_pcie_probe(struct platform_device *pdev)
>  	if (err)
>  		goto put_resources;
>  
> +	if (device_is_compatible(dev, "airoha,an7583-pcie")) {
> +		struct resource_entry *entry;
> +		struct regmap *pbus_regmap;
> +		resource_size_t addr;
> +		u32 args[2], size;
> +
> +		/*
> +		 * Configure PBus base address and base address mask to allow the
> +		 * hw to detect if a given address is accessible on PCIe controller.

Wrap to fit in 80 columns.

> +		 */
> +		pbus_regmap = syscon_regmap_lookup_by_phandle_args(dev->of_node,
> +								   "mediatek,pbus-csr",
> +								   ARRAY_SIZE(args),
> +								   args);
> +		if (IS_ERR(pbus_regmap))
> +			return PTR_ERR(pbus_regmap);
> +
> +		entry = resource_list_first_type(&host->windows, IORESOURCE_MEM);
> +		if (!entry)
> +			return -ENODEV;
> +
> +		addr = entry->res->start - entry->offset;
> +		regmap_write(pbus_regmap, args[0], lower_32_bits(addr));
> +		size = lower_32_bits(resource_size(entry->res));
> +		regmap_write(pbus_regmap, args[1], GENMASK(31, __fls(size)));
> +	}
> +
>  	return 0;
>  
>  put_resources:
> @@ -1205,6 +1236,15 @@ static const struct mtk_pcie_soc mtk_pcie_soc_mt7622 = {
>  	.setup_irq = mtk_pcie_setup_irq,
>  };
>  
> +static const struct mtk_pcie_soc mtk_pcie_soc_an7583 = {
> +	.skip_pcie_rstb = true,
> +	.need_fix_class_id = true,
> +	.need_fix_device_id = false,

No need to specify "false" items; things are false by default since
members not explicitly initialized are set to zero.

> +	.ops = &mtk_pcie_ops_v2,
> +	.startup = mtk_pcie_startup_port_v2,
> +	.setup_irq = mtk_pcie_setup_irq,
> +};
> +
>  static const struct mtk_pcie_soc mtk_pcie_soc_mt7629 = {
>  	.need_fix_class_id = true,
>  	.need_fix_device_id = true,
> @@ -1215,6 +1255,7 @@ static const struct mtk_pcie_soc mtk_pcie_soc_mt7629 = {
>  };
>  
>  static const struct of_device_id mtk_pcie_ids[] = {
> +	{ .compatible = "airoha,an7583-pcie", .data = &mtk_pcie_soc_an7583 },
>  	{ .compatible = "mediatek,mt2701-pcie", .data = &mtk_pcie_soc_v1 },
>  	{ .compatible = "mediatek,mt7623-pcie", .data = &mtk_pcie_soc_v1 },
>  	{ .compatible = "mediatek,mt2712-pcie", .data = &mtk_pcie_soc_mt2712 },
> -- 
> 2.51.0
>