linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] bus: mhi: core: Introduce sysfs ul chan id for mhi chan device
@ 2020-10-27  9:43 carl.yin
  2020-10-27 15:06 ` Jeffrey Hugo
  0 siblings, 1 reply; 6+ messages in thread
From: carl.yin @ 2020-10-27  9:43 UTC (permalink / raw)
  To: manivannan.sadhasivam, hemantk, sfr
  Cc: linux-arm-msm, linux-kernel, carl.yin, naveen.kumar

From: "carl.yin" <carl.yin@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.yin@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.yin@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);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] bus: mhi: core: Introduce sysfs ul chan id for mhi chan device
  2020-10-27  9:43 [PATCH] bus: mhi: core: Introduce sysfs ul chan id for mhi chan device carl.yin
@ 2020-10-27 15:06 ` Jeffrey Hugo
  2020-10-27 22:43   ` Hemant Kumar
  0 siblings, 1 reply; 6+ messages in thread
From: Jeffrey Hugo @ 2020-10-27 15:06 UTC (permalink / raw)
  To: carl.yin, manivannan.sadhasivam, hemantk, sfr
  Cc: linux-arm-msm, linux-kernel, naveen.kumar

On 10/27/2020 3:43 AM, carl.yin@quectel.com wrote:
> From: "carl.yin" <carl.yin@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.yin@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.yin@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.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] bus: mhi: core: Introduce sysfs ul chan id for mhi chan device
  2020-10-27 15:06 ` Jeffrey Hugo
@ 2020-10-27 22:43   ` Hemant Kumar
  2020-10-28  1:18     ` 答复: " Carl Yin(殷张成)
  0 siblings, 1 reply; 6+ messages in thread
From: Hemant Kumar @ 2020-10-27 22:43 UTC (permalink / raw)
  To: Jeffrey Hugo, carl.yin, manivannan.sadhasivam, sfr
  Cc: linux-arm-msm, linux-kernel, naveen.kumar

Hi Carl,

On 10/27/20 8:06 AM, Jeffrey Hugo wrote:
> On 10/27/2020 3:43 AM, carl.yin@quectel.com wrote:
>> From: "carl.yin" <carl.yin@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.yin@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.yin@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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* 答复: [PATCH] bus: mhi: core: Introduce sysfs ul chan id for mhi chan device
  2020-10-27 22:43   ` Hemant Kumar
@ 2020-10-28  1:18     ` Carl Yin(殷张成)
  2020-10-28 14:17       ` Jeffrey Hugo
  0 siblings, 1 reply; 6+ messages in thread
From: Carl Yin(殷张成) @ 2020-10-28  1:18 UTC (permalink / raw)
  To: Hemant Kumar, Jeffrey Hugo, manivannan.sadhasivam, sfr
  Cc: linux-arm-msm, linux-kernel, Naveen Kumar

Hi Jeffery and Hemant:

On Wednesday, October 28, 2020 6:44 AM, hemantk wrote:
> On 10/27/20 8:06 AM, Jeffrey Hugo wrote:
> Hi Carl,
> 
> On 10/27/20 8:06 AM, Jeffrey Hugo wrote:
> > On 10/27/2020 3:43 AM, carl.yin@quectel.com wrote:
> >> From: "carl.yin" <carl.yin@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.yin@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.yin@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 ?
[carl.yin] In my opinion, mhi chan id is something like 'bInterfaceNumber' of USB device.
A USB device and several USB interfaces, and a mhi devices have 128 mhi chans.
Chan id is a physical attribute of one mhi chan.

Next is the udev info of one mhi chan:
# udevadm info -a /dev/mhi_0000\:03\:00.0_EDL
  looking at parent device '/devices/pci0000:00/0000:00:1d.0/0000:03:00.0/0000:03:00.0_EDL':
    KERNELS=="0000:03:00.0_EDL"
    SUBSYSTEMS=="mhi"
    DRIVERS=="mhi_uci"
    ATTRS{serial_number}=="Serial Number: 2644481182"
    ATTRS{ul_chan_id}=="34"

If no ul_chan_id, the udev ruler will be ' KERNEL=="*_EDL" '
With ul_chan_id, the udev ruler will be ' ATTRS{ul_chan_id}=="34"'
I think the latter is more better.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: 答复: [PATCH] bus: mhi: core: Introduce sysfs ul chan id for mhi chan device
  2020-10-28  1:18     ` 答复: " Carl Yin(殷张成)
@ 2020-10-28 14:17       ` Jeffrey Hugo
  2020-10-29  1:33         ` 答复: " Carl Yin(殷张成)
  0 siblings, 1 reply; 6+ messages in thread
From: Jeffrey Hugo @ 2020-10-28 14:17 UTC (permalink / raw)
  To: Carl Yin(殷张成),
	Hemant Kumar, manivannan.sadhasivam, sfr
  Cc: linux-arm-msm, linux-kernel, Naveen Kumar

