Re: [PATCH v2 7/8] PCI: microchip: Rename and refactor mc_pcie_enable_msi()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux