Hi Hauke:
On Wed, 2012-04-25 at 00:15 +0200, Hauke Mehrtens wrote:
> Hi Nathan,
>
> It would make reviewing the patches a loot easier if you just but one
> thing into one patch. This patch changes a loot of different things that
> do not have many thing to do with each other. some part like
> bcma_core_irq() I understand and would like to apply and some other
> parts I do not understand.
I can split out the code related to "bcma_fix_i2s_irqflag" into a
separate patch, and maybe the calls to ""bcma_core_mips_set_irq(core,
0)" for the CC and I2S cores; but the remaining seems relevant to me.
If it's more than that, can you provide additional feedback? Do you
want the patch files posted independently to the mailing list, even
though there may be order dependencies?
>
> On 04/10/2012 05:56 AM, Nathan Hintz wrote:
> > Changes since v3: Fixed two references to bcma_core_mips_irq in
> > driver_pci_host.c
> >
> > Signed-off-by: Nathan Hintz <nlhi...@hotmail.com>
> >
> > --- /dev/null 2012-04-07 22:03:23.120698218 -0700
> > +++ target/linux/brcm47xx/patches-3.2/235-bcma-dont-expose-mips-irq.patch
> > 2012-04-09 20:18:16.988877496 -0700
> > @@ -0,0 +1,95 @@
> > +--- a/include/linux/bcma/bcma.h
> > ++++ b/include/linux/bcma/bcma.h
> > +@@ -295,6 +295,7 @@ extern struct bcma_device *bcma_find_cor
> > + extern bool bcma_core_is_enabled(struct bcma_device *core);
> > + extern void bcma_core_disable(struct bcma_device *core, u32 flags);
> > + extern int bcma_core_enable(struct bcma_device *core, u32 flags);
> > ++extern unsigned int bcma_core_irq(struct bcma_device *core);
> > + extern void bcma_core_set_clockmode(struct bcma_device *core,
> > + enum bcma_clkmode clkmode);
> > + extern void bcma_core_pll_ctl(struct bcma_device *core, u32 req, u32
> > status,
> > +--- a/include/linux/bcma/bcma_driver_mips.h
> > ++++ b/include/linux/bcma/bcma_driver_mips.h
> > +@@ -46,6 +46,4 @@ static inline void bcma_core_mips_init(s
> > +
> > + extern u32 bcma_cpu_clock(struct bcma_drv_mips *mcore);
> > +
> > +-extern unsigned int bcma_core_mips_irq(struct bcma_device *dev);
> > +-
> > + #endif /* LINUX_BCMA_DRIVER_MIPS_H_ */
> > +--- a/arch/mips/bcm47xx/gpio.c
> > ++++ b/arch/mips/bcm47xx/gpio.c
> > +@@ -94,7 +94,7 @@ int gpio_to_irq(unsigned gpio)
> > + #endif
> > + #ifdef CONFIG_BCM47XX_BCMA
> > + case BCM47XX_BUS_TYPE_BCMA:
> > +- return bcma_core_mips_irq(bcm47xx_bus.bcma.bus.drv_cc.core) + 2;
> > ++ return bcma_core_irq(bcm47xx_bus.bcma.bus.drv_cc.core);
> > + #endif
> > + }
> > + return -EINVAL;
> > +--- a/arch/mips/bcm47xx/serial.c
> > ++++ b/arch/mips/bcm47xx/serial.c
> > +@@ -62,7 +62,7 @@ static int __init uart8250_init_bcma(voi
> > +
> > + p->mapbase = (unsigned int) bcma_port->regs;
> > + p->membase = (void *) bcma_port->regs;
> > +- p->irq = bcma_port->irq + 2;
> > ++ p->irq = bcma_port->irq;
> > + p->uartclk = bcma_port->baud_base;
> > + p->regshift = bcma_port->reg_shift;
> > + p->iotype = UPIO_MEM;
> > +--- a/drivers/bcma/driver_chipcommon.c
> > ++++ b/drivers/bcma/driver_chipcommon.c
> > +@@ -147,7 +147,7 @@ void bcma_chipco_serial_init(struct bcma
> > + return;
> > + }
> > +
> > +- irq = bcma_core_mips_irq(cc->core);
> > ++ irq = bcma_core_irq(cc->core);
> > +
> > + /* Determine the registers of the UARTs */
> > + cc->nr_serial_ports = (cc->capabilities & BCMA_CC_CAP_NRUART);
> > +--- a/drivers/bcma/driver_mips.c
> > ++++ b/drivers/bcma/driver_mips.c
> > +@@ -81,7 +81,7 @@ static u32 bcma_core_mips_irqflag(struct
> > + /* Get the MIPS IRQ assignment for a specified device.
> > + * If unassigned, 0 is returned.
> > + */
> > +-unsigned int bcma_core_mips_irq(struct bcma_device *dev)
> > ++static unsigned int bcma_core_mips_irq(struct bcma_device *dev)
> > + {
> > + struct bcma_device *mdev = dev->bus->drv_mips.core;
> > + u32 irqflag;
> > +@@ -96,7 +96,11 @@ unsigned int bcma_core_mips_irq(struct b
> > +
> > + return 0;
> > + }
> > +-EXPORT_SYMBOL(bcma_core_mips_irq);
> > ++
> > ++unsigned int bcma_core_irq(struct bcma_device *dev) {
> > ++ return bcma_core_mips_irq(dev) + 2;
> > ++}
> > ++EXPORT_SYMBOL(bcma_core_irq);
> > +
> > + static void bcma_core_mips_set_irq(struct bcma_device *dev, unsigned int
> > irq)
> > + {
> > +--- a/drivers/bcma/driver_pci_host.c
> > ++++ b/drivers/bcma/driver_pci_host.c
> > +@@ -565,7 +565,7 @@
> > + pr_info("PCI: Fixing up device %s\n", pci_name(dev));
> > +
> > + /* Fix up interrupt lines */
> > +- dev->irq = bcma_core_mips_irq(pc_host->pdev->core) + 2;
> > ++ dev->irq = bcma_core_irq(pc_host->pdev->core);
> > + pci_write_config_byte(dev, PCI_INTERRUPT_LINE, dev->irq);
> > +
> > + return 0;
> > +@@ -584,6 +584,6 @@
> > +
> > + pc_host = container_of(dev->bus->ops, struct bcma_drv_pci_host,
> > + pci_ops);
> > +- return bcma_core_mips_irq(pc_host->pdev->core) + 2;
> > ++ return bcma_core_irq(pc_host->pdev->core);
> > + }
> > + EXPORT_SYMBOL(bcma_core_pci_pcibios_map_irq);
> > --- /dev/null 2012-03-21 21:16:41.055325718 -0700
> > +++ target/linux/brcm47xx/patches-3.2/237-bcma-fix-irq-assignment.patch
> > 2012-03-29 21:05:49.189660716 -0700
> > @@ -0,0 +1,173 @@
> > +--- a/include/linux/bcma/bcma_driver_mips.h
> > ++++ b/include/linux/bcma/bcma_driver_mips.h
> > +@@ -28,6 +28,7 @@
> > + #define BCMA_MIPS_MIPS74K_GPIOEN 0x0048
> > + #define BCMA_MIPS_MIPS74K_CLKCTLST 0x01E0
> > +
> > ++#define BCMA_MIPS_OOBSELINA74 0x004
> > + #define BCMA_MIPS_OOBSELOUTA30 0x100
> > +
> > + struct bcma_device;
> > +--- a/drivers/bcma/driver_mips.c
> > ++++ b/drivers/bcma/driver_mips.c
> > +@@ -65,6 +65,10 @@ static const u32 ipsflag_irq_shift[] = {
> > + BCMA_MIPS_IPSFLAG_IRQ4_SHIFT,
> > + };
> > +
> > ++#define IRQ_FLAG_MASK 0x1F
> > ++#define INVALID_IRQ_FLAG 0x3F
> > ++#define IRQ_NOT_ASSIGNED 5
> > ++#define IRQ_NOT_REQUIRED 6
> > + static u32 bcma_core_mips_irqflag(struct bcma_device *dev)
> > + {
> > + u32 flag;
> > +@@ -75,11 +79,11 @@ static u32 bcma_core_mips_irqflag(struct
> > + return dev->core_index;
> > + flag = bcma_aread32(dev, BCMA_MIPS_OOBSELOUTA30);
> > +
> > +- return flag & 0x1F;
> > ++ return ((flag && ((flag & IRQ_FLAG_MASK) <= 7)) ? (flag &
> > IRQ_FLAG_MASK) : INVALID_IRQ_FLAG);
> For what core do you get INVALID_IRQ_FLAG?
A value of INVALID_IRQ_FLAG is not expected to be read from any core,
valid IRQ Flags should always be <= 31. INVALID_IRQ_FLAG is just an
arbitrary (internal) value to indicate a valid IRQ Flag was not read for
the core.
> > + }
> > +
> > + /* Get the MIPS IRQ assignment for a specified device.
> > +- * If unassigned, 0 is returned.
> > ++ * If unassigned, 5 is returned; if no IRQ is required, 6 is returned
> > + */
> > + static unsigned int bcma_core_mips_irq(struct bcma_device *dev)
> > + {
> > +@@ -88,21 +92,24 @@ static unsigned int bcma_core_mips_irq(s
> > + unsigned int irq;
> > +
> > + irqflag = bcma_core_mips_irqflag(dev);
> > ++ if (irqflag == INVALID_IRQ_FLAG)
> > ++ return IRQ_NOT_REQUIRED;
> > +
> > +- 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 IRQ_NOT_ASSIGNED;
> > + }
> > +
> > + unsigned int bcma_core_irq(struct bcma_device *dev) {
> > +- return bcma_core_mips_irq(dev) + 2;
> > ++ unsigned int irq = bcma_core_mips_irq(dev);
> > ++ return ((irq > 4) ? 0 : (irq + 2));
> > + }
> > + EXPORT_SYMBOL(bcma_core_irq);
> > +
> > +-static void bcma_core_mips_set_irq(struct bcma_device *dev, unsigned int
> > irq)
> > ++static bool bcma_core_mips_set_irq(struct bcma_device *dev, unsigned int
> > irq)
> > + {
> > + unsigned int oldirq = bcma_core_mips_irq(dev);
> > + struct bcma_bus *bus = dev->bus;
> > +@@ -110,7 +117,8 @@ static void bcma_core_mips_set_irq(struc
> > + u32 irqflag;
> > +
> > + irqflag = bcma_core_mips_irqflag(dev);
> > +- BUG_ON(oldirq == 6);
> > ++ if (irqflag == INVALID_IRQ_FLAG)
> > ++ return false;
> > +
> > + dev->irq = irq + 2;
> > +
> > +@@ -119,8 +127,8 @@ static void bcma_core_mips_set_irq(struc
> > + bcma_write32(mdev, BCMA_MIPS_MIPS74K_INTMASK(0),
> > + bcma_read32(mdev, BCMA_MIPS_MIPS74K_INTMASK(0)) &
> > + ~(1 << irqflag));
> > +- else
> > +- bcma_write32(mdev, BCMA_MIPS_MIPS74K_INTMASK(irq), 0);
> > ++ else if (oldirq <= 4)
> > ++ bcma_write32(mdev, BCMA_MIPS_MIPS74K_INTMASK(oldirq), 0);
> > +
> > + /* assign the new one */
> > + if (irq == 0) {
> > +@@ -149,7 +157,9 @@ static void bcma_core_mips_set_irq(struc
> > + }
> > +
> > + pr_info("set_irq: core 0x%04x, irq %d => %d\n",
> > +- dev->id.id, oldirq + 2, irq + 2);
> > ++ dev->id.id, (oldirq > 4) ? 0 : (oldirq + 2), irq + 2);
> > ++
> > ++ return true;
> > + }
> > +
> > + static void bcma_core_mips_print_irq(struct bcma_device *dev, unsigned
> > int irq)
> > +@@ -226,6 +236,27 @@ static void bcma_core_mips_flash_detect(
> > + }
> > + }
> > +
> > ++static void bcma_fix_i2s_irqflag(struct bcma_bus *bus) {
> > ++ struct bcma_device *cpu, *pcie, *i2s;
> > ++
> > ++ /* IRQ flags >= 8 are not honored in the IRQ masks (2010 Broadcom SDK)
> > */
> > ++ if (bus->chipinfo.id != 0x4716 &&
> > ++ bus->chipinfo.id != 0x4748)
> > ++ return;
> > ++
> > ++ if ((cpu = bcma_find_core(bus, BCMA_CORE_MIPS_74K)) &&
> > ++ (pcie = bcma_find_core(bus, BCMA_CORE_PCIE)) &&
> > ++ (i2s = bcma_find_core(bus, BCMA_CORE_I2S))) {
> > ++ if (bcma_aread32(cpu, BCMA_MIPS_OOBSELINA74) == 0x08060504 &&
> > ++ bcma_aread32(pcie, BCMA_MIPS_OOBSELINA74) == 0x08060504 &&
> > ++ bcma_aread32(i2s, BCMA_MIPS_OOBSELOUTA30) == 0x88) {
> > ++ bcma_awrite32(cpu, BCMA_MIPS_OOBSELINA74, 0x07060504);
> > ++ bcma_awrite32(pcie, BCMA_MIPS_OOBSELINA74, 0x07060504);
> > ++ bcma_awrite32(i2s, BCMA_MIPS_OOBSELOUTA30, 0x87);
> > ++ }
> > ++ }
> > ++}
> > ++
> In the Broadcom SDK this code is guarded by #ifdef _CFE_ as I understand
> it this means that it will just be compiled into the cfe. Do you need
> this code for your device? Don't you have a CFE boot loader on your device?
The testing I have done has demonstrated that IRQ Flags > 7 are not
supported. Any attempt to set an IRQ Mask using an IRQ Flag value > 7
is ignored by the device; that is, the IRQ Mask remains un-set for that
IRQ Flag. I scratched my head for quite some time when I found that the
call to "bcma_core_mips_set_irq(core, 0)" for "BCMA_CORE_I2S" was not
working. I finally found the code in the Broadcom SDK that adjusts the
IRQ Flag value from 8 to 7 (in the CFE). Unfortunately, my device has
an older CFE version, so the code to make the adjustment is not present.
That's why I added it here. As to how important it is to set the IRQ
Mask properly for the I2S Core, I don't have an answer for that, but I
haven't noticed any bad behavior. Would this improve performance in
some way?
>
> > + void bcma_core_mips_init(struct bcma_drv_mips *mcore)
> > + {
> > + struct bcma_bus *bus;
> > +@@ -234,12 +265,14 @@ void bcma_core_mips_init(struct bcma_drv
> > +
> > + pr_info("Initializing MIPS core...\n");
> > +
> > ++ bcma_fix_i2s_irqflag(bus);
> > ++
> > + if (!mcore->setup_done)
> > + mcore->assigned_irqs = 1;
> > +
> > + /* Assign IRQs to all cores on the bus */
> > + list_for_each_entry_reverse(core, &bus->cores, list) {
> > +- int mips_irq;
> > ++ unsigned int mips_irq;
> > + if (core->irq)
> > + continue;
> > +
> > +@@ -248,8 +281,7 @@ void bcma_core_mips_init(struct bcma_drv
> > + 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:
> > +@@ -261,9 +293,17 @@ void bcma_core_mips_init(struct bcma_drv
> > + /* 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++);
> > ++ if (mcore->assigned_irqs <= 4) {
> > ++ if (bcma_core_mips_set_irq(core,
> > ++
> > mcore->assigned_irqs))
> > ++ mcore->assigned_irqs++;
> > ++ }
> > ++ else
> > ++ bcma_core_mips_set_irq(core, 0);
> > ++ break;
> > ++ case BCMA_CORE_CHIPCOMMON:
> > ++ case BCMA_CORE_I2S:
> > ++ bcma_core_mips_set_irq(core, 0);
> This looks good to me, expect for the formating.
Please elaborate on the formatting, as I'm oblivious to the problem. I
will fix if you will point me to the problem area(s).
> > + break;
> > + }
> > + }
> >
> > _______________________________________________
> > openwrt-devel mailing list
> > openwrt-devel@lists.openwrt.org
> > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>
>
Thanks for reviewing,
Nathan
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
