Re: [PATCH v8 2/3] PCI: microchip: Fix inbound address translation tables

Bjorn Helgaas
2024-08-21T17:17:14Z

On Wed, Aug 21, 2024 at 02:02:16PM +0100, daire.mcnamara@microchip.com wrote:
> From: Daire McNamara <daire.mcnamara@microchip.com>
>
> On Microchip PolarFire SoC the PCIe Root Port can be behind one of three
> general purpose Fabric Interface Controller (FIC) buses that encapsulates
> an AXI-S bus. Depending on which FIC(s) the Root Port is connected
> through to CPU space, and what address translation is done by that FIC,
> the Root Port driver's inbound address translation may vary.
>
> For all current supported designs and all future expected designs,
> inbound address translation done by a FIC on PolarFire SoC varies
> depending on whether PolarFire SoC in operating in coherent DMA mode or
> noncoherent DMA mode.

s/in operating/is operating/

> The setup of the outbound address translation tables in the Root Port
> driver only needs to handle these two cases.
>
> Setup the inbound address translation tables to one of two address
> translations, depending on whether the rootport is being used with coherent
> DMA or noncoherent DMA.

s/rootport/Root Port/ to match above

> +static void mc_pcie_setup_inbound_atr(int window_index, u64 axi_addr, u64 pcie_addr, u64 size)

Most of this file fits in 80 columns, maybe these new decls could, too.

> +static int mc_pcie_setup_inbound_ranges(struct platform_device *pdev, struct mc_pcie *port)

> @@ -525,13 +529,20 @@ void plda_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
> val = upper_32_bits(pci_addr);
> writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
> ATR0_AXI4_SLV0_TRSL_ADDR_UDW);
> +}
> +EXPORT_SYMBOL_GPL(plda_pcie_setup_window);

I think the caller that needs this export is in a previous patch?

I wish we didn't need to export symbols like these since they're
really private to the driver, but I didn't look into the module
structure here.

Also, I get this error when building after both patch 1/3 and 2/3:

drivers/pci/controller/plda/pcie-microchip-host.c:617:5: error: no previous prototype for ‘mc_pcie_setup_iomems’ [-Werror=missing-prototypes]
617 | int mc_pcie_setup_iomems(struct pci_host_bridge *bridge,
| ^~~~~~~~~~~~~~~~~~~~

> +++ b/drivers/pci/controller/plda/pcie-starfive.c
> @@ -355,6 +355,11 @@ static int starfive_pcie_host_init(struct plda_pcie_rp *plda)
> */
> plda_pcie_set_pref_win_64bit(plda);
>
> + /*
> + * Setup the inbound address translation
> + */

Could be a single-line comment: /* Setup the ... */

> + plda_pcie_setup_inbound_address_translation(plda);