Hi Geert, On Fri, Apr 29, 2022 at 10:59 AM Lad, Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote: > > Hi Geert, > > On Fri, Apr 29, 2022 at 10:53 AM Geert Uytterhoeven > <geert@xxxxxxxxxxxxxx> wrote: > > > > Hi Prabhakar, > > > > On Fri, Apr 29, 2022 at 11:43 AM Lad, Prabhakar > > <prabhakar.csengg@xxxxxxxxx> wrote: > > > On Thu, Apr 28, 2022 at 10:42 AM Geert Uytterhoeven > > > <geert@xxxxxxxxxxxxxx> wrote: > > > > On Fri, Apr 22, 2022 at 12:12 AM Lad Prabhakar > > > > <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> wrote: > > > > > Add a driver for the Renesas RZ/G2L Interrupt Controller. > > > > > > > > > > This supports external pins being used as interrupts. It supports > > > > > one line for NMI, 8 external pins and 32 GPIO pins (out of 123) > > > > > to be used as IRQ lines. > > > > > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > > > > > > --- /dev/null > > > > > +++ b/drivers/irqchip/irq-renesas-rzg2l.c > > > > > @@ -0,0 +1,447 @@ > > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > > +/* > > > > > + * Renesas RZ/G2L IRQC Driver > > > > > + * > > > > > + * Copyright (C) 2022 Renesas Electronics Corporation. > > > > > + * > > > > > + * Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > > > > + */ > > > > > + > > > > > +#include <linux/clk.h> > > > > > +#include <linux/err.h> > > > > > +#include <linux/io.h> > > > > > +#include <linux/irqchip.h> > > > > > +#include <linux/irqdomain.h> > > > > > +#include <linux/of_address.h> > > > > > +#include <linux/reset.h> > > > > > +#include <linux/spinlock.h> > > > > > + > > > > > +#define IRQC_IRQ_START 1 > > > > > +#define IRQC_IRQ_COUNT 8 > > > > > +#define IRQC_TINT_START 9 > > > > > > > > = IRQC_IRQ_START + IRQC_IRQ_COUNT > > > > > > > OK > > > > > > > > +#define IRQC_TINT_COUNT 32 > > > > > +#define IRQC_NUM_IRQ 41 > > > > > > > > = IRQC_TINT_START + IRQC_TINT_COUNT > > > > > > > OK. > > > > > > > Should these be in a DT binding header file? > > > > > > > > Combining all types into a single linear number space makes it hard > > > > to extend the range, when reusing for an SoC that supports more > > > > interrupt sources. > > > > > > > Or DT data maybe? > > > > Let's leave it for now. As I missed that DT consumers will refer to > > external interrupt numbers only (is that actually enforced?), there > > won't be an issue. > > > > The driver can be changed later to derive IRQC_IRQ_COUNT from the > > compatible value, when needed. > > > Agreed. > > > > > > + u32 reg; > > > > > + > > > > > + reg = readl_relaxed(priv->base + ISCR); > > > > > + if (reg & bit) > > > > > + writel_relaxed(GENMASK(IRQC_IRQ_COUNT - 1, 0) & ~bit, > > > > > > > > As writes to the unused upper bits are ignored, you can drop the > > > > masking with GENMASK(IRQC_IRQ_COUNT - 1, 0), and be prepared for more > > > > interrupt sources. > > > > > > > Agreed. > > > > > > > + u32 bit = BIT(hw_irq - IRQC_TINT_START); > > > > > + u32 reg; > > > > > + > > > > > + reg = readl_relaxed(priv->base + TSCR); > > > > > + if (reg & bit) > > > > > + writel_relaxed(GENMASK(IRQC_TINT_COUNT - 1, 0) & ~bit, > > > > > > > > Drop the masking with all-ones? > > > > > > > You mean instead of a mask just use the reg instead? > > > > No, I meant to drop the masking with GENMASK(IRQC_TINT_COUNT - 1, 0), > > cfr. for external interrupts. > > > Ahh right, I missed that. > I would need reg to clear off the bit if I drop the mask. Cheers, Prabhakar
