This improves IRQ assignment to cores on a mips SoC.
Hauke Mehrtens (6):
bcma: make bcma_find_core_unit() accessible
bcma: mips: explicit assign IRQ numbers
bcma: mips: make some info messages debug messages
bcma: mips: show also disabled IRQs
bcma: mips: remove assigned_irqs from structure
bcma: mips: rename oldirqflag to irqinitmask
drivers/bcma/bcma_private.h | 2 +
drivers/bcma/driver_mips.c | 119 +++++++++++++++++++++------------
drivers/bcma/main.c | 4 +-
include/linux/bcma/bcma_driver_mips.h | 1 -
4 files changed, 80 insertions(+), 46 deletions(-)
--
1.7.10.4
On Sun, 2013-01-06 at 21:35 +0100, Hauke Mehrtens wrote:
> On 01/04/2013 08:43 AM, Rafał Miłecki wrote:
> > 2013/1/4 Hauke Mehrtens <[email protected]>:
> >> The assignment of the IRQs to the cores of the chips by iterating over
> >> the cores is complicated and causes problems with SoC like the BCM4706
> >> with two GMAC core where just one should get a dedicated IRQ number.
> >
> > Well, the only problem on BCM4706 is that second (broken/dangling)
> > GMAC core gets it's own IRQ. With such a "waste" of one IRQ line we
> > may end up using shared IRQ for some other core like PCIE/USB.
> > This issue can be simply workarounded (with 2 LOC) and sounds sane:
> > bug in HW, specific workaround in a driver. We already have similar
> > workarounds for other "dangling" cores.
> >
> > I don't know about any other issues with BCM4706 IRQs.
> >
> > Are there any other (more serious?) issues on different SoCs? Any
> > advantages of hardcoding IRQs (per chipset) rather than keeping this
> > simple algorithm? Do the IRQ lines differ? Is this somehow better to
> > use (just an example) IRQ 3 instead of IRQ 4 for GMAC core?
>
> Normally the boot loader (CFE) assigns IRQs to the cores on the bus. The
> old code mostly worked because CFE did it correct most of the time.
>
> On my bcm4718 the I2S core (id 0x834) does not get any IRQ from CFE, but
> at least Nathan (nlhintz) wants to assign an IRQ (shared) to this core.
This is the way it is in the Broadcom SDK; whether it matters or not, I
don't know. It seemed to be working without it too. How would one tell
if it is important?
>
> It is also possible to extend the algorithm to solve all these things,
> but I think the new code is better to understand.
>
> I do not think that there will be any new SoC using bcma and a mips cpu,
> I haven't seen an announcement for such a SoC at the Broadcom web page,
> just arm based devices, so I assume this list is complete.
>
> Hauke
>
Nathan
This bcma_find_core_unit() is needed by the mips driver in the next
patch.
Signed-off-by: Hauke Mehrtens <[email protected]>
---
drivers/bcma/bcma_private.h | 2 ++
drivers/bcma/main.c | 4 ++--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/bcma/bcma_private.h b/drivers/bcma/bcma_private.h
index 4a2d72e..d35294e 100644
--- a/drivers/bcma/bcma_private.h
+++ b/drivers/bcma/bcma_private.h
@@ -31,6 +31,8 @@ int __init bcma_bus_early_register(struct bcma_bus *bus,
int bcma_bus_suspend(struct bcma_bus *bus);
int bcma_bus_resume(struct bcma_bus *bus);
#endif
+struct bcma_device *bcma_find_core_unit(struct bcma_bus *bus, u16 coreid,
+ u8 unit);
/* scan.c */
int bcma_bus_scan(struct bcma_bus *bus);
diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c
index 53ba20c..d12b7da 100644
--- a/drivers/bcma/main.c
+++ b/drivers/bcma/main.c
@@ -81,8 +81,8 @@ struct bcma_device *bcma_find_core(struct bcma_bus *bus, u16 coreid)
}
EXPORT_SYMBOL_GPL(bcma_find_core);
-static struct bcma_device *bcma_find_core_unit(struct bcma_bus *bus, u16 coreid,
- u8 unit)
+struct bcma_device *bcma_find_core_unit(struct bcma_bus *bus, u16 coreid,
+ u8 unit)
{
struct bcma_device *core;
--
1.7.10.4
Some cores do not have a IRQ assigned and some do not support when an
IRQ is assigned to them, this is now handled and printed out in the
code.
Signed-off-by: Hauke Mehrtens <[email protected]>
---
drivers/bcma/driver_mips.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/bcma/driver_mips.c b/drivers/bcma/driver_mips.c
index 486aa2d..6981507 100644
--- a/drivers/bcma/driver_mips.c
+++ b/drivers/bcma/driver_mips.c
@@ -74,11 +74,16 @@ static u32 bcma_core_mips_irqflag(struct bcma_device *dev)
return dev->core_index;
flag = bcma_aread32(dev, BCMA_MIPS_OOBSELOUTA30);
- return flag & 0x1F;
+ if (flag)
+ return flag & 0x1F;
+ else
+ return 0x3f;
}
/* Get the MIPS IRQ assignment for a specified device.
* If unassigned, 0 is returned.
+ * If disabled, 5 is returned.
+ * If not supported, 6 is returned.
*/
unsigned int bcma_core_mips_irq(struct bcma_device *dev)
{
@@ -87,13 +92,15 @@ unsigned int bcma_core_mips_irq(struct bcma_device *dev)
unsigned int irq;
irqflag = bcma_core_mips_irqflag(dev);
+ if (irqflag == 0x3f)
+ return 6;
- for (irq = 1; irq <= 4; irq++)
+ for (irq = 0; irq <= 4; irq++)
if (bcma_read32(mdev, BCMA_MIPS_MIPS74K_INTMASK(irq)) &
(1 << irqflag))
return irq;
- return 0;
+ return 5;
}
EXPORT_SYMBOL(bcma_core_mips_irq);
@@ -114,7 +121,7 @@ static void bcma_core_mips_set_irq(struct bcma_device *dev, unsigned int irq)
bcma_write32(mdev, BCMA_MIPS_MIPS74K_INTMASK(0),
bcma_read32(mdev, BCMA_MIPS_MIPS74K_INTMASK(0)) &
~(1 << irqflag));
- else
+ else if (oldirq != 5)
bcma_write32(mdev, BCMA_MIPS_MIPS74K_INTMASK(oldirq), 0);
/* assign the new one */
@@ -144,7 +151,7 @@ static void bcma_core_mips_set_irq(struct bcma_device *dev, unsigned int irq)
}
bcma_debug(bus, "set_irq: core 0x%04x, irq %d => %d\n",
- dev->id.id, oldirq + 2, irq + 2);
+ dev->id.id, oldirq <= 4 ? oldirq + 2 : 0, irq + 2);
}
static void bcma_core_mips_set_irq_name(struct bcma_bus *bus, unsigned int irq,
--
1.7.10.4
The assignment of the IRQs to the cores of the chips by iterating over
the cores is complicated and causes problems with SoC like the BCM4706
with two GMAC core where just one should get a dedicated IRQ number.
Now the code assigns the same IRQs to the cores as the code from the
Broadcom SDK does. If the SoC is not know the current assigned IRQs are
only read out and an error message is printed.
Signed-off-by: Hauke Mehrtens <[email protected]>
---
drivers/bcma/driver_mips.c | 84 +++++++++++++++++++++++++++++---------------
1 file changed, 56 insertions(+), 28 deletions(-)
diff --git a/drivers/bcma/driver_mips.c b/drivers/bcma/driver_mips.c
index 792daad..171be0f 100644
--- a/drivers/bcma/driver_mips.c
+++ b/drivers/bcma/driver_mips.c
@@ -147,6 +147,22 @@ static void bcma_core_mips_set_irq(struct bcma_device *dev, unsigned int irq)
dev->id.id, oldirq + 2, irq + 2);
}
+static void bcma_core_mips_set_irq_name(struct bcma_bus *bus, unsigned int irq,
+ u16 coreid, u8 unit)
+{
+ struct bcma_device *core;
+
+ core = bcma_find_core_unit(bus, coreid, unit);
+ if (!core) {
+ bcma_warn(bus,
+ "Can not find core (id: 0x%x, unit %i) for IRQ configuration.\n",
+ coreid, unit);
+ return;
+ }
+
+ bcma_core_mips_set_irq(core, irq);
+}
+
static void bcma_core_mips_print_irq(struct bcma_device *dev, unsigned int irq)
{
int i;
@@ -242,35 +258,47 @@ void bcma_core_mips_init(struct bcma_drv_mips *mcore)
mcore->assigned_irqs = 1;
- /* Assign IRQs to all cores on the bus */
- list_for_each_entry(core, &bus->cores, list) {
- int mips_irq;
- if (core->irq)
- continue;
-
- mips_irq = bcma_core_mips_irq(core);
- if (mips_irq > 4)
- core->irq = 0;
- else
- core->irq = mips_irq + 2;
- if (core->irq > 5)
- continue;
- switch (core->id.id) {
- case BCMA_CORE_PCI:
- case BCMA_CORE_PCIE:
- case BCMA_CORE_ETHERNET:
- case BCMA_CORE_ETHERNET_GBIT:
- case BCMA_CORE_MAC_GBIT:
- case BCMA_CORE_80211:
- case BCMA_CORE_USB20_HOST:
- /* These devices get their own IRQ line if available,
- * the rest goes on IRQ0
- */
- if (mcore->assigned_irqs <= 4)
- bcma_core_mips_set_irq(core,
- mcore->assigned_irqs++);
- break;
+ switch (bus->chipinfo.id) {
+ case BCMA_CHIP_ID_BCM4716:
+ case BCMA_CHIP_ID_BCM4748:
+ bcma_core_mips_set_irq_name(bus, 1, BCMA_CORE_80211, 0);
+ bcma_core_mips_set_irq_name(bus, 2, BCMA_CORE_MAC_GBIT, 0);
+ bcma_core_mips_set_irq_name(bus, 3, BCMA_CORE_USB20_HOST, 0);
+ bcma_core_mips_set_irq_name(bus, 4, BCMA_CORE_PCIE, 0);
+ bcma_core_mips_set_irq_name(bus, 0, BCMA_CORE_CHIPCOMMON, 0);
+ bcma_core_mips_set_irq_name(bus, 0, BCMA_CORE_I2S, 0);
+ break;
+ case BCMA_CHIP_ID_BCM5356:
+ case BCMA_CHIP_ID_BCM47162:
+ case BCMA_CHIP_ID_BCM53572:
+ bcma_core_mips_set_irq_name(bus, 1, BCMA_CORE_80211, 0);
+ bcma_core_mips_set_irq_name(bus, 2, BCMA_CORE_MAC_GBIT, 0);
+ bcma_core_mips_set_irq_name(bus, 0, BCMA_CORE_CHIPCOMMON, 0);
+ break;
+ case BCMA_CHIP_ID_BCM5357:
+ case BCMA_CHIP_ID_BCM4749:
+ bcma_core_mips_set_irq_name(bus, 1, BCMA_CORE_80211, 0);
+ bcma_core_mips_set_irq_name(bus, 2, BCMA_CORE_MAC_GBIT, 0);
+ bcma_core_mips_set_irq_name(bus, 3, BCMA_CORE_USB20_HOST, 0);
+ bcma_core_mips_set_irq_name(bus, 0, BCMA_CORE_CHIPCOMMON, 0);
+ bcma_core_mips_set_irq_name(bus, 0, BCMA_CORE_I2S, 0);
+ break;
+ case BCMA_CHIP_ID_BCM4706:
+ bcma_core_mips_set_irq_name(bus, 1, BCMA_CORE_PCIE, 0);
+ bcma_core_mips_set_irq_name(bus, 2, BCMA_CORE_4706_MAC_GBIT,
+ 0);
+ bcma_core_mips_set_irq_name(bus, 3, BCMA_CORE_PCIE, 1);
+ bcma_core_mips_set_irq_name(bus, 4, BCMA_CORE_USB20_HOST, 0);
+ bcma_core_mips_set_irq_name(bus, 0, BCMA_CORE_4706_CHIPCOMMON,
+ 0);
+ break;
+ default:
+ list_for_each_entry(core, &bus->cores, list) {
+ core->irq = bcma_core_irq(core);
}
+ bcma_err(bus,
+ "Unknown device (0x%x) found, can not configure IRQs\n",
+ bus->chipinfo.id);
}
bcma_info(bus, "IRQ reconfiguration done\n");
bcma_core_mips_dump_irq(bus);
--
1.7.10.4
This member is not needed any more.
Signed-off-by: Hauke Mehrtens <[email protected]>
---
drivers/bcma/driver_mips.c | 2 --
include/linux/bcma/bcma_driver_mips.h | 1 -
2 files changed, 3 deletions(-)
diff --git a/drivers/bcma/driver_mips.c b/drivers/bcma/driver_mips.c
index 6981507..c6d7be3 100644
--- a/drivers/bcma/driver_mips.c
+++ b/drivers/bcma/driver_mips.c
@@ -263,8 +263,6 @@ void bcma_core_mips_init(struct bcma_drv_mips *mcore)
bcma_core_mips_early_init(mcore);
- mcore->assigned_irqs = 1;
-
switch (bus->chipinfo.id) {
case BCMA_CHIP_ID_BCM4716:
case BCMA_CHIP_ID_BCM4748:
diff --git a/include/linux/bcma/bcma_driver_mips.h b/include/linux/bcma/bcma_driver_mips.h
index 0baf8a5..6495579 100644
--- a/include/linux/bcma/bcma_driver_mips.h
+++ b/include/linux/bcma/bcma_driver_mips.h
@@ -36,7 +36,6 @@ struct bcma_drv_mips {
struct bcma_device *core;
u8 setup_done:1;
u8 early_setup_done:1;
- unsigned int assigned_irqs;
};
#ifdef CONFIG_BCMA_DRIVER_MIPS
--
1.7.10.4
The new name better matches the use of this variable.
Signed-off-by: Hauke Mehrtens <[email protected]>
---
drivers/bcma/driver_mips.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/bcma/driver_mips.c b/drivers/bcma/driver_mips.c
index c6d7be3..8a51c79 100644
--- a/drivers/bcma/driver_mips.c
+++ b/drivers/bcma/driver_mips.c
@@ -130,9 +130,9 @@ static void bcma_core_mips_set_irq(struct bcma_device *dev, unsigned int irq)
bcma_read32(mdev, BCMA_MIPS_MIPS74K_INTMASK(0)) |
(1 << irqflag));
} else {
- u32 oldirqflag = bcma_read32(mdev,
- BCMA_MIPS_MIPS74K_INTMASK(irq));
- if (oldirqflag) {
+ u32 irqinitmask = bcma_read32(mdev,
+ BCMA_MIPS_MIPS74K_INTMASK(irq));
+ if (irqinitmask) {
struct bcma_device *core;
/* backplane irq line is in use, find out who uses
@@ -140,7 +140,7 @@ static void bcma_core_mips_set_irq(struct bcma_device *dev, unsigned int irq)
*/
list_for_each_entry(core, &bus->cores, list) {
if ((1 << bcma_core_mips_irqflag(core)) ==
- oldirqflag) {
+ irqinitmask) {
bcma_core_mips_set_irq(core, 0);
break;
}
--
1.7.10.4
Great, Ive used "Reply" instead of "Reply to all"... :/
2013/1/6 Hauke Mehrtens <[email protected]>:
> On 01/04/2013 08:43 AM, Rafał Miłecki wrote:
>> 2013/1/4 Hauke Mehrtens <[email protected]>:
>>> The assignment of the IRQs to the cores of the chips by iterating over
>>> the cores is complicated and causes problems with SoC like the BCM4706
>>> with two GMAC core where just one should get a dedicated IRQ number.
>>
>> Well, the only problem on BCM4706 is that second (broken/dangling)
>> GMAC core gets it's own IRQ. With such a "waste" of one IRQ line we
>> may end up using shared IRQ for some other core like PCIE/USB.
>> This issue can be simply workarounded (with 2 LOC) and sounds sane:
>> bug in HW, specific workaround in a driver. We already have similar
>> workarounds for other "dangling" cores.
>>
>> I don't know about any other issues with BCM4706 IRQs.
>>
>> Are there any other (more serious?) issues on different SoCs? Any
>> advantages of hardcoding IRQs (per chipset) rather than keeping this
>> simple algorithm? Do the IRQ lines differ? Is this somehow better to
>> use (just an example) IRQ 3 instead of IRQ 4 for GMAC core?
>
> Normally the boot loader (CFE) assigns IRQs to the cores on the bus. The
> old code mostly worked because CFE did it correct most of the time.
>
> On my bcm4718 the I2S core (id 0x834) does not get any IRQ from CFE, but
> at least Nathan (nlhintz) wants to assign an IRQ (shared) to this core.
So can we simply resolve this by
> default:
> bcma_core_mips_set_irq(core, mcore->assigned_irqs);
?
> It is also possible to extend the algorithm to solve all these things,
> but I think the new code is better to understand.
>
> I do not think that there will be any new SoC using bcma and a mips cpu,
> I haven't seen an announcement for such a SoC at the Broadcom web page,
> just arm based devices, so I assume this list is complete.
I just think we should leave some fallback for other devices... and
that probably means the algorithm. But if we decide to leave (a fixed)
algorithm there, why to use hadrcoded version at all?
--
Rafał
Signed-off-by: Hauke Mehrtens <[email protected]>
---
drivers/bcma/driver_mips.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/bcma/driver_mips.c b/drivers/bcma/driver_mips.c
index 171be0f..486aa2d 100644
--- a/drivers/bcma/driver_mips.c
+++ b/drivers/bcma/driver_mips.c
@@ -143,8 +143,8 @@ static void bcma_core_mips_set_irq(struct bcma_device *dev, unsigned int irq)
1 << irqflag);
}
- bcma_info(bus, "set_irq: core 0x%04x, irq %d => %d\n",
- dev->id.id, oldirq + 2, irq + 2);
+ bcma_debug(bus, "set_irq: core 0x%04x, irq %d => %d\n",
+ dev->id.id, oldirq + 2, irq + 2);
}
static void bcma_core_mips_set_irq_name(struct bcma_bus *bus, unsigned int irq,
@@ -167,7 +167,7 @@ static void bcma_core_mips_print_irq(struct bcma_device *dev, unsigned int irq)
{
int i;
static const char *irq_name[] = {"2(S)", "3", "4", "5", "6", "D", "I"};
- printk(KERN_INFO KBUILD_MODNAME ": core 0x%04x, irq :", dev->id.id);
+ printk(KERN_DEBUG KBUILD_MODNAME ": core 0x%04x, irq :", dev->id.id);
for (i = 0; i <= 6; i++)
printk(" %s%s", irq_name[i], i == irq ? "*" : " ");
printk("\n");
@@ -252,7 +252,7 @@ void bcma_core_mips_init(struct bcma_drv_mips *mcore)
if (mcore->setup_done)
return;
- bcma_info(bus, "Initializing MIPS core...\n");
+ bcma_debug(bus, "Initializing MIPS core...\n");
bcma_core_mips_early_init(mcore);
@@ -300,7 +300,7 @@ void bcma_core_mips_init(struct bcma_drv_mips *mcore)
"Unknown device (0x%x) found, can not configure IRQs\n",
bus->chipinfo.id);
}
- bcma_info(bus, "IRQ reconfiguration done\n");
+ bcma_debug(bus, "IRQ reconfiguration done\n");
bcma_core_mips_dump_irq(bus);
mcore->setup_done = true;
--
1.7.10.4
On 01/04/2013 08:43 AM, Rafał Miłecki wrote:
> 2013/1/4 Hauke Mehrtens <[email protected]>:
>> The assignment of the IRQs to the cores of the chips by iterating over
>> the cores is complicated and causes problems with SoC like the BCM4706
>> with two GMAC core where just one should get a dedicated IRQ number.
>
> Well, the only problem on BCM4706 is that second (broken/dangling)
> GMAC core gets it's own IRQ. With such a "waste" of one IRQ line we
> may end up using shared IRQ for some other core like PCIE/USB.
> This issue can be simply workarounded (with 2 LOC) and sounds sane:
> bug in HW, specific workaround in a driver. We already have similar
> workarounds for other "dangling" cores.
>
> I don't know about any other issues with BCM4706 IRQs.
>
> Are there any other (more serious?) issues on different SoCs? Any
> advantages of hardcoding IRQs (per chipset) rather than keeping this
> simple algorithm? Do the IRQ lines differ? Is this somehow better to
> use (just an example) IRQ 3 instead of IRQ 4 for GMAC core?
Normally the boot loader (CFE) assigns IRQs to the cores on the bus. The
old code mostly worked because CFE did it correct most of the time.
On my bcm4718 the I2S core (id 0x834) does not get any IRQ from CFE, but
at least Nathan (nlhintz) wants to assign an IRQ (shared) to this core.
It is also possible to extend the algorithm to solve all these things,
but I think the new code is better to understand.
I do not think that there will be any new SoC using bcma and a mips cpu,
I haven't seen an announcement for such a SoC at the Broadcom web page,
just arm based devices, so I assume this list is complete.
Hauke
2013/1/4 Hauke Mehrtens <[email protected]>:
> This improves IRQ assignment to cores on a mips SoC.
>
> Hauke Mehrtens (6):
> bcma: make bcma_find_core_unit() accessible
> bcma: mips: explicit assign IRQ numbers
> bcma: mips: make some info messages debug messages
> bcma: mips: show also disabled IRQs
> bcma: mips: remove assigned_irqs from structure
> bcma: mips: rename oldirqflag to irqinitmask
OK, well, the patchset was applied... did someone at least tried to compile it?
CC drivers/bcma/driver_mips.o
drivers/bcma/driver_mips.c: In function 'bcma_core_mips_init':
drivers/bcma/driver_mips.c:302:4: error: implicit declaration of
function 'bcma_core_irq' [-Werror=implicit-function-declaration]
cc1: some warnings being treated as errors
make[6]: *** [drivers/bcma/driver_mips.o] Error 1
Am I missing something?
--
Rafał
2013/1/4 Hauke Mehrtens <[email protected]>:
> The assignment of the IRQs to the cores of the chips by iterating over
> the cores is complicated and causes problems with SoC like the BCM4706
> with two GMAC core where just one should get a dedicated IRQ number.
Well, the only problem on BCM4706 is that second (broken/dangling)
GMAC core gets it's own IRQ. With such a "waste" of one IRQ line we
may end up using shared IRQ for some other core like PCIE/USB.
This issue can be simply workarounded (with 2 LOC) and sounds sane:
bug in HW, specific workaround in a driver. We already have similar
workarounds for other "dangling" cores.
I don't know about any other issues with BCM4706 IRQs.
Are there any other (more serious?) issues on different SoCs? Any
advantages of hardcoding IRQs (per chipset) rather than keeping this
simple algorithm? Do the IRQ lines differ? Is this somehow better to
use (just an example) IRQ 3 instead of IRQ 4 for GMAC core?
--
Rafał