On Tue, 03 Jan 2023 14:14:03 +0000,
Anup Patel <apatel@xxxxxxxxxxxxxxxx> wrote:
>
> The RISC-V advanced interrupt architecture (AIA) extends the per-HART
> local interrupts in following ways:
> 1. Minimum 64 local interrupts for both RV32 and RV64
> 2. Ability to process multiple pending local interrupts in same
> interrupt handler
> 3. Priority configuration for each local interrupts
> 4. Special CSRs to configure/access the per-HART MSI controller
>
> This patch adds support for RISC-V AIA in the RISC-V intc driver.
>
> Signed-off-by: Anup Patel <apatel@xxxxxxxxxxxxxxxx>
> ---
> drivers/irqchip/irq-riscv-intc.c | 37 ++++++++++++++++++++++++++------
> 1 file changed, 31 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
> index f229e3e66387..880d1639aadc 100644
> --- a/drivers/irqchip/irq-riscv-intc.c
> +++ b/drivers/irqchip/irq-riscv-intc.c
> @@ -16,6 +16,7 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/smp.h>
> +#include <asm/hwcap.h>
>
> static struct irq_domain *intc_domain;
>
> @@ -29,6 +30,15 @@ static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
> generic_handle_domain_irq(intc_domain, cause);
> }
>
> +static asmlinkage void riscv_intc_aia_irq(struct pt_regs *regs)
What does "static asmlinkage" in a C file even mean? And clearly, this
isn't the only instance in this file...
> +{
> + unsigned long topi;
> +
> + while ((topi = csr_read(CSR_TOPI)))
> + generic_handle_domain_irq(intc_domain,
> + topi >> TOPI_IID_SHIFT);
> +}
> +
> /*
> * On RISC-V systems local interrupts are masked or unmasked by writing
> * the SIE (Supervisor Interrupt Enable) CSR. As CSRs can only be written
> @@ -38,12 +48,18 @@ static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
>
> static void riscv_intc_irq_mask(struct irq_data *d)
> {
> - csr_clear(CSR_IE, BIT(d->hwirq));
> + if (d->hwirq < BITS_PER_LONG)
And what if BIT_PER_LONG is 32, as I expect it to be on 32bit, which
the commit message says is supported?
> + csr_clear(CSR_IE, BIT(d->hwirq));
> + else
> + csr_clear(CSR_IEH, BIT(d->hwirq - BITS_PER_LONG));
> }
>
> static void riscv_intc_irq_unmask(struct irq_data *d)
> {
> - csr_set(CSR_IE, BIT(d->hwirq));
> + if (d->hwirq < BITS_PER_LONG)
> + csr_set(CSR_IE, BIT(d->hwirq));
> + else
> + csr_set(CSR_IEH, BIT(d->hwirq - BITS_PER_LONG));
> }
>
> static void riscv_intc_irq_eoi(struct irq_data *d)
> @@ -115,7 +131,7 @@ static struct fwnode_handle *riscv_intc_hwnode(void)
> static int __init riscv_intc_init(struct device_node *node,
> struct device_node *parent)
> {
> - int rc;
> + int rc, nr_irqs;
> unsigned long hartid;
>
> rc = riscv_of_parent_hartid(node, &hartid);
> @@ -133,14 +149,21 @@ static int __init riscv_intc_init(struct device_node *node,
> if (riscv_hartid_to_cpuid(hartid) != smp_processor_id())
> return 0;
>
> - intc_domain = irq_domain_add_linear(node, BITS_PER_LONG,
> + nr_irqs = BITS_PER_LONG;
> + if (riscv_isa_extension_available(NULL, SxAIA) && BITS_PER_LONG == 32)
> + nr_irqs = nr_irqs * 2;
Really, please drop this BITS_PER_LONG stuff. Use explicit numbers.
M.
--
Without deviation from the norm, progress is not possible.
