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

[Jeffrey Hugo <jhugo@codeaurora.org>]

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.