[PATCH v3 0/5] armada-370-xp irqchip updates

[PATCH v3 0/5] armada-370-xp irqchip updates

[Marek Behún]

Hi Arnd, Andrew, et al.

this is v3 of updates for armada-370-xp irqchip. There is one small fix,
and another patch added which was previosly sent separately.

Also I realized that I did not send to the SoC mailing list, nor to
Arnd.

As written in previous cover letter: this driver is in need of a major
refactor in order to bring it to modern standards, but that is
unfortunately currently infeasible with my time constraints.

v1 and v2 at:
  https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=863473
  https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=863707

Changes since v2:
- dropped redundant assignment during declaration in patch 4
  (bool src0 = false, src1 = false)
- added patch 5 to this series, previously sent separately

Marek Behún (1):
  irqchip/armada-370-xp: Use atomic_io_modify() instead of another
    spinlock

Pali Rohár (4):
  irqchip/armada-370-xp: Do not allow mapping IRQ 0 and 1
  irqchip/armada-370-xp: Only call ipi_resume() if IPI is available
  irqchip/armada-370-xp: Do not touch IPI registers on platforms without
    IPI
  irqchip/armada-370-xp: Add support for 32 MSI interrupts on non-IPI
    platforms

 drivers/irqchip/irq-armada-370-xp.c | 121 ++++++++++++++++++++++------
 1 file changed, 95 insertions(+), 26 deletions(-)

-- 
2.44.2

[PATCH v3 1/5] irqchip/armada-370-xp: Do not allow mapping IRQ 0 and 1

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

IRQs 0 (IPI) and 1 (MSI) are handled internally by this driver,
generic_handle_domain_irq() is never called for these IRQs.

Disallow mapping these IRQs.

