[U-Boot] [RFC PATCH] dm: ensure device names are unique

[Simon Glass]

Hi Stephen,

On 29 April 2016 at 10:30, Stephen Warren <swarren at wwwdotorg.org> wrote:
> On 04/29/2016 10:28 AM, Simon Glass wrote:
>>
>> Hi Stephen,
>>
>> On 29 April 2016 at 10:23, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>>
>>> On 04/29/2016 07:23 AM, Simon Glass wrote:
>>>>
>>>>
>>>> Hi Stephen,
>>>>
>>>> On 28 April 2016 at 09:55, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>>>>
>>>>>
>>>>> On 04/27/2016 10:50 PM, Simon Glass wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> Hi Stephen,
>>>>>>
>>>>>> On 26 April 2016 at 15:30, Stephen Warren wrote:
>>>>>>    > It is possible for HW to contain multiple instances of the same
>>>>>> device.
>>>>>> In
>>>>>>    > this case, the name passed to device_bind() may not be unique
>>>>>> across
>>>>>> all
>>>>>>    > devices within its uclass. One example is a system with multiple
>>>>>> identical
>>>>>>    > PCI Ethernet devices. Another might be a system with multiple
>>>>>> identical
>>>>>>    > I2C GPIO expanders, each connected to a separate I2C bus, yet
>>>>>> using
>>>>>> the
>>>>>>    > same I2C address on that bus and hence having the same DT node
>>>>>> name.
>>>>>>    >
>>>>>>    > Enhance the code to detect this situation, and append a sequence
>>>>>> number so
>>>>>>    > the device name to ensure uniqueness.
>>>>>>    >
>>>>>>    > Signed-off-by: Stephen Warren <swarren at nvidia.com
>>>>>> <swarren at nvidia.com>>
>>>>>>
>>>>>> I would rather that the caller handles this. But failing this perhaps
>>>>>> a
>>>>>> new function that does it? Is this for the Ethernet use case?
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Wouldn't all callers of this function simply call the new function? I'm
>>>>> not
>>>>> aware of any case where the code to avoid duplicate names would not be
>>>>> desired.
>>>>>
>>>>> I hit this for the Ethernet case, but I believe it applies to any type
>>>>> of
>>>>> device at all; see another possible trigger case in the commit
>>>>> description.
>>>>
>>>>
>>>>
>>>> This does not happen with devices from the device tree. It only
>>>> happens with auto-probed devices. Your I2C GPIO example is odd but I'd
>>>> rather solve that by using the device tree node name.
>>>
>>>
>>>
>>> DT itself imposes no such rule; node names must be unique only within
>>> their
>>> parent node but there's no restriction on identical node names appearing
>>> in
>>> different parts of the tree.
>>>
>>> If U-Boot imposes that rule on DT, then there's no way in general that we
>>> can guarantee U-Boot will be able to use standard DTs (i.e. identical to
>>> those used by Linux or any other OS) for any platform; it'd be another
>>> change someone would need to make to transform a DT to be "U-Boot
>>> compatible", which rather reduces a potential benefit of DT for U-Boot;
>>> being able to just drop a DT in and have it work.
>>
>>
>> U-Boot does not impose a rule. If you want duplicate device names you
>> can have them. I think it is bad practice though.
>>
>>>
>>> It would be possible for U-Boot to decouple its internal device name from
>>> the DT node name. In which case, your statement would work. However, I
>>> don't
>>> think that's the case at the moment, and in fact it's effectively what
>>> this
>>> patch is doing, although admittedly there are other ways of doing this.
>>
>>
>> Anyway I believe my point stands. Whereas users can edit the device
>> tree and avoid conflicts they cannot do this with auto-probed devices.
>> So for that case we should have a way of allocating a name before
>> calling device_bind().
>
>
> OK. I'm just going to solve this by unplugging the second Ethernet card, or
> holding this patch locally for the case when I need to test the PCIe port.
>

Binding a device needs to be as cheap as possible. 99% of the time the
name is fine, and we know the cases where it might not be. This is
similar to the DM_UC_FLAG_SEQ_ALIAS - expensive functionality should
be separated out so we don't require everyone to use it. It just
creates bloat.

So please when contributing things to driver model (which is most
welcome and appreciated) be a little more flexible. It should not take
this much energy to explain the design principles and get a change to
a patch.

Also see u-boot-dm/blkb-working which includes the allocation of
device names. I need to resolve the comments and respin...

Regards,
Simon