On Wed, Aug 21, 2024 at 02:02:16PM +0100, daire.mcnamara@xxxxxxxxxxxxx wrote: > From: Daire McNamara <daire.mcnamara@xxxxxxxxxxxxx> > > 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);