| 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
| ||
|
Date: Thu, 23 Jun 2016 15:51:48 +0000
From: Jason Cooper <jason@...edaemon.net>
To: Binbin Zhou <zhoubb@...ote.com>
Cc: Ralf Baechle <ralf@...ux-mips.org>,
John Crispin <john@...ozen.org>,
"Steven J. Hill" <Steven.Hill@...tec.com>,
linux-mips@...ux-mips.org, Fuxin Zhang <zhangfx@...ote.com>,
Zhangjin Wu <wuzhangjin@...il.com>,
Kelvin Cheung <keguang.zhang@...il.com>,
Thomas Gleixner <tglx@...utronix.de>,
Marc Zyngier <marc.zyngier@....com>,
linux-kernel@...r.kernel.org, Chunbo Cui <cuichboo@....com>,
Huacai Chen <chenhc@...ote.com>
Subject: Re: [PATCH RESEND v4 6/9] irqchip/ls1x-cpu: Move the CPU IRQ driver
from arch/mips/loongson32/common/
Hi folks!
On Thu, May 19, 2016 at 09:44:17AM +0800, Binbin Zhou wrote:
Commit message?
> Signed-off-by: Chunbo Cui <cuichboo@....com>
> Signed-off-by: Binbin Zhou <zhoubb@...ote.com>
> Signed-off-by: Huacai Chen <chenhc@...ote.com>
> ---
> arch/mips/include/asm/irq_cpu.h | 1 +
> arch/mips/loongson32/common/irq.c | 128 +-------------------
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-ls1x-cpu.c | 242 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 245 insertions(+), 127 deletions(-)
> create mode 100644 drivers/irqchip/irq-ls1x-cpu.c
>
...
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index b03cfcb..ae53eb9 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -5,6 +5,7 @@ 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-bcm2836.o
> +obj-$(CONFIG_MACH_LOONGSON32) += irq-ls1x-cpu.o
> obj-$(CONFIG_ARCH_EXYNOS) += exynos-combiner.o
> obj-$(CONFIG_ARCH_HIP04) += irq-hip04.o
> obj-$(CONFIG_ARCH_MMP) += irq-mmp.o
This is a gentle reminder to myself to fix this file up at the end of
the merge window. It's making me twitch.
> diff --git a/drivers/irqchip/irq-ls1x-cpu.c b/drivers/irqchip/irq-ls1x-cpu.c
> new file mode 100644
> index 0000000..0df984b
> --- /dev/null
> +++ b/drivers/irqchip/irq-ls1x-cpu.c
> @@ -0,0 +1,242 @@
> +/*
> + * Copyright (c) 2011 Zhang, Keguang <keguang.zhang@...il.com>
> + * Copyright (c) 2016 Binbin Zhou <zhoubb@...ote.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
nit: paragraph needs to be reflowed.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqdomain.h>
> +#include <asm/irq_cpu.h>
> +
> +#include <loongson1.h>
> +#include <irq.h>
> +
> +#define LS1X_INTC_REG(n, x) \
> + ((void __iomem *)KSEG1ADDR(LS1X_INT_REG_BASE + (n * 0x18) + (x)))
> +
> +#define LS1X_INTC_INTISR(n) LS1X_INTC_REG(n, 0x0)
> +#define LS1X_INTC_INTIEN(n) LS1X_INTC_REG(n, 0x4)
> +#define LS1X_INTC_INTSET(n) LS1X_INTC_REG(n, 0x8)
> +#define LS1X_INTC_INTCLR(n) LS1X_INTC_REG(n, 0xc)
> +#define LS1X_INTC_INTPOL(n) LS1X_INTC_REG(n, 0x10)
> +#define LS1X_INTC_INTEDGE(n) LS1X_INTC_REG(n, 0x14)
> +
> +static void ls1x_irq_ack(struct irq_data *d)
> +{
> + unsigned int bit = (d->irq - LS1X_IRQ_BASE) & 0x1f;
> + unsigned int n = (d->irq - LS1X_IRQ_BASE) >> 5;
> +
> + ls1x_writel(ls1x_readl(LS1X_INTC_INTCLR(n))
> + | (1 << bit), LS1X_INTC_INTCLR(n));
Why can't we use the standard functions?
> +}
> +
> +static void ls1x_irq_mask(struct irq_data *d)
> +{
> + unsigned int bit = (d->irq - LS1X_IRQ_BASE) & 0x1f;
> + unsigned int n = (d->irq - LS1X_IRQ_BASE) >> 5;
> +
> + ls1x_writel(ls1x_readl(LS1X_INTC_INTIEN(n))
> + & ~(1 << bit), LS1X_INTC_INTIEN(n));
With this, and all the statements similar to it below, we do the same
calculations (LS1X_INTC_XXXXXX(n)) twice. The compiler can't optimize
it because it doesn't know n ahead of time. Let's just calculate it
once:
void __iomem *addr = LS1X_INTC_INTIEN((d->irq - LS1X_IRQ_BASE) >> 5);
xwrite(xread(addr) & ~(1 << bit), addr);
where x{write,read}() is whatever we conclude is appropriate for this
driver.
> +}
> +
> +static void ls1x_irq_mask_ack(struct irq_data *d)
> +{
> + unsigned int bit = (d->irq - LS1X_IRQ_BASE) & 0x1f;
> + unsigned int n = (d->irq - LS1X_IRQ_BASE) >> 5;
> +
> + ls1x_writel(ls1x_readl(LS1X_INTC_INTIEN(n))
> + & ~(1 << bit), LS1X_INTC_INTIEN(n));
> + ls1x_writel(ls1x_readl(LS1X_INTC_INTCLR(n))
> + | (1 << bit), LS1X_INTC_INTCLR(n));
> +}
> +
> +static void ls1x_irq_unmask(struct irq_data *d)
> +{
> + unsigned int bit = (d->irq - LS1X_IRQ_BASE) & 0x1f;
> + unsigned int n = (d->irq - LS1X_IRQ_BASE) >> 5;
> +
> + ls1x_writel(ls1x_readl(LS1X_INTC_INTIEN(n))
> + | (1 << bit), LS1X_INTC_INTIEN(n));
> +}
> +
> +static void ls1x_irq_edge_type(struct irq_data *data, unsigned int flow_type)
> +{
> + unsigned int bit = (data->irq - LS1X_IRQ_BASE) & 0x1f;
> + unsigned int n = (data->irq - LS1X_IRQ_BASE) >> 5;
> +
> + if (flow_type & IRQ_TYPE_EDGE_RISING)
> + ls1x_writel(ls1x_readl(LS1X_INTC_INTPOL(n)) | (1 << bit),
> + LS1X_INTC_INTPOL(n));
> + else
> + ls1x_writel(ls1x_readl(LS1X_INTC_INTPOL(n)) & ~(1 << bit),
> + LS1X_INTC_INTPOL(n));
> +
> + ls1x_writel(ls1x_readl(LS1X_INTC_INTEDGE(n)) | (1 << bit),
> + LS1X_INTC_INTEDGE(n));
> + ls1x_writel(ls1x_readl(LS1X_INTC_INTIEN(n)) | (1 << bit),
> + LS1X_INTC_INTIEN(n));
> +
> + irq_set_handler_locked(data, handle_edge_irq);
> +}
> +
> +static void ls1x_irq_level_type(struct irq_data *data, unsigned int flow_type)
> +{
> + unsigned int bit = (data->irq - LS1X_IRQ_BASE) & 0x1f;
> + unsigned int n = (data->irq - LS1X_IRQ_BASE) >> 5;
> +
> + if (flow_type & IRQ_TYPE_LEVEL_HIGH)
> + ls1x_writel(ls1x_readl(LS1X_INTC_INTPOL(n)) | (1 << bit),
> + LS1X_INTC_INTPOL(n));
> + else if (flow_type & IRQ_TYPE_LEVEL_LOW)
> + ls1x_writel(ls1x_readl(LS1X_INTC_INTPOL(n)) & ~(1 << bit),
> + LS1X_INTC_INTPOL(n));
> +
> + ls1x_writel(ls1x_readl(LS1X_INTC_INTEDGE(n)) & ~(1 << bit),
> + LS1X_INTC_INTEDGE(n));
> + ls1x_writel(ls1x_readl(LS1X_INTC_INTIEN(n)) | (1 << bit),
> + LS1X_INTC_INTIEN(n));
> +
> + irq_set_handler_locked(data, handle_level_irq);
> +}
> +
> +static int ls1x_irq_set_type(struct irq_data *data, unsigned int flow_type)
> +{
> + unsigned int bit = (data->irq - LS1X_IRQ_BASE) & 0x1f;
> + unsigned int n = (data->irq - LS1X_IRQ_BASE) >> 5;
> +
> + if ((flow_type & IRQ_TYPE_EDGE_BOTH) == IRQ_TYPE_EDGE_BOTH) {
> + pr_info("ls1x irq don't support both rising and falling\n");
Under what conditions have you encountered this? Badly written
devicetree? driver?
> + return -1;
> + }
> +
> + ls1x_writel(ls1x_readl(LS1X_INTC_INTCLR(n)) | (1 << bit),
> + LS1X_INTC_INTCLR(n));
> +
> + if (flow_type & IRQ_TYPE_EDGE_BOTH)
> + ls1x_irq_edge_type(data, flow_type);
> + else if (flow_type && IRQ_TYPE_LEVEL_MASK)
> + ls1x_irq_level_type(data, flow_type);
> +
> + return IRQ_SET_MASK_OK;
> +}
> +
> +static struct irq_chip ls1x_cpu_irq_chip = {
> + .name = "LS1X-INTC",
> + .irq_ack = ls1x_irq_ack,
> + .irq_mask = ls1x_irq_mask,
> + .irq_mask_ack = ls1x_irq_mask_ack,
> + .irq_unmask = ls1x_irq_unmask,
> + .irq_set_type = ls1x_irq_set_type,
> +};
> +
> +static void ls1x_irq_dispatch(int n)
> +{
> + u32 int_status, irq;
> +
> + /* Get pending sources, masked by current enables */
> + int_status = ls1x_readl(LS1X_INTC_INTISR(n)) &
> + ls1x_readl(LS1X_INTC_INTIEN(n));
> +
> + if (int_status) {
> + irq = LS1X_IRQ(n, __ffs(int_status));
> + do_IRQ(irq);
> + }
> +}
> +
> +asmlinkage void plat_irq_dispatch(void)
> +{
> + unsigned int pending;
> +
> + pending = read_c0_cause() & read_c0_status() & ST0_IM;
> +
> + if (pending & CAUSEF_IP7)
> + do_IRQ(TIMER_IRQ);
> + else if (pending & CAUSEF_IP2)
> + ls1x_irq_dispatch(0); /* INT0 */
> + else if (pending & CAUSEF_IP3)
> + ls1x_irq_dispatch(1); /* INT1 */
> + else if (pending & CAUSEF_IP4)
> + ls1x_irq_dispatch(2); /* INT2 */
> + else if (pending & CAUSEF_IP5)
> + ls1x_irq_dispatch(3); /* INT3 */
> + else if (pending & CAUSEF_IP6)
> + ls1x_irq_dispatch(4); /* INT4 */
> + else
> + spurious_interrupt();
> +
> +}
> +
> +struct irqaction cascade_irqaction = {
> + .handler = no_action,
> + .name = "cascade",
> + .flags = IRQF_NO_THREAD,
> +};
> +
> +static int ls1x_cpu_intc_map(struct irq_domain *d, unsigned int irq,
> + irq_hw_number_t hw)
> +{
> + int n;
> +
> + /*
> + * Disable interrupts and clear pending,
> + * setup all IRQs as high level triggered
> + */
> + for (n = 0; n < 4; n++) {
> + ls1x_writel(0x0, LS1X_INTC_INTIEN(n));
> + ls1x_writel(0xffffffff, LS1X_INTC_INTCLR(n));
> + ls1x_writel(0xffffffff, LS1X_INTC_INTPOL(n));
> + /* set DMA0, DMA1 and DMA2 to edge trigger */
> + ls1x_writel(n ? 0x0 : 0xe000, LS1X_INTC_INTEDGE(n));
> + }
> +
> +
> + for (n = LS1X_IRQ_BASE; n < LS1X_IRQS; n++) {
> + irq_set_chip_and_handler(n, &ls1x_cpu_irq_chip,
> + handle_level_irq);
> + }
> +
> + setup_irq(INT0_IRQ, &cascade_irqaction);
> + setup_irq(INT1_IRQ, &cascade_irqaction);
> + setup_irq(INT2_IRQ, &cascade_irqaction);
> + setup_irq(INT3_IRQ, &cascade_irqaction);
> + setup_irq(INT4_IRQ, &cascade_irqaction);
> +
> + return 0;
> +}
> +
> +static const struct irq_domain_ops ls1x_cpu_intc_domain_ops = {
> + .map = ls1x_cpu_intc_map,
> + .xlate = irq_domain_xlate_onecell,
> +};
> +
> +static void __init __ls1x_irq_init(struct device_node *of_node)
> +{
> + struct irq_domain *domain;
> +
> + domain = irq_domain_add_legacy(of_node, LS1X_IRQS, LS1X_IRQ_BASE, 0,
> + &ls1x_cpu_intc_domain_ops, NULL);
Please take a look at using _simple() or _linear().
> + if (!domain)
> + panic("Failed to add irqdomain for MIPS CPU");
> +}
> +
> +void __init ls1x_cpu_irq_init(void)
> +{
> + __ls1x_irq_init(NULL);
> +}
> +
> +static int __init ls1x_cpu_irq_of_init(
> + struct device_node *node, struct device_node *parent)
> +
> +{
> + __ls1x_irq_init(node);
> + return 0;
> +}
> +
> +IRQCHIP_DECLARE(ls1x_cpu_intc, "loongson-1A cpu-irq-intc",
> + ls1x_cpu_irq_of_init);
> --
> 1.9.1
thx,
Jason.
Powered by blists - more mailing lists