Signed-off-by: Pali Rohár <pali@kernel.org>
[ changed commit message ]
Signed-off-by: Marek Behún <kabel@kernel.org>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/irqchip/irq-armada-370-xp.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
index 4b021a67bdfe..f488c35d9130 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -566,6 +566,10 @@ static struct irq_chip armada_370_xp_irq_chip = {
 static int armada_370_xp_mpic_irq_map(struct irq_domain *h,
 				      unsigned int virq, irq_hw_number_t hw)
 {
+	/* IRQs 0 and 1 cannot be mapped, they are handled internally */
+	if (hw <= 1)
+		return -EINVAL;
+
 	armada_370_xp_irq_mask(irq_get_irq_data(virq));
 	if (!is_percpu_irq(hw))
 		writel(hw, per_cpu_int_base +
-- 
2.44.2

[PATCH v3 2/5] irqchip/armada-370-xp: Only call ipi_resume() if IPI is available

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

IPI is available only on systems where the mpic controller does not
have a parent IRQ defined (e.g. on Armada XP). If a parent IRQ is
defined, inter-processor interrupts are handled by an interrupt
controller higher in the hierarchy (most probably a parent GIC).

Only call ipi_resume() on systems where IPI is available in the mpic
controller.

Signed-off-by: Pali Rohár <pali@kernel.org>
[ refactored a little and changed commit message ]
Signed-off-by: Marek Behún <kabel@kernel.org>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/irqchip/irq-armada-370-xp.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
index f488c35d9130..ea95e327f672 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -29,6 +29,7 @@
 #include <linux/slab.h>
 #include <linux/syscore_ops.h>
 #include <linux/msi.h>
+#include <linux/types.h>
 #include <asm/mach/arch.h>
 #include <asm/exception.h>
 #include <asm/smp_plat.h>
@@ -156,6 +157,17 @@ static DEFINE_MUTEX(msi_used_lock);
 static phys_addr_t msi_doorbell_addr;
 #endif
 
+static inline bool is_ipi_available(void)
+{
+	/*
+	 * We distinguish IPI availability in the IC by the IC not having a
+	 * parent irq defined. If a parent irq is defined, there is a parent
+	 * interrupt controller (e.g. GIC) that takes care of inter-processor
+	 * interrupts.
+	 */
+	return parent_irq <= 0;
+}
+
 static inline bool is_percpu_irq(irq_hw_number_t irq)
 {
 	if (irq <= ARMADA_370_XP_MAX_PER_CPU_IRQS)
@@ -527,7 +539,8 @@ static void armada_xp_mpic_reenable_percpu(void)
 		armada_370_xp_irq_unmask(data);
 	}
 
-	ipi_resume();
+	if (is_ipi_available())
+		ipi_resume();
 
 	armada_370_xp_msi_reenable_percpu();
 }
@@ -750,7 +763,8 @@ static void armada_370_xp_mpic_resume(void)
 	if (doorbell_mask_reg & PCI_MSI_DOORBELL_MASK)
 		writel(1, per_cpu_int_base + ARMADA_370_XP_INT_CLEAR_MASK_OFFS);
 
-	ipi_resume();
+	if (is_ipi_available())
+		ipi_resume();
 }
 
 static struct syscore_ops armada_370_xp_mpic_syscore_ops = {
-- 
2.44.2

[PATCH v3 3/5] irqchip/armada-370-xp: Do not touch IPI registers on platforms without IPI

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

On platforms where IPI is not available in the MPIC, the IPI registers
instead represent an additional set of MSI interrupt registers (currently
unused by the driver).

Do not touch these registers on platforms where IPI is not available in
the MPIC.

Signed-off-by: Pali Rohár <pali@kernel.org>
[ refactored, changed commit message ]
Signed-off-by: Marek Behún <kabel@kernel.org>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/irqchip/irq-armada-370-xp.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
index ea95e327f672..aca64de4e3f8 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -508,6 +508,9 @@ static void armada_xp_mpic_smp_cpu_init(void)
 	for (i = 0; i < nr_irqs; i++)
 		writel(i, per_cpu_int_base + ARMADA_370_XP_INT_SET_MASK_OFFS);
 
+	if (!is_ipi_available())
+		return;
+
 	/* Disable all IPIs */
 	writel(0, per_cpu_int_base + ARMADA_370_XP_IN_DRBEL_MSK_OFFS);
 
@@ -758,7 +761,7 @@ static void armada_370_xp_mpic_resume(void)
 	/* Reconfigure doorbells for IPIs and MSIs */
 	writel(doorbell_mask_reg,
 	       per_cpu_int_base + ARMADA_370_XP_IN_DRBEL_MSK_OFFS);
-	if (doorbell_mask_reg & IPI_DOORBELL_MASK)
+	if (is_ipi_available() && (doorbell_mask_reg & IPI_DOORBELL_MASK))
 		writel(0, per_cpu_int_base + ARMADA_370_XP_INT_CLEAR_MASK_OFFS);
 	if (doorbell_mask_reg & PCI_MSI_DOORBELL_MASK)
 		writel(1, per_cpu_int_base + ARMADA_370_XP_INT_CLEAR_MASK_OFFS);
@@ -809,13 +812,18 @@ static int __init armada_370_xp_mpic_of_init(struct device_node *node,
 	BUG_ON(!armada_370_xp_mpic_domain);
 	irq_domain_update_bus_token(armada_370_xp_mpic_domain, DOMAIN_BUS_WIRED);
 
+	/*
+	 * Initialize parent_irq before calling any other functions, since it is
+	 * used to distinguish between IPI and non-IPI platforms.
+	 */
+	parent_irq = irq_of_parse_and_map(node, 0);
+
 	/* Setup for the boot CPU */
 	armada_xp_mpic_perf_init();
 	armada_xp_mpic_smp_cpu_init();
 
 	armada_370_xp_msi_init(node, main_int_res.start);
 
-	parent_irq = irq_of_parse_and_map(node, 0);
 	if (parent_irq <= 0) {
 		irq_set_default_host(armada_370_xp_mpic_domain);
 		set_handle_irq(armada_370_xp_handle_irq);
-- 
2.44.2

[PATCH v3 4/5] irqchip/armada-370-xp: Add support for 32 MSI interrupts on non-IPI platforms

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

The doorbell interrupts have the following layout on IPI vs no-IPI
platforms:

                    |  0...7  |  8...15  |       16...31       |
  ------------------+---------+----------+---------------------+
       IPI platform |   IPI   |   n/a    |         MSI         |
  ------------------+---------+----------+---------------------+
   non-IPI platform |                   MSI                    |
  ------------------+------------------------------------------+

Currently the driver only allows for the upper 16...31 interrupts for
MSI domain (i.e. the MSI domain has only 16 interrupts).

On platforms where IPI is not available, we can use whole 32 MSI
interrupts.

Implement support also for the lower 16 MSI interrupts on non-IPI
platforms.

Signed-off-by: Pali Rohár <pali@kernel.org>
[ refactored, changed commit message ]
Signed-off-by: Marek Behún <kabel@kernel.org>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/irqchip/irq-armada-370-xp.c | 77 +++++++++++++++++++++++------
 1 file changed, 63 insertions(+), 14 deletions(-)

diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
index aca64de4e3f8..ada257aeba78 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -13,6 +13,7 @@
  * warranty of any kind, whether express or implied.
  */
 
+#include <linux/bits.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -136,6 +137,7 @@
 
 #define ARMADA_370_XP_MAX_PER_CPU_IRQS		(28)
 
+/* IPI and MSI interrupt definitions for IPI platforms */
 #define IPI_DOORBELL_START                      (0)
 #define IPI_DOORBELL_END                        (8)
 #define IPI_DOORBELL_MASK                       0xFF
@@ -144,6 +146,14 @@
 #define PCI_MSI_DOORBELL_END                    (32)
 #define PCI_MSI_DOORBELL_MASK                   0xFFFF0000
 
+/* MSI interrupt definitions for non-IPI platforms */
+#define PCI_MSI_FULL_DOORBELL_START		0
+#define PCI_MSI_FULL_DOORBELL_NR		32
+#define PCI_MSI_FULL_DOORBELL_END		32
+#define PCI_MSI_FULL_DOORBELL_MASK		GENMASK(31, 0)
+#define PCI_MSI_FULL_DOORBELL_SRC0_MASK		GENMASK(15, 0)
+#define PCI_MSI_FULL_DOORBELL_SRC1_MASK		GENMASK(31, 16)
+
 static void __iomem *per_cpu_int_base;
 static void __iomem *main_int_base;
 static struct irq_domain *armada_370_xp_mpic_domain;
@@ -152,7 +162,7 @@ static int parent_irq;
 #ifdef CONFIG_PCI_MSI
 static struct irq_domain *armada_370_xp_msi_domain;
 static struct irq_domain *armada_370_xp_msi_inner_domain;
-static DECLARE_BITMAP(msi_used, PCI_MSI_DOORBELL_NR);
+static DECLARE_BITMAP(msi_used, PCI_MSI_FULL_DOORBELL_NR);
 static DEFINE_MUTEX(msi_used_lock);
 static phys_addr_t msi_doorbell_addr;
 #endif
@@ -168,6 +178,30 @@ static inline bool is_ipi_available(void)
 	return parent_irq <= 0;
 }
 
+static inline u32 msi_doorbell_mask(void)
+{
+	return is_ipi_available() ? PCI_MSI_DOORBELL_MASK :
+				    PCI_MSI_FULL_DOORBELL_MASK;
+}
+
+static inline unsigned int msi_doorbell_start(void)
+{
+	return is_ipi_available() ? PCI_MSI_DOORBELL_START :
+				    PCI_MSI_FULL_DOORBELL_START;
+}
+
+static inline unsigned int msi_doorbell_size(void)
+{
+	return is_ipi_available() ? PCI_MSI_DOORBELL_NR :
+				    PCI_MSI_FULL_DOORBELL_NR;
+}
+
+static inline unsigned int msi_doorbell_end(void)
+{
+	return is_ipi_available() ? PCI_MSI_DOORBELL_END :
+				    PCI_MSI_FULL_DOORBELL_END;
+}
+
 static inline bool is_percpu_irq(irq_hw_number_t irq)
 {
 	if (irq <= ARMADA_370_XP_MAX_PER_CPU_IRQS)
@@ -225,7 +259,7 @@ static void armada_370_xp_compose_msi_msg(struct irq_data *data, struct msi_msg
 
 	msg->address_lo = lower_32_bits(msi_doorbell_addr);
 	msg->address_hi = upper_32_bits(msi_doorbell_addr);
-	msg->data = BIT(cpu + 8) | (data->hwirq + PCI_MSI_DOORBELL_START);
+	msg->data = BIT(cpu + 8) | (data->hwirq + msi_doorbell_start());
 }
 
 static int armada_370_xp_msi_set_affinity(struct irq_data *irq_data,
@@ -258,7 +292,7 @@ static int armada_370_xp_msi_alloc(struct irq_domain *domain, unsigned int virq,
 	int hwirq, i;
 
 	mutex_lock(&msi_used_lock);
-	hwirq = bitmap_find_free_region(msi_used, PCI_MSI_DOORBELL_NR,
+	hwirq = bitmap_find_free_region(msi_used, msi_doorbell_size(),
 					order_base_2(nr_irqs));
 	mutex_unlock(&msi_used_lock);
 
@@ -295,9 +329,10 @@ static void armada_370_xp_msi_reenable_percpu(void)
 	u32 reg;
 
 	/* Enable MSI doorbell mask and combined cpu local interrupt */
-	reg = readl(per_cpu_int_base + ARMADA_370_XP_IN_DRBEL_MSK_OFFS)
-		| PCI_MSI_DOORBELL_MASK;
+	reg = readl(per_cpu_int_base + ARMADA_370_XP_IN_DRBEL_MSK_OFFS);
+	reg |= msi_doorbell_mask();
 	writel(reg, per_cpu_int_base + ARMADA_370_XP_IN_DRBEL_MSK_OFFS);
+
 	/* Unmask local doorbell interrupt */
 	writel(1, per_cpu_int_base + ARMADA_370_XP_INT_CLEAR_MASK_OFFS);
 }
@@ -309,7 +344,7 @@ static int armada_370_xp_msi_init(struct device_node *node,
 		ARMADA_370_XP_SW_TRIG_INT_OFFS;
 
 	armada_370_xp_msi_inner_domain =
-		irq_domain_add_linear(NULL, PCI_MSI_DOORBELL_NR,
+		irq_domain_add_linear(NULL, msi_doorbell_size(),
 				      &armada_370_xp_msi_domain_ops, NULL);
 	if (!armada_370_xp_msi_inner_domain)
 		return -ENOMEM;
@@ -325,6 +360,10 @@ static int armada_370_xp_msi_init(struct device_node *node,
 
 	armada_370_xp_msi_reenable_percpu();
 
+	/* Unmask low 16 MSI irqs on non-IPI platforms */
+	if (!is_ipi_available())
+		writel(0, per_cpu_int_base + ARMADA_370_XP_INT_CLEAR_MASK_OFFS);
+
 	return 0;
 }
 #else
@@ -619,20 +658,20 @@ static void armada_370_xp_handle_msi_irq(struct pt_regs *regs, bool is_chained)
 	u32 msimask, msinr;
 
 	msimask = readl_relaxed(per_cpu_int_base +
-				ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS)
-		& PCI_MSI_DOORBELL_MASK;
+				ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS);
+	msimask &= msi_doorbell_mask();
 
 	writel(~msimask, per_cpu_int_base +
 	       ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS);
 
-	for (msinr = PCI_MSI_DOORBELL_START;
-	     msinr < PCI_MSI_DOORBELL_END; msinr++) {
+	for (msinr = msi_doorbell_start();
+	     msinr < msi_doorbell_end(); msinr++) {
 		unsigned int irq;
 
 		if (!(msimask & BIT(msinr)))
 			continue;
 
-		irq = msinr - PCI_MSI_DOORBELL_START;
+		irq = msinr - msi_doorbell_start();
 
 		generic_handle_domain_irq(armada_370_xp_msi_inner_domain, irq);
 	}
@@ -661,7 +700,7 @@ static void armada_370_xp_mpic_handle_cascade_irq(struct irq_desc *desc)
 		if (!(irqsrc & ARMADA_370_XP_INT_IRQ_FIQ_MASK(cpuid)))
 			continue;
 
-		if (irqn == 1) {
+		if (irqn == 0 || irqn == 1) {
 			armada_370_xp_handle_msi_irq(NULL, true);
 			continue;
 		}
@@ -722,6 +761,7 @@ static int armada_370_xp_mpic_suspend(void)
 
 static void armada_370_xp_mpic_resume(void)
 {
+	bool src0, src1;
 	int nirqs;
 	irq_hw_number_t irq;
 
@@ -761,9 +801,18 @@ static void armada_370_xp_mpic_resume(void)
 	/* Reconfigure doorbells for IPIs and MSIs */
 	writel(doorbell_mask_reg,
 	       per_cpu_int_base + ARMADA_370_XP_IN_DRBEL_MSK_OFFS);
-	if (is_ipi_available() && (doorbell_mask_reg & IPI_DOORBELL_MASK))
+
+	if (is_ipi_available()) {
+		src0 = doorbell_mask_reg & IPI_DOORBELL_MASK;
+		src1 = doorbell_mask_reg & PCI_MSI_DOORBELL_MASK;
+	} else {
+		src0 = doorbell_mask_reg & PCI_MSI_FULL_DOORBELL_SRC0_MASK;
+		src1 = doorbell_mask_reg & PCI_MSI_FULL_DOORBELL_SRC1_MASK;
+	}
+
+	if (src0)
 		writel(0, per_cpu_int_base + ARMADA_370_XP_INT_CLEAR_MASK_OFFS);
-	if (doorbell_mask_reg & PCI_MSI_DOORBELL_MASK)
+	if (src1)
 		writel(1, per_cpu_int_base + ARMADA_370_XP_INT_CLEAR_MASK_OFFS);
 
 	if (is_ipi_available())
-- 
2.44.2

[PATCH v3 5/5] irqchip/armada-370-xp: Use atomic_io_modify() instead of another spinlock

[Marek Behún]

Use the dedicated atomic_io_modify() instead of hardcoded
spin_lock() + readl() + writel() + spin_unlock() sequence.

This allows us to drop the irq_controller_lock spinlock from the driver.

Signed-off-by: Marek Behún <kabel@kernel.org>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/irqchip/irq-armada-370-xp.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
index ada257aeba78..dce2b80bf439 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -512,24 +512,18 @@ static __init void armada_xp_ipi_init(struct device_node *node)
 	set_smp_ipi_range(base_ipi, IPI_DOORBELL_END);
 }
 
-static DEFINE_RAW_SPINLOCK(irq_controller_lock);
-
 static int armada_xp_set_affinity(struct irq_data *d,
 				  const struct cpumask *mask_val, bool force)
 {
 	irq_hw_number_t hwirq = irqd_to_hwirq(d);
-	unsigned long reg, mask;
 	int cpu;
 
 	/* Select a single core from the affinity mask which is online */
 	cpu = cpumask_any_and(mask_val, cpu_online_mask);
-	mask = 1UL << cpu_logical_map(cpu);
 
-	raw_spin_lock(&irq_controller_lock);
-	reg = readl(main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
-	reg = (reg & (~ARMADA_370_XP_INT_SOURCE_CPU_MASK)) | mask;
-	writel(reg, main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
-	raw_spin_unlock(&irq_controller_lock);
+	atomic_io_modify(main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq),
+			 ARMADA_370_XP_INT_SOURCE_CPU_MASK,
+			 BIT(cpu_logical_map(cpu)));
 
 	irq_data_update_effective_affinity(d, cpumask_of(cpu));
 
-- 
2.44.2

Re: [PATCH v3 0/5] armada-370-xp irqchip updates

[Marek Behún]

On Fri, 21 Jun 2024 11:38:27 +0200
Marek Behún <kabel@kernel.org> wrote:

> Hi Arnd, Andrew, et al.
> 
> this is v3 of updates for armada-370-xp irqchip. There is one small fix,
> and another patch added which was previosly sent separately.
> 
> Also I realized that I did not send to the SoC mailing list, nor to
> Arnd.
> 
> As written in previous cover letter: this driver is in need of a major
> refactor in order to bring it to modern standards, but that is
> unfortunately currently infeasible with my time constraints.
> 
> v1 and v2 at:
>   https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=863473
>   https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=863707
> 
> Changes since v2:
> - dropped redundant assignment during declaration in patch 4
>   (bool src0 = false, src1 = false)
> - added patch 5 to this series, previously sent separately
> 
> Marek Behún (1):
>   irqchip/armada-370-xp: Use atomic_io_modify() instead of another
>     spinlock
> 
> Pali Rohár (4):
>   irqchip/armada-370-xp: Do not allow mapping IRQ 0 and 1
>   irqchip/armada-370-xp: Only call ipi_resume() if IPI is available
>   irqchip/armada-370-xp: Do not touch IPI registers on platforms without
>     IPI
>   irqchip/armada-370-xp: Add support for 32 MSI interrupts on non-IPI
>     platforms
> 
>  drivers/irqchip/irq-armada-370-xp.c | 121 ++++++++++++++++++++++------
>  1 file changed, 95 insertions(+), 26 deletions(-)
> 

Hi Arnd,

just noting that these changes were also accepted, by Thomas Glexiner.
No need to apply to soc. (I marked them as Awainting Upstream on
patchwork).

Marek