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

From: Scott Wood <[email-masked]>
Date: Fri, 10 Aug 2012 11:00:04 -0500

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-
>> [email-masked]
>> 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.

> 3. The key advantage of my way is better unified for platform driver. If I use
> the Scott's way I have to make an routine and called in all boards code.

Only until all boards are converted, and this is *not* different with
your approach.

> The goal
> of my PCI patch is unifying all PCI initialization code and obviously primary
> determination is part of PCI code.
> 
> 4. The other advantage is efficiency. All my search for ISA node is just under
> PCI node instead of all device tree.

We do so many searches over the full device tree during boot that this
is meaningless.

Do you have benchmarks to show that device tree iteration is a
significant contributor to boot time?

-Scott