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

From: Scott Wood <[email-masked]>
Date: Tue, 21 Aug 2012 13:24:30 -0500

On 08/21/2012 01:09 PM, Li Yang-R58472 wrote:
> 
> 
> On Aug 22, 2012, at 1:16, "Wood Scott-B07421" <[email-masked]> wrote:
> 
>> On 08/21/2012 01:49 AM, Li Yang-R58472 wrote:
>>> If there is i8259 node in the device tree, it should be suggesting
>>> that there is a PCI to ISA bridge but not explicitly described in the
>>> device tree.  Then we need to fix the device tree to add the ISA
>>> nodes.
>>
>> No, we need to work with existing device trees.  Just because they're
>> hosted in the kernel tree doesn't mean they can be treated as an
>> internal kernel implementation detail.  They are stable API with
>> external entities such as bootloaders and hypervisors, get forked for
>> custom boards, etc.
> 
> You are right that device tree binding is a stable API but not a poorly written dts instance.
>
> We are not changing any binding definition.  It is just adding the
> somehow missed node that should be there in some poorly written dts
> in the first place according to current binding.  Is it making any
> sense to have ISA devices directly under pci bus?

I agree in general, but in this case it's easy enough to work with
existing trees, and the workaround is localized to platform code for the
boards with the problem.  Adding this workaround doesn't prevent adding
an isa node to the device tree.

>> Boards with this problem should set fsl_pci_primary in platform code
>> based on whatever criteria is relevant.
> 
> I don't know how legitimate it is to use a new kernel but not the
> device tree from that source. 

It's legitimate, if a bit risky because we have a history of not getting
device trees right the first time (but it should still be a goal).
Conisder that significant parts of the device tree often come from
firmware rather than the .dts.  As I've said a few times in the past, I
think the device trees should be kept in the U-Boot tree rather than
Linux (only those that are meant to be used with U-Boot, of course).
It's a U-Boot internal implementation issue what comes from the .dts and
what gets fixed up or generated at runtime.

-Scott