diff mbox series

[v1,5/9] PCI: microchip: Gather MSI information from hardware config registers

Message ID 20221116135504.258687-6-daire.mcnamara@microchip.com (mailing list archive)
State Superseded
Headers show
Series PCI: microchip: Partition address translations | expand

Checks

Context Check Description
conchuod/tree_selection fail Guessing tree name failed

Commit Message

Daire McNamara Nov. 16, 2022, 1:55 p.m. UTC 
From: Daire McNamara <daire.mcnamara@microchip.com>

The PCIe root complex on PolarFire SoC is configured at bitstream creation
time using Libero.  Key MSI-related parameters include the number of
MSIs (1/2/4/8/16/32) and the MSI address. In the device driver, extract
this information from hw registers at init time, and use it to configure
MSI system, including configuring MSI capability structure correctly in
configuration space.

Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/pci/controller/pcie-microchip-host.c | 73 +++++++++++---------
 1 file changed, 40 insertions(+), 33 deletions(-)

Comments

Bjorn Helgaas Nov. 16, 2022, 4:41 p.m. UTC | #1 
On Wed, Nov 16, 2022 at 01:55:00PM +0000, daire.mcnamara@microchip.com wrote:
> From: Daire McNamara <daire.mcnamara@microchip.com>
> 
> The PCIe root complex on PolarFire SoC is configured at bitstream creation
> time using Libero.  Key MSI-related parameters include the number of
> MSIs (1/2/4/8/16/32) and the MSI address. In the device driver, extract
> this information from hw registers at init time, and use it to configure
> MSI system, including configuring MSI capability structure correctly in
> configuration space.

Minor nits for v2.

> +	/* fixup msi enable flag */

s/msi/MSI/ here and comments below to match other usage.

> +	reg = readw_relaxed(ecam + MC_MSI_CAP_CTRL_OFFSET + PCI_MSI_FLAGS);
> +	reg |= PCI_MSI_FLAGS_ENABLE;
> +	writew_relaxed(reg, ecam + MC_MSI_CAP_CTRL_OFFSET + PCI_MSI_FLAGS);
> +
> +	/* fixup msi queue flags */
> +	queue_size = reg & PCI_MSI_FLAGS_QMASK;
> +	queue_size >>= 1;
> +	reg &= ~PCI_MSI_FLAGS_QSIZE;
> +	reg |= queue_size << 4;
> +	writew_relaxed(reg, ecam + MC_MSI_CAP_CTRL_OFFSET + PCI_MSI_FLAGS);
> +
> +	/* fixup msi addr fields */

> +	/* allow enabling msi by disabling msi-x */

s/msi-x/MSI-X/
Conor Dooley Nov. 23, 2022, 10:09 p.m. UTC | #2 
On Wed, Nov 16, 2022 at 01:55:00PM +0000, daire.mcnamara@microchip.com wrote:
> From: Daire McNamara <daire.mcnamara@microchip.com>
> 
> The PCIe root complex on PolarFire SoC is configured at bitstream creation
> time using Libero.  Key MSI-related parameters include the number of
> MSIs (1/2/4/8/16/32) and the MSI address. In the device driver, extract
> this information from hw registers at init time, and use it to configure

I don't know the answer here, so I am not being difficult:
Does the HSS program them in based on the XML produced alongside the
bitstream, or is that information encoded in the bitstream itself?

