| Message ID | 20210306023633.9579-6-zhangqing@loongson.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add basic support for Loongson-2K1000 | expand |
Hi, Qing, On Sat, Mar 6, 2021 at 10:36 AM Qing Zhang <zhangqing@loongson.cn> wrote: > > Add IO interrupt controller support for Loongson 2k1000, different > from the 3a series is that 2K1000 has 64 interrupt sources, 0-31 > correspond to the device tree liointc0 device node, and the other > correspond to liointc1 node. Use the formal name (Loongson-2K1000, Loongson-3A series), please. Huacai > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > Signed-off-by: Qing Zhang <zhangqing@loongson.cn> > --- > > v2-v3: No change > > drivers/irqchip/irq-loongson-liointc.c | 55 +++++++++++++++++++++----- > 1 file changed, 46 insertions(+), 9 deletions(-) > > diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c > index 09b91b81851c..1c3c80f7f9f5 100644 > --- a/drivers/irqchip/irq-loongson-liointc.c > +++ b/drivers/irqchip/irq-loongson-liointc.c > @@ -20,6 +20,7 @@ > > #define LIOINTC_CHIP_IRQ 32 > #define LIOINTC_NUM_PARENT 4 > +#define LIOINTC_NUM_CORES 4 > > #define LIOINTC_INTC_CHIP_START 0x20 > > @@ -42,6 +43,7 @@ struct liointc_handler_data { > struct liointc_priv { > struct irq_chip_generic *gc; > struct liointc_handler_data handler[LIOINTC_NUM_PARENT]; > + void __iomem *core_isr[LIOINTC_NUM_CORES]; > u8 map_cache[LIOINTC_CHIP_IRQ]; > bool has_lpc_irq_errata; > }; > @@ -51,11 +53,12 @@ static void liointc_chained_handle_irq(struct irq_desc *desc) > struct liointc_handler_data *handler = irq_desc_get_handler_data(desc); > struct irq_chip *chip = irq_desc_get_chip(desc); > struct irq_chip_generic *gc = handler->priv->gc; > + int core = get_ebase_cpunum() % LIOINTC_NUM_CORES; > u32 pending; > > chained_irq_enter(chip, desc); > > - pending = readl(gc->reg_base + LIOINTC_REG_INTC_STATUS); > + pending = readl(handler->priv->core_isr[core]); > > if (!pending) { > /* Always blame LPC IRQ if we have that bug */ > @@ -141,6 +144,15 @@ static void liointc_resume(struct irq_chip_generic *gc) > } > > static const char * const parent_names[] = {"int0", "int1", "int2", "int3"}; > +static const char * const core_reg_names[] = {"isr0", "isr1", "isr2", "isr3"}; > + > +static void __iomem *liointc_get_reg_byname(struct device_node *node, > + const char *name) > +{ > + int index = of_property_match_string(node, "reg-names", name); > + > + return of_iomap(node, index); > +} > > static int __init liointc_of_init(struct device_node *node, > struct device_node *parent) > @@ -159,10 +171,28 @@ static int __init liointc_of_init(struct device_node *node, > if (!priv) > return -ENOMEM; > > - base = of_iomap(node, 0); > - if (!base) { > - err = -ENODEV; > - goto out_free_priv; > + if (of_device_is_compatible(node, "loongson,liointc-2.0")) { > + base = liointc_get_reg_byname(node, "main"); > + if (!base) { > + err = -ENODEV; > + goto out_free_priv; > + } > + for (i = 0; i < LIOINTC_NUM_CORES; i++) { > + priv->core_isr[i] = > + liointc_get_reg_byname(node, core_reg_names[i]); > + } > + if (!priv->core_isr[0]) { > + err = -ENODEV; > + goto out_iounmap_base; > + } > + } else { > + base = of_iomap(node, 0); > + if (!base) { > + err = -ENODEV; > + goto out_free_priv; > + } > + for (i = 0; i < LIOINTC_NUM_CORES; i++) > + priv->core_isr[i] = base + LIOINTC_REG_INTC_STATUS; > } > > for (i = 0; i < LIOINTC_NUM_PARENT; i++) { > @@ -172,7 +202,7 @@ static int __init liointc_of_init(struct device_node *node, > } > if (!have_parent) { > err = -ENODEV; > - goto out_iounmap; > + goto out_iounmap_isr; > } > > sz = of_property_read_variable_u32_array(node, > @@ -183,7 +213,7 @@ static int __init liointc_of_init(struct device_node *node, > if (sz < 4) { > pr_err("loongson-liointc: No parent_int_map\n"); > err = -ENODEV; > - goto out_iounmap; > + goto out_iounmap_isr; > } > > for (i = 0; i < LIOINTC_NUM_PARENT; i++) > @@ -195,7 +225,7 @@ static int __init liointc_of_init(struct device_node *node, > if (!domain) { > pr_err("loongson-liointc: cannot add IRQ domain\n"); > err = -EINVAL; > - goto out_iounmap; > + goto out_iounmap_isr; > } > > err = irq_alloc_domain_generic_chips(domain, 32, 1, > @@ -260,7 +290,13 @@ static int __init liointc_of_init(struct device_node *node, > > out_free_domain: > irq_domain_remove(domain); > -out_iounmap: > +out_iounmap_isr: > + for (i = 0; i < LIOINTC_NUM_CORES; i++) { > + if (!priv->core_isr[i]) > + continue; > + iounmap(priv->core_isr[i]); > + } > +out_iounmap_base: > iounmap(base); > out_free_priv: > kfree(priv); > @@ -270,3 +306,4 @@ static int __init liointc_of_init(struct device_node *node, > > IRQCHIP_DECLARE(loongson_liointc_1_0, "loongson,liointc-1.0", liointc_of_init); > IRQCHIP_DECLARE(loongson_liointc_1_0a, "loongson,liointc-1.0a", liointc_of_init); > +IRQCHIP_DECLARE(loongson_liointc_2_0, "loongson,liointc-2.0", liointc_of_init); > -- > 2.20.1 >
On 03/06/2021 11:06 AM, Huacai Chen wrote: > Hi, Qing, > > On Sat, Mar 6, 2021 at 10:36 AM Qing Zhang <zhangqing@loongson.cn> wrote: >> Add IO interrupt controller support for Loongson 2k1000, different >> from the 3a series is that 2K1000 has 64 interrupt sources, 0-31 >> correspond to the device tree liointc0 device node, and the other >> correspond to liointc1 node. > Use the formal name (Loongson-2K1000, Loongson-3A series), please. Hi, Huacai Thank you very much for your reply. I will modify the submission information, Use the formal name (Loongson-2K1000, Loongson-3A series) in v4, Before that, I will wait for other people's opinions about this series. Thanks, Qing > > Huacai
On 03/06/2021 10:36 AM, Qing Zhang wrote: > Add IO interrupt controller support for Loongson 2k1000, different > from the 3a series is that 2K1000 has 64 interrupt sources, 0-31 > correspond to the device tree liointc0 device node, and the other > correspond to liointc1 node. > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > Signed-off-by: Qing Zhang <zhangqing@loongson.cn> Tested-by: Ming Wang <wangming01@loongson.cn> Thanks, Ming
On Sat, 06 Mar 2021 02:36:31 +0000, Qing Zhang <zhangqing@loongson.cn> wrote: > > Add IO interrupt controller support for Loongson 2k1000, different > from the 3a series is that 2K1000 has 64 interrupt sources, 0-31 > correspond to the device tree liointc0 device node, and the other > correspond to liointc1 node. > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > Signed-off-by: Qing Zhang <zhangqing@loongson.cn> > --- > > v2-v3: No change > > drivers/irqchip/irq-loongson-liointc.c | 55 +++++++++++++++++++++----- > 1 file changed, 46 insertions(+), 9 deletions(-) > > diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c > index 09b91b81851c..1c3c80f7f9f5 100644 > --- a/drivers/irqchip/irq-loongson-liointc.c > +++ b/drivers/irqchip/irq-loongson-liointc.c > @@ -20,6 +20,7 @@ > > #define LIOINTC_CHIP_IRQ 32 > #define LIOINTC_NUM_PARENT 4 > +#define LIOINTC_NUM_CORES 4 > > #define LIOINTC_INTC_CHIP_START 0x20 > > @@ -42,6 +43,7 @@ struct liointc_handler_data { > struct liointc_priv { > struct irq_chip_generic *gc; > struct liointc_handler_data handler[LIOINTC_NUM_PARENT]; > + void __iomem *core_isr[LIOINTC_NUM_CORES]; > u8 map_cache[LIOINTC_CHIP_IRQ]; > bool has_lpc_irq_errata; > }; > @@ -51,11 +53,12 @@ static void liointc_chained_handle_irq(struct irq_desc *desc) > struct liointc_handler_data *handler = irq_desc_get_handler_data(desc); > struct irq_chip *chip = irq_desc_get_chip(desc); > struct irq_chip_generic *gc = handler->priv->gc; > + int core = get_ebase_cpunum() % LIOINTC_NUM_CORES; > u32 pending; > > chained_irq_enter(chip, desc); > > - pending = readl(gc->reg_base + LIOINTC_REG_INTC_STATUS); > + pending = readl(handler->priv->core_isr[core]); > > if (!pending) { > /* Always blame LPC IRQ if we have that bug */ > @@ -141,6 +144,15 @@ static void liointc_resume(struct irq_chip_generic *gc) > } > > static const char * const parent_names[] = {"int0", "int1", "int2", "int3"}; > +static const char * const core_reg_names[] = {"isr0", "isr1", "isr2", "isr3"}; > + > +static void __iomem *liointc_get_reg_byname(struct device_node *node, > + const char *name) > +{ > + int index = of_property_match_string(node, "reg-names", name); > + > + return of_iomap(node, index); So if of_property_match_string() returns an error, you feed that error to of_iomap()? Somehow, I don't think that's a good idea. > +} > > static int __init liointc_of_init(struct device_node *node, > struct device_node *parent) > @@ -159,10 +171,28 @@ static int __init liointc_of_init(struct device_node *node, > if (!priv) > return -ENOMEM; > > - base = of_iomap(node, 0); > - if (!base) { > - err = -ENODEV; > - goto out_free_priv; > + if (of_device_is_compatible(node, "loongson,liointc-2.0")) { > + base = liointc_get_reg_byname(node, "main"); > + if (!base) { > + err = -ENODEV; > + goto out_free_priv; > + } > + for (i = 0; i < LIOINTC_NUM_CORES; i++) { > + priv->core_isr[i] = > + liointc_get_reg_byname(node, core_reg_names[i]); Please write assignments on a single line. Thanks, M.
On 03/09/2021 05:10 PM, Marc Zyngier wrote: > >> + >> +static void __iomem *liointc_get_reg_byname(struct device_node *node, >> + const char *name) >> +{ >> + int index = of_property_match_string(node, "reg-names", name); >> + >> + return of_iomap(node, index); > So if of_property_match_string() returns an error, you feed that error > to of_iomap()? Somehow, I don't think that's a good idea. Hi, Marc Thank you for your suggestion, error handling is missing here, + if (index <0) + return NULL; return of_iomap(node, index); It has been fixed in the fourth version, and I will send V4 soon. > + if (of_device_is_compatible(node, "loongson,liointc-2.0")) { > + base = liointc_get_reg_byname(node, "main"); > + if (!base) { > + err = -ENODEV; > + goto out_free_priv; > + } > + for (i = 0; i < LIOINTC_NUM_CORES; i++) { > + priv->core_isr[i] = > + liointc_get_reg_byname(node, core_reg_names[i]); > Please write assignments on a single line. In addition, write assignments on a single line for (i = 0; i <LIOINTC_NUM_CORES; i++) priv->core_isr[i] = liointc_get_reg_byname(node, core_reg_names[i]); It is 92 characters, more than 80 characters... Thanks -Qing > > Thanks, > > M. >
Hello! On 10.03.2021 5:26, zhangqing wrote: [...] >> + if (of_device_is_compatible(node, "loongson,liointc-2.0")) { >> + base = liointc_get_reg_byname(node, "main"); >> + if (!base) { >> + err = -ENODEV; >> + goto out_free_priv; >> + } >> + for (i = 0; i < LIOINTC_NUM_CORES; i++) { >> + priv->core_isr[i] = >> + liointc_get_reg_byname(node, core_reg_names[i]); >> Please write assignments on a single line. > > In addition, write assignments on a single line > > for (i = 0; i <LIOINTC_NUM_CORES; i++) > priv->core_isr[i] = liointc_get_reg_byname(node, > core_reg_names[i]); > > It is 92 characters, more than 80 characters... The new line length limit is 100 chars. :-) [...] MBR, Sergei
On Wed, 10 Mar 2021 02:26:24 +0000, zhangqing <zhangqing@loongson.cn> wrote: > > > On 03/09/2021 05:10 PM, Marc Zyngier wrote: > > > >> + > >> +static void __iomem *liointc_get_reg_byname(struct device_node *node, > >> + const char *name) > >> +{ > >> + int index = of_property_match_string(node, "reg-names", name); > >> + > >> + return of_iomap(node, index); > > So if of_property_match_string() returns an error, you feed that error > > to of_iomap()? Somehow, I don't think that's a good idea. > > Hi, Marc > > Thank you for your suggestion, error handling is missing here, > > + if (index <0) > + return NULL; > > return of_iomap(node, index); > > It has been fixed in the fourth version, and I will send V4 soon. > > > + if (of_device_is_compatible(node, "loongson,liointc-2.0")) { > > + base = liointc_get_reg_byname(node, "main"); > > + if (!base) { > > + err = -ENODEV; > > + goto out_free_priv; > > + } > > + for (i = 0; i < LIOINTC_NUM_CORES; i++) { > > + priv->core_isr[i] = > > + liointc_get_reg_byname(node, core_reg_names[i]); > > Please write assignments on a single line. > > In addition, write assignments on a single line > > for (i = 0; i <LIOINTC_NUM_CORES; i++) > priv->core_isr[i] = > liointc_get_reg_byname(node, core_reg_names[i]); > > It is 92 characters, more than 80 characters... I really don't care about whatever arbitrary limit people think there is. Please put it on a single line. Thanks, M.
On 03/10/2021 04:47 PM, Sergei Shtylyov wrote: > Hello! > > The new line length limit is 100 chars. :-) Hi,Sergei Thank you for your reply, I saw it my $max_line_length = 100; Soalready fixed it in v4:). Thanks, Qing > > > MBR, Sergei
> I really don't care about whatever arbitrary limit people think there > is. Please put it on a single line. Hi,Marc Thank you for your reply, I saw it my $max_line_length = 100; So already fixed it in v4:). Thanks, Qing > > Thanks, > > M. >
diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c index 09b91b81851c..1c3c80f7f9f5 100644 --- a/drivers/irqchip/irq-loongson-liointc.c +++ b/drivers/irqchip/irq-loongson-liointc.c @@ -20,6 +20,7 @@ #define LIOINTC_CHIP_IRQ 32 #define LIOINTC_NUM_PARENT 4 +#define LIOINTC_NUM_CORES 4 #define LIOINTC_INTC_CHIP_START 0x20 @@ -42,6 +43,7 @@ struct liointc_handler_data { struct liointc_priv { struct irq_chip_generic *gc; struct liointc_handler_data handler[LIOINTC_NUM_PARENT]; + void __iomem *core_isr[LIOINTC_NUM_CORES]; u8 map_cache[LIOINTC_CHIP_IRQ]; bool has_lpc_irq_errata; }; @@ -51,11 +53,12 @@ static void liointc_chained_handle_irq(struct irq_desc *desc) struct liointc_handler_data *handler = irq_desc_get_handler_data(desc); struct irq_chip *chip = irq_desc_get_chip(desc); struct irq_chip_generic *gc = handler->priv->gc; + int core = get_ebase_cpunum() % LIOINTC_NUM_CORES; u32 pending; chained_irq_enter(chip, desc); - pending = readl(gc->reg_base + LIOINTC_REG_INTC_STATUS); + pending = readl(handler->priv->core_isr[core]); if (!pending) { /* Always blame LPC IRQ if we have that bug */ @@ -141,6 +144,15 @@ static void liointc_resume(struct irq_chip_generic *gc) } static const char * const parent_names[] = {"int0", "int1", "int2", "int3"}; +static const char * const core_reg_names[] = {"isr0", "isr1", "isr2", "isr3"}; + +static void __iomem *liointc_get_reg_byname(struct device_node *node, + const char *name) +{ + int index = of_property_match_string(node, "reg-names", name); + + return of_iomap(node, index); +} static int __init liointc_of_init(struct device_node *node, struct device_node *parent) @@ -159,10 +171,28 @@ static int __init liointc_of_init(struct device_node *node, if (!priv) return -ENOMEM; - base = of_iomap(node, 0); - if (!base) { - err = -ENODEV; - goto out_free_priv; + if (of_device_is_compatible(node, "loongson,liointc-2.0")) { + base = liointc_get_reg_byname(node, "main"); + if (!base) { + err = -ENODEV; + goto out_free_priv; + } + for (i = 0; i < LIOINTC_NUM_CORES; i++) { + priv->core_isr[i] = + liointc_get_reg_byname(node, core_reg_names[i]); + } + if (!priv->core_isr[0]) { + err = -ENODEV; + goto out_iounmap_base; + } + } else { + base = of_iomap(node, 0); + if (!base) { + err = -ENODEV; + goto out_free_priv; + } + for (i = 0; i < LIOINTC_NUM_CORES; i++) + priv->core_isr[i] = base + LIOINTC_REG_INTC_STATUS; } for (i = 0; i < LIOINTC_NUM_PARENT; i++) { @@ -172,7 +202,7 @@ static int __init liointc_of_init(struct device_node *node, } if (!have_parent) { err = -ENODEV; - goto out_iounmap; + goto out_iounmap_isr; } sz = of_property_read_variable_u32_array(node, @@ -183,7 +213,7 @@ static int __init liointc_of_init(struct device_node *node, if (sz < 4) { pr_err("loongson-liointc: No parent_int_map\n"); err = -ENODEV; - goto out_iounmap; + goto out_iounmap_isr; } for (i = 0; i < LIOINTC_NUM_PARENT; i++) @@ -195,7 +225,7 @@ static int __init liointc_of_init(struct device_node *node, if (!domain) { pr_err("loongson-liointc: cannot add IRQ domain\n"); err = -EINVAL; - goto out_iounmap; + goto out_iounmap_isr; } err = irq_alloc_domain_generic_chips(domain, 32, 1, @@ -260,7 +290,13 @@ static int __init liointc_of_init(struct device_node *node, out_free_domain: irq_domain_remove(domain); -out_iounmap: +out_iounmap_isr: + for (i = 0; i < LIOINTC_NUM_CORES; i++) { + if (!priv->core_isr[i]) + continue; + iounmap(priv->core_isr[i]); + } +out_iounmap_base: iounmap(base); out_free_priv: kfree(priv); @@ -270,3 +306,4 @@ static int __init liointc_of_init(struct device_node *node, IRQCHIP_DECLARE(loongson_liointc_1_0, "loongson,liointc-1.0", liointc_of_init); IRQCHIP_DECLARE(loongson_liointc_1_0a, "loongson,liointc-1.0a", liointc_of_init); +IRQCHIP_DECLARE(loongson_liointc_2_0, "loongson,liointc-2.0", liointc_of_init);