On Wed, Jul 19, 2023 at 12:41:35PM -0500, Bjorn Helgaas wrote: > On Fri, Jun 30, 2023 at 04:48:58PM +0100, daire.mcnamara@xxxxxxxxxxxxx wrote: > > From: Daire McNamara <daire.mcnamara@xxxxxxxxxxxxx> > > > > After improving driver to get MSI-related information from > > configuration registers (set at power on from the Libero FPGA > > design), its now clear that mc_pcie_enable_msi() is not a good > > it's (contraction of "it is") > > > name for this function. The function is better named as > > mc_pcie_fixup_ecam() as its purpose is to correct the queue > > size of the MSI CAP CTRL. > > > -static void mc_pcie_enable_msi(struct mc_pcie *port, void __iomem *base) > > +static void mc_pcie_fixup_ecam(struct mc_pcie *port, void __iomem *ecam) > > Since the purpose of this seems to be to fix stuff in the MSI cap, > removing "msi" from the name seems weird. The fact that it uses ECAM > to access the registers is incidental. > > > - msg_ctrl &= ~PCI_MSI_FLAGS_QSIZE; > > - msg_ctrl |= queue_size << 4; > > - writew_relaxed(msg_ctrl, base + cap_offset + PCI_MSI_FLAGS); > > + reg &= ~PCI_MSI_FLAGS_QSIZE; > > + reg |= queue_size << 4; > > Could maybe use FIELD_PREP() instead of the shift? I guess this would > go in the "Gather MSI information" patch. Daire, can you follow up on these review comments please ? Thanks, Lorenzo > > Bjorn
