All of lore.kernel.org
 help / color / mirror / Atom feed
diff for duplicates of <a8ee65eb4d86963bd2b56e86eec0ab3e@kernel.org>

diff --git a/a/1.txt b/N1/1.txt
index 180ed49..71d40ff 100644
--- a/a/1.txt
+++ b/N1/1.txt
@@ -1,280 +1,111 @@
-On 2020-08-19 04:42, Mark-PK Tsai wrote:
-> Add MStar interrupt controller support using hierarchy irq
-> domain.
+From: Marc Zyngier <maz@kernel.org>
+
+> > +
+> > +static int mst_intc_domain_alloc(struct irq_domain *domain, unsigned 
+> > int virq,
+> > +				 unsigned int nr_irqs, void *data)
+> > +{
+> > +	int i;
+> > +	irq_hw_number_t hwirq;
+> > +	struct irq_fwspec parent_fwspec, *fwspec = data;
+> > +	struct mst_intc_chip_data *cd = (struct mst_intc_chip_data
+> > *)domain->host_data;
 > 
-> Signed-off-by: Mark-PK Tsai <mark-pk.tsai@mediatek.com>
-> ---
->  drivers/irqchip/Kconfig        |   7 ++
->  drivers/irqchip/Makefile       |   1 +
->  drivers/irqchip/irq-mst-intc.c | 195 +++++++++++++++++++++++++++++++++
->  3 files changed, 203 insertions(+)
->  create mode 100644 drivers/irqchip/irq-mst-intc.c
+> No cast necessary here.
 > 
-> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
-> index bb70b7177f94..c3a9d880a4ea 100644
-> --- a/drivers/irqchip/Kconfig
-> +++ b/drivers/irqchip/Kconfig
-> @@ -571,4 +571,11 @@ config LOONGSON_PCH_MSI
->  	help
->  	  Support for the Loongson PCH MSI Controller.
+> > +
+> > +	/* Not GIC compliant */
+> > +	if (fwspec->param_count != 3)
+> > +		return -EINVAL;
+> > +
+> > +	/* No PPI should point to this domain */
+> > +	if (fwspec->param[0])
+> > +		return -EINVAL;
+> > +
+> > +	if (fwspec->param[1] >= cd->nr_irqs)
+> 
+> This condition is bogus, as it doesn't take into account the nr_irqs
+> parameter.
 > 
-> +config MST_IRQ
-> +	bool "MStar Interrupt Controller"
-> +	select IRQ_DOMAIN
-> +	select IRQ_DOMAIN_HIERARCHY
-> +	help
-> +	  Support MStar Interrupt Controller.
-
-What selects it? Can it have a default for the relevant platforms?
-
-> +
->  endmenu
-> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
-> index 133f9c45744a..e2688a62403e 100644
-> --- a/drivers/irqchip/Makefile
-> +++ b/drivers/irqchip/Makefile
-> @@ -111,3 +111,4 @@ obj-$(CONFIG_LOONGSON_HTPIC)		+= 
-> irq-loongson-htpic.o
->  obj-$(CONFIG_LOONGSON_HTVEC)		+= irq-loongson-htvec.o
->  obj-$(CONFIG_LOONGSON_PCH_PIC)		+= irq-loongson-pch-pic.o
->  obj-$(CONFIG_LOONGSON_PCH_MSI)		+= irq-loongson-pch-msi.o
-> +obj-$(CONFIG_MST_IRQ)			+= irq-mst-intc.o
-> diff --git a/drivers/irqchip/irq-mst-intc.c 
-> b/drivers/irqchip/irq-mst-intc.c
-> new file mode 100644
-> index 000000000000..38d567741860
-> --- /dev/null
-> +++ b/drivers/irqchip/irq-mst-intc.c
-> @@ -0,0 +1,195 @@
-> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
-> +/*
-> + * Copyright (c) 2020 MediaTek Inc.
-> + * Author Mark-PK Tsai <mark-pk.tsai@mediatek.com>
-> + */
-> +#include <linux/interrupt.h>
-> +#include <linux/io.h>
-> +#include <linux/irq.h>
-> +#include <linux/irqchip.h>
-> +#include <linux/irqdomain.h>
-> +#include <linux/of.h>
-> +#include <linux/of_address.h>
-> +#include <linux/of_irq.h>
-> +#include <linux/slab.h>
-> +#include <linux/spinlock.h>
-> +
-> +#define INTC_MASK	0x0
-> +#define INTC_EOI	0x20
-> +
-> +struct mst_intc_chip_data {
-> +	raw_spinlock_t lock;
-> +	unsigned int irq_start, nr_irqs;
-> +	void __iomem *base;
-> +	bool no_eoi;
-
-nit: please align the fields of the data structure on a vertical line.
-
-> +};
-> +
-> +static void mst_poke_irq(struct irq_data *d, u32 offset)
-
-Given that this is supposed to be the opposite of "clear", this
-should be mst_set_irq().
-
-> +{
-> +	irq_hw_number_t hwirq = irqd_to_hwirq(d);
-> +	struct mst_intc_chip_data *cd = irq_data_get_irq_chip_data(d);
-> +	u16 val, mask;
-> +
-> +	mask = 1 << (hwirq % 16);
-> +	offset += (hwirq / 16) * 4;
-> +
-> +	raw_spin_lock(&cd->lock);
-> +	val = readw_relaxed(cd->base + offset) | mask;
-> +	writew_relaxed(val, cd->base + offset);
-> +	raw_spin_unlock(&cd->lock);
-
-Small improvement, but you still miss the fact that a masking
-operation can also occur from an interrupt handler, and this
-will cause a deadlock. You need to disable interrupts as well.
-
-> +}
-> +
-> +static void mst_clear_irq(struct irq_data *d, u32 offset)
-> +{
-> +	irq_hw_number_t hwirq = irqd_to_hwirq(d);
-> +	struct mst_intc_chip_data *cd = irq_data_get_irq_chip_data(d);
-> +	u16 val, mask;
-> +
-> +	mask = 1 << (hwirq % 16);
-> +	offset += (hwirq / 16) * 4;
-> +
-> +	raw_spin_lock(&cd->lock);
-> +	val = readw_relaxed(cd->base + offset) & ~mask;
-> +	writew_relaxed(val, cd->base + offset);
-> +	raw_spin_unlock(&cd->lock);
-> +}
-> +
-> +static void mst_intc_mask_irq(struct irq_data *d)
-> +{
-> +	mst_poke_irq(d, INTC_MASK);
-> +	irq_chip_mask_parent(d);
-> +}
-> +
-> +static void mst_intc_unmask_irq(struct irq_data *d)
-> +{
-> +	mst_clear_irq(d, INTC_MASK);
-> +	irq_chip_unmask_parent(d);
-> +}
-> +
-> +static void mst_intc_eoi_irq(struct irq_data *d)
-> +{
-> +	struct mst_intc_chip_data *cd = irq_data_get_irq_chip_data(d);
-> +
-> +	if (!cd->no_eoi)
-> +		mst_poke_irq(d, INTC_EOI);
-> +
-> +	irq_chip_eoi_parent(d);
-> +}
-> +
-> +static struct irq_chip mst_intc_chip = {
-> +	.name			= "mst-intc",
-> +	.irq_mask		= mst_intc_mask_irq,
-> +	.irq_unmask		= mst_intc_unmask_irq,
-> +	.irq_eoi		= mst_intc_eoi_irq,
-> +	.irq_get_irqchip_state	= irq_chip_get_parent_state,
-> +	.irq_set_irqchip_state	= irq_chip_set_parent_state,
-> +	.irq_set_affinity	= irq_chip_set_affinity_parent,
-> +	.irq_set_vcpu_affinity	= irq_chip_set_vcpu_affinity_parent,
-> +	.irq_set_type		= irq_chip_set_type_parent,
-> +	.irq_retrigger		= irq_chip_retrigger_hierarchy,
-> +	.flags			= IRQCHIP_SET_TYPE_MASKED |
-> +				  IRQCHIP_SKIP_SET_WAKE |
-> +				  IRQCHIP_MASK_ON_SUSPEND,
-> +};
-> +
-> +static int mst_intc_domain_translate(struct irq_domain *d,
-> +				     struct irq_fwspec *fwspec,
-> +				     unsigned long *hwirq,
-> +				     unsigned int *type)
-> +{
-> +	if (is_of_node(fwspec->fwnode)) {
-> +		if (fwspec->param_count != 3)
-> +			return -EINVAL;
-> +
-> +		/* No PPI should point to this domain */
-> +		if (fwspec->param[0] != 0)
-> +			return -EINVAL;
-> +
-> +		*hwirq = fwspec->param[1];
-> +		*type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;
-> +		return 0;
-> +	}
-> +
-> +	return -EINVAL;
-> +}
-> +
-> +static int mst_intc_domain_alloc(struct irq_domain *domain, unsigned 
-> int virq,
-> +				 unsigned int nr_irqs, void *data)
-> +{
-> +	int i;
-> +	irq_hw_number_t hwirq;
-> +	struct irq_fwspec parent_fwspec, *fwspec = data;
-> +	struct mst_intc_chip_data *cd = (struct mst_intc_chip_data
-> *)domain->host_data;
-
-No cast necessary here.
-
-> +
-> +	/* Not GIC compliant */
-> +	if (fwspec->param_count != 3)
-> +		return -EINVAL;
-> +
-> +	/* No PPI should point to this domain */
-> +	if (fwspec->param[0])
-> +		return -EINVAL;
-> +
-> +	if (fwspec->param[1] >= cd->nr_irqs)
-
-This condition is bogus, as it doesn't take into account the nr_irqs
-parameter.
-
-> +		return -EINVAL;
-> +
-> +	hwirq = fwspec->param[1];
-> +	for (i = 0; i < nr_irqs; i++)
-> +		irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
-> +					      &mst_intc_chip,
-> +					      domain->host_data);
-> +
-> +	parent_fwspec = *fwspec;
-> +	parent_fwspec.fwnode = domain->parent->fwnode;
-> +	parent_fwspec.param[1] = cd->irq_start + hwirq;
-> +	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, 
-> &parent_fwspec);
-> +}
-> +
-> +static const struct irq_domain_ops mst_intc_domain_ops = {
-> +	.translate	= mst_intc_domain_translate,
-> +	.alloc		= mst_intc_domain_alloc,
-> +	.free		= irq_domain_free_irqs_common,
-> +};
-> +
-> +int __init
-> +mst_intc_of_init(struct device_node *dn, struct device_node *parent)
-> +{
-> +	struct irq_domain *domain, *domain_parent;
-> +	struct mst_intc_chip_data *cd;
-> +	unsigned int irq_start, irq_end;
-
-unsigned int != u32.
-
-> +
-> +	domain_parent = irq_find_host(parent);
-> +	if (!domain_parent) {
-> +		pr_err("mst-intc: interrupt-parent not found\n");
-> +		return -EINVAL;
-> +	}
-> +
-> +	if (of_property_read_u32_index(dn, "mstar,irqs-map-range", 0, 
-> &irq_start) ||
-> +	    of_property_read_u32_index(dn, "mstar,irqs-map-range", 1, 
-> &irq_end))
-> +		return -EINVAL;
-> +
-> +	cd = kzalloc(sizeof(*cd), GFP_KERNEL);
-> +	if (!cd)
-> +		return -ENOMEM;
-> +
-> +	cd->base = of_iomap(dn, 0);
-> +	if (!cd->base) {
-> +		kfree(cd);
-> +		return -ENOMEM;
-> +	}
-> +
-> +	cd->no_eoi = of_property_read_bool(dn, "mstar,intc-no-eoi");
-> +	raw_spin_lock_init(&cd->lock);
-> +	cd->irq_start = irq_start;
-> +	cd->nr_irqs = irq_end - irq_start + 1;
-> +	domain = irq_domain_add_hierarchy(domain_parent, 0, cd->nr_irqs, dn,
-> +					  &mst_intc_domain_ops, cd);
-> +	if (!domain) {
-> +		iounmap(cd->base);
-> +		kfree(cd);
-> +		return -ENOMEM;
-> +	}
-> +
-> +	return 0;
-> +}
-> +
-> +IRQCHIP_DECLARE(mst_intc, "mstar,mst-intc", mst_intc_of_init);
-
-Thanks,
 
-         M.
--- 
-Jazz is not dead. It just smells funny...
 
+The hwirq number need to be in the irq map range. (property: mstar,irqs-map-range)
+If it's not, it must be incorrect configuration.
+So how about use the condition as following?
+
+if (hwirq >= cd->nr_irqs)
+	return -EINVAL;
+
+> > +		return -EINVAL;
+> > +
+> > +	hwirq = fwspec->param[1];
+> > +	for (i = 0; i < nr_irqs; i++)
+> > +		irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
+> > +					      &mst_intc_chip,
+> > +					      domain->host_data);
+> > +
+> > +	parent_fwspec = *fwspec;
+> > +	parent_fwspec.fwnode = domain->parent->fwnode;
+> > +	parent_fwspec.param[1] = cd->irq_start + hwirq;
+> > +	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, 
+> > &parent_fwspec);
+> > +}
+> > +
+> > +static const struct irq_domain_ops mst_intc_domain_ops = {
+> > +	.translate	= mst_intc_domain_translate,
+> > +	.alloc		= mst_intc_domain_alloc,
+> > +	.free		= irq_domain_free_irqs_common,
+> > +};
+> > +
+> > +int __init
+> > +mst_intc_of_init(struct device_node *dn, struct device_node *parent)
+> > +{
+> > +	struct irq_domain *domain, *domain_parent;
+> > +	struct mst_intc_chip_data *cd;
+> > +	unsigned int irq_start, irq_end;
+> 
+> unsigned int != u32.
+> 
+> > +
+> > +	domain_parent = irq_find_host(parent);
+> > +	if (!domain_parent) {
+> > +		pr_err("mst-intc: interrupt-parent not found\n");
+> > +		return -EINVAL;
+> > +	}
+> > +
+> > +	if (of_property_read_u32_index(dn, "mstar,irqs-map-range", 0, 
+> > &irq_start) ||
+> > +	    of_property_read_u32_index(dn, "mstar,irqs-map-range", 1, 
+> > &irq_end))
+> > +		return -EINVAL;
+> > +
+> > +	cd = kzalloc(sizeof(*cd), GFP_KERNEL);
+> > +	if (!cd)
+> > +		return -ENOMEM;
+> > +
+> > +	cd->base = of_iomap(dn, 0);
+> > +	if (!cd->base) {
+> > +		kfree(cd);
+> > +		return -ENOMEM;
+> > +	}
+> > +
+> > +	cd->no_eoi = of_property_read_bool(dn, "mstar,intc-no-eoi");
+> > +	raw_spin_lock_init(&cd->lock);
+> > +	cd->irq_start = irq_start;
+> > +	cd->nr_irqs = irq_end - irq_start + 1;
+> > +	domain = irq_domain_add_hierarchy(domain_parent, 0, cd->nr_irqs, dn,
+> > +					  &mst_intc_domain_ops, cd);
+> > +	if (!domain) {
+> > +		iounmap(cd->base);
+> > +		kfree(cd);
+> > +		return -ENOMEM;
+> > +	}
+> > +
+> > +	return 0;
+> > +}
+> > +
+> > +IRQCHIP_DECLARE(mst_intc, "mstar,mst-intc", mst_intc_of_init);
+> 
 _______________________________________________
 Linux-mediatek mailing list
 Linux-mediatek@lists.infradead.org
