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

[Jeffrey Hugo <jhugo@codeaurora.org>]

On 11/25/2020 9:15 AM, Loic Poulain wrote:
> On Wed, 25 Nov 2020 at 16:42, Jeffrey Hugo <jhugo@codeaurora.org> 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/ ?
> 
> That does change sysfs device dir names:
> $ ls -al /sys/bus/mhi/devices/
> lrwxrwxrwx 1 root root 0 nov.  25 16:27 mhi0 ->
> ../../../devices/pci0000:00/0000:00:01.2/0000:02:00.0/mhi0
> lrwxrwxrwx 1 root root 0 nov.  25 16:28 mhi0_DIAG ->
> ../../../devices/pci0000:00/0000:00:01.2/0000:02:00.0/mhi0/mhi0_DIAG
> lrwxrwxrwx 1 root root 0 nov.  25 16:28 mhi0_IPCR ->
> ../../../devices/pci0000:00/0000:00:01.2/0000:02:00.0/mhi0/mhi0_IPCR
> 
>> 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).
> 
> I see your point but it's not a problem specific to MHI bus, user can
> rely on sysfs/uevent to get device information such ass devices
> attributes, children, or parent devices. Do you have a concrete
> example in mind for your case?

3 I think -

1. I think its same to assume these names are going to trickle down to 
mhi_uci.  For a QAIC device, there is a /dev/qaic chardev for doing the 
AI work, and a /dev chardev from mhi_uci exposing the DIAG channel. 
Both of these are correlated to a specific PCI device by the BDF, which 
the user application will want to know so that relevant DIAG traffic for 
a qaic device is routed to the correct chardev, and thus the correct 
device.  For 1 device, this is simple.  For 12, some correlation needs 
to be done programmatically.

2. User is physically on the shell, and wants to look at 
/sys/bus/mhi/devices to see the state of a device.  Perhaps the expected 
mhi_uci devices are not present.

3. User is physically on the shell, and wants to invoke soc_reset on the 
controller.  The user needs to know which mhi controller instance 
(mhi1?) maps to a specific PCI BDF to make sure the right device is reset.

1 is used a lot by our userspace stack.  2/3 is used a lot by testers 
and developers.

> 
>>
>> Also, do we actually have some device that actually exposes multiple MHI
>> interfaces?
> 
> No, but better to fix possible problems ahead of time.

Good to know.  I was thinking this was not needed, and unlikely to be 
needed, but it just occurs to me I might have a usecase for this in 
development.

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