> MSI system, including configuring MSI capability structure correctly in
> configuration space.
> 
> Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  drivers/pci/controller/pcie-microchip-host.c | 73 +++++++++++---------
>  1 file changed, 40 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-microchip-host.c b/drivers/pci/controller/pcie-microchip-host.c
> index ecd4d3f3e3d4..faecf419ad6f 100644
> --- a/drivers/pci/controller/pcie-microchip-host.c
> +++ b/drivers/pci/controller/pcie-microchip-host.c
> @@ -20,8 +20,7 @@
>  #include "../pci.h"
>  
>  /* Number of MSI IRQs */
> -#define MC_NUM_MSI_IRQS				32
> -#define MC_NUM_MSI_IRQS_CODED			5
> +#define MC_MAX_NUM_MSI_IRQS			32
>  
>  /* PCIe Bridge Phy and Controller Phy offsets */
>  #define MC_PCIE1_BRIDGE_ADDR			0x00008000u
> @@ -31,6 +30,11 @@
>  #define MC_PCIE_CTRL_ADDR			(MC_PCIE1_CTRL_ADDR)
>  
>  /* PCIe Bridge Phy Regs */
> +#define PCIE_PCI_IRQ_DW0			0xa8
> +#define  MSIX_CAP_MASK				BIT(31)
> +#define  NUM_MSI_MSGS_MASK			GENMASK(6, 4)
> +#define  NUM_MSI_MSGS_SHIFT			4
> +
>  #define IMASK_LOCAL				0x180
>  #define  DMA_END_ENGINE_0_MASK			0x00000000u
>  #define  DMA_END_ENGINE_0_SHIFT			0
> @@ -79,7 +83,6 @@
>  #define IMASK_HOST				0x188
>  #define ISTATUS_HOST				0x18c
>  #define IMSI_ADDR				0x190
> -#define  MSI_ADDR				0x190
>  #define ISTATUS_MSI				0x194
>  
>  /* PCIe Master table init defines */
> @@ -156,8 +159,6 @@
>  
>  /* PCIe Config space MSI capability structure */
>  #define MC_MSI_CAP_CTRL_OFFSET			0xe0u
> -#define  MC_MSI_MAX_Q_AVAIL			(MC_NUM_MSI_IRQS_CODED << 1)
> -#define  MC_MSI_Q_SIZE				(MC_NUM_MSI_IRQS_CODED << 4)
>  
>  /* Events */
>  #define EVENT_PCIE_L2_EXIT			0
> @@ -257,7 +258,7 @@ struct mc_msi {
>  	struct irq_domain *dev_domain;
>  	u32 num_vectors;
>  	u64 vector_phy;
> -	DECLARE_BITMAP(used, MC_NUM_MSI_IRQS);
> +	DECLARE_BITMAP(used, MC_MAX_NUM_MSI_IRQS);
>  };
>  
>  struct mc_pcie {
> @@ -380,25 +381,29 @@ static struct {
>  
>  static char poss_clks[][5] = { "fic0", "fic1", "fic2", "fic3" };
>  
> -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)
>  {
>  	struct mc_msi *msi = &port->msi;
> -	u32 cap_offset = MC_MSI_CAP_CTRL_OFFSET;
> -	u16 msg_ctrl = readw_relaxed(base + cap_offset + PCI_MSI_FLAGS);
> -
> -	msg_ctrl |= PCI_MSI_FLAGS_ENABLE;
> -	msg_ctrl &= ~PCI_MSI_FLAGS_QMASK;
> -	msg_ctrl |= MC_MSI_MAX_Q_AVAIL;
> -	msg_ctrl &= ~PCI_MSI_FLAGS_QSIZE;
> -	msg_ctrl |= MC_MSI_Q_SIZE;
> -	msg_ctrl |= PCI_MSI_FLAGS_64BIT;
> -
> -	writew_relaxed(msg_ctrl, base + cap_offset + PCI_MSI_FLAGS);
> -
> +	u16 reg;
> +	u8 queue_size;
> +
> +	/* fixup msi enable flag */
> +	reg = readw_relaxed(ecam + MC_MSI_CAP_CTRL_OFFSET + PCI_MSI_FLAGS);
> +	reg |= PCI_MSI_FLAGS_ENABLE;
> +	writew_relaxed(reg, ecam + MC_MSI_CAP_CTRL_OFFSET + PCI_MSI_FLAGS);
> +
> +	/* fixup msi queue flags */
> +	queue_size = reg & PCI_MSI_FLAGS_QMASK;
> +	queue_size >>= 1;
> +	reg &= ~PCI_MSI_FLAGS_QSIZE;
> +	reg |= queue_size << 4;

Could you please explain where the >> 1 & << 4 come from? Maybe it's
sufficient to just make them defines, or the comment here could be
expanded on.

> +	writew_relaxed(reg, ecam + MC_MSI_CAP_CTRL_OFFSET + PCI_MSI_FLAGS);
> +
> +	/* fixup msi addr fields */
>  	writel_relaxed(lower_32_bits(msi->vector_phy),
> -		       base + cap_offset + PCI_MSI_ADDRESS_LO);
> +		       ecam + MC_MSI_CAP_CTRL_OFFSET + PCI_MSI_ADDRESS_LO);
>  	writel_relaxed(upper_32_bits(msi->vector_phy),
> -		       base + cap_offset + PCI_MSI_ADDRESS_HI);
> +		       ecam + MC_MSI_CAP_CTRL_OFFSET + PCI_MSI_ADDRESS_HI);
>  }
>  
>  static void mc_handle_msi(struct irq_desc *desc)
> @@ -471,10 +476,7 @@ static int mc_irq_msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
>  {
>  	struct mc_pcie *port = domain->host_data;
>  	struct mc_msi *msi = &port->msi;
> -	void __iomem *bridge_base_addr =
> -		port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
>  	unsigned long bit;
> -	u32 val;
>  
>  	mutex_lock(&msi->lock);
>  	bit = find_first_zero_bit(msi->used, msi->num_vectors);
> @@ -488,11 +490,6 @@ static int mc_irq_msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
>  	irq_domain_set_info(domain, virq, bit, &mc_msi_bottom_irq_chip,
>  			    domain->host_data, handle_edge_irq, NULL, NULL);
>  
> -	/* Enable MSI interrupts */
> -	val = readl_relaxed(bridge_base_addr + IMASK_LOCAL);
> -	val |= PM_MSI_INT_MSI_MASK;
> -	writel_relaxed(val, bridge_base_addr + IMASK_LOCAL);
> -
>  	mutex_unlock(&msi->lock);
>  
>  	return 0;
> @@ -1102,6 +1099,7 @@ static int mc_platform_init(struct pci_config_window *cfg)
>  	void __iomem *bridge_base_addr;
>  	void __iomem *ctrl_base_addr;
>  	int ret;
> +	u32 val;
>  
>  	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
>  	if (!port)
> @@ -1123,11 +1121,20 @@ static int mc_platform_init(struct pci_config_window *cfg)
>  	bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
>  	ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
>  
> -	port->msi.vector_phy = MSI_ADDR;
> -	port->msi.num_vectors = MC_NUM_MSI_IRQS;
> +	/* allow enabling msi by disabling msi-x */
> +	val = readl(bridge_base_addr + PCIE_PCI_IRQ_DW0);
> +	val &= ~MSIX_CAP_MASK;
> +	writel(val, bridge_base_addr + PCIE_PCI_IRQ_DW0);
> +
> +	/* pick num vectors from design */
> +	val = readl(bridge_base_addr + PCIE_PCI_IRQ_DW0);
> +	val &= NUM_MSI_MSGS_MASK;
> +	val >>= NUM_MSI_MSGS_SHIFT;
> +
> +	port->msi.num_vectors = 1 << val;
>  
> -	/* Hardware doesn't setup MSI by default */
> -	mc_pcie_enable_msi(port, cfg->win);
> +	/* pick vector address from design */

"Design" might make sense to you or I, but I think you could expand this
comment for the lay reader. Copy-paste from the commit message maybe
hah.

Anyways, as with the rest of the series - I'm just nitpicking.
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

> +	port->msi.vector_phy = readl_relaxed(bridge_base_addr + IMSI_ADDR);
>  
>  	/* Configure Address Translation Table 0 for PCIe config space */
>  	mc_pcie_setup_window(bridge_base_addr, 0, cfg->res.start & 0xffffffff,
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
diff mbox series

Patch

diff --git a/drivers/pci/controller/pcie-microchip-host.c b/drivers/pci/controller/pcie-microchip-host.c
index ecd4d3f3e3d4..faecf419ad6f 100644
--- a/drivers/pci/controller/pcie-microchip-host.c
+++ b/drivers/pci/controller/pcie-microchip-host.c
@@ -20,8 +20,7 @@ 
 #include "../pci.h"
 
 /* Number of MSI IRQs */
-#define MC_NUM_MSI_IRQS				32
-#define MC_NUM_MSI_IRQS_CODED			5
+#define MC_MAX_NUM_MSI_IRQS			32
 
 /* PCIe Bridge Phy and Controller Phy offsets */
 #define MC_PCIE1_BRIDGE_ADDR			0x00008000u
@@ -31,6 +30,11 @@ 
 #define MC_PCIE_CTRL_ADDR			(MC_PCIE1_CTRL_ADDR)
 
 /* PCIe Bridge Phy Regs */
+#define PCIE_PCI_IRQ_DW0			0xa8
+#define  MSIX_CAP_MASK				BIT(31)
+#define  NUM_MSI_MSGS_MASK			GENMASK(6, 4)
+#define  NUM_MSI_MSGS_SHIFT			4
+
 #define IMASK_LOCAL				0x180
 #define  DMA_END_ENGINE_0_MASK			0x00000000u
 #define  DMA_END_ENGINE_0_SHIFT			0
@@ -79,7 +83,6 @@ 
 #define IMASK_HOST				0x188
 #define ISTATUS_HOST				0x18c
 #define IMSI_ADDR				0x190
-#define  MSI_ADDR				0x190
 #define ISTATUS_MSI				0x194
 
 /* PCIe Master table init defines */
@@ -156,8 +159,6 @@ 
 
 /* PCIe Config space MSI capability structure */
 #define MC_MSI_CAP_CTRL_OFFSET			0xe0u
-#define  MC_MSI_MAX_Q_AVAIL			(MC_NUM_MSI_IRQS_CODED << 1)
-#define  MC_MSI_Q_SIZE				(MC_NUM_MSI_IRQS_CODED << 4)
 
 /* Events */
 #define EVENT_PCIE_L2_EXIT			0
@@ -257,7 +258,7 @@  struct mc_msi {
 	struct irq_domain *dev_domain;
 	u32 num_vectors;
 	u64 vector_phy;
-	DECLARE_BITMAP(used, MC_NUM_MSI_IRQS);
+	DECLARE_BITMAP(used, MC_MAX_NUM_MSI_IRQS);
 };
 
 struct mc_pcie {
@@ -380,25 +381,29 @@  static struct {
 
 static char poss_clks[][5] = { "fic0", "fic1", "fic2", "fic3" };
 
-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)
 {
 	struct mc_msi *msi = &port->msi;
-	u32 cap_offset = MC_MSI_CAP_CTRL_OFFSET;
-	u16 msg_ctrl = readw_relaxed(base + cap_offset + PCI_MSI_FLAGS);
-
-	msg_ctrl |= PCI_MSI_FLAGS_ENABLE;
-	msg_ctrl &= ~PCI_MSI_FLAGS_QMASK;
-	msg_ctrl |= MC_MSI_MAX_Q_AVAIL;
-	msg_ctrl &= ~PCI_MSI_FLAGS_QSIZE;
-	msg_ctrl |= MC_MSI_Q_SIZE;
-	msg_ctrl |= PCI_MSI_FLAGS_64BIT;
-
-	writew_relaxed(msg_ctrl, base + cap_offset + PCI_MSI_FLAGS);
-
+	u16 reg;
+	u8 queue_size;
+
+	/* fixup msi enable flag */
+	reg = readw_relaxed(ecam + MC_MSI_CAP_CTRL_OFFSET + PCI_MSI_FLAGS);
+	reg |= PCI_MSI_FLAGS_ENABLE;
+	writew_relaxed(reg, ecam + MC_MSI_CAP_CTRL_OFFSET + PCI_MSI_FLAGS);
+
+	/* fixup msi queue flags */
+	queue_size = reg & PCI_MSI_FLAGS_QMASK;
+	queue_size >>= 1;
+	reg &= ~PCI_MSI_FLAGS_QSIZE;
+	reg |= queue_size << 4;
+	writew_relaxed(reg, ecam + MC_MSI_CAP_CTRL_OFFSET + PCI_MSI_FLAGS);
+
+	/* fixup msi addr fields */
 	writel_relaxed(lower_32_bits(msi->vector_phy),
-		       base + cap_offset + PCI_MSI_ADDRESS_LO);
+		       ecam + MC_MSI_CAP_CTRL_OFFSET + PCI_MSI_ADDRESS_LO);
 	writel_relaxed(upper_32_bits(msi->vector_phy),
-		       base + cap_offset + PCI_MSI_ADDRESS_HI);
+		       ecam + MC_MSI_CAP_CTRL_OFFSET + PCI_MSI_ADDRESS_HI);
 }
 
 static void mc_handle_msi(struct irq_desc *desc)
@@ -471,10 +476,7 @@  static int mc_irq_msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
 {
 	struct mc_pcie *port = domain->host_data;
 	struct mc_msi *msi = &port->msi;
-	void __iomem *bridge_base_addr =
-		port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
 	unsigned long bit;
-	u32 val;
 
 	mutex_lock(&msi->lock);
 	bit = find_first_zero_bit(msi->used, msi->num_vectors);
@@ -488,11 +490,6 @@  static int mc_irq_msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
 	irq_domain_set_info(domain, virq, bit, &mc_msi_bottom_irq_chip,
 			    domain->host_data, handle_edge_irq, NULL, NULL);
 
-	/* Enable MSI interrupts */
-	val = readl_relaxed(bridge_base_addr + IMASK_LOCAL);
-	val |= PM_MSI_INT_MSI_MASK;
-	writel_relaxed(val, bridge_base_addr + IMASK_LOCAL);
-
 	mutex_unlock(&msi->lock);
 
 	return 0;
@@ -1102,6 +1099,7 @@  static int mc_platform_init(struct pci_config_window *cfg)
 	void __iomem *bridge_base_addr;
 	void __iomem *ctrl_base_addr;
 	int ret;
+	u32 val;
 
 	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
 	if (!port)
@@ -1123,11 +1121,20 @@  static int mc_platform_init(struct pci_config_window *cfg)
 	bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
 	ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
 
-	port->msi.vector_phy = MSI_ADDR;
-	port->msi.num_vectors = MC_NUM_MSI_IRQS;
+	/* allow enabling msi by disabling msi-x */
+	val = readl(bridge_base_addr + PCIE_PCI_IRQ_DW0);
+	val &= ~MSIX_CAP_MASK;
+	writel(val, bridge_base_addr + PCIE_PCI_IRQ_DW0);
+
+	/* pick num vectors from design */
+	val = readl(bridge_base_addr + PCIE_PCI_IRQ_DW0);
+	val &= NUM_MSI_MSGS_MASK;
+	val >>= NUM_MSI_MSGS_SHIFT;
+
+	port->msi.num_vectors = 1 << val;
 
-	/* Hardware doesn't setup MSI by default */
-	mc_pcie_enable_msi(port, cfg->win);
+	/* pick vector address from design */
+	port->msi.vector_phy = readl_relaxed(bridge_base_addr + IMSI_ADDR);
 
 	/* Configure Address Translation Table 0 for PCIe config space */
 	mc_pcie_setup_window(bridge_base_addr, 0, cfg->res.start & 0xffffffff,