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.