diff --git a/a/content_digest b/N1/content_digest
index 8223824..8833ef2 100644
--- a/a/content_digest
+++ b/N1/content_digest
@@ -1,9 +1,8 @@
- "ref\020200819034231.20726-1-mark-pk.tsai@mediatek.com\0"
- "ref\020200819034231.20726-2-mark-pk.tsai@mediatek.com\0"
- "From\0Marc Zyngier <maz@kernel.org>\0"
+ "From\0Mark-PK Tsai <mark-pk.tsai@mediatek.com>\0"
  "Subject\0Re: [PATCH 1/2] irqchip: irq-mst: Add MStar interrupt controller support\0"
- "Date\0Wed, 19 Aug 2020 09:14:24 +0100\0"
- "To\0Mark-PK Tsai <mark-pk.tsai@mediatek.com>\0"
+ "Date\0Wed, 19 Aug 2020 21:31:18 +0800\0"
+ "To\0<maz@kernel.org>"
+ " Mark-PK Tsai <mark-pk.tsai@mediatek.com>\0"
  "Cc\0devicetree@vger.kernel.org"
   alix.wu@mediatek.com
   jason@lakedaemon.net
@@ -17,286 +16,117 @@
  " linux-arm-kernel@lists.infradead.org\0"
  "\00:1\0"
  "b\0"
- "On 2020-08-19 04:42, Mark-PK Tsai wrote:\n"
- "> Add MStar interrupt controller support using hierarchy irq\n"
- "> domain.\n"
+ "From: Marc Zyngier <maz@kernel.org>\n"
+ "\n"
+ "> > +\n"
+ "> > +static int mst_intc_domain_alloc(struct irq_domain *domain, unsigned \n"
+ "> > int virq,\n"
+ "> > +\t\t\t\t unsigned int nr_irqs, void *data)\n"
+ "> > +{\n"
+ "> > +\tint i;\n"
+ "> > +\tirq_hw_number_t hwirq;\n"
+ "> > +\tstruct irq_fwspec parent_fwspec, *fwspec = data;\n"
+ "> > +\tstruct mst_intc_chip_data *cd = (struct mst_intc_chip_data\n"
+ "> > *)domain->host_data;\n"
  "> \n"
- "> Signed-off-by: Mark-PK Tsai <mark-pk.tsai@mediatek.com>\n"
- "> ---\n"
- ">  drivers/irqchip/Kconfig        |   7 ++\n"
- ">  drivers/irqchip/Makefile       |   1 +\n"
- ">  drivers/irqchip/irq-mst-intc.c | 195 +++++++++++++++++++++++++++++++++\n"
- ">  3 files changed, 203 insertions(+)\n"
- ">  create mode 100644 drivers/irqchip/irq-mst-intc.c\n"
+ "> No cast necessary here.\n"
  "> \n"
- "> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig\n"
- "> index bb70b7177f94..c3a9d880a4ea 100644\n"
- "> --- a/drivers/irqchip/Kconfig\n"
- "> +++ b/drivers/irqchip/Kconfig\n"
- "> @@ -571,4 +571,11 @@ config LOONGSON_PCH_MSI\n"
- ">  \thelp\n"
- ">  \t  Support for the Loongson PCH MSI Controller.\n"
+ "> > +\n"
+ "> > +\t/* Not GIC compliant */\n"
+ "> > +\tif (fwspec->param_count != 3)\n"
+ "> > +\t\treturn -EINVAL;\n"
+ "> > +\n"
+ "> > +\t/* No PPI should point to this domain */\n"
+ "> > +\tif (fwspec->param[0])\n"
+ "> > +\t\treturn -EINVAL;\n"
+ "> > +\n"
+ "> > +\tif (fwspec->param[1] >= cd->nr_irqs)\n"
+ "> \n"
+ "> This condition is bogus, as it doesn't take into account the nr_irqs\n"
+ "> parameter.\n"
  "> \n"
- "> +config MST_IRQ\n"
- "> +\tbool \"MStar Interrupt Controller\"\n"
- "> +\tselect IRQ_DOMAIN\n"
- "> +\tselect IRQ_DOMAIN_HIERARCHY\n"
- "> +\thelp\n"
- "> +\t  Support MStar Interrupt Controller.\n"
- "\n"
- "What selects it? Can it have a default for the relevant platforms?\n"
- "\n"
- "> +\n"
- ">  endmenu\n"
- "> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile\n"
- "> index 133f9c45744a..e2688a62403e 100644\n"
- "> --- a/drivers/irqchip/Makefile\n"
- "> +++ b/drivers/irqchip/Makefile\n"
- "> @@ -111,3 +111,4 @@ obj-$(CONFIG_LOONGSON_HTPIC)\t\t+= \n"
- "> irq-loongson-htpic.o\n"
- ">  obj-$(CONFIG_LOONGSON_HTVEC)\t\t+= irq-loongson-htvec.o\n"
- ">  obj-$(CONFIG_LOONGSON_PCH_PIC)\t\t+= irq-loongson-pch-pic.o\n"
- ">  obj-$(CONFIG_LOONGSON_PCH_MSI)\t\t+= irq-loongson-pch-msi.o\n"
- "> +obj-$(CONFIG_MST_IRQ)\t\t\t+= irq-mst-intc.o\n"
- "> diff --git a/drivers/irqchip/irq-mst-intc.c \n"
- "> b/drivers/irqchip/irq-mst-intc.c\n"
- "> new file mode 100644\n"
- "> index 000000000000..38d567741860\n"
- "> --- /dev/null\n"
- "> +++ b/drivers/irqchip/irq-mst-intc.c\n"
- "> @@ -0,0 +1,195 @@\n"
- "> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)\n"
- "> +/*\n"
- "> + * Copyright (c) 2020 MediaTek Inc.\n"
- "> + * Author Mark-PK Tsai <mark-pk.tsai@mediatek.com>\n"
- "> + */\n"
- "> +#include <linux/interrupt.h>\n"
- "> +#include <linux/io.h>\n"
- "> +#include <linux/irq.h>\n"
- "> +#include <linux/irqchip.h>\n"
- "> +#include <linux/irqdomain.h>\n"
- "> +#include <linux/of.h>\n"
- "> +#include <linux/of_address.h>\n"
- "> +#include <linux/of_irq.h>\n"
- "> +#include <linux/slab.h>\n"
- "> +#include <linux/spinlock.h>\n"
- "> +\n"
- "> +#define INTC_MASK\t0x0\n"
- "> +#define INTC_EOI\t0x20\n"
- "> +\n"
- "> +struct mst_intc_chip_data {\n"
- "> +\traw_spinlock_t lock;\n"
- "> +\tunsigned int irq_start, nr_irqs;\n"
- "> +\tvoid __iomem *base;\n"
- "> +\tbool no_eoi;\n"
- "\n"
- "nit: please align the fields of the data structure on a vertical line.\n"
- "\n"
- "> +};\n"
- "> +\n"
- "> +static void mst_poke_irq(struct irq_data *d, u32 offset)\n"
- "\n"
- "Given that this is supposed to be the opposite of \"clear\", this\n"
- "should be mst_set_irq().\n"
- "\n"
- "> +{\n"
- "> +\tirq_hw_number_t hwirq = irqd_to_hwirq(d);\n"
- "> +\tstruct mst_intc_chip_data *cd = irq_data_get_irq_chip_data(d);\n"
- "> +\tu16 val, mask;\n"
- "> +\n"
- "> +\tmask = 1 << (hwirq % 16);\n"
- "> +\toffset += (hwirq / 16) * 4;\n"
- "> +\n"
- "> +\traw_spin_lock(&cd->lock);\n"
- "> +\tval = readw_relaxed(cd->base + offset) | mask;\n"
- "> +\twritew_relaxed(val, cd->base + offset);\n"
- "> +\traw_spin_unlock(&cd->lock);\n"
- "\n"
- "Small improvement, but you still miss the fact that a masking\n"
- "operation can also occur from an interrupt handler, and this\n"
- "will cause a deadlock. You need to disable interrupts as well.\n"
- "\n"
- "> +}\n"
- "> +\n"
- "> +static void mst_clear_irq(struct irq_data *d, u32 offset)\n"
- "> +{\n"
- "> +\tirq_hw_number_t hwirq = irqd_to_hwirq(d);\n"
- "> +\tstruct mst_intc_chip_data *cd = irq_data_get_irq_chip_data(d);\n"
- "> +\tu16 val, mask;\n"
- "> +\n"
- "> +\tmask = 1 << (hwirq % 16);\n"
- "> +\toffset += (hwirq / 16) * 4;\n"
- "> +\n"
- "> +\traw_spin_lock(&cd->lock);\n"
- "> +\tval = readw_relaxed(cd->base + offset) & ~mask;\n"
- "> +\twritew_relaxed(val, cd->base + offset);\n"
- "> +\traw_spin_unlock(&cd->lock);\n"
- "> +}\n"
- "> +\n"
- "> +static void mst_intc_mask_irq(struct irq_data *d)\n"
- "> +{\n"
- "> +\tmst_poke_irq(d, INTC_MASK);\n"
- "> +\tirq_chip_mask_parent(d);\n"
- "> +}\n"
- "> +\n"
- "> +static void mst_intc_unmask_irq(struct irq_data *d)\n"
- "> +{\n"
- "> +\tmst_clear_irq(d, INTC_MASK);\n"
- "> +\tirq_chip_unmask_parent(d);\n"
- "> +}\n"
- "> +\n"
- "> +static void mst_intc_eoi_irq(struct irq_data *d)\n"
- "> +{\n"
- "> +\tstruct mst_intc_chip_data *cd = irq_data_get_irq_chip_data(d);\n"
- "> +\n"
- "> +\tif (!cd->no_eoi)\n"
- "> +\t\tmst_poke_irq(d, INTC_EOI);\n"
- "> +\n"
- "> +\tirq_chip_eoi_parent(d);\n"
- "> +}\n"
- "> +\n"
- "> +static struct irq_chip mst_intc_chip = {\n"
- "> +\t.name\t\t\t= \"mst-intc\",\n"
- "> +\t.irq_mask\t\t= mst_intc_mask_irq,\n"
- "> +\t.irq_unmask\t\t= mst_intc_unmask_irq,\n"
- "> +\t.irq_eoi\t\t= mst_intc_eoi_irq,\n"
- "> +\t.irq_get_irqchip_state\t= irq_chip_get_parent_state,\n"
- "> +\t.irq_set_irqchip_state\t= irq_chip_set_parent_state,\n"
- "> +\t.irq_set_affinity\t= irq_chip_set_affinity_parent,\n"
- "> +\t.irq_set_vcpu_affinity\t= irq_chip_set_vcpu_affinity_parent,\n"
- "> +\t.irq_set_type\t\t= irq_chip_set_type_parent,\n"
- "> +\t.irq_retrigger\t\t= irq_chip_retrigger_hierarchy,\n"
- "> +\t.flags\t\t\t= IRQCHIP_SET_TYPE_MASKED |\n"
- "> +\t\t\t\t  IRQCHIP_SKIP_SET_WAKE |\n"
- "> +\t\t\t\t  IRQCHIP_MASK_ON_SUSPEND,\n"
- "> +};\n"
- "> +\n"
- "> +static int mst_intc_domain_translate(struct irq_domain *d,\n"
- "> +\t\t\t\t     struct irq_fwspec *fwspec,\n"
- "> +\t\t\t\t     unsigned long *hwirq,\n"
- "> +\t\t\t\t     unsigned int *type)\n"
- "> +{\n"
- "> +\tif (is_of_node(fwspec->fwnode)) {\n"
- "> +\t\tif (fwspec->param_count != 3)\n"
- "> +\t\t\treturn -EINVAL;\n"
- "> +\n"
- "> +\t\t/* No PPI should point to this domain */\n"
- "> +\t\tif (fwspec->param[0] != 0)\n"
- "> +\t\t\treturn -EINVAL;\n"
- "> +\n"
- "> +\t\t*hwirq = fwspec->param[1];\n"
- "> +\t\t*type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;\n"
- "> +\t\treturn 0;\n"
- "> +\t}\n"
- "> +\n"
- "> +\treturn -EINVAL;\n"
- "> +}\n"
- "> +\n"
- "> +static int mst_intc_domain_alloc(struct irq_domain *domain, unsigned \n"
- "> int virq,\n"
- "> +\t\t\t\t unsigned int nr_irqs, void *data)\n"
- "> +{\n"
- "> +\tint i;\n"
- "> +\tirq_hw_number_t hwirq;\n"
- "> +\tstruct irq_fwspec parent_fwspec, *fwspec = data;\n"
- "> +\tstruct mst_intc_chip_data *cd = (struct mst_intc_chip_data\n"
- "> *)domain->host_data;\n"
- "\n"
- "No cast necessary here.\n"
- "\n"
- "> +\n"
- "> +\t/* Not GIC compliant */\n"
- "> +\tif (fwspec->param_count != 3)\n"
- "> +\t\treturn -EINVAL;\n"
- "> +\n"
- "> +\t/* No PPI should point to this domain */\n"
- "> +\tif (fwspec->param[0])\n"
- "> +\t\treturn -EINVAL;\n"
- "> +\n"
- "> +\tif (fwspec->param[1] >= cd->nr_irqs)\n"
- "\n"
- "This condition is bogus, as it doesn't take into account the nr_irqs\n"
- "parameter.\n"
- "\n"
- "> +\t\treturn -EINVAL;\n"
- "> +\n"
- "> +\thwirq = fwspec->param[1];\n"
- "> +\tfor (i = 0; i < nr_irqs; i++)\n"
- "> +\t\tirq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,\n"
- "> +\t\t\t\t\t      &mst_intc_chip,\n"
- "> +\t\t\t\t\t      domain->host_data);\n"
- "> +\n"
- "> +\tparent_fwspec = *fwspec;\n"
- "> +\tparent_fwspec.fwnode = domain->parent->fwnode;\n"
- "> +\tparent_fwspec.param[1] = cd->irq_start + hwirq;\n"
- "> +\treturn irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, \n"
- "> &parent_fwspec);\n"
- "> +}\n"
- "> +\n"
- "> +static const struct irq_domain_ops mst_intc_domain_ops = {\n"
- "> +\t.translate\t= mst_intc_domain_translate,\n"
- "> +\t.alloc\t\t= mst_intc_domain_alloc,\n"
- "> +\t.free\t\t= irq_domain_free_irqs_common,\n"
- "> +};\n"
- "> +\n"
- "> +int __init\n"
- "> +mst_intc_of_init(struct device_node *dn, struct device_node *parent)\n"
- "> +{\n"
- "> +\tstruct irq_domain *domain, *domain_parent;\n"
- "> +\tstruct mst_intc_chip_data *cd;\n"
- "> +\tunsigned int irq_start, irq_end;\n"
- "\n"
- "unsigned int != u32.\n"
- "\n"
- "> +\n"
- "> +\tdomain_parent = irq_find_host(parent);\n"
- "> +\tif (!domain_parent) {\n"
- "> +\t\tpr_err(\"mst-intc: interrupt-parent not found\\n\");\n"
- "> +\t\treturn -EINVAL;\n"
- "> +\t}\n"
- "> +\n"
- "> +\tif (of_property_read_u32_index(dn, \"mstar,irqs-map-range\", 0, \n"
- "> &irq_start) ||\n"
- "> +\t    of_property_read_u32_index(dn, \"mstar,irqs-map-range\", 1, \n"
- "> &irq_end))\n"
- "> +\t\treturn -EINVAL;\n"
- "> +\n"
- "> +\tcd = kzalloc(sizeof(*cd), GFP_KERNEL);\n"
- "> +\tif (!cd)\n"
- "> +\t\treturn -ENOMEM;\n"
- "> +\n"
- "> +\tcd->base = of_iomap(dn, 0);\n"
- "> +\tif (!cd->base) {\n"
- "> +\t\tkfree(cd);\n"
- "> +\t\treturn -ENOMEM;\n"
- "> +\t}\n"
- "> +\n"
- "> +\tcd->no_eoi = of_property_read_bool(dn, \"mstar,intc-no-eoi\");\n"
- "> +\traw_spin_lock_init(&cd->lock);\n"
- "> +\tcd->irq_start = irq_start;\n"
- "> +\tcd->nr_irqs = irq_end - irq_start + 1;\n"
- "> +\tdomain = irq_domain_add_hierarchy(domain_parent, 0, cd->nr_irqs, dn,\n"
- "> +\t\t\t\t\t  &mst_intc_domain_ops, cd);\n"
- "> +\tif (!domain) {\n"
- "> +\t\tiounmap(cd->base);\n"
- "> +\t\tkfree(cd);\n"
- "> +\t\treturn -ENOMEM;\n"
- "> +\t}\n"
- "> +\n"
- "> +\treturn 0;\n"
- "> +}\n"
- "> +\n"
- "> +IRQCHIP_DECLARE(mst_intc, \"mstar,mst-intc\", mst_intc_of_init);\n"
- "\n"
- "Thanks,\n"
  "\n"
