Re: [PATCH 2/8] Add uli1575 pci-bridge sector to MPC8641HPCN dts file.

[Segher Boessenkool <segher@kernel.crashing.org>]

>> "reg" included in "ranges"?  Something is wrong here.
>
> I think it's correct for soc nodes.

It is not.  "ranges" is the address space translation
between the parent and child busses; "reg" is the stuff
that's on the bridge device itself.

> At least, it appears that all of
> the dts files with soc nodes do similar things (including this one even
> without this patch).

Yes, but that doesn't make it right.

>>> +				pci_bridge@0 {
>>
>>> +					#size-cells = <2>;
>>> +					#address-cells = <3>;
>>> +					ranges = <02000000 0 80000000
>>> +						  02000000 0 80000000
>>> +						  0 20000000
>>> +						  01000000 0 00000000
>>> +						  01000000 0 00000000
>>> +						  0 00100000>;
>>> +
>>> +					isa@1e {
>>> +						#size-cells = <1>;
>>> +						#address-cells = <2>;
>>>
>>> +						ranges = <1 0 01000000 0 0
>>> +							  00001000>;
>>
>> You map the same range (4kB legacy I/O @ 0) for both
>> bridges here.
>
> There is a one-to-one mapping between the I/O spaces of "isa" and
> "pci_bridge", so wouldn't it be reasonable that a similar range be 
> used?

Oh wait, the ISA bridge is a child of the PCI bridge here.
It is obviously fine then.

>>> +						i8042@60 {
>>> +							reg = <1 60 1 1 64 1>;
>>
>> And this address space is included in both of those.
>
> Again, shouldn't the child's address space be in its parent's range?

Yes of course, I'm simply losing track of the nesting
here.  Sigh.

>>> +						i8259: i8259@4d0 {
>>
>> Needs "reg".  And 4d0 isn't the primary address
>> I think?
>
> Yes, this is a standard i8259 with additional registers at 0x20 and
> 0xa0.  I'll fix the address and add the registers.

That sounds good, thanks!


Segher