On 06/06/2011 01:23 PM, Rafał Miłecki wrote:

2011/6/6 Hauke Mehrtens [off-list ref]:

quoted

+/* driver_mips.c */
+extern unsigned int bcma_core_mips_irq(struct bcma_device *dev);

Does it compile without CONFIG_BCMA_DRIVER_MIPS?

No ;-) Thought about it after sending these patches, some other patches
will have the same problem.

quoted

+/* 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?

Some old comment, will fix this.

quoted

+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.

Will check all patches with scripts/checkpatch.sh

Why don't you just use "return irq;" instead of break?

yes this will be better.

quoted

+
+       if (irq == 5)
+               irq = 0;
+
+       return irq;

You can just make it "return 0;" after changing break to return.

agree

quoted

+                       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".

Is this needed after the coding guildlines? Shouldn't they be removed if
they are not needed?

quoted

+       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?

Sounds better.