- "         M.\n"
- "-- \n"
- "Jazz is not dead. It just smells funny...\n"
  "\n"
+ "The hwirq number need to be in the irq map range. (property: mstar,irqs-map-range)\n"
+ "If it's not, it must be incorrect configuration.\n"
+ "So how about use the condition as following?\n"
+ "\n"
+ "if (hwirq >= cd->nr_irqs)\n"
+ "\treturn -EINVAL;\n"
+ "\n"
+ "> > +\t\treturn -EINVAL;\n"
+ "> > +\n"
+ "> > +\thwirq = fwspec->param[1];\n"
+ "> > +\tfor (i = 0; i < nr_irqs; i++)\n"
+ "> > +\t\tirq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,\n"
+ "> > +\t\t\t\t\t      &mst_intc_chip,\n"
+ "> > +\t\t\t\t\t      domain->host_data);\n"
+ "> > +\n"
+ "> > +\tparent_fwspec = *fwspec;\n"
+ "> > +\tparent_fwspec.fwnode = domain->parent->fwnode;\n"
+ "> > +\tparent_fwspec.param[1] = cd->irq_start + hwirq;\n"
+ "> > +\treturn irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, \n"
+ "> > &parent_fwspec);\n"
+ "> > +}\n"
+ "> > +\n"
+ "> > +static const struct irq_domain_ops mst_intc_domain_ops = {\n"
+ "> > +\t.translate\t= mst_intc_domain_translate,\n"
+ "> > +\t.alloc\t\t= mst_intc_domain_alloc,\n"
+ "> > +\t.free\t\t= irq_domain_free_irqs_common,\n"
+ "> > +};\n"
+ "> > +\n"
+ "> > +int __init\n"
+ "> > +mst_intc_of_init(struct device_node *dn, struct device_node *parent)\n"
+ "> > +{\n"
+ "> > +\tstruct irq_domain *domain, *domain_parent;\n"
+ "> > +\tstruct mst_intc_chip_data *cd;\n"
+ "> > +\tunsigned int irq_start, irq_end;\n"
+ "> \n"
+ "> unsigned int != u32.\n"
+ "> \n"
+ "> > +\n"
+ "> > +\tdomain_parent = irq_find_host(parent);\n"
+ "> > +\tif (!domain_parent) {\n"
+ "> > +\t\tpr_err(\"mst-intc: interrupt-parent not found\\n\");\n"
+ "> > +\t\treturn -EINVAL;\n"
+ "> > +\t}\n"
+ "> > +\n"
+ "> > +\tif (of_property_read_u32_index(dn, \"mstar,irqs-map-range\", 0, \n"
+ "> > &irq_start) ||\n"
+ "> > +\t    of_property_read_u32_index(dn, \"mstar,irqs-map-range\", 1, \n"
+ "> > &irq_end))\n"
+ "> > +\t\treturn -EINVAL;\n"
+ "> > +\n"
+ "> > +\tcd = kzalloc(sizeof(*cd), GFP_KERNEL);\n"
+ "> > +\tif (!cd)\n"
+ "> > +\t\treturn -ENOMEM;\n"
+ "> > +\n"
+ "> > +\tcd->base = of_iomap(dn, 0);\n"
+ "> > +\tif (!cd->base) {\n"
+ "> > +\t\tkfree(cd);\n"
+ "> > +\t\treturn -ENOMEM;\n"
+ "> > +\t}\n"
+ "> > +\n"
+ "> > +\tcd->no_eoi = of_property_read_bool(dn, \"mstar,intc-no-eoi\");\n"
+ "> > +\traw_spin_lock_init(&cd->lock);\n"
+ "> > +\tcd->irq_start = irq_start;\n"
+ "> > +\tcd->nr_irqs = irq_end - irq_start + 1;\n"
+ "> > +\tdomain = irq_domain_add_hierarchy(domain_parent, 0, cd->nr_irqs, dn,\n"
+ "> > +\t\t\t\t\t  &mst_intc_domain_ops, cd);\n"
+ "> > +\tif (!domain) {\n"
+ "> > +\t\tiounmap(cd->base);\n"
+ "> > +\t\tkfree(cd);\n"
+ "> > +\t\treturn -ENOMEM;\n"
+ "> > +\t}\n"
+ "> > +\n"
+ "> > +\treturn 0;\n"
+ "> > +}\n"
+ "> > +\n"
+ "> > +IRQCHIP_DECLARE(mst_intc, \"mstar,mst-intc\", mst_intc_of_init);\n"
+ "> \n"
  "_______________________________________________\n"
  "Linux-mediatek mailing list\n"
  "Linux-mediatek@lists.infradead.org\n"
  http://lists.infradead.org/mailman/listinfo/linux-mediatek
 
-e094b2c5f51d6c831122683cd7f1f7290a94d496a7d3d6d56b860a22901c0e18
+3280b3c604be0374f341fe747b4a5c9c7679978da39d856395c28c50ec8bb313

diff --git a/a/1.txt b/N2/1.txt
index 180ed49..83deddc 100644
--- a/a/1.txt
+++ b/N2/1.txt
@@ -276,6 +276,6 @@ Thanks,
 Jazz is not dead. It just smells funny...
 
 _______________________________________________
-Linux-mediatek mailing list
-Linux-mediatek@lists.infradead.org
-http://lists.infradead.org/mailman/listinfo/linux-mediatek
+linux-arm-kernel mailing list
+linux-arm-kernel@lists.infradead.org
+http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/a/content_digest b/N2/content_digest
index 8223824..0ddae57 100644
--- a/a/content_digest
+++ b/N2/content_digest
@@ -295,8 +295,8 @@
  "Jazz is not dead. It just smells funny...\n"
  "\n"
  "_______________________________________________\n"
- "Linux-mediatek mailing list\n"
- "Linux-mediatek@lists.infradead.org\n"
- http://lists.infradead.org/mailman/listinfo/linux-mediatek
+ "linux-arm-kernel mailing list\n"
+ "linux-arm-kernel@lists.infradead.org\n"
+ http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
 
-e094b2c5f51d6c831122683cd7f1f7290a94d496a7d3d6d56b860a22901c0e18
+8f2fa51e75bd21d57b9198775d38ab94e9833fbdf5f80c519cc3e1848ed02db4

