diff mbox series

[v2,04/15] PCI: xilinx: Don't allocate extra memory for the MSI capture address

Message ID 20210322184614.802565-5-maz@kernel.org (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series PCI/MSI: Getting rid of msi_controller, and other cleanups | expand

Commit Message

Marc Zyngier March 22, 2021, 6:46 p.m. UTC
A long cargo-culted behaviour of PCI drivers is to allocate memory
to obtain an address that is fed to the controller as the MSI
capture address (i.e. the MSI doorbell).

But there is no actual requirement for this address to be RAM.
All it needs to be is a suitable aligned address that will
*not* be DMA'd to.

Use the physical address of the 'port' data structure as the MSI
capture address.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/pci/controller/pcie-xilinx.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

Comments

Bharat Kumar Gogada March 24, 2021, 12:35 p.m. UTC | #1
Thanks Marc for the patch.
> Subject: [PATCH v2 04/15] PCI: xilinx: Don't allocate extra memory for the
> MSI capture address
> 
> A long cargo-culted behaviour of PCI drivers is to allocate memory to obtain
> an address that is fed to the controller as the MSI capture address (i.e. the
> MSI doorbell).
> 
> But there is no actual requirement for this address to be RAM.
> All it needs to be is a suitable aligned address that will
> *not* be DMA'd to.
> 
> Use the physical address of the 'port' data structure as the MSI capture
> address.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  drivers/pci/controller/pcie-xilinx.c | 18 ++++++------------
>  1 file changed, 6 insertions(+), 12 deletions(-)

...
> -	msg.address_hi = 0;
> -	msg.address_lo = msg_addr;
> +	msg.address_hi = upper_32_bits(msg_addr);
> +	msg.address_lo = lower_32_bits(msg_addr);

The XILINX_PCIE_REG_MSIBASE2 register expects 4KB aligned address.
The lower 12-bits are always set to 0 in this register. So we need to mask the address 
while programming address to EP.

#define XILINX_PCIE_MSI_ADDR_MASK       GENMASK(31, 12)
msg.address_lo = lower_32_bits(msg_addr) & XILINX_PCIE_MSI_ADDR_MASK;


>  	msg.data = irq;
>

Regards,
Bharat
Marc Zyngier March 24, 2021, 12:55 p.m. UTC | #2
On Wed, 24 Mar 2021 12:35:58 +0000,
Bharat Kumar Gogada <bharatku@xilinx.com> wrote:
> 
> Thanks Marc for the patch.
> > Subject: [PATCH v2 04/15] PCI: xilinx: Don't allocate extra memory for the
> > MSI capture address
> > 
> > A long cargo-culted behaviour of PCI drivers is to allocate memory to obtain
> > an address that is fed to the controller as the MSI capture address (i.e. the
> > MSI doorbell).
> > 
> > But there is no actual requirement for this address to be RAM.
> > All it needs to be is a suitable aligned address that will
> > *not* be DMA'd to.
> > 
> > Use the physical address of the 'port' data structure as the MSI capture
> > address.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  drivers/pci/controller/pcie-xilinx.c | 18 ++++++------------
> >  1 file changed, 6 insertions(+), 12 deletions(-)
> 
> ...
> > -	msg.address_hi = 0;
> > -	msg.address_lo = msg_addr;
> > +	msg.address_hi = upper_32_bits(msg_addr);
> > +	msg.address_lo = lower_32_bits(msg_addr);
> 
> The XILINX_PCIE_REG_MSIBASE2 register expects 4KB aligned address.
> The lower 12-bits are always set to 0 in this register. So we need
> to mask the address while programming address to

Thanks for the heads up, I'll fix this up. Does it work correctly once
the address is aligned?

	M.
diff mbox series

Patch

diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
index fa5baeb82653..3a762ffb0cef 100644
--- a/drivers/pci/controller/pcie-xilinx.c
+++ b/drivers/pci/controller/pcie-xilinx.c
@@ -94,7 +94,6 @@ 
  * struct xilinx_pcie_port - PCIe port information
  * @reg_base: IO Mapped Register Base
  * @irq: Interrupt number
- * @msi_pages: MSI pages
  * @dev: Device pointer
  * @msi_domain: MSI IRQ domain pointer
  * @leg_domain: Legacy IRQ domain pointer
@@ -103,7 +102,6 @@ 
 struct xilinx_pcie_port {
 	void __iomem *reg_base;
 	u32 irq;
-	unsigned long msi_pages;
 	struct device *dev;
 	struct irq_domain *msi_domain;
 	struct irq_domain *leg_domain;
@@ -274,10 +272,10 @@  static int xilinx_pcie_msi_setup_irq(struct msi_controller *chip,
 
 	irq_set_msi_desc(irq, desc);
 
-	msg_addr = virt_to_phys((void *)port->msi_pages);
+	msg_addr = virt_to_phys(port);
 
-	msg.address_hi = 0;
-	msg.address_lo = msg_addr;
+	msg.address_hi = upper_32_bits(msg_addr);
+	msg.address_lo = lower_32_bits(msg_addr);
 	msg.data = irq;
 
 	pci_write_msi_msg(irq, &msg);
@@ -330,13 +328,9 @@  static int xilinx_pcie_enable_msi(struct xilinx_pcie_port *port)
 {
 	phys_addr_t msg_addr;
 
-	port->msi_pages = __get_free_pages(GFP_KERNEL, 0);
-	if (!port->msi_pages)
-		return -ENOMEM;
-
-	msg_addr = virt_to_phys((void *)port->msi_pages);
-	pcie_write(port, 0x0, XILINX_PCIE_REG_MSIBASE1);
-	pcie_write(port, msg_addr, XILINX_PCIE_REG_MSIBASE2);
+	msg_addr = virt_to_phys(port);
+	pcie_write(port, upper_32_bit(msg_addr), XILINX_PCIE_REG_MSIBASE1);
+	pcie_write(port, lower_32_bits(msg_addr), XILINX_PCIE_REG_MSIBASE2);
 
 	return 0;
 }