lkml.org 
[lkml]   [2017]   [Jun]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
On 14/06/17 17:29, Phil Elwell wrote:
> Devices in the BCM2835 AUX block share a common interrupt line, with a
> register indicating which devices have active IRQs. Expose this as a
> nested interrupt controller to avoid IRQ sharing problems (easily
> observed if UART1 and SPI1/2 are enabled simultaneously).
>
> Signed-off-by: Phil Elwell <phil@raspberrypi.org>
> ---
> drivers/irqchip/Makefile | 2 +-
> drivers/irqchip/irq-bcm2835-aux.c | 153 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 154 insertions(+), 1 deletion(-)
> create mode 100644 drivers/irqchip/irq-bcm2835-aux.c
>
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index b64c59b..cf01920 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -3,7 +3,7 @@ obj-$(CONFIG_IRQCHIP) += irqchip.o
> obj-$(CONFIG_ALPINE_MSI) += irq-alpine-msi.o
> obj-$(CONFIG_ATH79) += irq-ath79-cpu.o
> obj-$(CONFIG_ATH79) += irq-ath79-misc.o
> -obj-$(CONFIG_ARCH_BCM2835) += irq-bcm2835.o
> +obj-$(CONFIG_ARCH_BCM2835) += irq-bcm2835.o irq-bcm2835-aux.o
> obj-$(CONFIG_ARCH_BCM2835) += irq-bcm2836.o
> obj-$(CONFIG_ARCH_EXYNOS) += exynos-combiner.o
> obj-$(CONFIG_FARADAY_FTINTC010) += irq-ftintc010.o
> diff --git a/drivers/irqchip/irq-bcm2835-aux.c b/drivers/irqchip/irq-bcm2835-aux.c
> new file mode 100644
> index 0000000..2151d51
> --- /dev/null
> +++ b/drivers/irqchip/irq-bcm2835-aux.c
> @@ -0,0 +1,153 @@
> +/*
> + * Copyright (C) 2017 Raspberry Pi (Trading) Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/irqdomain.h>
> +#include <linux/module.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <dt-bindings/interrupt-controller/bcm2835-aux-intc.h>
> +
> +#define BCM2835_AUXIRQ 0x00
> +
> +#define BCM2835_AUX_IRQ_UART_MASK BIT(BCM2835_AUX_IRQ_UART)
> +#define BCM2835_AUX_IRQ_SPI1_MASK BIT(BCM2835_AUX_IRQ_SPI1)
> +#define BCM2835_AUX_IRQ_SPI2_MASK BIT(BCM2835_AUX_IRQ_SPI2)
> +
> +#define BCM2835_AUX_IRQ_ALL_MASK \
> + (BCM2835_AUX_IRQ_UART_MASK | \
> + BCM2835_AUX_IRQ_SPI1_MASK | \
> + BCM2835_AUX_IRQ_SPI2_MASK)
> +
> +struct aux_irq_state {
> + void __iomem *status;
> + struct irq_domain *domain;
> +};
> +
> +static struct aux_irq_state aux_irq __read_mostly;
> +
> +static irqreturn_t bcm2835_aux_irq_handler(int irq, void *dev_id)
> +{
> + u32 stat = readl_relaxed(aux_irq.status);
> +
> + if (stat & BCM2835_AUX_IRQ_UART_MASK)
> + generic_handle_irq(irq_linear_revmap(aux_irq.domain,
> + BCM2835_AUX_IRQ_UART));
> +
> + if (stat & BCM2835_AUX_IRQ_SPI1_MASK)
> + generic_handle_irq(irq_linear_revmap(aux_irq.domain,
> + BCM2835_AUX_IRQ_SPI1));
> +
> + if (stat & BCM2835_AUX_IRQ_SPI2_MASK)
> + generic_handle_irq(irq_linear_revmap(aux_irq.domain,
> + BCM2835_AUX_IRQ_SPI2));
> +
> + return (stat & BCM2835_AUX_IRQ_ALL_MASK) ? IRQ_HANDLED : IRQ_NONE;

That feels like the wrong conditions to return IRQ_HANDLED. Only the
last level handler actually knows whether this has been handled or not.

A vaguely better approach would be to reread the HW status and only
return IRQ_HANDLED if everything is clear. Yes, this is racy (another
interrupt could have come in the meantime). But I doubt you can do much
better given the quality of the HW...

Thanks,

M.
--
Jazz is not dead. It just smells funny...

\
 
 \ /
  Last update: 2017-06-22 20:03    [W:0.061 / U:0.300 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site