diff --git a/a/1.txt b/N3/1.txt
index 180ed49..cada067 100644
--- a/a/1.txt
+++ b/N3/1.txt
@@ -1,281 +1,112 @@
-On 2020-08-19 04:42, Mark-PK Tsai wrote:
-> Add MStar interrupt controller support using hierarchy irq
-> domain.
+From: Marc Zyngier <maz@kernel.org>
+
+> > +
+> > +static int mst_intc_domain_alloc(struct irq_domain *domain, unsigned 
+> > int virq,
+> > +				 unsigned int nr_irqs, void *data)
+> > +{
+> > +	int i;
+> > +	irq_hw_number_t hwirq;
+> > +	struct irq_fwspec parent_fwspec, *fwspec = data;
+> > +	struct mst_intc_chip_data *cd = (struct mst_intc_chip_data
+> > *)domain->host_data;
 > 
-> Signed-off-by: Mark-PK Tsai <mark-pk.tsai@mediatek.com>
-> ---
->  drivers/irqchip/Kconfig        |   7 ++
->  drivers/irqchip/Makefile       |   1 +
->  drivers/irqchip/irq-mst-intc.c | 195 +++++++++++++++++++++++++++++++++
->  3 files changed, 203 insertions(+)
->  create mode 100644 drivers/irqchip/irq-mst-intc.c
+> No cast necessary here.
 > 
-> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
-> index bb70b7177f94..c3a9d880a4ea 100644
-> --- a/drivers/irqchip/Kconfig
-> +++ b/drivers/irqchip/Kconfig
-> @@ -571,4 +571,11 @@ config LOONGSON_PCH_MSI
->  	help
->  	  Support for the Loongson PCH MSI Controller.
+> > +
+> > +	/* Not GIC compliant */
+> > +	if (fwspec->param_count != 3)
+> > +		return -EINVAL;
+> > +
+> > +	/* No PPI should point to this domain */
+> > +	if (fwspec->param[0])
+> > +		return -EINVAL;
+> > +
+> > +	if (fwspec->param[1] >= cd->nr_irqs)
+> 
+> This condition is bogus, as it doesn't take into account the nr_irqs
+> parameter.
 > 
-> +config MST_IRQ
-> +	bool "MStar Interrupt Controller"
-> +	select IRQ_DOMAIN
-> +	select IRQ_DOMAIN_HIERARCHY
-> +	help
-> +	  Support MStar Interrupt Controller.
-
-What selects it? Can it have a default for the relevant platforms?
-
-> +
->  endmenu
-> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
-> index 133f9c45744a..e2688a62403e 100644
-> --- a/drivers/irqchip/Makefile
-> +++ b/drivers/irqchip/Makefile
-> @@ -111,3 +111,4 @@ obj-$(CONFIG_LOONGSON_HTPIC)		+= 
-> irq-loongson-htpic.o
->  obj-$(CONFIG_LOONGSON_HTVEC)		+= irq-loongson-htvec.o
->  obj-$(CONFIG_LOONGSON_PCH_PIC)		+= irq-loongson-pch-pic.o
->  obj-$(CONFIG_LOONGSON_PCH_MSI)		+= irq-loongson-pch-msi.o
-> +obj-$(CONFIG_MST_IRQ)			+= irq-mst-intc.o
-> diff --git a/drivers/irqchip/irq-mst-intc.c 
-> b/drivers/irqchip/irq-mst-intc.c
-> new file mode 100644
-> index 000000000000..38d567741860
-> --- /dev/null
-> +++ b/drivers/irqchip/irq-mst-intc.c
-> @@ -0,0 +1,195 @@
-> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
-> +/*
-> + * Copyright (c) 2020 MediaTek Inc.
-> + * Author Mark-PK Tsai <mark-pk.tsai@mediatek.com>
-> + */
-> +#include <linux/interrupt.h>
-> +#include <linux/io.h>
-> +#include <linux/irq.h>
-> +#include <linux/irqchip.h>
-> +#include <linux/irqdomain.h>
-> +#include <linux/of.h>
-> +#include <linux/of_address.h>
-> +#include <linux/of_irq.h>
-> +#include <linux/slab.h>
-> +#include <linux/spinlock.h>
-> +
-> +#define INTC_MASK	0x0
-> +#define INTC_EOI	0x20
-> +
-> +struct mst_intc_chip_data {
-> +	raw_spinlock_t lock;
-> +	unsigned int irq_start, nr_irqs;
-> +	void __iomem *base;
-> +	bool no_eoi;
-
-nit: please align the fields of the data structure on a vertical line.
-
-> +};
-> +
-> +static void mst_poke_irq(struct irq_data *d, u32 offset)
-
-Given that this is supposed to be the opposite of "clear", this
-should be mst_set_irq().
-
-> +{
-> +	irq_hw_number_t hwirq = irqd_to_hwirq(d);
-> +	struct mst_intc_chip_data *cd = irq_data_get_irq_chip_data(d);
-> +	u16 val, mask;
-> +
-> +	mask = 1 << (hwirq % 16);
-> +	offset += (hwirq / 16) * 4;
-> +
-> +	raw_spin_lock(&cd->lock);
-> +	val = readw_relaxed(cd->base + offset) | mask;
-> +	writew_relaxed(val, cd->base + offset);
-> +	raw_spin_unlock(&cd->lock);
-
-Small improvement, but you still miss the fact that a masking
-operation can also occur from an interrupt handler, and this
-will cause a deadlock. You need to disable interrupts as well.
-
-> +}
-> +
-> +static void mst_clear_irq(struct irq_data *d, u32 offset)
-> +{
-> +	irq_hw_number_t hwirq = irqd_to_hwirq(d);
-> +	struct mst_intc_chip_data *cd = irq_data_get_irq_chip_data(d);
-> +	u16 val, mask;
-> +
-> +	mask = 1 << (hwirq % 16);
-> +	offset += (hwirq / 16) * 4;
-> +
-> +	raw_spin_lock(&cd->lock);
-> +	val = readw_relaxed(cd->base + offset) & ~mask;
-> +	writew_relaxed(val, cd->base + offset);
-> +	raw_spin_unlock(&cd->lock);
-> +}
-> +
-> +static void mst_intc_mask_irq(struct irq_data *d)
-> +{
-> +	mst_poke_irq(d, INTC_MASK);
-> +	irq_chip_mask_parent(d);
-> +}
-> +
-> +static void mst_intc_unmask_irq(struct irq_data *d)
-> +{
-> +	mst_clear_irq(d, INTC_MASK);
-> +	irq_chip_unmask_parent(d);
-> +}
-> +
-> +static void mst_intc_eoi_irq(struct irq_data *d)
-> +{
-> +	struct mst_intc_chip_data *cd = irq_data_get_irq_chip_data(d);
-> +
-> +	if (!cd->no_eoi)
-> +		mst_poke_irq(d, INTC_EOI);
-> +
-> +	irq_chip_eoi_parent(d);
-> +}
-> +
-> +static struct irq_chip mst_intc_chip = {
-> +	.name			= "mst-intc",
-> +	.irq_mask		= mst_intc_mask_irq,
-> +	.irq_unmask		= mst_intc_unmask_irq,
-> +	.irq_eoi		= mst_intc_eoi_irq,
-> +	.irq_get_irqchip_state	= irq_chip_get_parent_state,
-> +	.irq_set_irqchip_state	= irq_chip_set_parent_state,
-> +	.irq_set_affinity	= irq_chip_set_affinity_parent,
-> +	.irq_set_vcpu_affinity	= irq_chip_set_vcpu_affinity_parent,
-> +	.irq_set_type		= irq_chip_set_type_parent,
-> +	.irq_retrigger		= irq_chip_retrigger_hierarchy,
-> +	.flags			= IRQCHIP_SET_TYPE_MASKED |
-> +				  IRQCHIP_SKIP_SET_WAKE |
-> +				  IRQCHIP_MASK_ON_SUSPEND,
-> +};
-> +
-> +static int mst_intc_domain_translate(struct irq_domain *d,
-> +				     struct irq_fwspec *fwspec,
-> +				     unsigned long *hwirq,
-> +				     unsigned int *type)
-> +{
-> +	if (is_of_node(fwspec->fwnode)) {
-> +		if (fwspec->param_count != 3)
-> +			return -EINVAL;
-> +
-> +		/* No PPI should point to this domain */
-> +		if (fwspec->param[0] != 0)
-> +			return -EINVAL;
-> +
-> +		*hwirq = fwspec->param[1];
-> +		*type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;
-> +		return 0;
-> +	}
-> +
-> +	return -EINVAL;
-> +}
-> +
-> +static int mst_intc_domain_alloc(struct irq_domain *domain, unsigned 
-> int virq,
-> +				 unsigned int nr_irqs, void *data)
-> +{
-> +	int i;
-> +	irq_hw_number_t hwirq;
-> +	struct irq_fwspec parent_fwspec, *fwspec = data;
-> +	struct mst_intc_chip_data *cd = (struct mst_intc_chip_data
-> *)domain->host_data;
-
-No cast necessary here.
-
-> +
-> +	/* Not GIC compliant */
-> +	if (fwspec->param_count != 3)
-> +		return -EINVAL;
-> +
-> +	/* No PPI should point to this domain */
-> +	if (fwspec->param[0])
-> +		return -EINVAL;
-> +
-> +	if (fwspec->param[1] >= cd->nr_irqs)
-
-This condition is bogus, as it doesn't take into account the nr_irqs
-parameter.
-
-> +		return -EINVAL;
-> +
-> +	hwirq = fwspec->param[1];
-> +	for (i = 0; i < nr_irqs; i++)
-> +		irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
-> +					      &mst_intc_chip,
-> +					      domain->host_data);
-> +
-> +	parent_fwspec = *fwspec;
-> +	parent_fwspec.fwnode = domain->parent->fwnode;
-> +	parent_fwspec.param[1] = cd->irq_start + hwirq;
-> +	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, 
-> &parent_fwspec);
-> +}
-> +
-> +static const struct irq_domain_ops mst_intc_domain_ops = {
-> +	.translate	= mst_intc_domain_translate,
-> +	.alloc		= mst_intc_domain_alloc,
-> +	.free		= irq_domain_free_irqs_common,
-> +};
-> +
-> +int __init
-> +mst_intc_of_init(struct device_node *dn, struct device_node *parent)
-> +{
-> +	struct irq_domain *domain, *domain_parent;
-> +	struct mst_intc_chip_data *cd;
-> +	unsigned int irq_start, irq_end;
-
-unsigned int != u32.
-
-> +
-> +	domain_parent = irq_find_host(parent);
-> +	if (!domain_parent) {
-> +		pr_err("mst-intc: interrupt-parent not found\n");
-> +		return -EINVAL;
-> +	}
-> +
-> +	if (of_property_read_u32_index(dn, "mstar,irqs-map-range", 0, 
-> &irq_start) ||
-> +	    of_property_read_u32_index(dn, "mstar,irqs-map-range", 1, 
-> &irq_end))
-> +		return -EINVAL;
-> +
-> +	cd = kzalloc(sizeof(*cd), GFP_KERNEL);
-> +	if (!cd)
-> +		return -ENOMEM;
-> +
-> +	cd->base = of_iomap(dn, 0);
-> +	if (!cd->base) {
-> +		kfree(cd);
-> +		return -ENOMEM;
-> +	}
-> +
-> +	cd->no_eoi = of_property_read_bool(dn, "mstar,intc-no-eoi");
-> +	raw_spin_lock_init(&cd->lock);
-> +	cd->irq_start = irq_start;
-> +	cd->nr_irqs = irq_end - irq_start + 1;
-> +	domain = irq_domain_add_hierarchy(domain_parent, 0, cd->nr_irqs, dn,
-> +					  &mst_intc_domain_ops, cd);
-> +	if (!domain) {
-> +		iounmap(cd->base);
-> +		kfree(cd);
-> +		return -ENOMEM;
-> +	}
-> +
-> +	return 0;
-> +}
-> +
-> +IRQCHIP_DECLARE(mst_intc, "mstar,mst-intc", mst_intc_of_init);
-
-Thanks,
 
-         M.
--- 
-Jazz is not dead. It just smells funny...
 
+The hwirq number need to be in the irq map range. (property: mstar,irqs-map-range)
+If it's not, it must be incorrect configuration.
+So how about use the condition as following?
+
+if (hwirq >= cd->nr_irqs)
+	return -EINVAL;
+
+> > +		return -EINVAL;
+> > +
+> > +	hwirq = fwspec->param[1];
+> > +	for (i = 0; i < nr_irqs; i++)
+> > +		irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
+> > +					      &mst_intc_chip,
+> > +					      domain->host_data);
+> > +
+> > +	parent_fwspec = *fwspec;
+> > +	parent_fwspec.fwnode = domain->parent->fwnode;
+> > +	parent_fwspec.param[1] = cd->irq_start + hwirq;
+> > +	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, 
+> > &parent_fwspec);
+> > +}
+> > +
+> > +static const struct irq_domain_ops mst_intc_domain_ops = {
+> > +	.translate	= mst_intc_domain_translate,
+> > +	.alloc		= mst_intc_domain_alloc,
+> > +	.free		= irq_domain_free_irqs_common,
+> > +};
+> > +
+> > +int __init
+> > +mst_intc_of_init(struct device_node *dn, struct device_node *parent)
+> > +{
+> > +	struct irq_domain *domain, *domain_parent;
+> > +	struct mst_intc_chip_data *cd;
+> > +	unsigned int irq_start, irq_end;
+> 
+> unsigned int != u32.
+> 
+> > +
+> > +	domain_parent = irq_find_host(parent);
+> > +	if (!domain_parent) {
+> > +		pr_err("mst-intc: interrupt-parent not found\n");
+> > +		return -EINVAL;
+> > +	}
+> > +
+> > +	if (of_property_read_u32_index(dn, "mstar,irqs-map-range", 0, 
+> > &irq_start) ||
+> > +	    of_property_read_u32_index(dn, "mstar,irqs-map-range", 1, 
+> > &irq_end))
+> > +		return -EINVAL;
+> > +
+> > +	cd = kzalloc(sizeof(*cd), GFP_KERNEL);
+> > +	if (!cd)
+> > +		return -ENOMEM;
+> > +
+> > +	cd->base = of_iomap(dn, 0);
+> > +	if (!cd->base) {
+> > +		kfree(cd);
+> > +		return -ENOMEM;
+> > +	}
+> > +
+> > +	cd->no_eoi = of_property_read_bool(dn, "mstar,intc-no-eoi");
+> > +	raw_spin_lock_init(&cd->lock);
+> > +	cd->irq_start = irq_start;
+> > +	cd->nr_irqs = irq_end - irq_start + 1;
+> > +	domain = irq_domain_add_hierarchy(domain_parent, 0, cd->nr_irqs, dn,
+> > +					  &mst_intc_domain_ops, cd);
+> > +	if (!domain) {
+> > +		iounmap(cd->base);
+> > +		kfree(cd);
+> > +		return -ENOMEM;
+> > +	}
+> > +
+> > +	return 0;
+> > +}
+> > +
+> > +IRQCHIP_DECLARE(mst_intc, "mstar,mst-intc", mst_intc_of_init);
+> 
 _______________________________________________
-Linux-mediatek mailing list
-Linux-mediatek@lists.infradead.org
-http://lists.infradead.org/mailman/listinfo/linux-mediatek
+linux-arm-kernel mailing list
+linux-arm-kernel@lists.infradead.org
+http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/a/content_digest b/N3/content_digest
index 8223824..8f648ba 100644
--- a/a/content_digest
+++ b/N3/content_digest
@@ -1,9 +1,8 @@
- "ref\020200819034231.20726-1-mark-pk.tsai@mediatek.com\0"
- "ref\020200819034231.20726-2-mark-pk.tsai@mediatek.com\0"
- "From\0Marc Zyngier <maz@kernel.org>\0"
+ "From\0Mark-PK Tsai <mark-pk.tsai@mediatek.com>\0"
  "Subject\0Re: [PATCH 1/2] irqchip: irq-mst: Add MStar interrupt controller support\0"
- "Date\0Wed, 19 Aug 2020 09:14:24 +0100\0"
- "To\0Mark-PK Tsai <mark-pk.tsai@mediatek.com>\0"
+ "Date\0Wed, 19 Aug 2020 21:31:18 +0800\0"
+ "To\0<maz@kernel.org>"
+ " Mark-PK Tsai <mark-pk.tsai@mediatek.com>\0"
  "Cc\0devicetree@vger.kernel.org"
   alix.wu@mediatek.com
   jason@lakedaemon.net
@@ -17,286 +16,117 @@
  " linux-arm-kernel@lists.infradead.org\0"
  "\00:1\0"
  "b\0"
- "On 2020-08-19 04:42, Mark-PK Tsai wrote:\n"
- "> Add MStar interrupt controller support using hierarchy irq\n"
- "> domain.\n"
+ "From: Marc Zyngier <maz@kernel.org>\n"
+ "\n"
+ "> > +\n"
+ "> > +static int mst_intc_domain_alloc(struct irq_domain *domain, unsigned \n"
+ "> > int virq,\n"
+ "> > +\t\t\t\t unsigned int nr_irqs, void *data)\n"
+ "> > +{\n"
+ "> > +\tint i;\n"
+ "> > +\tirq_hw_number_t hwirq;\n"
+ "> > +\tstruct irq_fwspec parent_fwspec, *fwspec = data;\n"
+ "> > +\tstruct mst_intc_chip_data *cd = (struct mst_intc_chip_data\n"
+ "> > *)domain->host_data;\n"
  "> \n"
- "> Signed-off-by: Mark-PK Tsai <mark-pk.tsai@mediatek.com>\n"
- "> ---\n"
- ">  drivers/irqchip/Kconfig        |   7 ++\n"
- ">  drivers/irqchip/Makefile       |   1 +\n"
- ">  drivers/irqchip/irq-mst-intc.c | 195 +++++++++++++++++++++++++++++++++\n"
- ">  3 files changed, 203 insertions(+)\n"
- ">  create mode 100644 drivers/irqchip/irq-mst-intc.c\n"
+ "> No cast necessary here.\n"
  "> \n"
- "> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig\n"
- "> index bb70b7177f94..c3a9d880a4ea 100644\n"
- "> --- a/drivers/irqchip/Kconfig\n"
- "> +++ b/drivers/irqchip/Kconfig\n"
- "> @@ -571,4 +571,11 @@ config LOONGSON_PCH_MSI\n"
- ">  \thelp\n"
- ">  \t  Support for the Loongson PCH MSI Controller.\n"
+ "> > +\n"
+ "> > +\t/* Not GIC compliant */\n"
+ "> > +\tif (fwspec->param_count != 3)\n"
+ "> > +\t\treturn -EINVAL;\n"
+ "> > +\n"
+ "> > +\t/* No PPI should point to this domain */\n"
+ "> > +\tif (fwspec->param[0])\n"
+ "> > +\t\treturn -EINVAL;\n"
+ "> > +\n"
+ "> > +\tif (fwspec->param[1] >= cd->nr_irqs)\n"
+ "> \n"
+ "> This condition is bogus, as it doesn't take into account the nr_irqs\n"
+ "> parameter.\n"
  "> \n"
