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

[Bjorn Helgaas <helgaas@kernel.org>]

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);