[PATCH 2/5] powerpc/fsl_msi: add MSIIR1 support for MPIC v4.3

[Scott Wood]

On 06/17/2013 09:34:49 PM, Lian Minghuan-b31939 wrote:
> 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)

Using the larger bitmap unconditionally is fine.  What is not fine is,  
on older hardware, acting as if all 16 irqs are present.

In other words, I'm talking about the contents of the bitmap, not its  
size.

> we have two choice I think.
> 1. Use a variable assigned value 8 or 16 based on compatible, then  
> dynamically create bitmap

If we have the mpic4.3 compatible, then we don't even support  
msi-available-ranges, so we'd skip this code and free everything in the  
bitmap.

> 2. Add a new file for mpic v4.3.

No. :-)

-Scott