Re: [PATCH v1 8/9] PCI: microchip: Partition inbound address translation

Conor Dooley
2022-11-23T23:05:55Z

On Wed, Nov 16, 2022 at 01:55:03PM +0000, daire.mcnamara@microchip.com wrote:
> From: Daire McNamara <daire.mcnamara@microchip.com>
>
> On Microchip PolarFire SoC the PCIe rootport is behind a set of fabric
> inter connect (fic) busses that encapsulate busses like ABP/AHP, AXI-S
> and AXI-M. Depending on which fic(s) the rootport is wired through to
> cpu space, the rootport driver needs to take account of the address
> translation done by a parent (e.g. fabric) node before setting up its
> own inbound address translation tables from attached devices.
>
> Parse the dma-range properties to determine how much address translation
> to perform in the root port and how much is being provided by the
> fabric.
>
> Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> drivers/pci/controller/pcie-microchip-host.c | 184 ++++++++++++++++++-
> 1 file changed, 178 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-microchip-host.c b/drivers/pci/controller/pcie-microchip-host.c
> index 62f8c5edfd0e..a90a0a675f14 100644
> --- a/drivers/pci/controller/pcie-microchip-host.c
> +++ b/drivers/pci/controller/pcie-microchip-host.c

> @@ -940,6 +954,46 @@ static int mc_pcie_init_irq_domains(struct mc_pcie *port)
> return mc_allocate_msi_domains(port);
> }
>
> +static int mc_pcie_setup_inbound_ranges(struct platform_device *pdev, struct mc_pcie *port)
> +{
> + void __iomem *bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
> + phys_addr_t pcie_addr;
> + phys_addr_t axi_addr;
> + u32 atr_size;
> + u32 val;
> + int i;
> +
> + for (i = 0; i < port->num_inbound_windows; i++) {
> + atr_size = ilog2(port->inbound_windows[i].size) - 1;
> + atr_size &= GENMASK(5, 0);
> +
> + pcie_addr = port->inbound_windows[i].pci_addr;
> +
> + val = lower_32_bits(pcie_addr) & GENMASK(31, 12);
> + val |= (atr_size << ATR_SIZE_SHIFT);
> + val |= ATR_IMPL_ENABLE;
> + writel(val, bridge_base_addr +
> + ATR0_PCIE_WIN0_SRCADDR_PARAM + (i * ATR_WINDOW_DESC_SIZE));
> + writel(upper_32_bits(pcie_addr), bridge_base_addr +
> + ATR0_PCIE_WIN0_SRC_ADDR + (i * ATR_WINDOW_DESC_SIZE));
> +
> + axi_addr = port->inbound_windows[i].axi_addr;
> +
> + writel(lower_32_bits(axi_addr), bridge_base_addr +
> + ATR0_PCIE_WIN0_TRSL_ADDR_LSB + (i * ATR_WINDOW_DESC_SIZE));
> + writel(upper_32_bits(axi_addr), bridge_base_addr +
> + ATR0_PCIE_WIN0_TRSL_ADDR_UDW + (i * ATR_WINDOW_DESC_SIZE));
> +
> + writel(TRSL_ID_AXI4_MASTER_0, bridge_base_addr +
> + ATR0_PCIE_WIN0_TRSL_PARAM + (i * ATR_WINDOW_DESC_SIZE));
> +
> + dev_dbg(&pdev->dev, "0x%010llx..0x%010llx -> 0x%010llx\n",
> + pcie_addr, pcie_addr + port->inbound_windows[i].size - 1, axi_addr);

If you're going to leave the dbg print in, can you make it more verbose
so that the log message stands on its own?

Other than that, I made most of my comments before submission so LGTM..
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.