- "> +config MST_IRQ\n"
- "> +\tbool \"MStar Interrupt Controller\"\n"
- "> +\tselect IRQ_DOMAIN\n"
- "> +\tselect IRQ_DOMAIN_HIERARCHY\n"
- "> +\thelp\n"
- "> +\t  Support MStar Interrupt Controller.\n"
- "\n"
- "What selects it? Can it have a default for the relevant platforms?\n"
- "\n"
- "> +\n"
- ">  endmenu\n"
- "> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile\n"
- "> index 133f9c45744a..e2688a62403e 100644\n"
- "> --- a/drivers/irqchip/Makefile\n"
- "> +++ b/drivers/irqchip/Makefile\n"
- "> @@ -111,3 +111,4 @@ obj-$(CONFIG_LOONGSON_HTPIC)\t\t+= \n"
- "> irq-loongson-htpic.o\n"
- ">  obj-$(CONFIG_LOONGSON_HTVEC)\t\t+= irq-loongson-htvec.o\n"
- ">  obj-$(CONFIG_LOONGSON_PCH_PIC)\t\t+= irq-loongson-pch-pic.o\n"
- ">  obj-$(CONFIG_LOONGSON_PCH_MSI)\t\t+= irq-loongson-pch-msi.o\n"
- "> +obj-$(CONFIG_MST_IRQ)\t\t\t+= irq-mst-intc.o\n"
- "> diff --git a/drivers/irqchip/irq-mst-intc.c \n"
- "> b/drivers/irqchip/irq-mst-intc.c\n"
- "> new file mode 100644\n"
- "> index 000000000000..38d567741860\n"
- "> --- /dev/null\n"
- "> +++ b/drivers/irqchip/irq-mst-intc.c\n"
- "> @@ -0,0 +1,195 @@\n"
- "> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)\n"
- "> +/*\n"
- "> + * Copyright (c) 2020 MediaTek Inc.\n"
- "> + * Author Mark-PK Tsai <mark-pk.tsai@mediatek.com>\n"
- "> + */\n"
- "> +#include <linux/interrupt.h>\n"
- "> +#include <linux/io.h>\n"
- "> +#include <linux/irq.h>\n"
- "> +#include <linux/irqchip.h>\n"
- "> +#include <linux/irqdomain.h>\n"
- "> +#include <linux/of.h>\n"
- "> +#include <linux/of_address.h>\n"
- "> +#include <linux/of_irq.h>\n"
- "> +#include <linux/slab.h>\n"
- "> +#include <linux/spinlock.h>\n"
- "> +\n"
- "> +#define INTC_MASK\t0x0\n"
- "> +#define INTC_EOI\t0x20\n"
- "> +\n"
- "> +struct mst_intc_chip_data {\n"
- "> +\traw_spinlock_t lock;\n"
- "> +\tunsigned int irq_start, nr_irqs;\n"
- "> +\tvoid __iomem *base;\n"
- "> +\tbool no_eoi;\n"
- "\n"
- "nit: please align the fields of the data structure on a vertical line.\n"
- "\n"
- "> +};\n"
- "> +\n"
- "> +static void mst_poke_irq(struct irq_data *d, u32 offset)\n"
- "\n"
- "Given that this is supposed to be the opposite of \"clear\", this\n"
- "should be mst_set_irq().\n"
- "\n"
- "> +{\n"
- "> +\tirq_hw_number_t hwirq = irqd_to_hwirq(d);\n"
- "> +\tstruct mst_intc_chip_data *cd = irq_data_get_irq_chip_data(d);\n"
- "> +\tu16 val, mask;\n"
- "> +\n"
- "> +\tmask = 1 << (hwirq % 16);\n"
- "> +\toffset += (hwirq / 16) * 4;\n"
- "> +\n"
- "> +\traw_spin_lock(&cd->lock);\n"
- "> +\tval = readw_relaxed(cd->base + offset) | mask;\n"
- "> +\twritew_relaxed(val, cd->base + offset);\n"
- "> +\traw_spin_unlock(&cd->lock);\n"
- "\n"
- "Small improvement, but you still miss the fact that a masking\n"
- "operation can also occur from an interrupt handler, and this\n"
- "will cause a deadlock. You need to disable interrupts as well.\n"
- "\n"
- "> +}\n"
- "> +\n"
- "> +static void mst_clear_irq(struct irq_data *d, u32 offset)\n"
- "> +{\n"
- "> +\tirq_hw_number_t hwirq = irqd_to_hwirq(d);\n"
- "> +\tstruct mst_intc_chip_data *cd = irq_data_get_irq_chip_data(d);\n"
- "> +\tu16 val, mask;\n"
- "> +\n"
- "> +\tmask = 1 << (hwirq % 16);\n"
- "> +\toffset += (hwirq / 16) * 4;\n"
- "> +\n"
- "> +\traw_spin_lock(&cd->lock);\n"
- "> +\tval = readw_relaxed(cd->base + offset) & ~mask;\n"
- "> +\twritew_relaxed(val, cd->base + offset);\n"
- "> +\traw_spin_unlock(&cd->lock);\n"
- "> +}\n"
- "> +\n"
- "> +static void mst_intc_mask_irq(struct irq_data *d)\n"
- "> +{\n"
- "> +\tmst_poke_irq(d, INTC_MASK);\n"
- "> +\tirq_chip_mask_parent(d);\n"
- "> +}\n"
- "> +\n"
- "> +static void mst_intc_unmask_irq(struct irq_data *d)\n"
- "> +{\n"
- "> +\tmst_clear_irq(d, INTC_MASK);\n"
- "> +\tirq_chip_unmask_parent(d);\n"
- "> +}\n"
- "> +\n"
- "> +static void mst_intc_eoi_irq(struct irq_data *d)\n"
- "> +{\n"
- "> +\tstruct mst_intc_chip_data *cd = irq_data_get_irq_chip_data(d);\n"
- "> +\n"
- "> +\tif (!cd->no_eoi)\n"
- "> +\t\tmst_poke_irq(d, INTC_EOI);\n"
- "> +\n"
- "> +\tirq_chip_eoi_parent(d);\n"
- "> +}\n"
- "> +\n"
- "> +static struct irq_chip mst_intc_chip = {\n"
- "> +\t.name\t\t\t= \"mst-intc\",\n"
- "> +\t.irq_mask\t\t= mst_intc_mask_irq,\n"
- "> +\t.irq_unmask\t\t= mst_intc_unmask_irq,\n"
- "> +\t.irq_eoi\t\t= mst_intc_eoi_irq,\n"
- "> +\t.irq_get_irqchip_state\t= irq_chip_get_parent_state,\n"
- "> +\t.irq_set_irqchip_state\t= irq_chip_set_parent_state,\n"
- "> +\t.irq_set_affinity\t= irq_chip_set_affinity_parent,\n"
- "> +\t.irq_set_vcpu_affinity\t= irq_chip_set_vcpu_affinity_parent,\n"
- "> +\t.irq_set_type\t\t= irq_chip_set_type_parent,\n"
- "> +\t.irq_retrigger\t\t= irq_chip_retrigger_hierarchy,\n"
- "> +\t.flags\t\t\t= IRQCHIP_SET_TYPE_MASKED |\n"
- "> +\t\t\t\t  IRQCHIP_SKIP_SET_WAKE |\n"
- "> +\t\t\t\t  IRQCHIP_MASK_ON_SUSPEND,\n"
- "> +};\n"
- "> +\n"
- "> +static int mst_intc_domain_translate(struct irq_domain *d,\n"
- "> +\t\t\t\t     struct irq_fwspec *fwspec,\n"
- "> +\t\t\t\t     unsigned long *hwirq,\n"
- "> +\t\t\t\t     unsigned int *type)\n"
- "> +{\n"
- "> +\tif (is_of_node(fwspec->fwnode)) {\n"
- "> +\t\tif (fwspec->param_count != 3)\n"
- "> +\t\t\treturn -EINVAL;\n"
- "> +\n"
- "> +\t\t/* No PPI should point to this domain */\n"
- "> +\t\tif (fwspec->param[0] != 0)\n"
- "> +\t\t\treturn -EINVAL;\n"
- "> +\n"
- "> +\t\t*hwirq = fwspec->param[1];\n"
- "> +\t\t*type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;\n"
- "> +\t\treturn 0;\n"
- "> +\t}\n"
- "> +\n"
- "> +\treturn -EINVAL;\n"
- "> +}\n"
- "> +\n"
- "> +static int mst_intc_domain_alloc(struct irq_domain *domain, unsigned \n"
- "> int virq,\n"
- "> +\t\t\t\t unsigned int nr_irqs, void *data)\n"
- "> +{\n"
- "> +\tint i;\n"
- "> +\tirq_hw_number_t hwirq;\n"
- "> +\tstruct irq_fwspec parent_fwspec, *fwspec = data;\n"
- "> +\tstruct mst_intc_chip_data *cd = (struct mst_intc_chip_data\n"
- "> *)domain->host_data;\n"
- "\n"
- "No cast necessary here.\n"
- "\n"
- "> +\n"
- "> +\t/* Not GIC compliant */\n"
- "> +\tif (fwspec->param_count != 3)\n"
- "> +\t\treturn -EINVAL;\n"
- "> +\n"
- "> +\t/* No PPI should point to this domain */\n"
- "> +\tif (fwspec->param[0])\n"
- "> +\t\treturn -EINVAL;\n"
- "> +\n"
- "> +\tif (fwspec->param[1] >= cd->nr_irqs)\n"
- "\n"
- "This condition is bogus, as it doesn't take into account the nr_irqs\n"
- "parameter.\n"
- "\n"
- "> +\t\treturn -EINVAL;\n"
- "> +\n"
- "> +\thwirq = fwspec->param[1];\n"
- "> +\tfor (i = 0; i < nr_irqs; i++)\n"
- "> +\t\tirq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,\n"
- "> +\t\t\t\t\t      &mst_intc_chip,\n"
- "> +\t\t\t\t\t      domain->host_data);\n"
- "> +\n"
- "> +\tparent_fwspec = *fwspec;\n"
- "> +\tparent_fwspec.fwnode = domain->parent->fwnode;\n"
- "> +\tparent_fwspec.param[1] = cd->irq_start + hwirq;\n"
- "> +\treturn irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, \n"
- "> &parent_fwspec);\n"
- "> +}\n"
- "> +\n"
- "> +static const struct irq_domain_ops mst_intc_domain_ops = {\n"
- "> +\t.translate\t= mst_intc_domain_translate,\n"
- "> +\t.alloc\t\t= mst_intc_domain_alloc,\n"
- "> +\t.free\t\t= irq_domain_free_irqs_common,\n"
- "> +};\n"
- "> +\n"
- "> +int __init\n"
- "> +mst_intc_of_init(struct device_node *dn, struct device_node *parent)\n"
- "> +{\n"
- "> +\tstruct irq_domain *domain, *domain_parent;\n"
- "> +\tstruct mst_intc_chip_data *cd;\n"
- "> +\tunsigned int irq_start, irq_end;\n"
- "\n"
- "unsigned int != u32.\n"
- "\n"
- "> +\n"
- "> +\tdomain_parent = irq_find_host(parent);\n"
- "> +\tif (!domain_parent) {\n"
- "> +\t\tpr_err(\"mst-intc: interrupt-parent not found\\n\");\n"
- "> +\t\treturn -EINVAL;\n"
- "> +\t}\n"
- "> +\n"
- "> +\tif (of_property_read_u32_index(dn, \"mstar,irqs-map-range\", 0, \n"
- "> &irq_start) ||\n"
- "> +\t    of_property_read_u32_index(dn, \"mstar,irqs-map-range\", 1, \n"
- "> &irq_end))\n"
- "> +\t\treturn -EINVAL;\n"
- "> +\n"
- "> +\tcd = kzalloc(sizeof(*cd), GFP_KERNEL);\n"
- "> +\tif (!cd)\n"
- "> +\t\treturn -ENOMEM;\n"
- "> +\n"
- "> +\tcd->base = of_iomap(dn, 0);\n"
- "> +\tif (!cd->base) {\n"
- "> +\t\tkfree(cd);\n"
- "> +\t\treturn -ENOMEM;\n"
- "> +\t}\n"
- "> +\n"
- "> +\tcd->no_eoi = of_property_read_bool(dn, \"mstar,intc-no-eoi\");\n"
- "> +\traw_spin_lock_init(&cd->lock);\n"
- "> +\tcd->irq_start = irq_start;\n"
- "> +\tcd->nr_irqs = irq_end - irq_start + 1;\n"
- "> +\tdomain = irq_domain_add_hierarchy(domain_parent, 0, cd->nr_irqs, dn,\n"
- "> +\t\t\t\t\t  &mst_intc_domain_ops, cd);\n"
- "> +\tif (!domain) {\n"
- "> +\t\tiounmap(cd->base);\n"
- "> +\t\tkfree(cd);\n"
- "> +\t\treturn -ENOMEM;\n"
- "> +\t}\n"
- "> +\n"
- "> +\treturn 0;\n"
- "> +}\n"
- "> +\n"
- "> +IRQCHIP_DECLARE(mst_intc, \"mstar,mst-intc\", mst_intc_of_init);\n"
- "\n"
- "Thanks,\n"
  "\n"
- "         M.\n"
- "-- \n"
- "Jazz is not dead. It just smells funny...\n"
  "\n"
