Re: [PATCH V5 3/3] powerpc/fsl-pci: Unify pci/pcie initialization code

[Scott Wood <scottwood@freescale.com>]

On 08/15/2012 04:22 AM, Jia Hongtao-B38951 wrote:
> 
> 
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Saturday, August 11, 2012 12:00 AM
>> To: Jia Hongtao-B38951
>> Cc: Gala Kumar-B11780; Wood Scott-B07421; Li Yang-R58472; linuxppc-
>> dev@lists.ozlabs.org
>> Subject: Re: [PATCH V5 3/3] powerpc/fsl-pci: Unify pci/pcie
>> initialization code
>>
>> On 08/10/2012 03:47 AM, Jia Hongtao-B38951 wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Gala Kumar-B11780
>>>> Sent: Thursday, August 09, 2012 3:04 AM
>>>> To: Wood Scott-B07421
>>>> Cc: Jia Hongtao-B38951; Wood Scott-B07421; Li Yang-R58472; linuxppc-
>>>> dev@lists.ozlabs.org
>>>> Subject: Re: [PATCH V5 3/3] powerpc/fsl-pci: Unify pci/pcie
>>>> initialization code
>>>>
>>>>>>>>>>> As I explained before, this has to be done globally, not from
>>>>>>>>>>> the probe function, so we can assign a default primary bus if
>>>>>>>>>>> there
>>>>>>> isn't any ISA.
>>>>>>>>>>> There are bugs in the Linux PPC PCI code relating to not
>>>>>>>>>>> having any primary bus.
>>>>>>>>>>>
>>>>>>>>>>> -Scott
>>>>>>>>>>
>>>>>>>>>> In my way of searching ISA you can also assign a default
>>>>>>>>>> primary bus in board specific files.
>>>>>>>>>
>>>>>>>>> That was meant for when the board file had an alternate way of
>>>>>>>>> searching for the primary bus (e.g. look for i8259), not as a
>>>>>>>>> replacement for the mechanism that guarantees there's a primary
>> bus.
>>>>>>>>>
>>>>>>>>> You are causing a regression in the qemu_e500.c platform.
>>>>>>>>
>>>>>>>> Can we fix the qemu device tree to address the problem if we do
>>>>>>>> make it a rule to use the ISA node to indicate the primary bus?
>>>>>>>
>>>>>>> No.  There is no ISA, and we're not going to lie and say there is.
>>>>>>
>>>>>> But we can assign a default primary for qemu.
>>>>>
>>>>> Not in the device tree.  What other mechanism do you propose?  And
>>>>> why do you want to fix it only for QEMU and not other boards, where
>>>>> things happen to work but not as designed?
>>>>>
>>>>> Kumar, can you speak up here as maintainer so we can stop going back
>>>>> and forth endlessly?
>>>>
>>>> I'd rather we stick with the code that works for this purpose at this
>>>> point.  That would be Scott's current upstream code.  Lets get the
>>>> other aspects of this patchset closed (SWIOTLB, conversion to
>>>> platform driver, PM, etc.).  The primary bus code Scott wrote does
>>>> NOT need to change at this point.
>>>>
>>>> - k
>>>
>>>
>>> I just submitted a new version of PCI patch in which I improve the
>> primary part.
>>> The reasons I want to change the way of primary assignment listed below:
>>>
>>> 1. This approach is functionally equivalent to the Scott's code. In my
>>> approach there might be no primary assigned but it fixed by "quick fix"
>> introduced by Ben.
>>> Please refer to this link:
>>> https://lists.ozlabs.org/pipermail/linuxppc-dev/2012-June/098586.html
>>
>> You might want to get Ben's input as to whether he actually wants to see
>> that "quick fix" applied.
>>
>>> 2. Scott's and my way could not handle the situation that "the primary
>>> is not the first PCI bus detected". I found that only ge_imp3a got
>>> this problem so I fixed it by adding ISA node to its device tree. By
>>> adding this I think the solution is logically completed.
>>
>> How did my approach not handle this case?  As I said, ge_imp3a platform
>> code needs to set fsl_pci_primary manually before PCI init runs.
>>
>> Adding a node to the device tree is not the answer, since that will break
>> compatibility with old device trees.
>>
> 
> I assume that kernel image and dtb image are from the same tree.

That's a bad assumption.  Device trees get forked off for custom boards,
modified by firmware, generated by firmware, etc.

-Scott