Re: [PATCH] bus: mhi: core: Introduce sysfs ul chan id for mhi chan device

[Hemant Kumar]

Hi Carl,

On 10/27/20 8:06 AM, Jeffrey Hugo wrote:
On 10/27/2020 3:43 AM, carl....@quectel.com wrote:
From: "carl.yin" <carl....@quectel.com>
User space software like ModemManager can identify the function
of the mhi chan device by ul_chan_id.

Signed-off-by: carl.yin <carl....@quectel.com>
---
  Documentation/ABI/stable/sysfs-bus-mhi | 10 ++++++++++
  drivers/bus/mhi/core/init.c            | 15 +++++++++++++++
  2 files changed, 25 insertions(+)

diff --git a/Documentation/ABI/stable/sysfs-bus-mhi 
b/Documentation/ABI/stable/sysfs-bus-mhi
index ecfe766..6d52768 100644
--- a/Documentation/ABI/stable/sysfs-bus-mhi
+++ b/Documentation/ABI/stable/sysfs-bus-mhi
@@ -19,3 +19,13 @@ Description:    The file holds the OEM PK Hash 
value of the endpoint device
          read without having the device power on at least once, the file
          will read all 0's.
  Users:        Any userspace application or clients interested in 
device info.
+
+What:        /sys/bus/mhi/devices/.../ul_chan_id
+Date:        November 2020
+KernelVersion:    5.10
+Contact:    Carl Yin <carl....@quectel.com>
+Description:    The file holds the uplink chan id of the mhi chan 
device.
+        User space software like ModemManager can identify the 
function of
+        the mhi chan device. If the mhi device is not a chan device,
+        eg mhi controller device, the file read -1.
+Users:        Any userspace application or clients interested in 
device info.
diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index c6b43e9..ac4aa5c 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -105,9 +105,24 @@ static ssize_t oem_pk_hash_show(struct device *dev,
  }
  static DEVICE_ATTR_RO(oem_pk_hash);
+static ssize_t ul_chan_id_show(struct device *dev,
+                struct device_attribute *attr,
+                char *buf)
+{
+    struct mhi_device *mhi_dev = to_mhi_device(dev);
+    int ul_chan_id = -1;
+
+    if (mhi_dev->ul_chan)
+        ul_chan_id = mhi_dev->ul_chan_id;
+
+    return snprintf(buf, PAGE_SIZE, "%d\n", ul_chan_id);
+}
+static DEVICE_ATTR_RO(ul_chan_id);
+
  static struct attribute *mhi_dev_attrs[] = {
      &dev_attr_serial_number.attr,
      &dev_attr_oem_pk_hash.attr,
+    &dev_attr_ul_chan_id.attr,
      NULL,
  };
  ATTRIBUTE_GROUPS(mhi_dev);


NACK

Channel ID is a device specific detail.  Userspace should be basing 
decisions on the channel name.

I agree with Jeff, why do you need to know the channel id, if you need 
to poll for any device node to get created you can try to open the 
device node from user space and wait until the device gets opened.
Are you trying to wait for EDL channels to get started using UCI ?

Thanks,
Hemant
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project