Linux-MIPS Archive on lore.kernel.org
 help / color / Atom feed
From: Jiaxun Yang <jiaxun.yang@flygoat.com>
To: Marc Zyngier <maz@kernel.org>, linux-mips@vger.kernel.org
Cc: chenhc@lemote.com, paul.burton@mips.com, tglx@linutronix.de,
	jason@lakedaemon.net, linux-kernel@vger.kernel.org,
	robh+dt@kernel.org, mark.rutland@arm.co,
	devicetree@vger.kernel.org
Subject: Re: [PATCH 04/13] irqchip: Add driver for Loongson-3 I/O interrupt controller
Date: Wed, 28 Aug 2019 08:27:05 +0800
Message-ID: <82c4b9ed-7270-74ce-6e10-165182e540dd@flygoat.com> (raw)
In-Reply-To: <e6a5862f-0f6c-cab0-9f4a-51b7889d38e7@kernel.org>


On 2019/8/28 上午12:45, Marc Zyngier wrote:
> On 27/08/2019 09:52, Jiaxun Yang wrote:
>> This controller appeared on Loongson-3 family of chips as the primary
>> package interrupt source.
>>
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>>   drivers/irqchip/Kconfig          |   9 ++
>>   drivers/irqchip/Makefile         |   1 +
>>   drivers/irqchip/irq-ls3-iointc.c | 216 +++++++++++++++++++++++++++++++
>>   3 files changed, 226 insertions(+)
>>   create mode 100644 drivers/irqchip/irq-ls3-iointc.c
>>
>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>> index 80e10f4e213a..8d9eac5fd4a7 100644
>> --- a/drivers/irqchip/Kconfig
>> +++ b/drivers/irqchip/Kconfig
>> @@ -471,6 +471,15 @@ config TI_SCI_INTA_IRQCHIP
>>   	  If you wish to use interrupt aggregator irq resources managed by the
>>   	  TI System Controller, say Y here. Otherwise, say N.
>>   
>> +config LS3_IOINTC
>> +	bool "Loongson3 I/O Interrupt Controller"
>> +	depends on MACH_LOONGSON64
>> +	default y
>> +	select IRQ_DOMAIN
>> +	select GENERIC_IRQ_CHIP
>> +	help
>> +	  Support for the Loongson-3 I/O Interrupt Controller.
>> +
>>   endmenu
>>   
>>   config SIFIVE_PLIC
>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>> index 8d0fcec6ab23..49ecb8d38138 100644
>> --- a/drivers/irqchip/Makefile
>> +++ b/drivers/irqchip/Makefile
>> @@ -102,3 +102,4 @@ obj-$(CONFIG_MADERA_IRQ)		+= irq-madera.o
>>   obj-$(CONFIG_LS1X_IRQ)			+= irq-ls1x.o
>>   obj-$(CONFIG_TI_SCI_INTR_IRQCHIP)	+= irq-ti-sci-intr.o
>>   obj-$(CONFIG_TI_SCI_INTA_IRQCHIP)	+= irq-ti-sci-inta.o
>> +obj-$(CONFIG_LS3_IOINTC)			+= irq-ls3-iointc.o
>> diff --git a/drivers/irqchip/irq-ls3-iointc.c b/drivers/irqchip/irq-ls3-iointc.c
>> new file mode 100644
>> index 000000000000..1fc3c41c57d9
>> --- /dev/null
>> +++ b/drivers/irqchip/irq-ls3-iointc.c
>> @@ -0,0 +1,216 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + *  Copyright (C) 2019, Jiaxun Yang <jiaxun.yang@flygoat.com>
>> + *  Loongson-3 IOINTC IRQ support
>> + */
>> +
>> +#include <linux/errno.h>
>> +#include <linux/init.h>
>> +#include <linux/types.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/ioport.h>
>> +#include <linux/irqchip.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/io.h>
>> +#include <linux/smp.h>
>> +#include <linux/irqchip/chained_irq.h>
>> +
>> +
>> +#define LS3_CHIP_IRQ	32
>> +
>> +#define LS3_INTC_CHIP_START	0x20
>> +
>> +#define LS3_REG_INTC_STATUS	0x00
>> +#define LS3_REG_INTC_EN_STATUS	0x04
>> +#define LS3_REG_INTC_ENABLE	0x08
>> +#define LS3_REG_INTC_DISABLE	0x0c
>> +#define LS3_REG_INTC_POL	0x10
>> +#define LS3_REG_INTC_EDGE	0x18
>> +
>> +#define LS3_MAP_CORE_INT(x, y)	(u8)(BIT(x) | (BIT(y) << 4))
>> +
>> +
>> +struct ls3_iointc_priv {
>> +	struct irq_domain	*domain;
>> +	void __iomem		*intc_base;
>> +};
>> +
>> +
>> +static void ls3_io_chained_handle_irq(struct irq_desc *desc)
>> +{
>> +	struct ls3_iointc_priv *priv = irq_desc_get_handler_data(desc);
>> +	struct irq_chip *chip = irq_desc_get_chip(desc);
>> +	u32 pending;
>> +
>> +	chained_irq_enter(chip, desc);
>> +
>> +	pending = readl(priv->intc_base + LS3_REG_INTC_EN_STATUS) &
>> +		readl(priv->intc_base + LS3_REG_INTC_STATUS);
> Reading the enabled status from the HW on each interrupt? I'm sure
> that's pretty cheap...
Seems expensive but to deal with a buggy hardware... That's worthy.
>
>> +
>> +	if (!pending)
>> +		spurious_interrupt();
>> +
>> +	while (pending) {
>> +		int bit = __ffs(pending);
>> +
>> +		generic_handle_irq(irq_find_mapping(priv->domain, bit));
>> +		pending &= ~BIT(bit);
>> +	}
>> +
>> +	chained_irq_exit(chip, desc);
>> +}
>> +
>> +
>> +static void ls_intc_set_bit(struct irq_chip_generic *gc,
>> +							unsigned int offset,
>> +							u32 mask, bool set)
>> +{
>> +	if (set)
>> +		writel(readl(gc->reg_base + offset) | mask,
>> +		gc->reg_base + offset);
> Please correctly align the second line.
>
>> +	else
>> +		writel(readl(gc->reg_base + offset) & ~mask,
>> +		gc->reg_base + offset);
>> +}
> Have you tried this on a SMP system? A RMW without locking is unlikely
> to go down very well.
>
>> +
>> +static int ls_intc_set_type(struct irq_data *data, unsigned int type)
>> +{
>> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
>> +	u32 mask = data->mask;
>> +
>> +	switch (type) {
>> +	case IRQ_TYPE_LEVEL_HIGH:
>> +		ls_intc_set_bit(gc, LS3_REG_INTC_EDGE, mask, false);
>> +		ls_intc_set_bit(gc, LS3_REG_INTC_POL, mask, true);
>> +		break;
>> +	case IRQ_TYPE_LEVEL_LOW:
>> +		ls_intc_set_bit(gc, LS3_REG_INTC_EDGE, mask, false);
>> +		ls_intc_set_bit(gc, LS3_REG_INTC_POL, mask, false);
>> +		break;
>> +	case IRQ_TYPE_EDGE_RISING:
>> +		ls_intc_set_bit(gc, LS3_REG_INTC_EDGE, mask, true);
>> +		ls_intc_set_bit(gc, LS3_REG_INTC_POL, mask, true);
>> +		break;
>> +	case IRQ_TYPE_EDGE_FALLING:
>> +		ls_intc_set_bit(gc, LS3_REG_INTC_EDGE, mask, true);
>> +		ls_intc_set_bit(gc, LS3_REG_INTC_POL, mask, false);
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	irqd_set_trigger_type(data, type);
>> +	return 0;
>> +}
>> +
>> +int __init ls3_iointc_of_init(struct device_node *node,
>> +				       struct device_node *parent)
>> +{
>> +	struct irq_chip_generic *gc;
>> +	struct irq_chip_type *ct;
>> +	struct ls3_iointc_priv *priv;
>> +	int parent_irq, err = 0;
>> +	int core = cpu_logical_map(smp_processor_id());
> Are you guaranteed to be in a non-preemptible section here?
Yes, as irqchip will be initialized even earlier than clockevent. There 
must be non-preemptible.
>
>> +	int ip = 0;
>> +	int i;
>> +	const u32 *map_ip, *map_core;
>> +
>> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +
>> +	priv->intc_base = of_iomap(node, 0);
>> +	if (!priv->intc_base) {
>> +		err = -ENODEV;
>> +		goto out_free_priv;
>> +	}
>> +
>> +	map_ip = of_get_property(node, "loongson,map-ip", NULL);
>> +	if (!map_ip)
>> +		goto no_ip;
>> +	else if ((*map_ip) > 5)
>> +		pr_err("* %pOF loongson,map-ip is invalid\n", node);
>> +	else
>> +		ip = (*map_ip);
> What is this "ip"?
The interrupt line number of CPU (parent intc). As MIPS called CPU 
interrupt registers "CAUSE_IP".
>
>> +no_ip:
>> +
>> +	/* If this property does not exist or invalid,
>> +	 * we map all IRQs to bootcore.
>> +	 */
> Comment format.
>
>> +	map_core = of_get_property(node, "loongson,map-core", NULL);
>> +	if (!map_core)
>> +		goto no_core;
>> +	else if ((*map_core) > 3)
>> +		pr_err("* %pOF loongson,map-core is invalid\n", node);
>> +	else
>> +		core = (*map_core);
>> +no_core:
>> +
>> +	parent_irq = irq_of_parse_and_map(node, 0);
>> +	if (!parent_irq) {
>> +		pr_err("ls3-iointc: unable to get parent irq\n");
>> +		err =  -ENODEV;
>> +		goto out_iounmap;
>> +	}
>> +	/* Set up an IRQ domain */
>> +	priv->domain = irq_domain_add_linear(node, 32, &irq_generic_chip_ops,
>> +					     NULL);
>> +	if (!priv->domain) {
>> +		pr_err("ls3-iointc: cannot add IRQ domain\n");
>> +		err = -ENOMEM;
>> +		goto out_iounmap;
>> +	}
>> +
>> +	err = irq_alloc_domain_generic_chips(priv->domain, 32, 1,
>> +		node->full_name, handle_level_irq,
>> +		IRQ_NOPROBE, 0, 0);
>> +	if (err) {
>> +		pr_err("ls3-iointc: unable to register IRQ domain\n");
>> +		err = -ENOMEM;
>> +		goto out_free_domain;
>> +	}
>> +
>> +	/*
>> +	 * Q: Why don't we set IRQ affinity by these registers?
>> +	 * A: Hardware IRQ delivery is seriously broken,
>> +	 *    so we map all IRQs to a fixed core.
>> +	 */
>> +	pr_info("ls3-iointc: Mapping All ls3-iointc IRQ to core %d, IP %d\n", core, ip);
>> +	for (i = 0; i < LS3_CHIP_IRQ; i++)
>> +		writeb(LS3_MAP_CORE_INT(core, ip), priv->intc_base + 0x1 * i);
> This doesn't make much sense. If this is a chained irqchip, all
> interrupts end-up on a single CPU (the one that handle the parent IRQ).
> So how comes there is even a choice of picking a target CPU?

It's parent IRQ (mti,cpu-interrupt-controller) is a percpu IRQ.

In design, it allows us to decide affinity at runtime but actually 
hardware is seriously broken.

> Also, you still need to define a set_affinity() callback, even if it
> returns -EINVAL.
>
>> +	priv->intc_base += LS3_INTC_CHIP_START;
>> +
>> +	/* Disable all IRQs */
>> +	writel(0xffffffff, priv->intc_base + LS3_REG_INTC_DISABLE);
>> +	/* Set to level triggered */
>> +	writel(0x0, priv->intc_base + LS3_REG_INTC_EDGE);
>> +
>> +	gc = irq_get_domain_generic_chip(priv->domain, 0);
>> +	gc->reg_base = priv->intc_base;
>> +	gc->domain = priv->domain;
>> +
>> +	ct = gc->chip_types;
>> +	ct->regs.enable = LS3_REG_INTC_ENABLE;
>> +	ct->regs.disable = LS3_REG_INTC_DISABLE;
>> +	ct->chip.irq_unmask = irq_gc_unmask_enable_reg;
>> +	ct->chip.irq_mask = irq_gc_mask_disable_reg;
>> +	ct->chip.irq_mask_ack = irq_gc_mask_disable_reg;
>> +	ct->chip.irq_set_type = ls_intc_set_type;
>> +
>> +	irq_set_chained_handler_and_data(parent_irq,
>> +		ls3_io_chained_handle_irq, priv);
>> +
>> +	return 0;
>> +
>> +out_free_domain:
>> +	irq_domain_remove(priv->domain);
>> +out_iounmap:
>> +	iounmap(priv->intc_base);
>> +out_free_priv:
>> +	kfree(priv);
>> +
>> +	return err;
>> +}
>> +
>> +IRQCHIP_DECLARE(ls3_iointc, "loongson,ls3-iointc", ls3_iointc_of_init);
>>
> Thanks,
>
> 	M.

  reply index

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-27  8:52 [PATCH 00/13] Modernize Loongson64 Machine Jiaxun Yang
2019-08-27  8:52 ` [PATCH 01/13] MIPS: Loongson64: Rename CPU TYPES Jiaxun Yang
2019-08-27  8:52 ` [PATCH 02/13] MIPS: Loongson64: Sepreate loongson2ef/loongson64 code Jiaxun Yang
2019-08-27 22:05   ` Aaro Koskinen
2019-08-28  0:37     ` Jiaxun Yang
2019-09-02 17:51       ` Aaro Koskinen
2019-08-27  8:52 ` [PATCH 03/13] MAINTAINERS: Fix entries for new loongson64 path Jiaxun Yang
2019-08-27  8:52 ` [PATCH 04/13] irqchip: Add driver for Loongson-3 I/O interrupt controller Jiaxun Yang
2019-08-27 16:45   ` Marc Zyngier
2019-08-28  0:27     ` Jiaxun Yang [this message]
2019-08-28  6:59       ` Marc Zyngier
2019-08-28 15:31         ` Jiaxun Yang
2019-08-28 16:02           ` Marc Zyngier
2019-08-27  8:52 ` [PATCH 05/13] dt-bindings: interrupt-controller: Add Loongson-3 IOINTC Jiaxun Yang
2019-08-27 13:08   ` Rob Herring
2019-08-27  8:52 ` [PATCH 06/13] irqchip: Add driver for Loongson-3 HyperTransport interrupt controller Jiaxun Yang
2019-08-27 12:53   ` Marc Zyngier
2019-08-27  8:52 ` [PATCH 07/13] dt-bindings: interrupt-controller: Add Loongson-3 HTINTC Jiaxun Yang
2019-08-27 12:49   ` Rob Herring
2019-08-27 14:08     ` Jiaxun Yang
2019-08-27  8:52 ` [PATCH 08/13] irqchip: i8259: Add plat-poll support Jiaxun Yang
2019-08-27  8:52 ` [PATCH 09/13] irqchip: mips-cpu: Convert to simple domain Jiaxun Yang
2019-08-27  8:52 ` [PATCH 10/13] MIPS: Loongson64: Drop legacy IRQ code Jiaxun Yang
2019-08-27  8:53 ` [PATCH 11/13] dt-bindings: mips: Add loongson cpus & boards Jiaxun Yang
2019-08-27 12:45   ` Rob Herring
2019-08-27 14:18     ` Jiaxun Yang
2019-08-27 20:41       ` Paul Burton
2019-08-28  0:15         ` Jiaxun Yang
2019-09-02 14:50         ` Rob Herring
2019-09-03  9:07           ` Paul Burton
2019-09-04  3:33             ` Huacai Chen
2019-08-27  8:53 ` [PATCH 12/13] MIPS: Loongson64: Add generic dts Jiaxun Yang
2019-08-27  8:53 ` [PATCH 13/13] MIPS: Loongson64: Load built-in dtbs Jiaxun Yang
2019-08-27 13:25 ` Re:[PATCH 00/13] Modernize Loongson64 Machine 陈华才
2019-08-27 14:39   ` [PATCH " Jiaxun Yang
2019-08-30  4:25 ` [PATCH v1 00/18] " Jiaxun Yang
2019-08-30  4:25   ` [PATCH v1 01/18] MIPS: Loongson64: Rename CPU TYPES Jiaxun Yang
2019-08-30  4:25   ` [PATCH v1 02/18] MIPS: Loongson64: separate loongson2ef/loongson64 code Jiaxun Yang
2019-08-30  4:25   ` [PATCH v1 03/18] MAINTAINERS: Fix entries for new loongson64 path Jiaxun Yang
2019-08-30  4:25   ` [PATCH v1 04/18] irqchip: Export generic chip domain map/unmap functions Jiaxun Yang
2019-08-30  4:25   ` [PATCH v1 05/18] irqchip: Add driver for Loongson-3 I/O interrupt controller Jiaxun Yang
2019-08-30  4:25   ` [PATCH v1 06/18] dt-bindings: interrupt-controller: Add Loongson-3 IOINTC Jiaxun Yang
2019-09-05 14:42 ` [PATCH v2 00/19] Modernize Loongson64 Machine Jiaxun Yang
2019-09-05 14:42   ` [PATCH v2 01/19] MIPS: Loongson64: Rename CPU TYPES Jiaxun Yang
2019-09-05 14:42   ` [PATCH v2 02/19] MIPS: Loongson64: separate loongson2ef/loongson64 code Jiaxun Yang
2019-09-10  0:12     ` Paul Burton
2019-09-16  2:23       ` Huacai Chen
2019-09-18  0:38       ` Jiaxun Yang
2019-09-05 14:43   ` [PATCH v2 03/19] MAINTAINERS: Fix entries for new loongson64 path Jiaxun Yang
2019-09-05 14:43   ` [PATCH v2 04/19] irqchip: Export generic chip domain map/unmap functions Jiaxun Yang
2019-09-05 14:43   ` [PATCH v2 05/19] irqchip: Add driver for Loongson-3 I/O interrupt controller Jiaxun Yang
2019-09-05 14:43   ` [PATCH v2 06/19] dt-bindings: interrupt-controller: Add Loongson-3 IOINTC Jiaxun Yang
2019-09-17 21:26     ` Rob Herring
2019-09-05 14:43   ` [PATCH v2 07/19] irqchip: Add driver for Loongson-3 HyperTransport interrupt controller Jiaxun Yang
2019-09-05 14:43   ` [PATCH v2 08/19] dt-bindings: interrupt-controller: Add Loongson-3 HTINTC Jiaxun Yang
2019-09-05 14:43   ` [PATCH v2 09/19] irqchip: i8259: Add plat-poll support Jiaxun Yang
2019-09-05 14:43   ` [PATCH v2 10/19] irqchip: mips-cpu: Convert to simple domain Jiaxun Yang
2019-09-05 14:43   ` [PATCH v2 11/19] MIPS: Loongson64: Drop legacy IRQ code Jiaxun Yang
2019-09-05 14:43   ` [PATCH v2 12/19] dt-bindings: mips: Add loongson boards Jiaxun Yang
2019-09-30 13:23     ` Rob Herring
2019-09-05 14:43   ` [PATCH v2 13/19] dt-bindings: Document loongson vendor-prefix Jiaxun Yang
2019-09-05 14:43   ` [PATCH v2 14/19] MIPS: Loongson64: Add generic dts Jiaxun Yang
2019-09-07  2:53     ` Huacai Chen
2019-09-07  3:14       ` Jiaxun Yang
2019-09-30 13:20     ` Rob Herring
2019-09-05 14:43   ` [PATCH v2 15/19] MIPS: Loongson64: Load built-in dtbs Jiaxun Yang
2019-09-05 14:43   ` [PATCH v2 16/19] GPIO: loongson: Drop Loongson-3A/3B support Jiaxun Yang
2019-09-05 14:43   ` [PATCH v2 17/19] MIPS: Loongson: Regenerate defconfigs Jiaxun Yang
2019-09-05 14:43   ` [PATCH v2 18/19] MAINTAINERS: Add new pathes to LOONGSON64 ARCHITECTURE Jiaxun Yang
2019-09-05 14:43   ` [PATCH v2 19/19] MAINTAINERS: Add myself as maintainer of LOONGSON64 Jiaxun Yang
2019-09-07  3:13   ` [PATCH v2 00/19] Modernize Loongson64 Machine Huacai Chen
2019-09-12  6:30 ` [PATCH 00/13] " Matt Turner
2019-09-18  0:33   ` Jiaxun Yang

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=82c4b9ed-7270-74ce-6e10-165182e540dd@flygoat.com \
    --to=jiaxun.yang@flygoat.com \
    --cc=chenhc@lemote.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jason@lakedaemon.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=mark.rutland@arm.co \
    --cc=maz@kernel.org \
    --cc=paul.burton@mips.com \
    --cc=robh+dt@kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-MIPS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mips/0 linux-mips/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mips linux-mips/ https://lore.kernel.org/linux-mips \
		linux-mips@vger.kernel.org
	public-inbox-index linux-mips

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-mips


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git