lkml.org 
[lkml]   [2019]   [Mar]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH] irqchip/gic: fix passing wrong start irq number to irq_alloc_descs() for secondary GICs
From
Date
On 11/03/2019 14:52, Liu Xiang wrote:
> For secondary GICs, the start irq number should skip over SGIs
> and PPIs. Its value should be 32. So we should pass hwirq_base to
> irq_alloc_descs() rather than a constant number 16.
>
> Signed-off-by: Liu Xiang <liu.xiang6@zte.com.cn>
> ---
> drivers/irqchip/irq-gic.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index ba2a37a..351f576 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -1157,7 +1157,7 @@ static int gic_init_bases(struct gic_chip_data *gic, int irq_start,
>
> gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
>
> - irq_base = irq_alloc_descs(irq_start, 16, gic_irqs,
> + irq_base = irq_alloc_descs(irq_start, hwirq_base, gic_irqs,
> numa_node_id());
> if (irq_base < 0) {
> WARN(1, "Cannot allocate irq_descs @ IRQ%d, assuming pre-allocated\n",
>

I suggest you look at __irq_alloc_descs(), and understand what the
various parameters mean. What you're doing here has absolutely no
effect.

The right thing to do would be to get rid of this altogether, except
that we have exactly *one* platform in the tree that is still non-DT
(some unmaintained Cavium piece of junk). But we can still simplify it,
as this guy doesn't have a secondary GIC (it is braindead enough).

What I'm suggesting instead is:

From b41fdc4a7bf9045e4871c5b15905ea732ffd044f Mon Sep 17 00:00:00 2001
From: Marc Zyngier <marc.zyngier@arm.com>
Date: Mon, 11 Mar 2019 15:38:10 +0000
Subject: [PATCH] irqchip/gic: Drop support for secondary GIC in non-DT systems

We do not have any in-tree platform with this pathological setup,
and only a single system (Cavium's cns3xxx) isn't DT aware.

Let's drop the secondary GIC support for now, until we remove
the above horror altogether.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm/mach-cns3xxx/core.c | 2 +-
drivers/irqchip/irq-gic.c | 45 ++++++++++++---------------------
include/linux/irqchip/arm-gic.h | 3 +--
3 files changed, 18 insertions(+), 32 deletions(-)

diff --git a/arch/arm/mach-cns3xxx/core.c b/arch/arm/mach-cns3xxx/core.c
index 7d5a44a06648..f676592d8402 100644
--- a/arch/arm/mach-cns3xxx/core.c
+++ b/arch/arm/mach-cns3xxx/core.c
@@ -90,7 +90,7 @@ void __init cns3xxx_map_io(void)
/* used by entry-macro.S */
void __init cns3xxx_init_irq(void)
{
- gic_init(0, 29, IOMEM(CNS3XXX_TC11MP_GIC_DIST_BASE_VIRT),
+ gic_init(IOMEM(CNS3XXX_TC11MP_GIC_DIST_BASE_VIRT),
IOMEM(CNS3XXX_TC11MP_GIC_CPU_BASE_VIRT));
}

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index ba2a37a27a54..fd3110c171ba 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1089,11 +1089,10 @@ static void gic_init_chip(struct gic_chip_data *gic, struct device *dev,
#endif
}

-static int gic_init_bases(struct gic_chip_data *gic, int irq_start,
+static int gic_init_bases(struct gic_chip_data *gic,
struct fwnode_handle *handle)
{
- irq_hw_number_t hwirq_base;
- int gic_irqs, irq_base, ret;
+ int gic_irqs, ret;

if (IS_ENABLED(CONFIG_GIC_NON_BANKED) && gic->percpu_offset) {
/* Frankein-GIC without banked registers... */
@@ -1145,28 +1144,21 @@ static int gic_init_bases(struct gic_chip_data *gic, int irq_start,
} else { /* Legacy support */
/*
* For primary GICs, skip over SGIs.
- * For secondary GICs, skip over PPIs, too.
+ * No secondary GIC support whatsoever.
*/
- if (gic == &gic_data[0] && (irq_start & 31) > 0) {
- hwirq_base = 16;
- if (irq_start != -1)
- irq_start = (irq_start & ~31) + 16;
- } else {
- hwirq_base = 32;
- }
+ int irq_base;

- gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
+ gic_irqs -= 16; /* calculate # of irqs to allocate */

- irq_base = irq_alloc_descs(irq_start, 16, gic_irqs,
+ irq_base = irq_alloc_descs(16, 16, gic_irqs,
numa_node_id());
if (irq_base < 0) {
- WARN(1, "Cannot allocate irq_descs @ IRQ%d, assuming pre-allocated\n",
- irq_start);
- irq_base = irq_start;
+ WARN(1, "Cannot allocate irq_descs @ IRQ16, assuming pre-allocated\n");
+ irq_base = 16;
}

gic->domain = irq_domain_add_legacy(NULL, gic_irqs, irq_base,
- hwirq_base, &gic_irq_domain_ops, gic);
+ 16, &gic_irq_domain_ops, gic);
}

if (WARN_ON(!gic->domain)) {
@@ -1195,7 +1187,6 @@ static int gic_init_bases(struct gic_chip_data *gic, int irq_start,
}

static int __init __gic_init_bases(struct gic_chip_data *gic,
- int irq_start,
struct fwnode_handle *handle)
{
char *name;
@@ -1231,32 +1222,28 @@ static int __init __gic_init_bases(struct gic_chip_data *gic,
gic_init_chip(gic, NULL, name, false);
}

- ret = gic_init_bases(gic, irq_start, handle);
+ ret = gic_init_bases(gic, handle);
if (ret)
kfree(name);

return ret;
}

-void __init gic_init(unsigned int gic_nr, int irq_start,
- void __iomem *dist_base, void __iomem *cpu_base)
+void __init gic_init(void __iomem *dist_base, void __iomem *cpu_base)
{
struct gic_chip_data *gic;

- if (WARN_ON(gic_nr >= CONFIG_ARM_GIC_MAX_NR))
- return;
-
/*
* Non-DT/ACPI systems won't run a hypervisor, so let's not
* bother with these...
*/
static_branch_disable(&supports_deactivate_key);

- gic = &gic_data[gic_nr];
+ gic = &gic_data[0];
gic->raw_dist_base = dist_base;
gic->raw_cpu_base = cpu_base;

- __gic_init_bases(gic, irq_start, NULL);
+ __gic_init_bases(gic, NULL);
}

static void gic_teardown(struct gic_chip_data *gic)
@@ -1399,7 +1386,7 @@ int gic_of_init_child(struct device *dev, struct gic_chip_data **gic, int irq)
if (ret)
return ret;

- ret = gic_init_bases(*gic, -1, &dev->of_node->fwnode);
+ ret = gic_init_bases(*gic, &dev->of_node->fwnode);
if (ret) {
gic_teardown(*gic);
return ret;
@@ -1459,7 +1446,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
if (gic_cnt == 0 && !gic_check_eoimode(node, &gic->raw_cpu_base))
static_branch_disable(&supports_deactivate_key);

- ret = __gic_init_bases(gic, -1, &node->fwnode);
+ ret = __gic_init_bases(gic, &node->fwnode);
if (ret) {
gic_teardown(gic);
return ret;
@@ -1650,7 +1637,7 @@ static int __init gic_v2_acpi_init(struct acpi_subtable_header *header,
return -ENOMEM;
}

- ret = __gic_init_bases(gic, -1, domain_handle);
+ ret = __gic_init_bases(gic, domain_handle);
if (ret) {
pr_err("Failed to initialise GIC\n");
irq_domain_free_fwnode(domain_handle);
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 626179077bb0..0f049b384ccd 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -158,8 +158,7 @@ int gic_of_init_child(struct device *dev, struct gic_chip_data **gic, int irq);
* Legacy platforms not converted to DT yet must use this to init
* their GIC
*/
-void gic_init(unsigned int nr, int start,
- void __iomem *dist , void __iomem *cpu);
+void gic_init(void __iomem *dist , void __iomem *cpu);

int gicv2m_init(struct fwnode_handle *parent_handle,
struct irq_domain *parent);
--
2.20.1
Thanks,

M.
--
Jazz is not dead. It just smells funny...

\
 
 \ /
  Last update: 2019-03-11 16:56    [W:0.055 / U:0.156 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site