Hi Soctt,
please see my comments inline.
On 06/18/2013 08:15 AM, Scott Wood wrote:
On 06/16/2013 10:00:01 PM, Lian Minghuan-b31939 wrote:
Hi Scott,
please see my comments inline.
On 06/15/2013 06:09 AM, Scott Wood wrote:
On 06/14/2013 02:15:56 AM, Minghuan Lian wrote:
diff --git a/arch/powerpc/sysdev/fsl_msi.h
b/arch/powerpc/sysdev/fsl_msi.h
index 8225f86..43a9d99 100644
--- a/arch/powerpc/sysdev/fsl_msi.h
+++ b/arch/powerpc/sysdev/fsl_msi.h
@@ -16,7 +16,7 @@
#include <linux/of.h>
#include <asm/msi_bitmap.h>
-#define NR_MSI_REG 8
+#define NR_MSI_REG 16
#define IRQS_PER_MSI_REG 32
#define NR_MSI_IRQS (NR_MSI_REG * IRQS_PER_MSI_REG)
I don't see where you update all_avail in fsl_of_msi_probe.
We should also be bounds-checking the contents of msi-available-ranges.
Currently it looks like we just silently overrun the bitmap if we
get bad
input from the device tree.
[Minghuan] all_avail definition: static const u32 all_avail[] = { 0,
NR_MSI_IRQS };
When changing NR_MSI_REG to 16, NR_MSI_IRQS has been changed to
16*32, and all_avail also is updated.
That's my point. It shouldn't change for older hardware.
[Minghaun] the older hardware has 8 registers, mipcv4.3 has 16
registers. If we do not use 16*32 bitmap to indicate 8*32 irqs.(this way
just only wastes some memory and has no other harm)
we have two choice I think.
1. Use a variable assigned value 8 or 16 based on compatible, then
dynamically create bitmap
2. Add a new file for mpic v4.3.
What do you think?
Before calling fsl_msi_setup_hwirq(), the code has checked
'msi-available-ranges', only the interrupts lied in
'msi-available-ranges' will be initialized by call
fsl_msi_setup_hwirq() , and the corresponding bitmap will be freed. I
moved msi_bitmap_free_hwirqs() to fsl_msi_setup_hwirq(), because the
code would generate different bitmap when using MSIIR or MSIIR1.
And what happens if msi-available-ranges is bad, and refers to
non-existent MSIs past the end of the bitmap?
[Minghuan] If msi-available-ranges is bad, the below code will get error.
virt_msir = irq_of_parse_and_map(dev->dev.of_node, irq_index);
And the related error will be printed out and fsl_msi_setup_hwirq() will
return error directly. There is no chance to set non-existent MSIs past
the end of the bitmap.
-Scott
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
