diff mbox series

[v6,10/17] ACPI: RISC-V: Implement function to reorder irqchip probe entries

Message ID 20240601150411.1929783-11-sunilvl@ventanamicro.com (mailing list archive)
State New
Headers show
Series RISC-V: ACPI: Add external interrupt controller support | expand

Commit Message

Sunil V L June 1, 2024, 3:04 p.m. UTC
ACPI MADT entries for interrupt controllers don't have a way to describe
the hierarchy. However, the hierarchy is known to the architecture and
on RISC-V platforms, the MADT sub table types are ordered in the
incremental order from the root controller which is RINTC. So, add
architecture function for RISC-V to reorder the interrupt controller
probing as per the hierarchy as below.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
---
 drivers/acpi/riscv/Makefile |  2 +-
 drivers/acpi/riscv/irq.c    | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 drivers/acpi/riscv/irq.c

Comments

Bjorn Helgaas June 6, 2024, 10:07 p.m. UTC | #1
On Sat, Jun 01, 2024 at 08:34:04PM +0530, Sunil V L wrote:
> ACPI MADT entries for interrupt controllers don't have a way to describe
> the hierarchy. However, the hierarchy is known to the architecture and
> on RISC-V platforms, the MADT sub table types are ordered in the
> incremental order from the root controller which is RINTC. So, add
> architecture function for RISC-V to reorder the interrupt controller
> probing as per the hierarchy as below.

Is this ordering requirement documented anywhere?  I don't see it in
the RISC-V ECRs to the ACPI r6.5 spec.

I guess the implication is that you need to process MADT structures in
order of ascending acpi_madt_type:

  ACPI_MADT_TYPE_RINTC = 24,
  ACPI_MADT_TYPE_IMSIC = 25,
  ACPI_MADT_TYPE_APLIC = 26,
  ACPI_MADT_TYPE_PLIC = 27,

regardless of the order they appear in the MADT?  I.e., process all
the RINTC structures (in order of appearance in MADT), followed by all
the IMSIC structures, then all the APLIC structures, etc?

> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> ---
>  drivers/acpi/riscv/Makefile |  2 +-
>  drivers/acpi/riscv/irq.c    | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/acpi/riscv/irq.c
> 
> diff --git a/drivers/acpi/riscv/Makefile b/drivers/acpi/riscv/Makefile
> index 877de00d1b50..a96fdf1e2cb8 100644
> --- a/drivers/acpi/riscv/Makefile
> +++ b/drivers/acpi/riscv/Makefile
> @@ -1,4 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0-only
> -obj-y					+= rhct.o init.o
> +obj-y					+= rhct.o init.o irq.o
>  obj-$(CONFIG_ACPI_PROCESSOR_IDLE)	+= cpuidle.o
>  obj-$(CONFIG_ACPI_CPPC_LIB)		+= cppc.o
> diff --git a/drivers/acpi/riscv/irq.c b/drivers/acpi/riscv/irq.c
> new file mode 100644
> index 000000000000..f56e103a501f
> --- /dev/null
> +++ b/drivers/acpi/riscv/irq.c
> @@ -0,0 +1,32 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2023-2024, Ventana Micro Systems Inc
> + *	Author: Sunil V L <sunilvl@ventanamicro.com>
> + *

Spurious blank line.

> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/sort.h>
> +
> +static int irqchip_cmp_func(const void *in0, const void *in1)
> +{
> +	struct acpi_probe_entry *elem0 = (struct acpi_probe_entry *)in0;
> +	struct acpi_probe_entry *elem1 = (struct acpi_probe_entry *)in1;
> +
> +	return (elem0->type > elem1->type) - (elem0->type < elem1->type);
> +}
> +
> +/*
> + * RISC-V irqchips in MADT of ACPI spec are defined in the same order how
> + * they should be probed. Since IRQCHIP_ACPI_DECLARE doesn't define any
> + * order, this arch function will reorder the probe functions as per the
> + * required order for the architecture.

But this comment seems to contradict the commit log.  This comment
says you should process MADT structures in the order they appear in
the MADT.  But if that were the case, you wouldn't need to sort
anything.

Maybe "defined in the order they should be probed" means "in order of
increasing Interrupt Controller structure type value"?

> + */
> +void arch_sort_irqchip_probe(struct acpi_probe_entry *ap_head, int nr)
> +{
> +	struct acpi_probe_entry *ape = ap_head;
> +
> +	if (nr == 1 || !ACPI_COMPARE_NAMESEG(ACPI_SIG_MADT, ape->id))
> +		return;
> +	sort(ape, nr, sizeof(*ape), irqchip_cmp_func, NULL);
> +}
> -- 
> 2.40.1
>
Sunil V L July 10, 2024, 1:55 p.m. UTC | #2
Hi Bjorn,

On Thu, Jun 06, 2024 at 05:07:43PM -0500, Bjorn Helgaas wrote:
> On Sat, Jun 01, 2024 at 08:34:04PM +0530, Sunil V L wrote:
> > ACPI MADT entries for interrupt controllers don't have a way to describe
> > the hierarchy. However, the hierarchy is known to the architecture and
> > on RISC-V platforms, the MADT sub table types are ordered in the
> > incremental order from the root controller which is RINTC. So, add
> > architecture function for RISC-V to reorder the interrupt controller
> > probing as per the hierarchy as below.
> 
> Is this ordering requirement documented anywhere?  I don't see it in
> the RISC-V ECRs to the ACPI r6.5 spec.
> 