On 10/27/2020 7:18 PM, Carl Yin(殷张成) wrote:
> Hi Jeffery and Hemant:
> 
> On Wednesday, October 28, 2020 6:44 AM, hemantk wrote:
>> On 10/27/20 8:06 AM, Jeffrey Hugo wrote:
>> Hi Carl,
>>
>> On 10/27/20 8:06 AM, Jeffrey Hugo wrote:
>>> On 10/27/2020 3:43 AM, carl.yin@quectel.com wrote:
>>>> From: "carl.yin" <carl.yin@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.yin@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.yin@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 ?
> [carl.yin] In my opinion, mhi chan id is something like 'bInterfaceNumber' of USB device.
> A USB device and several USB interfaces, and a mhi devices have 128 mhi chans.
> Chan id is a physical attribute of one mhi chan.
> 
> Next is the udev info of one mhi chan:
> # udevadm info -a /dev/mhi_0000\:03\:00.0_EDL
>    looking at parent device '/devices/pci0000:00/0000:00:1d.0/0000:03:00.0/0000:03:00.0_EDL':
>      KERNELS=="0000:03:00.0_EDL"
>      SUBSYSTEMS=="mhi"
>      DRIVERS=="mhi_uci"
>      ATTRS{serial_number}=="Serial Number: 2644481182"
>      ATTRS{ul_chan_id}=="34"
> 
> If no ul_chan_id, the udev ruler will be ' KERNEL=="*_EDL" '

I have several usecases where this works just fine today.

> With ul_chan_id, the udev ruler will be ' ATTRS{ul_chan_id}=="34"'

This breaks when there is some new device that has the EDL channel on 
some different chan_id, like 7.  The above does not.  Additionally if 
there is a different device that is using chan_id 34 for a different 
purpose, say Diag, then your udev rule also breaks.

The name of the channel is the interface to the channel.  Not the 
chan_id.  This holds true within the kernel, and should be the same for 
userspace.  I still oppose this change.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* 答复: 答复: [PATCH] bus: mhi: core: Introduce sysfs ul chan id for mhi chan device
  2020-10-28 14:17       ` Jeffrey Hugo
@ 2020-10-29  1:33         ` Carl Yin(殷张成)
  0 siblings, 0 replies; 6+ messages in thread
From: Carl Yin(殷张成) @ 2020-10-29  1:33 UTC (permalink / raw)
  To: Jeffrey Hugo, Hemant Kumar, manivannan.sadhasivam, sfr
  Cc: linux-arm-msm, linux-kernel, Naveen Kumar

Hi Jeffrey:

On October 28, 2020 10:18 PM, jhugo wrote:
> On 10/27/2020 7:18 PM, Carl Yin(殷张成) wrote:
> > Hi Jeffery and Hemant:
> >
> > On Wednesday, October 28, 2020 6:44 AM, hemantk wrote:
> >> On 10/27/20 8:06 AM, Jeffrey Hugo wrote:
> >> Hi Carl,
> >>
> >> On 10/27/20 8:06 AM, Jeffrey Hugo wrote:
> >>> On 10/27/2020 3:43 AM, carl.yin@quectel.com wrote:
> >>>> From: "carl.yin" <carl.yin@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.yin@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.yin@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 ?
> > [carl.yin] In my opinion, mhi chan id is something like 'bInterfaceNumber' of
> USB device.
> > A USB device and several USB interfaces, and a mhi devices have 128 mhi
> chans.
> > Chan id is a physical attribute of one mhi chan.
> >
> > Next is the udev info of one mhi chan:
> > # udevadm info -a /dev/mhi_0000\:03\:00.0_EDL
> >    looking at parent device
> '/devices/pci0000:00/0000:00:1d.0/0000:03:00.0/0000:03:00.0_EDL':
> >      KERNELS=="0000:03:00.0_EDL"
> >      SUBSYSTEMS=="mhi"
> >      DRIVERS=="mhi_uci"
> >      ATTRS{serial_number}=="Serial Number: 2644481182"
> >      ATTRS{ul_chan_id}=="34"
> >
> > If no ul_chan_id, the udev ruler will be ' KERNEL=="*_EDL" '
> 
> I have several usecases where this works just fine today.
> 
> > With ul_chan_id, the udev ruler will be ' ATTRS{ul_chan_id}=="34"'
> 
> This breaks when there is some new device that has the EDL channel on some
> different chan_id, like 7.  The above does not.  Additionally if there is a
> different device that is using chan_id 34 for a different purpose, say Diag, then
> your udev rule also breaks.
> 
> The name of the channel is the interface to the channel.  Not the chan_id.  This
> holds true within the kernel, and should be the same for userspace.  I still
> oppose this change.
[carl.yin] ok, I agree that chan id is not a good and necessary choice.
	ModemManager also work well with ' KERNEL=="mhi_*_<chan name>"'.
> 
> --
> Jeffrey Hugo
> Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux
> Foundation Collaborative Project.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-10-29  1:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-27  9:43 [PATCH] bus: mhi: core: Introduce sysfs ul chan id for mhi chan device carl.yin
2020-10-27 15:06 ` Jeffrey Hugo
2020-10-27 22:43   ` Hemant Kumar
2020-10-28  1:18     ` 答复: " Carl Yin(殷张成)
2020-10-28 14:17       ` Jeffrey Hugo
2020-10-29  1:33         ` 答复: " Carl Yin(殷张成)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).