+ "The hwirq number need to be in the irq map range. (property: mstar,irqs-map-range)\n"
+ "If it's not, it must be incorrect configuration.\n"
+ "So how about use the condition as following?\n"
+ "\n"
+ "if (hwirq >= cd->nr_irqs)\n"
+ "\treturn -EINVAL;\n"
+ "\n"
+ "> > +\t\treturn -EINVAL;\n"
+ "> > +\n"
+ "> > +\thwirq = fwspec->param[1];\n"
+ "> > +\tfor (i = 0; i < nr_irqs; i++)\n"
+ "> > +\t\tirq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,\n"
+ "> > +\t\t\t\t\t      &mst_intc_chip,\n"
+ "> > +\t\t\t\t\t      domain->host_data);\n"
+ "> > +\n"
+ "> > +\tparent_fwspec = *fwspec;\n"
+ "> > +\tparent_fwspec.fwnode = domain->parent->fwnode;\n"
+ "> > +\tparent_fwspec.param[1] = cd->irq_start + hwirq;\n"
+ "> > +\treturn irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, \n"
+ "> > &parent_fwspec);\n"
+ "> > +}\n"
+ "> > +\n"
+ "> > +static const struct irq_domain_ops mst_intc_domain_ops = {\n"
+ "> > +\t.translate\t= mst_intc_domain_translate,\n"
+ "> > +\t.alloc\t\t= mst_intc_domain_alloc,\n"
+ "> > +\t.free\t\t= irq_domain_free_irqs_common,\n"
+ "> > +};\n"
+ "> > +\n"
+ "> > +int __init\n"
+ "> > +mst_intc_of_init(struct device_node *dn, struct device_node *parent)\n"
+ "> > +{\n"
+ "> > +\tstruct irq_domain *domain, *domain_parent;\n"
+ "> > +\tstruct mst_intc_chip_data *cd;\n"
+ "> > +\tunsigned int irq_start, irq_end;\n"
+ "> \n"
+ "> unsigned int != u32.\n"
+ "> \n"
+ "> > +\n"
+ "> > +\tdomain_parent = irq_find_host(parent);\n"
+ "> > +\tif (!domain_parent) {\n"
+ "> > +\t\tpr_err(\"mst-intc: interrupt-parent not found\\n\");\n"
+ "> > +\t\treturn -EINVAL;\n"
+ "> > +\t}\n"
+ "> > +\n"
+ "> > +\tif (of_property_read_u32_index(dn, \"mstar,irqs-map-range\", 0, \n"
+ "> > &irq_start) ||\n"
+ "> > +\t    of_property_read_u32_index(dn, \"mstar,irqs-map-range\", 1, \n"
+ "> > &irq_end))\n"
+ "> > +\t\treturn -EINVAL;\n"
+ "> > +\n"
+ "> > +\tcd = kzalloc(sizeof(*cd), GFP_KERNEL);\n"
+ "> > +\tif (!cd)\n"
+ "> > +\t\treturn -ENOMEM;\n"
+ "> > +\n"
+ "> > +\tcd->base = of_iomap(dn, 0);\n"
+ "> > +\tif (!cd->base) {\n"
+ "> > +\t\tkfree(cd);\n"
+ "> > +\t\treturn -ENOMEM;\n"
+ "> > +\t}\n"
+ "> > +\n"
+ "> > +\tcd->no_eoi = of_property_read_bool(dn, \"mstar,intc-no-eoi\");\n"
+ "> > +\traw_spin_lock_init(&cd->lock);\n"
+ "> > +\tcd->irq_start = irq_start;\n"
+ "> > +\tcd->nr_irqs = irq_end - irq_start + 1;\n"
+ "> > +\tdomain = irq_domain_add_hierarchy(domain_parent, 0, cd->nr_irqs, dn,\n"
+ "> > +\t\t\t\t\t  &mst_intc_domain_ops, cd);\n"
+ "> > +\tif (!domain) {\n"
+ "> > +\t\tiounmap(cd->base);\n"
+ "> > +\t\tkfree(cd);\n"
+ "> > +\t\treturn -ENOMEM;\n"
+ "> > +\t}\n"
+ "> > +\n"
+ "> > +\treturn 0;\n"
+ "> > +}\n"
+ "> > +\n"
+ "> > +IRQCHIP_DECLARE(mst_intc, \"mstar,mst-intc\", mst_intc_of_init);\n"
+ "> \n"
  "_______________________________________________\n"
- "Linux-mediatek mailing list\n"
- "Linux-mediatek@lists.infradead.org\n"
- http://lists.infradead.org/mailman/listinfo/linux-mediatek
+ "linux-arm-kernel mailing list\n"
+ "linux-arm-kernel@lists.infradead.org\n"
+ http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
 
-e094b2c5f51d6c831122683cd7f1f7290a94d496a7d3d6d56b860a22901c0e18
+05cb3c1c4825e09cabeaeaa0208d89e5ab10caa1aa3cfbd67272bc9fc4aa8e2a

diff --git a/a/1.txt b/N4/1.txt
index 180ed49..e65a147 100644
--- a/a/1.txt
+++ b/N4/1.txt
@@ -274,8 +274,3 @@ Thanks,
          M.
 -- 
 Jazz is not dead. It just smells funny...
-
-_______________________________________________
-Linux-mediatek mailing list
-Linux-mediatek@lists.infradead.org
-http://lists.infradead.org/mailman/listinfo/linux-mediatek
diff --git a/a/content_digest b/N4/content_digest
index 8223824..fde6394 100644
--- a/a/content_digest
+++ b/N4/content_digest
@@ -4,17 +4,17 @@
  "Subject\0Re: [PATCH 1/2] irqchip: irq-mst: Add MStar interrupt controller support\0"
  "Date\0Wed, 19 Aug 2020 09:14:24 +0100\0"
  "To\0Mark-PK Tsai <mark-pk.tsai@mediatek.com>\0"
- "Cc\0devicetree@vger.kernel.org"
-  alix.wu@mediatek.com
+ "Cc\0tglx@linutronix.de"
   jason@lakedaemon.net
-  yj.chiang@mediatek.com
-  daniel@0x0f.com
-  linux-kernel@vger.kernel.org
   robh+dt@kernel.org
-  linux-mediatek@lists.infradead.org
   matthias.bgg@gmail.com
-  tglx@linutronix.de
- " linux-arm-kernel@lists.infradead.org\0"
+  linux-kernel@vger.kernel.org
+  devicetree@vger.kernel.org
+  linux-arm-kernel@lists.infradead.org
+  linux-mediatek@lists.infradead.org
+  yj.chiang@mediatek.com
+  alix.wu@mediatek.com
+ " daniel@0x0f.com\0"
  "\00:1\0"
  "b\0"
  "On 2020-08-19 04:42, Mark-PK Tsai wrote:\n"
@@ -292,11 +292,6 @@
  "\n"
  "         M.\n"
  "-- \n"
- "Jazz is not dead. It just smells funny...\n"
- "\n"
- "_______________________________________________\n"
- "Linux-mediatek mailing list\n"
- "Linux-mediatek@lists.infradead.org\n"
- http://lists.infradead.org/mailman/listinfo/linux-mediatek
+ Jazz is not dead. It just smells funny...
 
-e094b2c5f51d6c831122683cd7f1f7290a94d496a7d3d6d56b860a22901c0e18
+382dfd7d3b48f477fe566b9178f2008c6d5dff31a13cd3a5874c5e3fbf9c6ad6

diff --git a/a/1.txt b/N5/1.txt
index 180ed49..fec9c1f 100644
--- a/a/1.txt
+++ b/N5/1.txt
@@ -1,281 +1,108 @@
-On 2020-08-19 04:42, Mark-PK Tsai wrote:
-> Add MStar interrupt controller support using hierarchy irq
-> domain.
+From: Marc Zyngier <maz@kernel.org>
+
+> > +
+> > +static int mst_intc_domain_alloc(struct irq_domain *domain, unsigned 
+> > int virq,
+> > +				 unsigned int nr_irqs, void *data)
+> > +{
+> > +	int i;
+> > +	irq_hw_number_t hwirq;
+> > +	struct irq_fwspec parent_fwspec, *fwspec = data;
+> > +	struct mst_intc_chip_data *cd = (struct mst_intc_chip_data
+> > *)domain->host_data;
 > 
-> Signed-off-by: Mark-PK Tsai <mark-pk.tsai@mediatek.com>
-> ---
->  drivers/irqchip/Kconfig        |   7 ++
->  drivers/irqchip/Makefile       |   1 +
->  drivers/irqchip/irq-mst-intc.c | 195 +++++++++++++++++++++++++++++++++
->  3 files changed, 203 insertions(+)
->  create mode 100644 drivers/irqchip/irq-mst-intc.c
+> No cast necessary here.
 > 
-> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
-> index bb70b7177f94..c3a9d880a4ea 100644
-> --- a/drivers/irqchip/Kconfig
-> +++ b/drivers/irqchip/Kconfig
-> @@ -571,4 +571,11 @@ config LOONGSON_PCH_MSI
->  	help
->  	  Support for the Loongson PCH MSI Controller.
+> > +
+> > +	/* Not GIC compliant */
+> > +	if (fwspec->param_count != 3)
+> > +		return -EINVAL;
+> > +
+> > +	/* No PPI should point to this domain */
+> > +	if (fwspec->param[0])
+> > +		return -EINVAL;
+> > +
+> > +	if (fwspec->param[1] >= cd->nr_irqs)
+> 
+> This condition is bogus, as it doesn't take into account the nr_irqs
+> parameter.
 > 
-> +config MST_IRQ
-> +	bool "MStar Interrupt Controller"
-> +	select IRQ_DOMAIN
-> +	select IRQ_DOMAIN_HIERARCHY
-> +	help
-> +	  Support MStar Interrupt Controller.
-
-What selects it? Can it have a default for the relevant platforms?
-
-> +
->  endmenu
-> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
-> index 133f9c45744a..e2688a62403e 100644
-> --- a/drivers/irqchip/Makefile
-> +++ b/drivers/irqchip/Makefile
-> @@ -111,3 +111,4 @@ obj-$(CONFIG_LOONGSON_HTPIC)		+= 
-> irq-loongson-htpic.o
->  obj-$(CONFIG_LOONGSON_HTVEC)		+= irq-loongson-htvec.o
->  obj-$(CONFIG_LOONGSON_PCH_PIC)		+= irq-loongson-pch-pic.o
->  obj-$(CONFIG_LOONGSON_PCH_MSI)		+= irq-loongson-pch-msi.o
-> +obj-$(CONFIG_MST_IRQ)			+= irq-mst-intc.o
-> diff --git a/drivers/irqchip/irq-mst-intc.c 
-> b/drivers/irqchip/irq-mst-intc.c
-> new file mode 100644
-> index 000000000000..38d567741860
-> --- /dev/null
-> +++ b/drivers/irqchip/irq-mst-intc.c
-> @@ -0,0 +1,195 @@
-> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
-> +/*
-> + * Copyright (c) 2020 MediaTek Inc.
-> + * Author Mark-PK Tsai <mark-pk.tsai@mediatek.com>
-> + */
-> +#include <linux/interrupt.h>
-> +#include <linux/io.h>
-> +#include <linux/irq.h>
-> +#include <linux/irqchip.h>
-> +#include <linux/irqdomain.h>
-> +#include <linux/of.h>
-> +#include <linux/of_address.h>
-> +#include <linux/of_irq.h>
-> +#include <linux/slab.h>
-> +#include <linux/spinlock.h>
-> +
-> +#define INTC_MASK	0x0
-> +#define INTC_EOI	0x20
-> +
-> +struct mst_intc_chip_data {
-> +	raw_spinlock_t lock;
-> +	unsigned int irq_start, nr_irqs;
-> +	void __iomem *base;
-> +	bool no_eoi;
-
-nit: please align the fields of the data structure on a vertical line.
-
-> +};
-> +
-> +static void mst_poke_irq(struct irq_data *d, u32 offset)
-
-Given that this is supposed to be the opposite of "clear", this
-should be mst_set_irq().
-
-> +{
-> +	irq_hw_number_t hwirq = irqd_to_hwirq(d);
-> +	struct mst_intc_chip_data *cd = irq_data_get_irq_chip_data(d);
-> +	u16 val, mask;
-> +
-> +	mask = 1 << (hwirq % 16);
-> +	offset += (hwirq / 16) * 4;
-> +
-> +	raw_spin_lock(&cd->lock);
-> +	val = readw_relaxed(cd->base + offset) | mask;
-> +	writew_relaxed(val, cd->base + offset);
-> +	raw_spin_unlock(&cd->lock);
-
-Small improvement, but you still miss the fact that a masking
-operation can also occur from an interrupt handler, and this
-will cause a deadlock. You need to disable interrupts as well.
-
-> +}
-> +
-> +static void mst_clear_irq(struct irq_data *d, u32 offset)
-> +{
-> +	irq_hw_number_t hwirq = irqd_to_hwirq(d);
-> +	struct mst_intc_chip_data *cd = irq_data_get_irq_chip_data(d);
-> +	u16 val, mask;
-> +
-> +	mask = 1 << (hwirq % 16);
-> +	offset += (hwirq / 16) * 4;
-> +
-> +	raw_spin_lock(&cd->lock);
-> +	val = readw_relaxed(cd->base + offset) & ~mask;
-> +	writew_relaxed(val, cd->base + offset);
-> +	raw_spin_unlock(&cd->lock);
-> +}
-> +
-> +static void mst_intc_mask_irq(struct irq_data *d)
-> +{
-> +	mst_poke_irq(d, INTC_MASK);
-> +	irq_chip_mask_parent(d);
-> +}
-> +
-> +static void mst_intc_unmask_irq(struct irq_data *d)
-> +{
-> +	mst_clear_irq(d, INTC_MASK);
-> +	irq_chip_unmask_parent(d);
-> +}
-> +
-> +static void mst_intc_eoi_irq(struct irq_data *d)
-> +{
-> +	struct mst_intc_chip_data *cd = irq_data_get_irq_chip_data(d);
-> +
-> +	if (!cd->no_eoi)
-> +		mst_poke_irq(d, INTC_EOI);
-> +
-> +	irq_chip_eoi_parent(d);
-> +}
-> +
-> +static struct irq_chip mst_intc_chip = {
-> +	.name			= "mst-intc",
-> +	.irq_mask		= mst_intc_mask_irq,
-> +	.irq_unmask		= mst_intc_unmask_irq,
-> +	.irq_eoi		= mst_intc_eoi_irq,
-> +	.irq_get_irqchip_state	= irq_chip_get_parent_state,
-> +	.irq_set_irqchip_state	= irq_chip_set_parent_state,
-> +	.irq_set_affinity	= irq_chip_set_affinity_parent,
-> +	.irq_set_vcpu_affinity	= irq_chip_set_vcpu_affinity_parent,
-> +	.irq_set_type		= irq_chip_set_type_parent,
-> +	.irq_retrigger		= irq_chip_retrigger_hierarchy,
-> +	.flags			= IRQCHIP_SET_TYPE_MASKED |
-> +				  IRQCHIP_SKIP_SET_WAKE |
-> +				  IRQCHIP_MASK_ON_SUSPEND,
-> +};
-> +
-> +static int mst_intc_domain_translate(struct irq_domain *d,
-> +				     struct irq_fwspec *fwspec,
-> +				     unsigned long *hwirq,
-> +				     unsigned int *type)
-> +{
-> +	if (is_of_node(fwspec->fwnode)) {
-> +		if (fwspec->param_count != 3)
-> +			return -EINVAL;
-> +
-> +		/* No PPI should point to this domain */
-> +		if (fwspec->param[0] != 0)
-> +			return -EINVAL;
-> +
-> +		*hwirq = fwspec->param[1];
-> +		*type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;
-> +		return 0;
-> +	}
-> +
-> +	return -EINVAL;
-> +}
-> +
-> +static int mst_intc_domain_alloc(struct irq_domain *domain, unsigned 
-> int virq,
-> +				 unsigned int nr_irqs, void *data)
-> +{
-> +	int i;
-> +	irq_hw_number_t hwirq;
-> +	struct irq_fwspec parent_fwspec, *fwspec = data;
-> +	struct mst_intc_chip_data *cd = (struct mst_intc_chip_data
-> *)domain->host_data;
-
-No cast necessary here.
-
-> +
-> +	/* Not GIC compliant */
-> +	if (fwspec->param_count != 3)
-> +		return -EINVAL;
-> +
-> +	/* No PPI should point to this domain */
-> +	if (fwspec->param[0])
-> +		return -EINVAL;
-> +
-> +	if (fwspec->param[1] >= cd->nr_irqs)
-
-This condition is bogus, as it doesn't take into account the nr_irqs
-parameter.
-
-> +		return -EINVAL;
-> +
-> +	hwirq = fwspec->param[1];
-> +	for (i = 0; i < nr_irqs; i++)
-> +		irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
-> +					      &mst_intc_chip,
-> +					      domain->host_data);
-> +
-> +	parent_fwspec = *fwspec;
-> +	parent_fwspec.fwnode = domain->parent->fwnode;
-> +	parent_fwspec.param[1] = cd->irq_start + hwirq;
-> +	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, 
-> &parent_fwspec);
-> +}
-> +
-> +static const struct irq_domain_ops mst_intc_domain_ops = {
-> +	.translate	= mst_intc_domain_translate,
-> +	.alloc		= mst_intc_domain_alloc,
-> +	.free		= irq_domain_free_irqs_common,
-> +};
-> +
-> +int __init
-> +mst_intc_of_init(struct device_node *dn, struct device_node *parent)
-> +{
-> +	struct irq_domain *domain, *domain_parent;
-> +	struct mst_intc_chip_data *cd;
-> +	unsigned int irq_start, irq_end;
-
-unsigned int != u32.
-
-> +
-> +	domain_parent = irq_find_host(parent);
-> +	if (!domain_parent) {
-> +		pr_err("mst-intc: interrupt-parent not found\n");
-> +		return -EINVAL;
-> +	}
-> +
-> +	if (of_property_read_u32_index(dn, "mstar,irqs-map-range", 0, 
-> &irq_start) ||
-> +	    of_property_read_u32_index(dn, "mstar,irqs-map-range", 1, 
-> &irq_end))
-> +		return -EINVAL;
-> +
-> +	cd = kzalloc(sizeof(*cd), GFP_KERNEL);
-> +	if (!cd)
-> +		return -ENOMEM;
-> +
-> +	cd->base = of_iomap(dn, 0);
-> +	if (!cd->base) {
-> +		kfree(cd);
-> +		return -ENOMEM;
-> +	}
-> +
-> +	cd->no_eoi = of_property_read_bool(dn, "mstar,intc-no-eoi");
-> +	raw_spin_lock_init(&cd->lock);
-> +	cd->irq_start = irq_start;
-> +	cd->nr_irqs = irq_end - irq_start + 1;
-> +	domain = irq_domain_add_hierarchy(domain_parent, 0, cd->nr_irqs, dn,
-> +					  &mst_intc_domain_ops, cd);
-> +	if (!domain) {
-> +		iounmap(cd->base);
-> +		kfree(cd);
-> +		return -ENOMEM;
-> +	}
-> +
-> +	return 0;
-> +}
-> +
-> +IRQCHIP_DECLARE(mst_intc, "mstar,mst-intc", mst_intc_of_init);
-
-Thanks,
 
