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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Wed, 21 Feb 2024 18:37:56 +0100
From: Thomas Gleixner <tglx@...utronix.de>
To: Changhuang Liang <changhuang.liang@...rfivetech.com>, Rob Herring
 <robh+dt@...nel.org>, Krzysztof Kozlowski
 <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>,
 Philipp Zabel <p.zabel@...gutronix.de>
Cc: Ley Foon Tan <leyfoon.tan@...rfivetech.com>, Jack Zhu
 <jack.zhu@...rfivetech.com>, Changhuang Liang
 <changhuang.liang@...rfivetech.com>, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org
Subject: Re: [PATCH v3 2/2] irqchip: Add StarFive external interrupt controller

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