| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAMuHMdXxJhQ2Ra+PiR-UUv1HhL69Zpva2b-N9KygSMKUApHdwQ@mail.gmail.com>
Date: Thu, 24 Oct 2024 10:43:20 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Fabrizio Castro <fabrizio.castro.jz@...esas.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Philipp Zabel <p.zabel@...gutronix.de>,
Magnus Damm <magnus.damm@...il.com>, linux-kernel@...r.kernel.org,
linux-renesas-soc@...r.kernel.org,
Chris Paterson <Chris.Paterson2@...esas.com>, Biju Das <biju.das.jz@...renesas.com>,
Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [PATCH v3 2/3] irqchip: Add RZ/V2H(P) Interrupt Control Unit
(ICU) driver
Hi Fabrizio,
On Thu, Oct 10, 2024 at 1:08 AM Fabrizio Castro
<fabrizio.castro.jz@...esas.com> wrote:
> Add driver for the Renesas RZ/V2H(P) Interrupt Control Unit (ICU).
>
> This driver supports the external interrupts NMI, IRQn, and TINTn.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@...esas.com>
> ---
>
> v2->v3:
> * Reworked line breaks
> * Improved performance of rzv2h_icu_eoi
> * Replaced raw_spin_lock with guards
> * Improved the style of rzv2h_icu_domain_ops
> * Removed put_device from the successful path of rzv2h_icu_init
Thanks for the update!
> --- /dev/null
> +++ b/drivers/irqchip/irq-renesas-rzv2h.c
> +static void rzv2h_clear_irq_int(struct rzv2h_icu_priv *priv, unsigned int hwirq)
> +{
> + unsigned int irq_nr = hwirq - ICU_IRQ_START;
> + u32 isctr, iitsr, iitsel;
> + u32 bit = BIT(irq_nr);
> +
> + isctr = readl_relaxed(priv->base + ICU_ISCTR);
> + iitsr = readl_relaxed(priv->base + ICU_IITSR);
> + iitsel = ICU_IITSR_IITSEL_GET(iitsr, irq_nr);
> +
> + /*
> + * When level sensing is used, the interrupt flag gets automatically cleared when the
> + * interrupt signal is de-asserted by the source of the interrupt request, therefore clear
> + * the interrupt only for edge triggered interrupts.
> + */
> + if ((isctr & bit) && (iitsel != ICU_IRQ_LEVEL_LOW))
> + writel_relaxed(bit, priv->base + ICU_ISCLR);
> +}
Given you already manually inlined one call of this function in this v2,
I am not sure it's worthwhile to keep this helper. Manually inlining the
single remaining call would drop some boilerplate.
> +
> +static int rzv2h_irq_set_type(struct irq_data *d, unsigned int type)
> +{
> + struct rzv2h_icu_priv *priv = irq_data_to_priv(d);
> + unsigned int hwirq = irqd_to_hwirq(d);
> + u32 irq_nr = hwirq - ICU_IRQ_START;
> + u32 iitsr, sense;
> +
> + switch (type & IRQ_TYPE_SENSE_MASK) {
> + case IRQ_TYPE_LEVEL_LOW:
> + sense = ICU_IRQ_LEVEL_LOW;
> + break;
> +
> + case IRQ_TYPE_EDGE_FALLING:
> + sense = ICU_IRQ_EDGE_FALLING;
> + break;
> +
> + case IRQ_TYPE_EDGE_RISING:
> + sense = ICU_IRQ_EDGE_RISING;
> + break;
> +
> + case IRQ_TYPE_EDGE_BOTH:
> + sense = ICU_IRQ_EDGE_BOTH;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + guard(raw_spinlock)(&priv->lock);
> + iitsr = readl_relaxed(priv->base + ICU_IITSR);
> + iitsr &= ~ICU_IITSR_IITSEL_MASK(irq_nr);
> + iitsr |= ICU_IITSR_IITSEL_PREP(sense, irq_nr);
> + rzv2h_clear_irq_int(priv, hwirq);
> + writel_relaxed(iitsr, priv->base + ICU_IITSR);
> +
> + return 0;
> +}
> +
> +static void rzv2h_clear_tint_int(struct rzv2h_icu_priv *priv, unsigned int hwirq)
> +{
> + unsigned int tint_nr = hwirq - ICU_TINT_START;
> + int titsel_n = ICU_TITSR_TITSEL_N(tint_nr);
> + u32 tsctr, titsr, titsel;
> + u32 bit = BIT(tint_nr);
> + int k = tint_nr / 16;
> +
> + tsctr = readl_relaxed(priv->base + ICU_TSCTR);
> + titsr = readl_relaxed(priv->base + ICU_TITSR(k));
> + titsel = ICU_TITSR_TITSEL_GET(titsr, titsel_n);
> +
> + /*
> + * Writing 1 to the corresponding flag from register ICU_TSCTR only has effect if
> + * TSTATn = 1b and if it's a rising edge or a falling edge interrupt.
> + */
> + if ((tsctr & bit) && ((titsel == ICU_TINT_EDGE_RISING) ||
> + (titsel == ICU_TINT_EDGE_FALLING)))
> + writel_relaxed(bit, priv->base + ICU_TSCLR);
> +}
Likewise.
> +
> +static int rzv2h_tint_set_type(struct irq_data *d, unsigned int type)
> +{
> + u32 titsr, titsr_k, titsel_n, tien;
> + struct rzv2h_icu_priv *priv;
> + u32 tssr, tssr_k, tssel_n;
> + unsigned int hwirq;
> + u32 tint, sense;
> + int tint_nr;
> +
> + switch (type & IRQ_TYPE_SENSE_MASK) {
> + case IRQ_TYPE_LEVEL_LOW:
> + sense = ICU_TINT_LEVEL_LOW;
> + break;
> +
> + case IRQ_TYPE_LEVEL_HIGH:
> + sense = ICU_TINT_LEVEL_HIGH;
> + break;
> +
> + case IRQ_TYPE_EDGE_RISING:
> + sense = ICU_TINT_EDGE_RISING;
> + break;
> +
> + case IRQ_TYPE_EDGE_FALLING:
> + sense = ICU_TINT_EDGE_FALLING;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + tint = (u32)(uintptr_t)irq_data_get_irq_chip_data(d);
> + if (tint > ICU_PB5_TINT)
> + return -EINVAL;
> +
> + priv = irq_data_to_priv(d);
> + hwirq = irqd_to_hwirq(d);
> +
> + tint_nr = hwirq - ICU_TINT_START;
> +
> + tssr_k = ICU_TSSR_K(tint_nr);
> + tssel_n = ICU_TSSR_TSSEL_N(tint_nr);
> +
> + titsr_k = ICU_TITSR_K(tint_nr);
> + titsel_n = ICU_TITSR_TITSEL_N(tint_nr);
> + tien = ICU_TSSR_TIEN(titsel_n);
> +
> + guard(raw_spinlock)(&priv->lock);
> +
> + tssr = readl_relaxed(priv->base + ICU_TSSR(tssr_k));
> + tssr &= ~(ICU_TSSR_TSSEL_MASK(tssel_n) | tien);
> + tssr |= ICU_TSSR_TSSEL_PREP(tint, tssel_n);
> +
> + writel_relaxed(tssr, priv->base + ICU_TSSR(tssr_k));
> +
> + titsr = readl_relaxed(priv->base + ICU_TITSR(titsr_k));
> + titsr &= ~ICU_TITSR_TITSEL_MASK(titsel_n);
> + titsr |= ICU_TITSR_TITSEL_PREP(sense, titsel_n);
> +
> + writel_relaxed(titsr, priv->base + ICU_TITSR(titsr_k));
> +
> + rzv2h_clear_tint_int(priv, hwirq);
> +
> + writel_relaxed(tssr | tien, priv->base + ICU_TSSR(tssr_k));
> +
> + return 0;
> +}
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Powered by blists - more mailing lists