2023-12-05 10:15:01

by Max Filippov

Subject: [PATCH] irqchip/irq-xtensa-pic: clean up

- get rid of the cached_irq_mask variable
- make mask/unmask operations atomic
- use BIT() macro instead of bit shifts
- drop .disable and .enable as they are equivalent to the default
implementations

Signed-off-by: Max Filippov <[email protected]>
---
drivers/irqchip/irq-xtensa-pic.c | 37 ++++++++++++++++----------------
1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/irqchip/irq-xtensa-pic.c b/drivers/irqchip/irq-xtensa-pic.c
index 0c18d1f1e264..ebd1b77c4d11 100644
--- a/drivers/irqchip/irq-xtensa-pic.c
+++ b/drivers/irqchip/irq-xtensa-pic.c
@@ -12,6 +12,7 @@
* Kevin Chea
*/

+#include <linux/bits.h>
#include <linux/interrupt.h>
#include <linux/irqdomain.h>
#include <linux/irq.h>
@@ -19,8 +20,6 @@
#include <linux/irqchip/xtensa-pic.h>
#include <linux/of.h>

-unsigned int cached_irq_mask;
-
/*
* Device Tree IRQ specifier translation function which works with one or
* two cell bindings. First cell value maps directly to the hwirq number.
@@ -44,34 +43,36 @@ static const struct irq_domain_ops xtensa_irq_domain_ops = {

static void xtensa_irq_mask(struct irq_data *d)
{
- cached_irq_mask &= ~(1 << d->hwirq);
- xtensa_set_sr(cached_irq_mask, intenable);
-}
+ unsigned long flags;
+ u32 irq_mask;

-static void xtensa_irq_unmask(struct irq_data *d)
-{
- cached_irq_mask |= 1 << d->hwirq;
- xtensa_set_sr(cached_irq_mask, intenable);
+ local_irq_save(flags);
+ irq_mask = xtensa_get_sr(intenable);
+ irq_mask &= ~BIT(d->hwirq);
+ xtensa_set_sr(irq_mask, intenable);
+ local_irq_restore(flags);
}

-static void xtensa_irq_enable(struct irq_data *d)
+static void xtensa_irq_unmask(struct irq_data *d)
{
- xtensa_irq_unmask(d);
-}
+ unsigned long flags;
+ u32 irq_mask;

-static void xtensa_irq_disable(struct irq_data *d)
-{
- xtensa_irq_mask(d);
+ local_irq_save(flags);
+ irq_mask = xtensa_get_sr(intenable);
+ irq_mask |= BIT(d->hwirq);
+ xtensa_set_sr(irq_mask, intenable);
+ local_irq_restore(flags);
}

static void xtensa_irq_ack(struct irq_data *d)
{
- xtensa_set_sr(1 << d->hwirq, intclear);
+ xtensa_set_sr(BIT(d->hwirq), intclear);
}

static int xtensa_irq_retrigger(struct irq_data *d)
{
- unsigned int mask = 1u << d->hwirq;
+ unsigned int mask = BIT(d->hwirq);

if (WARN_ON(mask & ~XCHAL_INTTYPE_MASK_SOFTWARE))
return 0;
@@ -81,8 +82,6 @@ static int xtensa_irq_retrigger(struct irq_data *d)

static struct irq_chip xtensa_irq_chip = {
.name = "xtensa",
- .irq_enable = xtensa_irq_enable,
- .irq_disable = xtensa_irq_disable,
.irq_mask = xtensa_irq_mask,
.irq_unmask = xtensa_irq_unmask,
.irq_ack = xtensa_irq_ack,
--
2.39.2

2023-12-08 13:56:51

by Thomas Gleixner

Subject: Re: [PATCH] irqchip/irq-xtensa-pic: clean up

On Tue, Dec 05 2023 at 02:13, Max Filippov wrote:
> - get rid of the cached_irq_mask variable
> - make mask/unmask operations atomic
> - use BIT() macro instead of bit shifts
> - drop .disable and .enable as they are equivalent to the default
> implementations
> static void xtensa_irq_mask(struct irq_data *d)
> {
> - cached_irq_mask &= ~(1 << d->hwirq);
> - xtensa_set_sr(cached_irq_mask, intenable);
> -}
> + unsigned long flags;
> + u32 irq_mask;
>
> -static void xtensa_irq_unmask(struct irq_data *d)
> -{
> - cached_irq_mask |= 1 << d->hwirq;
> - xtensa_set_sr(cached_irq_mask, intenable);
> + local_irq_save(flags);

All of these callbacks are invoked with interrupts disabled already. No
point in disabling them some more.

> + irq_mask = xtensa_get_sr(intenable);
> + irq_mask &= ~BIT(d->hwirq);
> + xtensa_set_sr(irq_mask, intenable);
> + local_irq_restore(flags);
> }

Thanks,

tglx