[U-Boot] [PATCH v2 2/7] fdt: Add function to allow aliases to refer to multiple nodes

[Stephen Warren]

On 01/19/2012 04:45 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Thu, Jan 19, 2012 at 12:49 PM, Stephen Warren <swarren at nvidia.com> wrote:
>> On 01/12/2012 12:00 PM, Simon Glass wrote:
>>> Some devices can deal with multiple compatible properties. The devices
>>> need to know which nodes to bind to which features. For example an
>>> I2C driver which supports two different controller types will want to
>>> know which type it is dealing with in each case.
>>>
>>> The new fdtdec_add_aliases_for_id() function deals with this by allowing
>>> the driver to search for additional compatible nodes for a different ID.
>>> It can then detect the new ones and perform appropriate processing.
>>>
>>> Another option considered was to return a tuple (node offset, compat id)
>>> and have the function be passed a list of compatible IDs. This is more
>>> overhead for the common case though. We may add such a function later if
>>> more drivers in U-Boot require it.
...
>> I'm not convinced this patch is correct in all cases. It'll probably
>> work fine for simply device-trees though. Consider the following case:
>>
>> 4 device nodes
>> A: compat=i2c alias for usb0
>> B: compat=i2c no alias
>> C: compat=i2c alias for usb2
>> D: compat=i2cx alias for usb1
>>
>> First, we search for all compat=i2c, the list comes back:
>>
>> 0=A
>> 1=B
>> 2=C
>>
>> Then we search for all compat=i2cx, the list comes back:
>>
>> -1
>> -1
>> -1
>> D
>>
>> So D's alias of 1 isn't honored even though if both IDs were searched
>> for together, it would have been.
> 
> Do we really want that behaviour? Overall I think it is a bit odd to
> have devices with no aliases if you actually care about the ordering.
> At least in U-Boot ordering is important. The simple fix above is to
> provide an alias for everything, which is what we do. If we do want
> the behaviour you suggest then I will need the change to doing it in
> one pass.

I think that behaviour is the intended given that DT content. Despite
the fact that content is admittedly a little twisted and perhaps not
commonly useful, since this is user-supplied content in general, I don't
think we can simply not accept it or not implement the fairly obvious
intent.

>> Also, what about the scenario where a driver searches for both
>> nvidia,tegra20-i2c then later nvidia,tegra30-i2c, given that all the DT
>> nodes are probably compatible with both?
> 
> If all DT notes are compatible with both, why are you searching for
> one and then the other? Why not just search for one?

I suppose one /shouldn't/ need to.

I was thinking of supporting the case where somebody only wrote
compatible = "nvidia,tegra30-i2c" and left out the Tegra20 variant.
Still, is arguably a bug, and that won't work in the kernel either since
we're not going around adding all the new compatible values into the
drivers when the devices are compatible with the existing HW anyway.

I guess if we do get some legitimate case that trips this issue, we can
just fix it then, since it's entirely isolated to the code and doesn't
impact anything externally visible etc.

-- 
nvpublic