Re: [PATCH v3 2/2] PCI: mediatek-gen3: Configure PBUS_CSR registers for EN7581 SoC

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Sat, Feb 22, 2025 at 11:43:45AM +0100, Lorenzo Bianconi wrote:
> Configure PBus base address and address mask to allow the hw
> to detect if a given address is accessible on PCIe controller.
> 
> Fixes: f6ab898356dd ("PCI: mediatek-gen3: Add Airoha EN7581 support")
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  drivers/pci/controller/pcie-mediatek-gen3.c | 34 ++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
> index 0f64e76e2111468e6a453889ead7fbc75804faf7..51103b7624c09ca957c22a25536dc9da25428e48 100644
> --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> @@ -15,6 +15,7 @@
>  #include <linux/irqchip/chained_irq.h>
>  #include <linux/irqdomain.h>
>  #include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/msi.h>
>  #include <linux/of_device.h>
> @@ -24,6 +25,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/pm_domain.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
>  #include <linux/reset.h>
>  
>  #include "../pci.h"
> @@ -930,9 +932,13 @@ static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie)
>  
>  static int mtk_pcie_en7581_power_up(struct mtk_gen3_pcie *pcie)
>  {
> +	struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie);
>  	struct device *dev = pcie->dev;
> +	struct resource_entry *entry;
> +	u32 val, args[2], size, mask;
> +	struct regmap *pbus_regmap;
> +	resource_size_t addr;
>  	int err;
> -	u32 val;
>  
>  	/*
>  	 * The controller may have been left out of reset by the bootloader
> @@ -944,6 +950,32 @@ static int mtk_pcie_en7581_power_up(struct mtk_gen3_pcie *pcie)
>  	/* Wait for the time needed to complete the reset lines assert. */
>  	msleep(PCIE_EN7581_RESET_TIME_MS);
>  
> +	/*
> +	 * Configure PBus base address and base address mask to allow the
> +	 * hw to detect if a given address is accessible on PCIe controller.
> +	 */
> +	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 -EINVAL;

-ENODEV or -ENOMEM

> +
> +	addr = entry->res->start - entry->offset;
> +	err = regmap_write(pbus_regmap, args[0], lower_32_bits(addr));
> +	if (err)

MMIO write is not supposed to fail.

> +		return err;
> +
> +	size = lower_32_bits(resource_size(entry->res));
> +	mask = size ? GENMASK(31, __fls(size)) : 0;

Size of MEM region could be 0?

> +	err = regmap_write(pbus_regmap, args[1], mask);
> +	if (err)

MMIO write is not supposed to fail.

- Mani

-- 
மணிவண்ணன் சதாசிவம்