Re: [RFC][PATCH 04/10] bcma: add mips driver
From: Rafał Miłecki <zajec5@gmail.com>
Date: 2011-06-06 11:23:51
Also in:
linux-wireless
2011/6/6 Hauke Mehrtens [off-list ref]:
+/* driver_mips.c */ +extern unsigned int bcma_core_mips_irq(struct bcma_device *dev);
Does it compile without CONFIG_BCMA_DRIVER_MIPS?
+/* 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. + */
Does it ever return 6?
+unsigned int bcma_core_mips_irq(struct bcma_device *dev)
+{
+ struct bcma_device *mdev = dev->bus->drv_mips.core;
+ u32 irqflag;
+ unsigned int irq;
+
+ irqflag = bcma_core_mips_irqflag(dev);
+
+ for (irq = 1; irq <= 4; irq++)
+ if (bcma_read32(mdev, BCMA_MIPS_MIPS74K_INTMASK(irq)) & (1 << irqflag))
+ break;Use scripts/checkpatch*. Braces around "for" and split line to match 80 chars width. Why don't you just use "return irq;" instead of break?
+ + if (irq == 5) + irq = 0; + + return irq;
You can just make it "return 0;" after changing break to return.
+ for (i = 0; i < bus->nr_cores; i++)
+ if ((1 << bcma_core_mips_irqflag(&bus->cores[i])) == oldirqflag) {
+ bcma_core_mips_set_irq(&bus->cores[i], 0);
+ break;
+ }Braces for "for".
+ pr_info("after irq reconfiguration\n");Make first letter uppercase. I'm not English expert, but doesn't something like "IRQ reconfiguration done" sound better? -- Rafał