Thanks. We have added this clarity in a new mantis request.

> I guess the implication is that you need to process MADT structures in
> order of ascending acpi_madt_type:
> 
>   ACPI_MADT_TYPE_RINTC = 24,
>   ACPI_MADT_TYPE_IMSIC = 25,
>   ACPI_MADT_TYPE_APLIC = 26,
>   ACPI_MADT_TYPE_PLIC = 27,
> 
> regardless of the order they appear in the MADT?  I.e., process all
> the RINTC structures (in order of appearance in MADT), followed by all
> the IMSIC structures, then all the APLIC structures, etc?
> 
Correct!

> > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> > ---
> >  drivers/acpi/riscv/Makefile |  2 +-
> >  drivers/acpi/riscv/irq.c    | 32 ++++++++++++++++++++++++++++++++
> >  2 files changed, 33 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/acpi/riscv/irq.c
> > 
> > diff --git a/drivers/acpi/riscv/Makefile b/drivers/acpi/riscv/Makefile
> > index 877de00d1b50..a96fdf1e2cb8 100644
> > --- a/drivers/acpi/riscv/Makefile
> > +++ b/drivers/acpi/riscv/Makefile
> > @@ -1,4 +1,4 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> > -obj-y					+= rhct.o init.o
> > +obj-y					+= rhct.o init.o irq.o
> >  obj-$(CONFIG_ACPI_PROCESSOR_IDLE)	+= cpuidle.o
> >  obj-$(CONFIG_ACPI_CPPC_LIB)		+= cppc.o
> > diff --git a/drivers/acpi/riscv/irq.c b/drivers/acpi/riscv/irq.c
> > new file mode 100644
> > index 000000000000..f56e103a501f
> > --- /dev/null
> > +++ b/drivers/acpi/riscv/irq.c
> > @@ -0,0 +1,32 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2023-2024, Ventana Micro Systems Inc
> > + *	Author: Sunil V L <sunilvl@ventanamicro.com>
> > + *
> 
> Spurious blank line.
> 
Okay.

> > + */
> > +
> > +#include <linux/acpi.h>
> > +#include <linux/sort.h>
> > +
> > +static int irqchip_cmp_func(const void *in0, const void *in1)
> > +{
> > +	struct acpi_probe_entry *elem0 = (struct acpi_probe_entry *)in0;
> > +	struct acpi_probe_entry *elem1 = (struct acpi_probe_entry *)in1;
> > +
> > +	return (elem0->type > elem1->type) - (elem0->type < elem1->type);
> > +}
> > +
> > +/*
> > + * RISC-V irqchips in MADT of ACPI spec are defined in the same order how
> > + * they should be probed. Since IRQCHIP_ACPI_DECLARE doesn't define any
> > + * order, this arch function will reorder the probe functions as per the
> > + * required order for the architecture.
> 
> But this comment seems to contradict the commit log.  This comment
> says you should process MADT structures in the order they appear in
> the MADT.  But if that were the case, you wouldn't need to sort
> anything.
> 
> Maybe "defined in the order they should be probed" means "in order of
> increasing Interrupt Controller structure type value"?
>
Agree. Let me update.

Thanks!
Sunil
diff mbox series

Patch

diff --git a/drivers/acpi/riscv/Makefile b/drivers/acpi/riscv/Makefile
index 877de00d1b50..a96fdf1e2cb8 100644
--- a/drivers/acpi/riscv/Makefile
+++ b/drivers/acpi/riscv/Makefile
@@ -1,4 +1,4 @@ 
 # SPDX-License-Identifier: GPL-2.0-only
-obj-y					+= rhct.o init.o
+obj-y					+= rhct.o init.o irq.o
 obj-$(CONFIG_ACPI_PROCESSOR_IDLE)	+= cpuidle.o
 obj-$(CONFIG_ACPI_CPPC_LIB)		+= cppc.o
diff --git a/drivers/acpi/riscv/irq.c b/drivers/acpi/riscv/irq.c
new file mode 100644
index 000000000000..f56e103a501f
--- /dev/null
+++ b/drivers/acpi/riscv/irq.c
@@ -0,0 +1,32 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2023-2024, Ventana Micro Systems Inc
+ *	Author: Sunil V L <sunilvl@ventanamicro.com>
+ *
+ */
+
+#include <linux/acpi.h>
+#include <linux/sort.h>
+
+static int irqchip_cmp_func(const void *in0, const void *in1)
+{
+	struct acpi_probe_entry *elem0 = (struct acpi_probe_entry *)in0;
+	struct acpi_probe_entry *elem1 = (struct acpi_probe_entry *)in1;
+
+	return (elem0->type > elem1->type) - (elem0->type < elem1->type);
+}
+
+/*
+ * RISC-V irqchips in MADT of ACPI spec are defined in the same order how
+ * they should be probed. Since IRQCHIP_ACPI_DECLARE doesn't define any
+ * order, this arch function will reorder the probe functions as per the
+ * required order for the architecture.
+ */
+void arch_sort_irqchip_probe(struct acpi_probe_entry *ap_head, int nr)
+{
+	struct acpi_probe_entry *ape = ap_head;
+
+	if (nr == 1 || !ACPI_COMPARE_NAMESEG(ACPI_SIG_MADT, ape->id))
+		return;
+	sort(ape, nr, sizeof(*ape), irqchip_cmp_func, NULL);
+}