Re: [PATCH] bus: mhi: core: Indexed MHI controller name

[Jeffrey Hugo <jhugo@codeaurora.org>]

On 11/25/2020 9:23 AM, Loic Poulain wrote:
> On Wed, 25 Nov 2020 at 16:49, Jeffrey Hugo <jhugo@codeaurora.org> wrote:
>>
>> On 11/25/2020 8:42 AM, Jeffrey Hugo wrote:
>>> On 11/25/2020 8:43 AM, Loic Poulain wrote:
>>>> Today the MHI controller name is simply cloned from the underlying
>>>> bus device (its parent), that gives the following device structure
>>>> for e.g. a MHI/PCI controller:
>>>> devices/pci0000:00/0000:00:01.2/0000:02:00.0/0000:02:00.0
>>>> devices/pci0000:00/0000:00:01.2/0000:02:00.0/0000:02:00.0/0000:02:00.0_IPCR
>>>>
>>>> ...
>>>>
>>>> That's quite misleading/confusing and can cause device registering
>>>> issues because of duplicate dev name (e.g. if a PCI device register
>>>> two different MHI instances).
>>>>
>>>> This patch changes MHI core to create indexed mhi controller names
>>>> (mhi0, mhi1...) in the same way as other busses (i2c0, usb0...).
>>>>
>>>> The previous example becomes:
>>>> devices/pci0000:00/0000:00:01.2/0000:02:00.0/mhi0
>>>> devices/pci0000:00/0000:00:01.2/0000:02:00.0/mhi0/mhi0_IPCR
>>>> ...
>>>>
>>>> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
>>>
>>>
>>> How does this change /sys/bus/mhi/devices/ ?
>>>
>>> The point of having the bus name in the mhi device name is to give an
>>> easy way to correlate those devices back to the "root" device (I have a
>>> lot of users which do that).
>>>
>>> Also, do we actually have some device that actually exposes multiple MHI
>>> interfaces?
>>>
>>>
>>
>> Looking at the code change itself, looks like the controller index is
>> essentially random, and not persistent.  Is that expected?
>>
>> I'm thinking it might be confusing if you have say 12 MHI controllers
>> from 12 different devices, and some of those devices crash at roughly
>> the same time.  The controllers get removed, and re-initialized, which
>> means that you have essentially a race condition where the controller
>> for the same device now has a different index.
> 
> There is no race since ID is atomically allocated, but yes your
> controller can get a different index on unregister/register, like e.g
> network interfaces index, usb devices ID, video devices, etc... can be
> enumerated in various order. Not sure why the user should take care of
> the MHI controller index.

You said there is no race, but then literally described a race.

It appears we agree, the enumeration first time around may not be the 
same enumeration order later on.

The point is not that the user cares that they always talk to mhi0.  The 
point is that the user was talking to mhi0, that crashed, re-enumerated, 
and is now mhi3.

-- 
Jeffrey Hugo
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.