From: Paul Burton <[email protected]>
A few pieces of code in the MIPS GIC driver operate on the GIC local
register block for each online CPU, accessing each via the GIC's
other/redirect register block. This patch abstracts the process of
iterating over online CPUs & configuring the other/redirect region to
access their registers through a new gic_with_each_online_cpu() macro.
This simplifies users of the new macro slightly, and more importantly
prepares us for handling multi-cluster systems where the register
configuration will be done via the CM's GCR_CL_REDIRECT register. By
abstracting all other/redirect block configuration through this macro,
and the __gic_with_next_online_cpu() function which backs it, users will
trivially gain support for multi-cluster when it is implemented in
__gic_with_next_online_cpu().
Signed-off-by: Paul Burton <[email protected]>
Signed-off-by: Chao-ying Fu <[email protected]>
diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index ff89b36267dd..4872bebe24cf 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -65,6 +65,45 @@ static struct gic_all_vpes_chip_data {
bool mask;
} gic_all_vpes_chip_data[GIC_NUM_LOCAL_INTRS];
+static int __gic_with_next_online_cpu(int prev)
+{
+ unsigned int cpu;
+
+ /* Discover the next online CPU */
+ cpu = cpumask_next(prev, cpu_online_mask);
+
+ /* If there isn't one, we're done */
+ if (cpu >= nr_cpu_ids)
+ return cpu;
+
+ /*
+ * Lock access to the next CPU's GIC local register block.
+ *
+ * In the single cluster case we simply set GIC_VL_OTHER. The caller
+ * holds gic_lock so nothing can clobber the value we write.
+ */
+ write_gic_vl_other(mips_cm_vp_id(cpu));
+
+ return cpu;
+}
+
+/**
+ * gic_with_each_online_cpu() - Iterate over online CPUs, access local registers
+ * @cpu: An integer variable to hold the current CPU number
+ *
+ * Iterate over online CPUs & configure the other/redirect register region to
+ * access each CPUs GIC local register block, which can be accessed from the
+ * loop body using read_gic_vo_*() or write_gic_vo_*() accessor functions or
+ * their derivatives.
+ *
+ * The caller must hold gic_lock throughout the loop, such that GIC_VL_OTHER
+ * cannot be clobbered.
+ */
+#define gic_with_each_online_cpu(cpu) \
+ for ((cpu) = __gic_with_next_online_cpu(-1); \
+ (cpu) = __gic_with_next_online_cpu(cpu), \
+ (cpu) < nr_cpu_ids;)
+
static void gic_clear_pcpu_masks(unsigned int intr)
{
unsigned int i;
@@ -357,10 +396,8 @@ static void gic_mask_local_irq_all_vpes(struct irq_data *d)
cd->mask = false;
spin_lock_irqsave(&gic_lock, flags);
- for_each_online_cpu(cpu) {
- write_gic_vl_other(mips_cm_vp_id(cpu));
+ gic_with_each_online_cpu(cpu)
write_gic_vo_rmask(BIT(intr));
- }
spin_unlock_irqrestore(&gic_lock, flags);
}
@@ -375,10 +412,8 @@ static void gic_unmask_local_irq_all_vpes(struct irq_data *d)
cd->mask = true;
spin_lock_irqsave(&gic_lock, flags);
- for_each_online_cpu(cpu) {
- write_gic_vl_other(mips_cm_vp_id(cpu));
+ gic_with_each_online_cpu(cpu)
write_gic_vo_smask(BIT(intr));
- }
spin_unlock_irqrestore(&gic_lock, flags);
}
@@ -532,10 +567,8 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int virq,
return -EPERM;
spin_lock_irqsave(&gic_lock, flags);
- for_each_online_cpu(cpu) {
- write_gic_vl_other(mips_cm_vp_id(cpu));
+ gic_with_each_online_cpu(cpu)
write_gic_vo_map(mips_gic_vx_map_reg(intr), map);
- }
spin_unlock_irqrestore(&gic_lock, flags);
return 0;
--
2.17.1
On Thu, 19 May 2022 19:51:16 +0100,
Dragan Mladjenovic <[email protected]> wrote:
>
> From: Paul Burton <[email protected]>
>
> A few pieces of code in the MIPS GIC driver operate on the GIC local
> register block for each online CPU, accessing each via the GIC's
> other/redirect register block. This patch abstracts the process of
> iterating over online CPUs & configuring the other/redirect region to
> access their registers through a new gic_with_each_online_cpu() macro.
>
> This simplifies users of the new macro slightly, and more importantly
> prepares us for handling multi-cluster systems where the register
> configuration will be done via the CM's GCR_CL_REDIRECT register. By
> abstracting all other/redirect block configuration through this macro,
> and the __gic_with_next_online_cpu() function which backs it, users will
> trivially gain support for multi-cluster when it is implemented in
> __gic_with_next_online_cpu().
>
> Signed-off-by: Paul Burton <[email protected]>
> Signed-off-by: Chao-ying Fu <[email protected]>
>
> diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
> index ff89b36267dd..4872bebe24cf 100644
> --- a/drivers/irqchip/irq-mips-gic.c
> +++ b/drivers/irqchip/irq-mips-gic.c
No SoB from the sender, odd patch format (no ---), and I didn't get a
complete series, which makes it impossible to review things in context
(I don't even know what the series is about).
Please fix things and resend.
M.
--
Without deviation from the norm, progress is not possible.
On Fri, May 20, 2022 at 09:38:55AM +0100, Marc Zyngier wrote:
> On Thu, 19 May 2022 19:51:16 +0100,
> Dragan Mladjenovic <[email protected]> wrote:
> >
> > From: Paul Burton <[email protected]>
> >
> > A few pieces of code in the MIPS GIC driver operate on the GIC local
> > register block for each online CPU, accessing each via the GIC's
> > other/redirect register block. This patch abstracts the process of
> > iterating over online CPUs & configuring the other/redirect region to
> > access their registers through a new gic_with_each_online_cpu() macro.
> >
> > This simplifies users of the new macro slightly, and more importantly
> > prepares us for handling multi-cluster systems where the register
> > configuration will be done via the CM's GCR_CL_REDIRECT register. By
> > abstracting all other/redirect block configuration through this macro,
> > and the __gic_with_next_online_cpu() function which backs it, users will
> > trivially gain support for multi-cluster when it is implemented in
> > __gic_with_next_online_cpu().
> >
> > Signed-off-by: Paul Burton <[email protected]>
> > Signed-off-by: Chao-ying Fu <[email protected]>
> >
> > diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
> > index ff89b36267dd..4872bebe24cf 100644
> > --- a/drivers/irqchip/irq-mips-gic.c
> > +++ b/drivers/irqchip/irq-mips-gic.c
>
> No SoB from the sender, odd patch format (no ---), and I didn't get a
> complete series, which makes it impossible to review things in context
> (I don't even know what the series is about).
Hi Marc,
Here is the link to the whole series:
https://lore.kernel.org/linux-mips/[email protected]/
but yes, having a full patchset in the inbox would be much better.
@Dragan, if you are still willing to get the series reviewed, please
add both Marc and me to the lists of the recipients of each patch and
resend. Of course make sure Thomas Bogendoerfer gets the whole series too.
@Dragan, as Marc already noted the sender's SoB must be added.
Who is Chao-ying and why do you need to add his SoB to each patch?
For instance you have the patch
"[PATCH 05/12] irqchip: mips-gic: Setup defaults in each cluster"
in this series which is stated being authored by Chao-ying but
you've also added Paul' SoB there. What did Paul do for that patch
development?
-Sergey
>
> Please fix things and resend.
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.