On Tue, Feb 20 2024 at 18:26, Changhuang Liang wrote:
> +static void starfive_intc_unmask(struct irq_data *d)
> +{
> + struct starfive_irq_chip *irqc = irq_data_get_irq_chip_data(d);
> + unsigned long flags;
> +
> + raw_spin_lock_irqsave(&irqc->lock, flags);
This does not need the _irqsave() variant as this is guaranteed to be
called with interrupts disabled from the core code.
> + starfive_intc_bit_clear(irqc, STARFIVE_INTC_SRC0_MASK, BIT(d->hwirq));
> + raw_spin_unlock_irqrestore(&irqc->lock, flags);
> +}
> + chained_irq_enter(chip, desc);
> +
> + value = ioread32(irqc->base + STARFIVE_INTC_SRC0_INT);
> + while (value) {
> + hwirq = ffs(value) - 1;
> +
> + generic_handle_domain_irq(irqc->domain, hwirq);
> +
> + starfive_intc_bit_set(irqc, STARFIVE_INTC_SRC0_CLEAR, BIT(hwirq));
> + starfive_intc_bit_clear(irqc, STARFIVE_INTC_SRC0_CLEAR, BIT(hwirq));
> +
> + clear_bit(hwirq, &value);
As this is a local variable you really don't want to have the atomic
variant for clearing the bit. __clear_bit() is your friend.
Other than those nitpicks this looks good.
Thanks,
tglx
