Re: PCI bus node location

[Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>]

On Wed, Nov 11, 2009 at 7:16 AM, Rafal Jaworowski <raj-nYOzD4b6Jr9Wk0Htik3J/w@public.gmane.org> wrote:
>
> On 2009-11-11, at 01:05, David Gibson wrote:
>
>>> The current approach seems a bit of a maintenance problem: the PCI
>>> bridges control reg need to specify the whole address instead of
>>> just an offset, which is more error prone in case of changes (when a
>>
>> Well, yes.  And worse, it means there's two places that need to be
>> adjusted rather than one, if the the IMMR is relocated (which it can
>> be).  But it's a trade-off of this versus the inconvenience of dealing
>> with separate "control" and "bridge" nodes for the PCI and following
>> phandles between them.
>
> Would the technique with additional control node and a phandle complicate
> bindings handling much? The clear benefit is the ability to truly reflect
> hierarchy of devices available within IMMR/CCSR block.

No, it wouldn't be particularly more complex.  It would require a
couple of lines of extra code to follow the phandle to obtain some of
the data.

>>> number of places need to be adjusted etc.). What would need to be
>>> done/extended for the ranges prop you mention to allow for better
>>> handling cases like this?
>>
>> I don't really understand the question.  As Grant has said the
>> "correct" approach is to have one node representing the control
>> registers - located under the IMMR ("soc") node - and another
>> representing the PCI host bridge itself (which would be in its present
>> location).  There would need to be phandles linking the two.  It
>> doesn't really need any extension to the device tree semantics itself
>> - just a more complex binding for this device.
>
> Maybe I misunderstood Grant, my impression was that there was possible some
> 'fixing' of ranges properties (which would be alternative to the control
> node approach).

Yes, the PCI node could be moved back into the IMMR node, but, no, it
wouldn't improve the accuracy of the device tree.  It would just be
different.

However, It sounds like we're debating the wrong thing.  Yes, we
should debate how best to describe hardware, but something you said
earlier in this thread raised a warning flag for me:

>>>> Thanks a lot for the historic perspective and explanations.
>>>> My concern with integrating this model into FreeBSD device
>>>> scheme abstraction was that I could not reflect the hierarchy
>>>> of DT resources directly, because two peer bus entities at the
>>>> same level (soc, pci) would ask for overlapping areas to manage
>>>> (which shouldn't happen); but I believe I got an idea how to
>>>> resolve this.

It *sounds* like you may be depending too heavily on the device tree
for making internal FreeBSD decisions about how to manage your
devices.  But there is a fair bit of expressive latitude in device
tree authorship, and you've got no guarantees that you won't have
things like overlapping ranges between nodes, or 'peer' devices having
completely different parent nodes.  It is risky to model internal
kernel implementation details on device tree structure.

For example, in the early days of device tree work, the of_platform
stuff was created for registering of_devices on the of_bus where there
is a 1:1 relationship between device tree nodes and of_device
instances.  However, in hindsight it was a mistake because of_platform
ends up being a clone of the platform bus, but of_drivers and
platform_drivers are not compatible.  What we should do is extract and
adapt the data in the device tree and convert it to a form usable by
the existing kernel infrastructure.

> Please note we are targetting ARM (and other arches in the future) besides
> PowerPC, so if there can be any lessons learnt from previous encounters I'd
> rather embrace them.

Biggest lesson: document and post bindings for new hardware early;
before merging drivers that use them, and definitely before shipping
equipment.  Common errors are caught easily by review.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.