On Wed, 2013-01-02 at 19:22 +0100, Hauke Mehrtens wrote: > On 12/19/2012 05:09 AM, Nathan Hintz wrote: > > The existing IRQ logic incorrectly indicates cores that have no IRQ > > assignment as being assigned to the shared IRQ; this includes setting > > the "irq" field in the bcma_device struct to "2". This is evidenced by > > output of bcma_core_mips_print_irq, which is as follows: > > > > [ 0.112000] bcma: bus0: Initializing MIPS core... > > [ 0.112000] bcma: bus0: Moved i2s interrupt to oob line 7 instead of 8 > > [ 0.112000] bcma: bus0: set_irq: core 0x0800, irq 2 => 2 > > [ 0.112000] bcma: bus0: set_irq: core 0x0812, irq 3 => 3 > > [ 0.112000] bcma: bus0: set_irq: core 0x082d, irq 4 => 4 > > [ 0.112000] bcma: bus0: set_irq: core 0x0819, irq 5 => 5 > > [ 0.112000] bcma: bus0: set_irq: core 0x0820, irq 6 => 6 > > [ 0.112000] bcma: bus0: set_irq: core 0x0834, irq 2 => 2 > > [ 0.112000] bcma: bus0: IRQ reconfiguration done > > [ 0.112000] bcma: core 0x0800, irq : 2(S)* 3 4 5 6 D I > > [ 0.112000] bcma: core 0x082c, irq : 2(S)* 3 4 5 6 D I <-- Incorrect > > [ 0.112000] bcma: core 0x0812, irq : 2(S) 3* 4 5 6 D I > > [ 0.112000] bcma: core 0x082d, irq : 2(S) 3 4* 5 6 D I > > [ 0.112000] bcma: core 0x0819, irq : 2(S) 3 4 5* 6 D I > > [ 0.112000] bcma: core 0x0820, irq : 2(S) 3 4 5 6* D I > > [ 0.112000] bcma: core 0x082e, irq : 2(S)* 3 4 5 6 D I <-- Incorrect > > [ 0.112000] bcma: core 0x080e, irq : 2(S)* 3 4 5 6 D I <-- Incorrect > > [ 0.112000] bcma: core 0x0834, irq : 2(S)* 3 4 5 6 D I > > [ 0.116000] bcma: bus0: PCIEcore in host mode found > > > > With this patch applied I get the following on my bcm4706: > > [ 0.000000] bcma: bus0: Found chip with id 0x5300, rev 0x01 and > package 0x00 > [ 0.000000] bcma: bus0: Core 0 found: BCM4706 ChipCommon (manuf > 0x4BF, id 0x500, rev 0x1F, class 0x0) > [ 0.000000] bcma: bus0: Core 3 found: MIPS 74K (manuf 0x4A7, id > 0x82C, rev 0x00, class 0x0) > [ 1.956000] bcma: bus0: Core 1 found: BCM4706 GBit MAC (manuf 0x4BF, > id 0x52D, rev 0x00, class 0x0) > [ 1.956000] bcma: bus0: Core 2 found: BCM4706 GBit MAC (manuf 0x4BF, > id 0x52D, rev 0x00, class 0x0) > [ 1.956000] bcma: bus0: Core 4 found: USB 2.0 Host (manuf 0x4BF, id > 0x819, rev 0x04, class 0x0) > [ 1.956000] bcma: bus0: Core 5 found: PCIe (manuf 0x4BF, id 0x820, > rev 0x0E, class 0x0) > [ 1.956000] bcma: bus0: Core 6 found: PCIe (manuf 0x4BF, id 0x820, > rev 0x0E, class 0x0) > [ 1.956000] bcma: bus0: Core 7 found: AMEMC (DDR) (manuf 0x4BF, id > 0x52E, rev 0x00, class 0x0) > [ 1.956000] bcma: bus0: Core 8 found: BCM4706 SOC RAM (manuf 0x4BF, > id 0x50E, rev 0x05, class 0x0) > [ 1.956000] bcma: bus0: Core 9 found: ALTA (I2S) (manuf 0x4BF, id > 0x534, rev 0x00, class 0x0) > [ 1.956000] bcma: bus0: Core 10 found: BCM4706 GBit MAC Common (manuf > 0x43B, id 0x5DC, rev 0x00, class 0x0) > [ 2.120000] bcma: bus0: Initializing MIPS core... > [ 2.120000] bcma: bus0: set_irq: core 0x0820, irq 3 => 2 > [ 2.120000] bcma: bus0: set_irq: core 0x0819, irq 6 => 3 > [ 2.120000] bcma: bus0: set_irq: core 0x052d, irq 4 => 2 > [ 2.120000] bcma: bus0: set_irq: core 0x0820, irq 5 => 4 > [ 2.120000] bcma: bus0: IRQ reconfiguration done > [ 2.120000] bcma: core 0x0500, irq : I D 2(S)* 3 4 5 6 > [ 2.120000] bcma: core 0x082c, irq : I* D 2(S) 3 4 5 6 > [ 2.120000] bcma: core 0x052d, irq : I D 2(S)* 3 4 5 6 > [ 2.120000] bcma: core 0x052d, irq : I D* 2(S) 3 4 5 6 > [ 2.120000] bcma: core 0x0819, irq : I D 2(S) 3* 4 5 6 > [ 2.120000] bcma: core 0x0820, irq : I D 2(S)* 3 4 5 6 > [ 2.120000] bcma: core 0x0820, irq : I D 2(S) 3 4* 5 6 > [ 2.120000] bcma: core 0x052e, irq : I D* 2(S) 3 4 5 6 > [ 2.124000] bcma: core 0x050e, irq : I* D 2(S) 3 4 5 6 > [ 2.124000] bcma: core 0x0534, irq : I D* 2(S) 3 4 5 6 > [ 2.124000] bcma: core 0x05dc, irq : I* D 2(S) 3 4 5 6 > > This does not look correct to me, I will change the irq code to explicit > assign the irqs based on the chip used, like it is done in the Broadcom > gpl code, currently these are just 4 cases. > > Hauke > I can't imagine how the 4706 could have been correct before the patch either; as there would have needed to be a new 'case' selector within the switch statement for at least one of the 4706 specific cores (0x52d) in order for them all to be assigned correctly.
I agree with your assessment that it would be better to scrap this and just do it like in the Broadcom GPL code. There doesn't seem to be a way that this could be written in a generic sense and account for all possibilities. If you come across unknown H/W, I guess you can either BUG out; or just don't set anything and instead read back what was set in the default H/W configuration and hope it works. Nathan _______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/mailman/listinfo/openwrt-devel
