linux-kernel - Re: [PATCH] irqchip/sifive-plic: add irq_mask and irq

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <mhng-c06cc89b-42d9-4f95-b090-2db96628d5fb@palmer-si-x1e>
Date:   Sat, 14 Sep 2019 12:00:12 -0700 (PDT)
From:   Palmer Dabbelt <palmer@...ive.com>
To:     Darius Rad <darius@...espec.com>
CC:     linux-riscv@...ts.infradead.org,
        Paul Walmsley <paul.walmsley@...ive.com>,
        linux-kernel@...r.kernel.org, tglx@...utronix.de,
        jason@...edaemon.net, maz@...nel.org
Subject:     Re: [PATCH] irqchip/sifive-plic: add irq_mask and irq_unmask

On Thu, 12 Sep 2019 14:40:34 PDT (-0700), Darius Rad wrote:
> As per the existing comment, irq_mask and irq_unmask do not need
> to do anything for the PLIC.  However, the functions must exist
> (the pointers cannot be NULL) as they are not optional, based on
> the documentation (Documentation/core-api/genericirq.rst) as well
> as existing usage (e.g., include/linux/irqchip/chained_irq.h).
>
> Signed-off-by: Darius Rad <darius@...espec.com>
> ---
>  drivers/irqchip/irq-sifive-plic.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index cf755964f2f8..52d5169f924f 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -111,6 +111,13 @@ static void plic_irq_disable(struct irq_data *d)
>  	plic_irq_toggle(cpu_possible_mask, d->hwirq, 0);
>  }
>
> +/*
> + * There is no need to mask/unmask PLIC interrupts.  They are "masked"
> + * by reading claim and "unmasked" when writing it back.
> + */
> +static void plic_irq_mask(struct irq_data *d) { }
> +static void plic_irq_unmask(struct irq_data *d) { }
> +
>  #ifdef CONFIG_SMP
>  static int plic_set_affinity(struct irq_data *d,
>  			     const struct cpumask *mask_val, bool force)
> @@ -138,12 +145,10 @@ static int plic_set_affinity(struct irq_data *d,
>
>  static struct irq_chip plic_chip = {
>  	.name		= "SiFive PLIC",
> -	/*
> -	 * There is no need to mask/unmask PLIC interrupts.  They are "masked"
> -	 * by reading claim and "unmasked" when writing it back.
> -	 */
>  	.irq_enable	= plic_irq_enable,
>  	.irq_disable	= plic_irq_disable,
> +	.irq_mask	= plic_irq_mask,
> +	.irq_unmask	= plic_irq_unmask,
>  #ifdef CONFIG_SMP
>  	.irq_set_affinity = plic_set_affinity,
>  #endif

I can't find any other drivers in irqchip with empty irq_mask/irq_unmask.  I'm 
not well versed in irqchip stuff, so I'll leave it up to the irqchip 
maintainers to comment on if this is the right way to do this.  Either way, I'm 
assuming it'll go in through some the irqchip tree so

Acked-by: Palmer Dabbelt <palmer@...ive.com>

just to make sure I don't get in the way if it is the right way to do it :).