-         M.
--- 
-Jazz is not dead. It just smells funny...
 
-_______________________________________________
-Linux-mediatek mailing list
-Linux-mediatek@lists.infradead.org
-http://lists.infradead.org/mailman/listinfo/linux-mediatek
+The hwirq number need to be in the irq map range. (property: mstar,irqs-map-range)
+If it's not, it must be incorrect configuration.
+So how about use the condition as following?
+
+if (hwirq >= cd->nr_irqs)
+	return -EINVAL;
+
+> > +		return -EINVAL;
+> > +
+> > +	hwirq = fwspec->param[1];
+> > +	for (i = 0; i < nr_irqs; i++)
+> > +		irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
+> > +					      &mst_intc_chip,
+> > +					      domain->host_data);
+> > +
+> > +	parent_fwspec = *fwspec;
+> > +	parent_fwspec.fwnode = domain->parent->fwnode;
+> > +	parent_fwspec.param[1] = cd->irq_start + hwirq;
+> > +	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, 
+> > &parent_fwspec);
+> > +}
+> > +
+> > +static const struct irq_domain_ops mst_intc_domain_ops = {
+> > +	.translate	= mst_intc_domain_translate,
+> > +	.alloc		= mst_intc_domain_alloc,
+> > +	.free		= irq_domain_free_irqs_common,
+> > +};
+> > +
+> > +int __init
+> > +mst_intc_of_init(struct device_node *dn, struct device_node *parent)
+> > +{
+> > +	struct irq_domain *domain, *domain_parent;
+> > +	struct mst_intc_chip_data *cd;
+> > +	unsigned int irq_start, irq_end;
+> 
+> unsigned int != u32.
+> 
+> > +
+> > +	domain_parent = irq_find_host(parent);
+> > +	if (!domain_parent) {
+> > +		pr_err("mst-intc: interrupt-parent not found\n");
+> > +		return -EINVAL;
+> > +	}
+> > +
+> > +	if (of_property_read_u32_index(dn, "mstar,irqs-map-range", 0, 
+> > &irq_start) ||
+> > +	    of_property_read_u32_index(dn, "mstar,irqs-map-range", 1, 
+> > &irq_end))
+> > +		return -EINVAL;
+> > +
+> > +	cd = kzalloc(sizeof(*cd), GFP_KERNEL);
+> > +	if (!cd)
+> > +		return -ENOMEM;
+> > +
+> > +	cd->base = of_iomap(dn, 0);
+> > +	if (!cd->base) {
+> > +		kfree(cd);
+> > +		return -ENOMEM;
+> > +	}
+> > +
+> > +	cd->no_eoi = of_property_read_bool(dn, "mstar,intc-no-eoi");
+> > +	raw_spin_lock_init(&cd->lock);
+> > +	cd->irq_start = irq_start;
+> > +	cd->nr_irqs = irq_end - irq_start + 1;
+> > +	domain = irq_domain_add_hierarchy(domain_parent, 0, cd->nr_irqs, dn,
+> > +					  &mst_intc_domain_ops, cd);
+> > +	if (!domain) {
+> > +		iounmap(cd->base);
+> > +		kfree(cd);
+> > +		return -ENOMEM;
+> > +	}
+> > +
+> > +	return 0;
+> > +}
+> > +
+> > +IRQCHIP_DECLARE(mst_intc, "mstar,mst-intc", mst_intc_of_init);
+>
diff --git a/a/content_digest b/N5/content_digest
index 8223824..4a356af 100644
--- a/a/content_digest
+++ b/N5/content_digest
@@ -1,302 +1,128 @@
- "ref\020200819034231.20726-1-mark-pk.tsai@mediatek.com\0"
- "ref\020200819034231.20726-2-mark-pk.tsai@mediatek.com\0"
- "From\0Marc Zyngier <maz@kernel.org>\0"
+ "From\0Mark-PK Tsai <mark-pk.tsai@mediatek.com>\0"
  "Subject\0Re: [PATCH 1/2] irqchip: irq-mst: Add MStar interrupt controller support\0"
- "Date\0Wed, 19 Aug 2020 09:14:24 +0100\0"
- "To\0Mark-PK Tsai <mark-pk.tsai@mediatek.com>\0"
- "Cc\0devicetree@vger.kernel.org"
-  alix.wu@mediatek.com
-  jason@lakedaemon.net
-  yj.chiang@mediatek.com
-  daniel@0x0f.com
-  linux-kernel@vger.kernel.org
-  robh+dt@kernel.org
-  linux-mediatek@lists.infradead.org
-  matthias.bgg@gmail.com
-  tglx@linutronix.de
- " linux-arm-kernel@lists.infradead.org\0"
+ "Date\0Wed, 19 Aug 2020 21:31:18 +0800\0"
+ "To\0<maz@kernel.org>"
+ " Mark-PK Tsai <mark-pk.tsai@mediatek.com>\0"
+ "Cc\0<alix.wu@mediatek.com>"
+  <daniel@0x0f.com>
+  <devicetree@vger.kernel.org>
+  <jason@lakedaemon.net>
+  <linux-arm-kernel@lists.infradead.org>
+  <linux-kernel@vger.kernel.org>
+  <linux-mediatek@lists.infradead.org>
+  <matthias.bgg@gmail.com>
+  <robh+dt@kernel.org>
+  <tglx@linutronix.de>
+ " <yj.chiang@mediatek.com>\0"
  "\00:1\0"
  "b\0"
- "On 2020-08-19 04:42, Mark-PK Tsai wrote:\n"
- "> Add MStar interrupt controller support using hierarchy irq\n"
- "> domain.\n"
+ "From: Marc Zyngier <maz@kernel.org>\n"
+ "\n"
+ "> > +\n"
+ "> > +static int mst_intc_domain_alloc(struct irq_domain *domain, unsigned \n"
+ "> > int virq,\n"
+ "> > +\t\t\t\t unsigned int nr_irqs, void *data)\n"
+ "> > +{\n"
+ "> > +\tint i;\n"
+ "> > +\tirq_hw_number_t hwirq;\n"
+ "> > +\tstruct irq_fwspec parent_fwspec, *fwspec = data;\n"
+ "> > +\tstruct mst_intc_chip_data *cd = (struct mst_intc_chip_data\n"
+ "> > *)domain->host_data;\n"
  "> \n"
- "> Signed-off-by: Mark-PK Tsai <mark-pk.tsai@mediatek.com>\n"
- "> ---\n"
- ">  drivers/irqchip/Kconfig        |   7 ++\n"
- ">  drivers/irqchip/Makefile       |   1 +\n"
- ">  drivers/irqchip/irq-mst-intc.c | 195 +++++++++++++++++++++++++++++++++\n"
- ">  3 files changed, 203 insertions(+)\n"
- ">  create mode 100644 drivers/irqchip/irq-mst-intc.c\n"
+ "> No cast necessary here.\n"
  "> \n"
- "> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig\n"
- "> index bb70b7177f94..c3a9d880a4ea 100644\n"
- "> --- a/drivers/irqchip/Kconfig\n"
- "> +++ b/drivers/irqchip/Kconfig\n"
- "> @@ -571,4 +571,11 @@ config LOONGSON_PCH_MSI\n"
- ">  \thelp\n"
- ">  \t  Support for the Loongson PCH MSI Controller.\n"
+ "> > +\n"
+ "> > +\t/* Not GIC compliant */\n"
+ "> > +\tif (fwspec->param_count != 3)\n"
+ "> > +\t\treturn -EINVAL;\n"
+ "> > +\n"
+ "> > +\t/* No PPI should point to this domain */\n"
+ "> > +\tif (fwspec->param[0])\n"
+ "> > +\t\treturn -EINVAL;\n"
+ "> > +\n"
+ "> > +\tif (fwspec->param[1] >= cd->nr_irqs)\n"
+ "> \n"
+ "> This condition is bogus, as it doesn't take into account the nr_irqs\n"
+ "> parameter.\n"
  "> \n"
