Re: [PATCH v4 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface
From: Sumit Kumar
Date: Tue Jun 23 2026 - 06:29:04 EST
On 6/23/2026 2:15 PM, Manivannan Sadhasivam wrote:
On Mon, Jun 22, 2026 at 10:39:15AM +0530, Sumit Kumar wrote:since start_store() is holding the mutex for its entire duration,
The MHI specification defines a LOOPBACK channel. The endpoint firmwareThis comment just duplicates Kconfig help text. So drop it.
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.
Co-developed-by: Krishna Chaitanya Chundru <krishna.chundru@xxxxxxxxxxxxxxxx>
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@xxxxxxxxxxxxxxxx>
Signed-off-by: Sumit Kumar <sumit.kumar@xxxxxxxxxxxxxxxx>
---
.../ABI/testing/sysfs-bus-mhi-devices-loopback | 51 ++++
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 | 329 +++++++++++++++++++++
7 files changed, 402 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..3bd770747799a3341a23903cc1a108e650e915b8
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-mhi-devices-loopback
@@ -0,0 +1,51 @@
+What: /sys/bus/mhi/devices/mhi<N>_LOOPBACK/tre_size
+Date: April 2026
+KernelVersion: 7.1
+Contact: mhi@xxxxxxxxxxxxxxx
+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: April 2026
+KernelVersion: 7.1
+Contact: mhi@xxxxxxxxxxxxxxx
+Description:
+ (RO) Maximum allowed 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: April 2026
+KernelVersion: 7.1
+Contact: mhi@xxxxxxxxxxxxxxx
+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.
+
+What: /sys/bus/mhi/devices/mhi<N>_LOOPBACK/start
+Date: April 2026
+KernelVersion: 7.1
+Contact: mhi@xxxxxxxxxxxxxxx
+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.
+
+What: /sys/bus/mhi/devices/mhi<N>_LOOPBACK/status
+Date: April 2026
+KernelVersion: 7.1
+Contact: mhi@xxxxxxxxxxxxxxx
+Description:
+ (RO) Result of the last loopback test. Returns one of:
+ "pass" - last test completed successfully
+ "fail" - last test failed
+ "not started" - no test has been run yet
+
+ Reading this file while a test is in progress will block
+ until the test completes.
diff --git a/MAINTAINERS b/MAINTAINERS
index 6dcfbd11efef87927041f5cf58d70633dbb4b18d..ff12a6da48947ac853bc638359a7046fea85fc21 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16441,6 +16441,7 @@ L: linux-arm-msm@xxxxxxxxxxxxxxx
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..d1463c3e0df0da461c815afaec623ba349b51dda
--- /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 (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, trigger a loopback test, and read the result.
+
+ 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..693691fff26dc8fa0d58931b98ce5f287fbd5c3e
--- /dev/null
+++ b/drivers/bus/mhi/host/clients/loopback.c
@@ -0,0 +1,329 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+/*
+ * The MHI LOOPBACK channel is defined in the MHI specification and is
+ * implemented by MHI-based devices (modems, WLAN) already deployed in the
+ * field. The endpoint firmware echoes back whatever the host sends on this
+ * channel. This driver binds to the LOOPBACK channel and exposes a sysfs
+ * interface for testing MHI data path integrity between host and endpoint.
+ * The sysfs interface is stable ABI because the wire protocol is fixed by
+ * the endpoint firmware and cannot be changed.
+ */
+tre_pending
+#include <linux/atomic.h>
+#include <linux/cleanup.h>
+#include <linux/completion.h>
+#include <linux/errno.h>
+#include <linux/mhi.h>
+#include <linux/mod_devicetable.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;
+ struct mutex lb_mutex;
+ struct completion comp;
+ atomic_t tres_pending;
+ const char *result;I think you have this check here to avoid race between sysfs cleanup and driver
+ 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);
+
+ if (!loopback)
+ return -ENODEV;
remove due to the use of devm_device_add_group(). But you can drop these by
switching to non-devm helpers and freeing the sysfs entries directly in
mhi_loopback_remove().
+So once this function exits, both buffers will get freed due to the destructor.
+ 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 (!loopback)
+ return -ENODEV;
+
+ 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);
+
+ if (!loopback)
+ return -ENODEV;
+
+ 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 (!loopback)
+ return -ENODEV;
+
+ if (kstrtou32(buf, 0, &val))
+ return -EINVAL;
+
+ if (val == 0)
+ return -EINVAL;
+
+ guard(mutex)(&loopback->lb_mutex);
+
+ el_num = mhi_get_free_desc_count(loopback->mdev, DMA_TO_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;
+
+ if (!loopback)
+ return -ENODEV;
+
+ 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;
+
+ 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->tres_pending, tre_count);
+ reinit_completion(&loopback->comp);
+
+ for (i = 0; i < tre_count; i++) {
+ int 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);
+ loopback->result = "fail";
+ if (atomic_sub_and_test(tre_count - i, &loopback->tres_pending))
+ complete(&loopback->comp);
+ return ret;
+ }
+ }
+
+ for (i = 0; i < tre_count - 1; i++) {
+ int 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);
+ loopback->result = "fail";
+ return ret;
+ }
+ }
+
+ int 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);
+ loopback->result = "fail";
+ return ret;
+ }
+
+ if (!wait_for_completion_timeout(&loopback->comp,
+ msecs_to_jiffies(MHI_LOOPBACK_TIMEOUT_MS))) {
+ dev_err(dev, "Loopback test timed out\n");
+ loopback->result = "fail";
+ return -ETIMEDOUT;
But the device may still hold the reference to the buffers in TRE and may
read/write to it later. So you need to make sure that you flush the buffers in
the error path. But we don't have any explicit APIs to do that, so maybe you can
call mhi_unprepare_from_transfer() followed by mhi_prepare_for_transfer() in the
error path?
+ }I don't see a need for this separate 'status' attribute. 'start' attribute
+
+ if (memcmp(send_buf, recv_buf, total_size)) {
+ dev_err(dev, "Loopback data mismatch\n");
+ loopback->result = "fail";
+ return -EIO;
+ }
+
+ loopback->result = "pass";
+
+ return count;
+}
+static DEVICE_ATTR_WO(start);
+
+static ssize_t status_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct mhi_loopback *loopback = dev_get_drvdata(dev);
+
+ if (!loopback)
+ return -ENODEV;
+
+ guard(mutex)(&loopback->lb_mutex);
+
+ return sysfs_emit(buf, "%s\n", loopback->result);
blocks until the write is completed or timesout, prints an error message with
relevant errno and returns the error code. Though the syscall interface converts
all error code to (-1), it is sufficient for the userspace to know whether the
test has passed or not.
+}'int ret'
+static DEVICE_ATTR_RO(status);
+
+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 (!loopback)
+ return;
+
+ 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->tres_pending))
+ complete(&loopback->comp);
+}
+
+static void mhi_loopback_ul_callback(struct mhi_device *mhi_dev,
+ struct mhi_result *mhi_res)
+{
+}
+
+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,
+ &dev_attr_status.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 rc;
+nit: Capitalize 'Failed' and print the errno. Applies to all error prints.
+ 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;
+ loopback->result = "not started";
+
+ mutex_init(&loopback->lb_mutex);
+ init_completion(&loopback->comp);
+
+ dev_set_drvdata(&mhi_dev->dev, loopback);
+
+ rc = mhi_prepare_for_transfer(mhi_dev);
+ if (rc) {
+ dev_err(&mhi_dev->dev, "failed to prepare for transfers\n");
+ return rc;'return 0'
+ }
+
+ rc = devm_device_add_group(&mhi_dev->dev, &mhi_loopback_group);
+ if (rc) {
+ dev_err(&mhi_dev->dev, "failed to create sysfs attributes\n");
+ mhi_unprepare_from_transfer(mhi_dev);
+ }
+
+ return rc;
+}What does this locking protect?
+
+static void mhi_loopback_remove(struct mhi_device *mhi_dev)
+{
+ struct mhi_loopback *loopback = dev_get_drvdata(&mhi_dev->dev);
+
+ complete(&loopback->comp);
+
+ mutex_lock(&loopback->lb_mutex);
+ mutex_unlock(&loopback->lb_mutex);
this lock will block until any in-progress start_store() has fully
completed before teardown, ensuring mhi_unprepare_from_transfer()
does not race with an active test still accessing the channel.
Will fix other things you have mentioned.
- Sumit
+As I mentioned above, once you call sysfs_create_group() in probe() and
+ mhi_unprepare_from_transfer(mhi_dev);
+ dev_set_drvdata(&mhi_dev->dev, NULL);
sysfs_remove_group() before mhi_unprepare_from_transfer(), you can drop setting
drvdata to NULL.
- Mani