Re: [PATCH v6 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface

[Sumit Kumar <sumit.kumar@oss.qualcomm.com>]

On 9/26/2026 4:36 PM, Manivannan Sadhasivam wrote:
> On Tue, Sep 08, 2026 at 02:57:22PM +0530, Sumit Kumar wrote:
>> The MHI specification defines a LOOPBACK channel. The endpoint firmware
>> echoes back whatever the host sends on this channel. Without a host-side
>> driver, there is no way to exercise this channel to validate MHI data path
>> integrity between host and endpoint.
>>
>> Add a host-side loopback driver that binds to the LOOPBACK channel and
>> expose a sysfs interface for data path testing. The sysfs interface allows
>> users to configure TRE buffer size and count, trigger a loopback test, and
>> read the result.
>>
>> The new sysfs ABI is documented under Documentation/ABI/testing/, not
>> stable/, since this is a new interface without established API
>> guarantees yet. It is unrelated to the existing stable MHI sysfs ABI
>> documented in Documentation/ABI/stable/sysfs-bus-mhi, despite both
>> being covered by the same MAINTAINERS entry.
>>
>> Co-developed-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> Signed-off-by: Sumit Kumar <sumit.kumar@oss.qualcomm.com>
>> ---
>>   .../ABI/testing/sysfs-bus-mhi-devices-loopback     |  40 +++
>>   MAINTAINERS                                        |   1 +
>>   drivers/bus/mhi/host/Kconfig                       |   1 +
>>   drivers/bus/mhi/host/Makefile                      |   1 +
>>   drivers/bus/mhi/host/clients/Kconfig               |  17 ++
>>   drivers/bus/mhi/host/clients/Makefile              |   2 +
>>   drivers/bus/mhi/host/clients/loopback.c            | 287 +++++++++++++++++++++
>>   7 files changed, 349 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-mhi-devices-loopback b/Documentation/ABI/testing/sysfs-bus-mhi-devices-loopback
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..9e47e8443f309199691a50d70adadf84f3237037
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-mhi-devices-loopback
>> @@ -0,0 +1,40 @@
>> +What:		/sys/bus/mhi/devices/mhi<N>_LOOPBACK/tre_size
>> +Date:		August 2026
>> +KernelVersion:	7.4
>> +Contact:	mhi@lists.linux.dev
>> +Description:
>> +		(RW) Size of each Transfer Ring Element (TRE) buffer in bytes
>> +		used for the loopback test. Valid range is 1 to the value
>> +		reported by max_tre_size. Default value is 32 bytes.
>> +
>> +What:		/sys/bus/mhi/devices/mhi<N>_LOOPBACK/max_tre_size
>> +Date:		August 2026
>> +KernelVersion:	7.4
>> +Contact:	mhi@lists.linux.dev
>> +Description:
>> +		(RO) Maximum allowed Transfer Ring Element (TRE) size in bytes.
>> +		Reading this file returns the upper bound for the tre_size
>> +		attribute.
>> +
>> +What:		/sys/bus/mhi/devices/mhi<N>_LOOPBACK/num_tre
>> +Date:		August 2026
>> +KernelVersion:	7.4
>> +Contact:	mhi@lists.linux.dev
>> +Description:
>> +		(RW) Number of Transfer Ring Elements (TREs) to use per
>> +		loopback test. Must be greater than zero and must not exceed
>> +		the channel ring capacity. Default value is 1.
> How does the user know 'channel ring capacity'?
If the user enters a number greater than the ring capacity the error 
will log the max num_tre value.
Would you prefer having a new sysfs entry similar to 'max_tre_size'? 
something like 'max_num_tre'.
>
>> +
>> +What:		/sys/bus/mhi/devices/mhi<N>_LOOPBACK/start
>> +Date:		August 2026
>> +KernelVersion:	7.4
>> +Contact:	mhi@lists.linux.dev
>> +Description:
>> +		(WO) Write any value to trigger a loopback test. The driver
>> +		sends random data to the endpoint using the configured tre_size
>> +		and num_tre parameters, waits for the endpoint to echo it back,
>> +		and verifies the received data matches what was sent.
>> +
>> +		This is a blocking write that returns when the test completes
>> +		or times out after 5 seconds. The write returns an error code
>> +		if the test fails or times out.
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 70663089f071c4eaa07fe7b9baf9003d8b981c07..b10f8cd592954e1450e5851045962bf05dcf1a50 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -17628,6 +17628,7 @@ L:	linux-arm-msm@vger.kernel.org
>>   S:	Maintained
>>   T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mani/mhi.git
>>   F:	Documentation/ABI/stable/sysfs-bus-mhi
>> +F:	Documentation/ABI/testing/sysfs-bus-mhi-devices-loopback
>>   F:	Documentation/mhi/
>>   F:	drivers/bus/mhi/
>>   F:	drivers/pci/endpoint/functions/pci-epf-mhi.c
>> diff --git a/drivers/bus/mhi/host/Kconfig b/drivers/bus/mhi/host/Kconfig
>> index da5cd0c9fc620ab595e742c422f1a22a2a84c7b9..627c57948235aa52348179ae8b2d0826ebaed01e 100644
>> --- a/drivers/bus/mhi/host/Kconfig
>> +++ b/drivers/bus/mhi/host/Kconfig
>> @@ -29,3 +29,4 @@ config MHI_BUS_PCI_GENERIC
>>   	  This driver provides MHI PCI controller driver for devices such as
>>   	  Qualcomm SDX55 based PCIe modems.
>>   
>> +source "drivers/bus/mhi/host/clients/Kconfig"
>> diff --git a/drivers/bus/mhi/host/Makefile b/drivers/bus/mhi/host/Makefile
>> index 859c2f38451c669b3d3014c374b2b957c99a1cfe..2a16008aeb38127494782bbff4e1656428d2b776 100644
>> --- a/drivers/bus/mhi/host/Makefile
>> +++ b/drivers/bus/mhi/host/Makefile
>> @@ -4,3 +4,4 @@ mhi-$(CONFIG_MHI_BUS_DEBUG) += debugfs.o
>>   
>>   obj-$(CONFIG_MHI_BUS_PCI_GENERIC) += mhi_pci_generic.o
>>   mhi_pci_generic-y += pci_generic.o
>> +obj-y += clients/
>> diff --git a/drivers/bus/mhi/host/clients/Kconfig b/drivers/bus/mhi/host/clients/Kconfig
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..8bb5715e61d0d6e64a51012b4e1594ba46e5bfc1
>> --- /dev/null
>> +++ b/drivers/bus/mhi/host/clients/Kconfig
>> @@ -0,0 +1,17 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +
>> +config MHI_BUS_LOOPBACK
>> +	tristate "MHI LOOPBACK client driver"
>> +	depends on MHI_BUS
>> +	help
>> +	  MHI LOOPBACK client driver that binds to the MHI LOOPBACK channel
>> +	  as defined in the MHI specification. The LOOPBACK channel is
>> +	  implemented by MHI-based devices (e.g. modems, WLAN) in the field,
>> +	  where the endpoint firmware echoes back whatever the host sends.
>> +
>> +	  This driver exposes a sysfs interface for testing MHI data path
>> +	  integrity between host and endpoint. Users can configure the TRE
>> +	  size and count, and trigger a loopback test.
>> +
>> +	  To compile this driver as a module, choose M here. The module
>> +	  will be called mhi_loopback.
>> diff --git a/drivers/bus/mhi/host/clients/Makefile b/drivers/bus/mhi/host/clients/Makefile
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..3811b6928f42b38f94b1167941cf3b0fe512d32b
>> --- /dev/null
>> +++ b/drivers/bus/mhi/host/clients/Makefile
>> @@ -0,0 +1,2 @@
>> +obj-$(CONFIG_MHI_BUS_LOOPBACK) += mhi_loopback.o
>> +mhi_loopback-y += loopback.o
>> diff --git a/drivers/bus/mhi/host/clients/loopback.c b/drivers/bus/mhi/host/clients/loopback.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..058a8b587933245230eaae61bb7fe3361d4a362a
>> --- /dev/null
>> +++ b/drivers/bus/mhi/host/clients/loopback.c
>> @@ -0,0 +1,287 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +
>> +#include <linux/atomic.h>
>> +#include <linux/cleanup.h>
>> +#include <linux/completion.h>
>> +#include <linux/errno.h>
>> +#include <linux/mhi.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/random.h>
>> +#include <linux/sizes.h>
>> +#include <linux/slab.h>
>> +#include <linux/string.h>
>> +#include <linux/sysfs.h>
>> +#include <linux/types.h>
>> +
>> +#define MHI_LOOPBACK_DEFAULT_TRE_SIZE	32
>> +#define MHI_LOOPBACK_DEFAULT_NUM_TRE	1
>> +#define MHI_LOOPBACK_TIMEOUT_MS		5000
>> +#define MHI_LOOPBACK_MAX_TRE_SIZE	(SZ_64K - 1)
>> +
>> +struct mhi_loopback {
>> +	struct mhi_device *mdev;
>> +	/* Serializes the sysfs attributes against a running test */
>> +	struct mutex lb_mutex;
>> +	struct completion comp;
>> +	/* Tracks outstanding DL+UL completions; comp fires when it hits zero */
> This variable is only used in dl_callback.
>
>> +	atomic_t tre_pending;
>> +	u32 num_tre;
>> +	u32 tre_size;
>> +};
>> +
>> +static ssize_t tre_size_show(struct device *dev,
>> +			     struct device_attribute *attr, char *buf)
>> +{
>> +	struct mhi_loopback *loopback = dev_get_drvdata(dev);
>> +
>> +	return sysfs_emit(buf, "%u\n", loopback->tre_size);
>> +}
>> +
>> +static ssize_t tre_size_store(struct device *dev,
>> +			      struct device_attribute *attr,
>> +			      const char *buf, size_t count)
>> +{
>> +	struct mhi_loopback *loopback = dev_get_drvdata(dev);
>> +	u32 val;
>> +
>> +	if (kstrtou32(buf, 0, &val))
>> +		return -EINVAL;
>> +
>> +	if (val == 0 || val > MHI_LOOPBACK_MAX_TRE_SIZE)
>> +		return -EINVAL;
>> +
>> +	guard(mutex)(&loopback->lb_mutex);
>> +	loopback->tre_size = val;
>> +
>> +	return count;
>> +}
>> +static DEVICE_ATTR_RW(tre_size);
>> +
>> +static ssize_t max_tre_size_show(struct device *dev,
>> +				 struct device_attribute *attr, char *buf)
>> +{
>> +	return sysfs_emit(buf, "%u\n", MHI_LOOPBACK_MAX_TRE_SIZE);
>> +}
>> +static DEVICE_ATTR_RO(max_tre_size);
>> +
>> +static ssize_t num_tre_show(struct device *dev,
>> +			    struct device_attribute *attr, char *buf)
>> +{
>> +	struct mhi_loopback *loopback = dev_get_drvdata(dev);
>> +
>> +	return sysfs_emit(buf, "%u\n", loopback->num_tre);
>> +}
>> +
>> +static ssize_t num_tre_store(struct device *dev,
>> +			     struct device_attribute *attr,
>> +			     const char *buf, size_t count)
>> +{
>> +	struct mhi_loopback *loopback = dev_get_drvdata(dev);
>> +	u32 val;
>> +	int el_num;
>> +
>> +	if (kstrtou32(buf, 0, &val))
>> +		return -EINVAL;
>> +
>> +	if (val == 0)
>> +		return -EINVAL;
>> +
>> +	guard(mutex)(&loopback->lb_mutex);
>> +
>> +	el_num = min(mhi_get_free_desc_count(loopback->mdev, DMA_TO_DEVICE),
>> +		     mhi_get_free_desc_count(loopback->mdev, DMA_FROM_DEVICE));
>> +	if (val > el_num) {
>> +		dev_err(dev, "num_tre (%u) exceeds ring capacity (%d)\n", val, el_num);
>> +		return -EINVAL;
>> +	}
>> +
>> +	loopback->num_tre = val;
>> +
>> +	return count;
>> +}
>> +static DEVICE_ATTR_RW(num_tre);
>> +
>> +static ssize_t start_store(struct device *dev,
>> +			   struct device_attribute *attr,
>> +			   const char *buf, size_t count)
>> +{
>> +	struct mhi_loopback *loopback = dev_get_drvdata(dev);
>> +	u32 total_size, tre_count, tre_size;
>> +	int i, ret;
>> +
>> +	guard(mutex)(&loopback->lb_mutex);
>> +
>> +	tre_size = loopback->tre_size;
>> +	tre_count = loopback->num_tre;
>> +	total_size = size_mul(tre_count, tre_size);
>> +
>> +	if (total_size > KMALLOC_MAX_SIZE)
>> +		return -EINVAL;
>> +
>> +	if (tre_count > mhi_get_free_desc_count(loopback->mdev, DMA_TO_DEVICE) ||
>> +	    tre_count > mhi_get_free_desc_count(loopback->mdev, DMA_FROM_DEVICE)) {
>> +		dev_err(dev, "Not enough ring space for %u TREs\n", tre_count);
>> +		return -ENOSPC;
>> +	}
>> +
>> +	void *recv_buf __free(kfree) = kzalloc(total_size, GFP_KERNEL);
>> +	if (!recv_buf)
>> +		return -ENOMEM;
>> +
>> +	void *send_buf __free(kfree) = kzalloc(total_size, GFP_KERNEL);
>> +	if (!send_buf)
>> +		return -ENOMEM;
>> +
>> +	get_random_bytes(send_buf, total_size);
>> +
>> +	atomic_set(&loopback->tre_pending, tre_count);
>> +	reinit_completion(&loopback->comp);
>> +
>> +	for (i = 0; i < tre_count; i++) {
>> +		ret = mhi_queue_buf(loopback->mdev, DMA_FROM_DEVICE,
>> +				    recv_buf + (i * tre_size), tre_size, MHI_EOT);
>> +		if (ret) {
>> +			dev_err(dev, "Unable to queue read TRE %d: %d\n", i, ret);
>> +			if (atomic_sub_and_test(tre_count - i, &loopback->tre_pending))
>> +				complete(&loopback->comp);
>> +			return ret;
> Both recv_buf and send_buf are allocated with '__free' cleanup. So these buffers
> will get freed once this function exits. But these buffer might've already
> queued to the device in past iterations, making these pointers dangling.
>
> Maybe you should reset the channel as you've done below for completion timeout?
will create a reset channel function and will call that in both paths.
>
>> +		}
>> +	}
>> +
>> +	for (i = 0; i < tre_count - 1; i++) {
>> +		ret = mhi_queue_buf(loopback->mdev, DMA_TO_DEVICE,
>> +				    send_buf + (i * tre_size), tre_size, MHI_CHAIN);
>> +		if (ret) {
>> +			dev_err(dev, "Unable to queue send TRE %d: %d\n", i, ret);
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	ret = mhi_queue_buf(loopback->mdev, DMA_TO_DEVICE,
>> +			    send_buf + (i * tre_size), tre_size, MHI_EOT);
>> +	if (ret) {
>> +		dev_err(dev, "Unable to queue final TRE: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	if (!wait_for_completion_timeout(&loopback->comp,
>> +					 msecs_to_jiffies(MHI_LOOPBACK_TIMEOUT_MS))) {
>> +		dev_err(dev, "Loopback test timed out\n");
>> +		/* Reset the channel to reclaim the TREs still pointing at the buffers */
>> +		mhi_unprepare_from_transfer(loopback->mdev);
>> +		ret = mhi_prepare_for_transfer(loopback->mdev);
>> +		if (ret)
>> +			dev_err(dev, "Failed to re-prepare channel for transfers: %d\n", ret);
>> +
>> +		return -ETIMEDOUT;
>> +	}
>> +
>> +	if (memcmp(send_buf, recv_buf, total_size)) {
>> +		dev_err(dev, "Loopback data mismatch\n");
>> +		return -EIO;
>> +	}
>> +
>> +	return count;
>> +}
>> +static DEVICE_ATTR_WO(start);
>> +
>> +static void mhi_loopback_dl_callback(struct mhi_device *mhi_dev,
>> +				     struct mhi_result *mhi_res)
>> +{
>> +	struct mhi_loopback *loopback = dev_get_drvdata(&mhi_dev->dev);
>> +
>> +	if (mhi_res->transaction_status && mhi_res->transaction_status != -ENOTCONN)
>> +		dev_err(&mhi_dev->dev, "DL callback error: status %d\n",
>> +			mhi_res->transaction_status);
>> +
>> +	if (atomic_dec_and_test(&loopback->tre_pending))
>> +		complete(&loopback->comp);
>> +}
>> +
>> +static void mhi_loopback_ul_callback(struct mhi_device *mhi_dev,
>> +				     struct mhi_result *mhi_res)
>> +{
>> +	if (mhi_res->transaction_status && mhi_res->transaction_status != -ENOTCONN)
>> +		dev_err(&mhi_dev->dev, "UL callback error: status %d\n",
>> +			mhi_res->transaction_status);
>> +}
>> +
>> +static struct attribute *mhi_loopback_attrs[] = {
>> +	&dev_attr_tre_size.attr,
>> +	&dev_attr_max_tre_size.attr,
>> +	&dev_attr_num_tre.attr,
>> +	&dev_attr_start.attr,
>> +	NULL
>> +};
>> +
>> +static const struct attribute_group mhi_loopback_group = {
>> +	.attrs = mhi_loopback_attrs,
>> +};
>> +
>> +static int mhi_loopback_probe(struct mhi_device *mhi_dev,
>> +			      const struct mhi_device_id *id)
>> +{
>> +	struct mhi_loopback *loopback;
>> +	int ret;
>> +
>> +	loopback = devm_kzalloc(&mhi_dev->dev, sizeof(*loopback), GFP_KERNEL);
>> +	if (!loopback)
>> +		return -ENOMEM;
>> +
>> +	loopback->mdev = mhi_dev;
>> +	loopback->tre_size = MHI_LOOPBACK_DEFAULT_TRE_SIZE;
>> +	loopback->num_tre = MHI_LOOPBACK_DEFAULT_NUM_TRE;
>> +
>> +	mutex_init(&loopback->lb_mutex);
>> +	init_completion(&loopback->comp);
>> +
>> +	dev_set_drvdata(&mhi_dev->dev, loopback);
>> +
>> +	ret = mhi_prepare_for_transfer(mhi_dev);
>> +	if (ret)
>> +		return dev_err_probe(&mhi_dev->dev, ret, "Failed to prepare for transfers\n");
>> +
>> +	ret = sysfs_create_group(&mhi_dev->dev.kobj, &mhi_loopback_group);
>> +	if (ret) {
>> +		dev_err(&mhi_dev->dev, "Failed to create sysfs attributes: %d\n", ret);
>> +		mhi_unprepare_from_transfer(mhi_dev);
>> +		return ret;
>> +	}
> You can use mhi_driver->driver.dev_groups to let the driver core create sysfs
> groups for you.
>
> - Mani
>