- "> +config MST_IRQ\n"
- "> +\tbool \"MStar Interrupt Controller\"\n"
- "> +\tselect IRQ_DOMAIN\n"
- "> +\tselect IRQ_DOMAIN_HIERARCHY\n"
- "> +\thelp\n"
- "> +\t  Support MStar Interrupt Controller.\n"
- "\n"
- "What selects it? Can it have a default for the relevant platforms?\n"
- "\n"
- "> +\n"
- ">  endmenu\n"
- "> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile\n"
- "> index 133f9c45744a..e2688a62403e 100644\n"
- "> --- a/drivers/irqchip/Makefile\n"
- "> +++ b/drivers/irqchip/Makefile\n"
- "> @@ -111,3 +111,4 @@ obj-$(CONFIG_LOONGSON_HTPIC)\t\t+= \n"
- "> irq-loongson-htpic.o\n"
- ">  obj-$(CONFIG_LOONGSON_HTVEC)\t\t+= irq-loongson-htvec.o\n"
- ">  obj-$(CONFIG_LOONGSON_PCH_PIC)\t\t+= irq-loongson-pch-pic.o\n"
- ">  obj-$(CONFIG_LOONGSON_PCH_MSI)\t\t+= irq-loongson-pch-msi.o\n"
- "> +obj-$(CONFIG_MST_IRQ)\t\t\t+= irq-mst-intc.o\n"
- "> diff --git a/drivers/irqchip/irq-mst-intc.c \n"
- "> b/drivers/irqchip/irq-mst-intc.c\n"
- "> new file mode 100644\n"
- "> index 000000000000..38d567741860\n"
- "> --- /dev/null\n"
- "> +++ b/drivers/irqchip/irq-mst-intc.c\n"
- "> @@ -0,0 +1,195 @@\n"
- "> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)\n"
- "> +/*\n"
- "> + * Copyright (c) 2020 MediaTek Inc.\n"
- "> + * Author Mark-PK Tsai <mark-pk.tsai@mediatek.com>\n"
- "> + */\n"
- "> +#include <linux/interrupt.h>\n"
- "> +#include <linux/io.h>\n"
- "> +#include <linux/irq.h>\n"
- "> +#include <linux/irqchip.h>\n"
- "> +#include <linux/irqdomain.h>\n"
- "> +#include <linux/of.h>\n"
- "> +#include <linux/of_address.h>\n"
- "> +#include <linux/of_irq.h>\n"
- "> +#include <linux/slab.h>\n"
- "> +#include <linux/spinlock.h>\n"
- "> +\n"
- "> +#define INTC_MASK\t0x0\n"
- "> +#define INTC_EOI\t0x20\n"
- "> +\n"
- "> +struct mst_intc_chip_data {\n"
- "> +\traw_spinlock_t lock;\n"
- "> +\tunsigned int irq_start, nr_irqs;\n"
- "> +\tvoid __iomem *base;\n"
- "> +\tbool no_eoi;\n"
- "\n"
- "nit: please align the fields of the data structure on a vertical line.\n"
- "\n"
- "> +};\n"
- "> +\n"
- "> +static void mst_poke_irq(struct irq_data *d, u32 offset)\n"
- "\n"
- "Given that this is supposed to be the opposite of \"clear\", this\n"
- "should be mst_set_irq().\n"
- "\n"
- "> +{\n"
- "> +\tirq_hw_number_t hwirq = irqd_to_hwirq(d);\n"
- "> +\tstruct mst_intc_chip_data *cd = irq_data_get_irq_chip_data(d);\n"
- "> +\tu16 val, mask;\n"
- "> +\n"
- "> +\tmask = 1 << (hwirq % 16);\n"
- "> +\toffset += (hwirq / 16) * 4;\n"
- "> +\n"
- "> +\traw_spin_lock(&cd->lock);\n"
- "> +\tval = readw_relaxed(cd->base + offset) | mask;\n"
- "> +\twritew_relaxed(val, cd->base + offset);\n"
- "> +\traw_spin_unlock(&cd->lock);\n"
- "\n"
- "Small improvement, but you still miss the fact that a masking\n"
- "operation can also occur from an interrupt handler, and this\n"
- "will cause a deadlock. You need to disable interrupts as well.\n"
- "\n"
- "> +}\n"
- "> +\n"
- "> +static void mst_clear_irq(struct irq_data *d, u32 offset)\n"
- "> +{\n"
- "> +\tirq_hw_number_t hwirq = irqd_to_hwirq(d);\n"
- "> +\tstruct mst_intc_chip_data *cd = irq_data_get_irq_chip_data(d);\n"
- "> +\tu16 val, mask;\n"
- "> +\n"
- "> +\tmask = 1 << (hwirq % 16);\n"
- "> +\toffset += (hwirq / 16) * 4;\n"
- "> +\n"
- "> +\traw_spin_lock(&cd->lock);\n"
- "> +\tval = readw_relaxed(cd->base + offset) & ~mask;\n"
- "> +\twritew_relaxed(val, cd->base + offset);\n"
- "> +\traw_spin_unlock(&cd->lock);\n"
- "> +}\n"
- "> +\n"
- "> +static void mst_intc_mask_irq(struct irq_data *d)\n"
- "> +{\n"
- "> +\tmst_poke_irq(d, INTC_MASK);\n"
- "> +\tirq_chip_mask_parent(d);\n"
- "> +}\n"
- "> +\n"
- "> +static void mst_intc_unmask_irq(struct irq_data *d)\n"
- "> +{\n"
- "> +\tmst_clear_irq(d, INTC_MASK);\n"
- "> +\tirq_chip_unmask_parent(d);\n"
- "> +}\n"
- "> +\n"
- "> +static void mst_intc_eoi_irq(struct irq_data *d)\n"
- "> +{\n"
- "> +\tstruct mst_intc_chip_data *cd = irq_data_get_irq_chip_data(d);\n"
- "> +\n"
- "> +\tif (!cd->no_eoi)\n"
- "> +\t\tmst_poke_irq(d, INTC_EOI);\n"
- "> +\n"
- "> +\tirq_chip_eoi_parent(d);\n"
- "> +}\n"
- "> +\n"
- "> +static struct irq_chip mst_intc_chip = {\n"
- "> +\t.name\t\t\t= \"mst-intc\",\n"
- "> +\t.irq_mask\t\t= mst_intc_mask_irq,\n"
- "> +\t.irq_unmask\t\t= mst_intc_unmask_irq,\n"
- "> +\t.irq_eoi\t\t= mst_intc_eoi_irq,\n"
- "> +\t.irq_get_irqchip_state\t= irq_chip_get_parent_state,\n"
- "> +\t.irq_set_irqchip_state\t= irq_chip_set_parent_state,\n"
- "> +\t.irq_set_affinity\t= irq_chip_set_affinity_parent,\n"
- "> +\t.irq_set_vcpu_affinity\t= irq_chip_set_vcpu_affinity_parent,\n"
- "> +\t.irq_set_type\t\t= irq_chip_set_type_parent,\n"
- "> +\t.irq_retrigger\t\t= irq_chip_retrigger_hierarchy,\n"
- "> +\t.flags\t\t\t= IRQCHIP_SET_TYPE_MASKED |\n"
- "> +\t\t\t\t  IRQCHIP_SKIP_SET_WAKE |\n"
- "> +\t\t\t\t  IRQCHIP_MASK_ON_SUSPEND,\n"
- "> +};\n"
- "> +\n"
- "> +static int mst_intc_domain_translate(struct irq_domain *d,\n"
- "> +\t\t\t\t     struct irq_fwspec *fwspec,\n"
- "> +\t\t\t\t     unsigned long *hwirq,\n"
- "> +\t\t\t\t     unsigned int *type)\n"
- "> +{\n"
- "> +\tif (is_of_node(fwspec->fwnode)) {\n"
- "> +\t\tif (fwspec->param_count != 3)\n"
- "> +\t\t\treturn -EINVAL;\n"
- "> +\n"
- "> +\t\t/* No PPI should point to this domain */\n"
- "> +\t\tif (fwspec->param[0] != 0)\n"
- "> +\t\t\treturn -EINVAL;\n"
- "> +\n"
- "> +\t\t*hwirq = fwspec->param[1];\n"
- "> +\t\t*type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;\n"
- "> +\t\treturn 0;\n"
- "> +\t}\n"
- "> +\n"
- "> +\treturn -EINVAL;\n"
- "> +}\n"
- "> +\n"
- "> +static int mst_intc_domain_alloc(struct irq_domain *domain, unsigned \n"
- "> int virq,\n"
- "> +\t\t\t\t unsigned int nr_irqs, void *data)\n"
- "> +{\n"
- "> +\tint i;\n"
- "> +\tirq_hw_number_t hwirq;\n"
- "> +\tstruct irq_fwspec parent_fwspec, *fwspec = data;\n"
- "> +\tstruct mst_intc_chip_data *cd = (struct mst_intc_chip_data\n"
- "> *)domain->host_data;\n"
- "\n"
- "No cast necessary here.\n"
- "\n"
- "> +\n"
- "> +\t/* Not GIC compliant */\n"
- "> +\tif (fwspec->param_count != 3)\n"
- "> +\t\treturn -EINVAL;\n"
- "> +\n"
- "> +\t/* No PPI should point to this domain */\n"
- "> +\tif (fwspec->param[0])\n"
- "> +\t\treturn -EINVAL;\n"
- "> +\n"
- "> +\tif (fwspec->param[1] >= cd->nr_irqs)\n"
- "\n"
- "This condition is bogus, as it doesn't take into account the nr_irqs\n"
- "parameter.\n"
- "\n"
- "> +\t\treturn -EINVAL;\n"
- "> +\n"
- "> +\thwirq = fwspec->param[1];\n"
- "> +\tfor (i = 0; i < nr_irqs; i++)\n"
- "> +\t\tirq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,\n"
- "> +\t\t\t\t\t      &mst_intc_chip,\n"
- "> +\t\t\t\t\t      domain->host_data);\n"
- "> +\n"
- "> +\tparent_fwspec = *fwspec;\n"
- "> +\tparent_fwspec.fwnode = domain->parent->fwnode;\n"
- "> +\tparent_fwspec.param[1] = cd->irq_start + hwirq;\n"
- "> +\treturn irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, \n"
- "> &parent_fwspec);\n"
- "> +}\n"
- "> +\n"
- "> +static const struct irq_domain_ops mst_intc_domain_ops = {\n"
- "> +\t.translate\t= mst_intc_domain_translate,\n"
- "> +\t.alloc\t\t= mst_intc_domain_alloc,\n"
- "> +\t.free\t\t= irq_domain_free_irqs_common,\n"
- "> +};\n"
- "> +\n"
- "> +int __init\n"
- "> +mst_intc_of_init(struct device_node *dn, struct device_node *parent)\n"
- "> +{\n"
- "> +\tstruct irq_domain *domain, *domain_parent;\n"
- "> +\tstruct mst_intc_chip_data *cd;\n"
- "> +\tunsigned int irq_start, irq_end;\n"
- "\n"
- "unsigned int != u32.\n"
- "\n"
- "> +\n"
- "> +\tdomain_parent = irq_find_host(parent);\n"
- "> +\tif (!domain_parent) {\n"
- "> +\t\tpr_err(\"mst-intc: interrupt-parent not found\\n\");\n"
- "> +\t\treturn -EINVAL;\n"
- "> +\t}\n"
- "> +\n"
- "> +\tif (of_property_read_u32_index(dn, \"mstar,irqs-map-range\", 0, \n"
- "> &irq_start) ||\n"
- "> +\t    of_property_read_u32_index(dn, \"mstar,irqs-map-range\", 1, \n"
- "> &irq_end))\n"
- "> +\t\treturn -EINVAL;\n"
- "> +\n"
- "> +\tcd = kzalloc(sizeof(*cd), GFP_KERNEL);\n"
- "> +\tif (!cd)\n"
- "> +\t\treturn -ENOMEM;\n"
- "> +\n"
- "> +\tcd->base = of_iomap(dn, 0);\n"
- "> +\tif (!cd->base) {\n"
- "> +\t\tkfree(cd);\n"
- "> +\t\treturn -ENOMEM;\n"
- "> +\t}\n"
- "> +\n"
- "> +\tcd->no_eoi = of_property_read_bool(dn, \"mstar,intc-no-eoi\");\n"
- "> +\traw_spin_lock_init(&cd->lock);\n"
- "> +\tcd->irq_start = irq_start;\n"
- "> +\tcd->nr_irqs = irq_end - irq_start + 1;\n"
- "> +\tdomain = irq_domain_add_hierarchy(domain_parent, 0, cd->nr_irqs, dn,\n"
- "> +\t\t\t\t\t  &mst_intc_domain_ops, cd);\n"
- "> +\tif (!domain) {\n"
- "> +\t\tiounmap(cd->base);\n"
- "> +\t\tkfree(cd);\n"
- "> +\t\treturn -ENOMEM;\n"
- "> +\t}\n"
- "> +\n"
- "> +\treturn 0;\n"
- "> +}\n"
- "> +\n"
- "> +IRQCHIP_DECLARE(mst_intc, \"mstar,mst-intc\", mst_intc_of_init);\n"
- "\n"
- "Thanks,\n"
  "\n"
- "         M.\n"
- "-- \n"
- "Jazz is not dead. It just smells funny...\n"
  "\n"
- "_______________________________________________\n"
- "Linux-mediatek mailing list\n"
- "Linux-mediatek@lists.infradead.org\n"
- http://lists.infradead.org/mailman/listinfo/linux-mediatek
+ "The hwirq number need to be in the irq map range. (property: mstar,irqs-map-range)\n"
+ "If it's not, it must be incorrect configuration.\n"
+ "So how about use the condition as following?\n"
+ "\n"
+ "if (hwirq >= cd->nr_irqs)\n"
+ "\treturn -EINVAL;\n"
+ "\n"
+ "> > +\t\treturn -EINVAL;\n"
+ "> > +\n"
+ "> > +\thwirq = fwspec->param[1];\n"
+ "> > +\tfor (i = 0; i < nr_irqs; i++)\n"
+ "> > +\t\tirq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,\n"
+ "> > +\t\t\t\t\t      &mst_intc_chip,\n"
+ "> > +\t\t\t\t\t      domain->host_data);\n"
+ "> > +\n"
+ "> > +\tparent_fwspec = *fwspec;\n"
+ "> > +\tparent_fwspec.fwnode = domain->parent->fwnode;\n"
+ "> > +\tparent_fwspec.param[1] = cd->irq_start + hwirq;\n"
+ "> > +\treturn irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, \n"
+ "> > &parent_fwspec);\n"
+ "> > +}\n"
+ "> > +\n"
+ "> > +static const struct irq_domain_ops mst_intc_domain_ops = {\n"
+ "> > +\t.translate\t= mst_intc_domain_translate,\n"
+ "> > +\t.alloc\t\t= mst_intc_domain_alloc,\n"
+ "> > +\t.free\t\t= irq_domain_free_irqs_common,\n"
+ "> > +};\n"
+ "> > +\n"
+ "> > +int __init\n"
+ "> > +mst_intc_of_init(struct device_node *dn, struct device_node *parent)\n"
+ "> > +{\n"
+ "> > +\tstruct irq_domain *domain, *domain_parent;\n"
+ "> > +\tstruct mst_intc_chip_data *cd;\n"
+ "> > +\tunsigned int irq_start, irq_end;\n"
+ "> \n"
+ "> unsigned int != u32.\n"
+ "> \n"
+ "> > +\n"
+ "> > +\tdomain_parent = irq_find_host(parent);\n"
+ "> > +\tif (!domain_parent) {\n"
+ "> > +\t\tpr_err(\"mst-intc: interrupt-parent not found\\n\");\n"
+ "> > +\t\treturn -EINVAL;\n"
+ "> > +\t}\n"
+ "> > +\n"
+ "> > +\tif (of_property_read_u32_index(dn, \"mstar,irqs-map-range\", 0, \n"
+ "> > &irq_start) ||\n"
+ "> > +\t    of_property_read_u32_index(dn, \"mstar,irqs-map-range\", 1, \n"
+ "> > &irq_end))\n"
+ "> > +\t\treturn -EINVAL;\n"
+ "> > +\n"
+ "> > +\tcd = kzalloc(sizeof(*cd), GFP_KERNEL);\n"
+ "> > +\tif (!cd)\n"
+ "> > +\t\treturn -ENOMEM;\n"
+ "> > +\n"
+ "> > +\tcd->base = of_iomap(dn, 0);\n"
+ "> > +\tif (!cd->base) {\n"
+ "> > +\t\tkfree(cd);\n"
+ "> > +\t\treturn -ENOMEM;\n"
+ "> > +\t}\n"
+ "> > +\n"
+ "> > +\tcd->no_eoi = of_property_read_bool(dn, \"mstar,intc-no-eoi\");\n"
+ "> > +\traw_spin_lock_init(&cd->lock);\n"
+ "> > +\tcd->irq_start = irq_start;\n"
+ "> > +\tcd->nr_irqs = irq_end - irq_start + 1;\n"
+ "> > +\tdomain = irq_domain_add_hierarchy(domain_parent, 0, cd->nr_irqs, dn,\n"
+ "> > +\t\t\t\t\t  &mst_intc_domain_ops, cd);\n"
+ "> > +\tif (!domain) {\n"
+ "> > +\t\tiounmap(cd->base);\n"
+ "> > +\t\tkfree(cd);\n"
+ "> > +\t\treturn -ENOMEM;\n"
+ "> > +\t}\n"
+ "> > +\n"
+ "> > +\treturn 0;\n"
+ "> > +}\n"
+ "> > +\n"
+ "> > +IRQCHIP_DECLARE(mst_intc, \"mstar,mst-intc\", mst_intc_of_init);\n"
+ >
 
-e094b2c5f51d6c831122683cd7f1f7290a94d496a7d3d6d56b860a22901c0e18
+542de3fd9e6eaa5d4aca949d0b33657364ae71e37bf8c34689b4d716c2a97a0d

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.