mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v5 0/3] bus: mhi: Add loopback driver
@ 2026-08-17 10:14 Sumit Kumar
  2026-08-17 10:14 ` [PATCH v5 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface Sumit Kumar
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Sumit Kumar @ 2026-08-17 10:14 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Jeff Hugo
  Cc: mhi, linux-arm-msm, linux-kernel, Sumit Kumar, Krishna Chaitanya Chundru

The MHI specification defines a LOOPBACK channel that is already
implemented by MHI-based devices (modems, WLAN) deployed in the field.
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.

This series adds drivers to exercise the LOOPBACK channel from both the
host and endpoint sides. The host driver (patch 1) binds to the LOOPBACK
channel and provides a sysfs interface for configuring transfer parameters,
triggering a test, and reading the result. The sysfs interface is stable
ABI because the wire protocol is fixed by the endpoint firmware already
deployed in the field and cannot be changed.

The endpoint driver (patch 3) echoes received data back to the host using
a workqueue for asynchronous processing. Patch 2 introduces the
mhi_ep_queue_buf() API needed by the endpoint driver for raw buffer
queuing without an skb dependency.

Signed-off-by: Sumit Kumar <sumit.kumar@oss.qualcomm.com>
---
Changes in v5:
- Rebase onto mhi-next to pick up the EP flush_async() support, which
  disables the channels and flushes the in-flight transfers before calling
  the client driver's remove()
- Drop the file header comment block duplicating the Kconfig help text (Mani)
- Rename tres_pending to tre_pending (Mani)
- Replace devm_device_add_group() with sysfs_create_group() and remove it
  explicitly in remove(); this drops the !loopback guards from all sysfs
  show/store callbacks (Mani)
- Reset the channel in the start_store() timeout path so the device releases
  the queued TREs before the buffers are freed (Mani)
- Drop the status sysfs attribute; start already blocks and returns the
  errno (Mani)
- Capitalize the error strings and print the errno (Mani)
- Drop the mutex_lock()/mutex_unlock() pair and dev_set_drvdata(NULL) from
  mhi_loopback_remove() (Mani)
- Use ret instead of rc and return 0 on the probe() success path (Mani)
- Validate num_tre against both the UL and DL rings, and check both for free
  space in start_store() before queuing any TRE
- Log the transaction status in the host ul_xfer_cb() callback
- ep: rename loopback_wq to wq, log the kmemdup()/kmalloc() failures, and
  reword the error strings (Mani)
- Link to v4: https://lore.kernel.org/r/20260622-loopback_mhi-v4-0-782b3a0f2eef@oss.qualcomm.com

Changes in v4:
- Fix MHI_LOOPBACK_MAX_TRE_SIZE: change SZ_64K to (SZ_64K - 1) since the TRE
  length field is 16 bits and cannot encode 65536 (sashiko)
- Move mhi_prepare_for_transfer() to probe() so ring->el_size is initialized
  before num_tre_store() calls mhi_get_free_desc_count() (sashiko)
- Add mhi_unprepare_from_transfer() in mhi_loopback_remove() (sashiko)
- Add NULL guard in all sysfs show/store callbacks against post-remove drvdata
  race with devres teardown (sashiko)
- Add KMALLOC_MAX_SIZE check before kzalloc() to prevent page allocator WARN
  on large tre_count * tre_size values (sashiko)
- Fix start_store() to use __free(kfree) locals instead of goto-based cleanup
  to comply with cleanup.h guard+goto mixing rule (sashiko)
- Change buf_left and read_offset from u32 to size_t in mhi_ep_queue() to
  avoid truncation of size_t len parameter (sashiko)
- Add zero-length guard in mhi_ep_loopback_ul_callback() before kmemdup() to
  handle 0-byte transfers returning ZERO_SIZE_PTR (sashiko)
- Add NULL guard in mhi_ep_loopback_ul_callback() against post-remove drvdata
  race (sashiko)
- Link to v3: https://lore.kernel.org/r/20260610-loopback_mhi-v3-0-a733c0cef61a@oss.qualcomm.com

Changes in v3:
- Move ep driver to drivers/bus/mhi/ep/clients/loopback.c (Mani)
- Move host driver to drivers/bus/mhi/host/clients/loopback.c; keep
  module name mhi_loopback (Bjorn, Mani)
- Add ABI documentation in Documentation/ABI/testing/sysfs-bus-mhi-devices-loopback
  (Bjorn, Mani)
- Rename sysfs attribute 'size' to 'tre_size'; add 'max_tre_size' attribute
- Update Kconfig title to 'MHI LOOPBACK client driver' and describe that
  the driver binds to the MHI LOOPBACK channel defined in the MHI spec
  (Mani).
- Fix memory leak in ep loopback DL transfer error path.
- Rename mhi_ep_skb_completion() to mhi_ep_buf_completion().
- Document buffer ownership semantics in mhi_ep_queue_buf() kernel-doc
- Fix use-after-free in host loopback
- Fix completion race: arm completion before queuing recv TREs
- Fix teardown race: synchronize mhi_loopback_remove() with start_store()
  via lb_mutex
- Fix u32 multiplication overflow in total_size: use size_mul()
- Replace kmalloc+memcpy with kmemdup in ep loopback UL callback
- Update mhi_ep_queue_buf() kernel-doc: note per-TRE callback behavior
  when buffer length spans multiple host DL TREs
- Move mhi_prepare_for_transfer()/mhi_unprepare_from_transfer() into
  start_store() to avoid holding the channel open when idle
- Link to v2: https://lore.kernel.org/r/20251104-loopback_mhi-v2-0-727a3fd9aa74@oss.qualcomm.com

Changes in v2:
- Use __free(kfree) macro for buffers
- Removed NET layer socket buffer dependency, now using buffer and len
- Created a New Api for queuing buffers for clients which do not use skb
- Link to v1: https://lore.kernel.org/r/20250923-loopback_mhi-v1-0-8618f31f44aa@oss.qualcomm.com

---
Sumit Kumar (3):
      bus: mhi: host: clients: Add loopback driver with sysfs interface
      bus: mhi: ep: Add mhi_ep_queue_buf() API for raw buffer queuing
      bus: mhi: ep: clients: Add loopback driver for data path testing

 .../ABI/testing/sysfs-bus-mhi-devices-loopback     |  39 +++
 MAINTAINERS                                        |   1 +
 drivers/bus/mhi/ep/Kconfig                         |   2 +
 drivers/bus/mhi/ep/Makefile                        |   1 +
 drivers/bus/mhi/ep/clients/Kconfig                 |  16 ++
 drivers/bus/mhi/ep/clients/Makefile                |   2 +
 drivers/bus/mhi/ep/clients/loopback.c              | 130 +++++++++
 drivers/bus/mhi/ep/main.c                          |  29 ++-
 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            | 289 +++++++++++++++++++++
 include/linux/mhi_ep.h                             |  16 ++
 14 files changed, 537 insertions(+), 9 deletions(-)
---
base-commit: 9656bcd4c321a799148d00dd830ce7ebf20011da
change-id: 20250903-loopback_mhi-dee55ff0d462

Best regards,
-- 
Sumit Kumar <sumit.kumar@oss.qualcomm.com>


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

* [PATCH v5 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface
  2026-08-17 10:14 [PATCH v5 0/3] bus: mhi: Add loopback driver Sumit Kumar
@ 2026-08-17 10:14 ` Sumit Kumar
  2026-08-17 20:14   ` Jeff Hugo
  2026-08-18  8:50   ` Uwe Kleine-König
  2026-08-17 10:14 ` [PATCH v5 2/3] bus: mhi: ep: Add mhi_ep_queue_buf() API for raw buffer queuing Sumit Kumar
  2026-08-17 10:14 ` [PATCH v5 3/3] bus: mhi: ep: clients: Add loopback driver for data path testing Sumit Kumar
  2 siblings, 2 replies; 10+ messages in thread
From: Sumit Kumar @ 2026-08-17 10:14 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Jeff Hugo
  Cc: mhi, linux-arm-msm, linux-kernel, Sumit Kumar, Krishna Chaitanya Chundru

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.

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     |  39 +++
 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            | 289 +++++++++++++++++++++
 7 files changed, 350 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..1622a146110c2069c8b51e5bdf665c882b853303
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-mhi-devices-loopback
@@ -0,0 +1,39 @@
+What:		/sys/bus/mhi/devices/mhi<N>_LOOPBACK/tre_size
+Date:		August 2026
+KernelVersion:	7.2
+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.2
+Contact:	mhi@lists.linux.dev
+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:		August 2026
+KernelVersion:	7.2
+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.
+
+What:		/sys/bus/mhi/devices/mhi<N>_LOOPBACK/start
+Date:		August 2026
+KernelVersion:	7.2
+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 a4b89bd497ad5051c190025838e0e93d257ed061..098b12692140514da52565a3200801f5e24a43b7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17469,6 +17469,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..312a0b5a8e83b6ad7246453908bd552841a89188
--- /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, 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..02a952e8e01222cd8cb842c9f9490479df8ea2bd
--- /dev/null
+++ b/drivers/bus/mhi/host/clients/loopback.c
@@ -0,0 +1,289 @@
+// 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/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;
+	/* Serializes the sysfs attributes against a running test */
+	struct mutex lb_mutex;
+	struct completion comp;
+	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;
+		}
+	}
+
+	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) {
+		dev_err(&mhi_dev->dev, "Failed to prepare for transfers: %d\n", ret);
+		return ret;
+	}
+
+	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;
+	}
+
+	return 0;
+}
+
+static void mhi_loopback_remove(struct mhi_device *mhi_dev)
+{
+	/* Blocks until any in-progress store() has returned */
+	sysfs_remove_group(&mhi_dev->dev.kobj, &mhi_loopback_group);
+	mhi_unprepare_from_transfer(mhi_dev);
+}
+
+static const struct mhi_device_id mhi_loopback_id_table[] = {
+	{ .chan = "LOOPBACK"},
+	{}
+};
+MODULE_DEVICE_TABLE(mhi, mhi_loopback_id_table);
+
+static struct mhi_driver mhi_loopback_driver = {
+	.probe = mhi_loopback_probe,
+	.remove = mhi_loopback_remove,
+	.dl_xfer_cb = mhi_loopback_dl_callback,
+	.ul_xfer_cb = mhi_loopback_ul_callback,
+	.id_table = mhi_loopback_id_table,
+	.driver = {
+		.name = "mhi_loopback",
+	},
+};
+
+module_mhi_driver(mhi_loopback_driver);
+
+MODULE_AUTHOR("Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>");
+MODULE_AUTHOR("Sumit Kumar <sumit.kumar@oss.qualcomm.com>");
+MODULE_DESCRIPTION("MHI Host Loopback Driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1


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

* [PATCH v5 2/3] bus: mhi: ep: Add mhi_ep_queue_buf() API for raw buffer queuing
  2026-08-17 10:14 [PATCH v5 0/3] bus: mhi: Add loopback driver Sumit Kumar
  2026-08-17 10:14 ` [PATCH v5 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface Sumit Kumar
@ 2026-08-17 10:14 ` Sumit Kumar
  2026-08-17 10:14 ` [PATCH v5 3/3] bus: mhi: ep: clients: Add loopback driver for data path testing Sumit Kumar
  2 siblings, 0 replies; 10+ messages in thread
From: Sumit Kumar @ 2026-08-17 10:14 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Jeff Hugo
  Cc: mhi, linux-arm-msm, linux-kernel, Sumit Kumar

Some MHI endpoint clients do not use socket buffers and need a way to queue
raw buffers for DL transfers. Add mhi_ep_queue_buf() to support this use
case.

Refactor mhi_ep_queue_skb() to delegate to a new internal mhi_ep_queue()
helper shared by both APIs, and rename mhi_ep_skb_completion() to
mhi_ep_buf_completion() to reflect its broader use.

Signed-off-by: Sumit Kumar <sumit.kumar@oss.qualcomm.com>
---
 drivers/bus/mhi/ep/main.c | 29 ++++++++++++++++++++---------
 include/linux/mhi_ep.h    | 16 ++++++++++++++++
 2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
index 038b47158f0e0c905194779e24da1ad851e76061..7be1d3bbf5339156fee85d6b47603a5c8d46a8d5 100644
--- a/drivers/bus/mhi/ep/main.c
+++ b/drivers/bus/mhi/ep/main.c
@@ -505,7 +505,7 @@ static int mhi_ep_process_ch_ring(struct mhi_ep_ring *ring)
 	return 0;
 }
 
-static void mhi_ep_skb_completion(struct mhi_ep_buf_info *buf_info)
+static void mhi_ep_buf_completion(struct mhi_ep_buf_info *buf_info)
 {
 	struct mhi_ep_device *mhi_dev = buf_info->mhi_dev;
 	struct mhi_ep_cntrl *mhi_cntrl = mhi_dev->mhi_cntrl;
@@ -533,22 +533,22 @@ static void mhi_ep_skb_completion(struct mhi_ep_buf_info *buf_info)
 
 	mhi_ep_ring_inc_index(ring);
 }
-
 /* TODO: Handle partially formed TDs */
-int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
+static int mhi_ep_queue(struct mhi_ep_device *mhi_dev, void *buf, size_t len,
+			void *cb_buf)
 {
 	struct mhi_ep_cntrl *mhi_cntrl = mhi_dev->mhi_cntrl;
 	struct mhi_ep_chan *mhi_chan = mhi_dev->dl_chan;
 	struct device *dev = &mhi_chan->mhi_dev->dev;
 	struct mhi_ep_buf_info buf_info = {};
 	struct mhi_ring_element *el;
-	u32 buf_left, read_offset;
+	size_t buf_left, read_offset;
 	struct mhi_ep_ring *ring;
 	size_t tr_len;
 	u32 tre_len;
 	int ret;
 
-	buf_left = skb->len;
+	buf_left = len;
 	ring = &mhi_cntrl->mhi_chan[mhi_chan->chan].ring;
 
 	mutex_lock(&mhi_chan->lock);
@@ -571,13 +571,13 @@ int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
 		tre_len = MHI_TRE_DATA_GET_LEN(el);
 
 		tr_len = min(buf_left, tre_len);
-		read_offset = skb->len - buf_left;
+		read_offset = len - buf_left;
 
-		buf_info.dev_addr = skb->data + read_offset;
+		buf_info.dev_addr = buf + read_offset;
 		buf_info.host_addr = MHI_TRE_DATA_GET_PTR(el);
 		buf_info.size = tr_len;
-		buf_info.cb = mhi_ep_skb_completion;
-		buf_info.cb_buf = skb;
+		buf_info.cb = mhi_ep_buf_completion;
+		buf_info.cb_buf = cb_buf;
 		buf_info.mhi_dev = mhi_dev;
 
 		/*
@@ -616,8 +616,19 @@ int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
 
 	return ret;
 }
+
+int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
+{
+	return mhi_ep_queue(mhi_dev, skb->data, skb->len, skb);
+}
 EXPORT_SYMBOL_GPL(mhi_ep_queue_skb);
 
+int mhi_ep_queue_buf(struct mhi_ep_device *mhi_dev, void *buf, size_t len)
+{
+	return mhi_ep_queue(mhi_dev, buf, len, buf);
+}
+EXPORT_SYMBOL_GPL(mhi_ep_queue_buf);
+
 static int mhi_ep_cache_host_cfg(struct mhi_ep_cntrl *mhi_cntrl)
 {
 	size_t cmd_ctx_host_size, ch_ctx_host_size, ev_ctx_host_size;
diff --git a/include/linux/mhi_ep.h b/include/linux/mhi_ep.h
index f6383a57a872bf3cdea236ae9fe65f4ec8747b3e..852f331f03fda5c0212585de7d6f4eb9f8270609 100644
--- a/include/linux/mhi_ep.h
+++ b/include/linux/mhi_ep.h
@@ -304,4 +304,20 @@ bool mhi_ep_queue_is_empty(struct mhi_ep_device *mhi_dev, enum dma_data_directio
  */
 int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb);
 
+/**
+ * mhi_ep_queue_buf - Transfer buffer contents to host over MHI Endpoint
+ * @mhi_dev: Device associated with the DL channel
+ * @buf: Buffer to be queued. On success, ownership passes to the MHI stack;
+ *       the caller must not free @buf until the DL transfer callback fires
+ *       with result->buf_addr equal to @buf. On failure, the caller retains
+ *       ownership and must free @buf.
+ *       Note: if @len spans multiple host DL TREs, the DL transfer callback
+ *       fires once per TRE, each time with result->buf_addr equal to @buf.
+ * @len: Size of the buffer
+ *
+ * Return: 0 if the buffer contents have been transferred successfully, a
+ * negative error code otherwise.
+ */
+int mhi_ep_queue_buf(struct mhi_ep_device *mhi_dev, void *buf, size_t len);
+
 #endif

-- 
2.34.1


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

* [PATCH v5 3/3] bus: mhi: ep: clients: Add loopback driver for data path testing
  2026-08-17 10:14 [PATCH v5 0/3] bus: mhi: Add loopback driver Sumit Kumar
  2026-08-17 10:14 ` [PATCH v5 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface Sumit Kumar
  2026-08-17 10:14 ` [PATCH v5 2/3] bus: mhi: ep: Add mhi_ep_queue_buf() API for raw buffer queuing Sumit Kumar
@ 2026-08-17 10:14 ` Sumit Kumar
  2026-08-18  8:53   ` Uwe Kleine-König
  2 siblings, 1 reply; 10+ messages in thread
From: Sumit Kumar @ 2026-08-17 10:14 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Jeff Hugo
  Cc: mhi, linux-arm-msm, linux-kernel, Sumit Kumar, Krishna Chaitanya Chundru

When an MHI endpoint device runs Linux, there is no firmware to implement
the LOOPBACK channel echo that real modem firmware provides. Without an
endpoint-side driver, the host loopback test has no software echo partner
and cannot exercise the full end-to-end MHI data path.

Add an endpoint-side loopback driver that binds to the LOOPBACK channel and
echoes received data back to the host. An ordered workqueue is used for
asynchronous processing to preserve packet ordering. Together with the
host-side loopback driver, this enables complete MHI data path validation
for Linux-based endpoint devices.

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>
---
 drivers/bus/mhi/ep/Kconfig            |   2 +
 drivers/bus/mhi/ep/Makefile           |   1 +
 drivers/bus/mhi/ep/clients/Kconfig    |  16 +++++
 drivers/bus/mhi/ep/clients/Makefile   |   2 +
 drivers/bus/mhi/ep/clients/loopback.c | 130 ++++++++++++++++++++++++++++++++++
 5 files changed, 151 insertions(+)

diff --git a/drivers/bus/mhi/ep/Kconfig b/drivers/bus/mhi/ep/Kconfig
index 90ab3b040672e0f04181d4802e3062afcc7cf782..9edb81b39890e093a51138465a4d7705767eafa5 100644
--- a/drivers/bus/mhi/ep/Kconfig
+++ b/drivers/bus/mhi/ep/Kconfig
@@ -8,3 +8,5 @@ config MHI_BUS_EP
 
 	  MHI_BUS_EP implements the MHI protocol for the endpoint devices,
 	  such as SDX55 modem connected to the host machine over PCIe.
+
+source "drivers/bus/mhi/ep/clients/Kconfig"
diff --git a/drivers/bus/mhi/ep/Makefile b/drivers/bus/mhi/ep/Makefile
index aad85f180b707fb997fcb541837eda9bbbb67437..ab36ef2a40ab8174e5ddae44a3e6ccb8eb31168d 100644
--- a/drivers/bus/mhi/ep/Makefile
+++ b/drivers/bus/mhi/ep/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_MHI_BUS_EP) += mhi_ep.o
 mhi_ep-y := main.o mmio.o ring.o sm.o
+obj-y += clients/
diff --git a/drivers/bus/mhi/ep/clients/Kconfig b/drivers/bus/mhi/ep/clients/Kconfig
new file mode 100644
index 0000000000000000000000000000000000000000..4cf27184058ca2be020885b6f57b4cc44b5054b6
--- /dev/null
+++ b/drivers/bus/mhi/ep/clients/Kconfig
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0
+
+config MHI_BUS_EP_LOOPBACK
+	tristate "MHI Endpoint LOOPBACK client driver"
+	depends on MHI_BUS_EP
+	help
+	  MHI Endpoint LOOPBACK client driver that binds to the MHI LOOPBACK
+	  channel as defined in the MHI specification. The LOOPBACK channel is
+	  implemented by MHI-based endpoint devices (modems, WLAN) in the field,
+	  where the endpoint firmware echoes back whatever the host sends.
+
+	  This driver receives data on the uplink channel and echoes it back on
+	  the downlink channel for testing the MHI endpoint data path.
+
+	  To compile this driver as a module, choose M here. The module
+	  will be called mhi_ep_loopback.
diff --git a/drivers/bus/mhi/ep/clients/Makefile b/drivers/bus/mhi/ep/clients/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..71dc91cc63b02592b177cf66db6090748c0653a6
--- /dev/null
+++ b/drivers/bus/mhi/ep/clients/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_MHI_BUS_EP_LOOPBACK) += mhi_ep_loopback.o
+mhi_ep_loopback-y += loopback.o
diff --git a/drivers/bus/mhi/ep/clients/loopback.c b/drivers/bus/mhi/ep/clients/loopback.c
new file mode 100644
index 0000000000000000000000000000000000000000..18a92d6a40e0f7de277d155cdb5495d14a60c19d
--- /dev/null
+++ b/drivers/bus/mhi/ep/clients/loopback.c
@@ -0,0 +1,130 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/mhi_ep.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/string.h>
+
+struct mhi_ep_loopback {
+	struct workqueue_struct *wq;
+	struct mhi_ep_device *mdev;
+};
+
+struct mhi_ep_loopback_work {
+	struct mhi_ep_device *mdev;
+	struct work_struct work;
+	void *buf;
+	size_t len;
+};
+
+static void mhi_ep_loopback_work_handler(struct work_struct *work)
+{
+	struct mhi_ep_loopback_work *mhi_ep_lb_work = container_of(work,
+								struct mhi_ep_loopback_work, work);
+	int ret;
+
+	ret = mhi_ep_queue_buf(mhi_ep_lb_work->mdev, mhi_ep_lb_work->buf,
+			       mhi_ep_lb_work->len);
+	if (ret) {
+		dev_err(&mhi_ep_lb_work->mdev->dev, "Failed to queue buffer: %d\n", ret);
+		kfree(mhi_ep_lb_work->buf);
+	}
+
+	kfree(mhi_ep_lb_work);
+}
+
+static void mhi_ep_loopback_ul_callback(struct mhi_ep_device *mhi_dev,
+					struct mhi_result *mhi_res)
+{
+	struct mhi_ep_loopback *mhi_ep_lb = dev_get_drvdata(&mhi_dev->dev);
+	struct mhi_ep_loopback_work *mhi_ep_lb_work;
+	void *buf;
+
+	if (!mhi_ep_lb)
+		return;
+
+	if (!mhi_res->transaction_status) {
+		if (!mhi_res->bytes_xferd)
+			return;
+
+		buf = kmemdup(mhi_res->buf_addr, mhi_res->bytes_xferd, GFP_KERNEL);
+		if (!buf) {
+			dev_err(&mhi_dev->dev, "Failed to allocate loopback buffer\n");
+			return;
+		}
+
+		mhi_ep_lb_work = kmalloc(sizeof(*mhi_ep_lb_work), GFP_KERNEL);
+		if (!mhi_ep_lb_work) {
+			dev_err(&mhi_dev->dev, "Failed to allocate loopback work\n");
+			kfree(buf);
+			return;
+		}
+
+		INIT_WORK(&mhi_ep_lb_work->work, mhi_ep_loopback_work_handler);
+		mhi_ep_lb_work->mdev = mhi_dev;
+		mhi_ep_lb_work->buf = buf;
+		mhi_ep_lb_work->len = mhi_res->bytes_xferd;
+
+		queue_work(mhi_ep_lb->wq, &mhi_ep_lb_work->work);
+	}
+}
+
+static void mhi_ep_loopback_dl_callback(struct mhi_ep_device *mhi_dev,
+					struct mhi_result *mhi_res)
+{
+	kfree(mhi_res->buf_addr);
+}
+
+static int mhi_ep_loopback_probe(struct mhi_ep_device *mhi_dev, const struct mhi_device_id *id)
+{
+	struct mhi_ep_loopback *mhi_ep_lb;
+
+	mhi_ep_lb = devm_kzalloc(&mhi_dev->dev, sizeof(*mhi_ep_lb), GFP_KERNEL);
+	if (!mhi_ep_lb)
+		return -ENOMEM;
+
+	mhi_ep_lb->wq = alloc_ordered_workqueue("mhi_ep_loopback", WQ_MEM_RECLAIM);
+	if (!mhi_ep_lb->wq) {
+		dev_err(&mhi_dev->dev, "Failed to create workqueue\n");
+		return -ENOMEM;
+	}
+
+	mhi_ep_lb->mdev = mhi_dev;
+	dev_set_drvdata(&mhi_dev->dev, mhi_ep_lb);
+
+	return 0;
+}
+
+static void mhi_ep_loopback_remove(struct mhi_ep_device *mhi_dev)
+{
+	struct mhi_ep_loopback *mhi_ep_lb = dev_get_drvdata(&mhi_dev->dev);
+
+	destroy_workqueue(mhi_ep_lb->wq);
+}
+
+static const struct mhi_device_id mhi_ep_loopback_id_table[] = {
+	{ .chan = "LOOPBACK"},
+	{}
+};
+MODULE_DEVICE_TABLE(mhi, mhi_ep_loopback_id_table);
+
+static struct mhi_ep_driver mhi_ep_loopback_driver = {
+	.probe = mhi_ep_loopback_probe,
+	.remove = mhi_ep_loopback_remove,
+	.dl_xfer_cb = mhi_ep_loopback_dl_callback,
+	.ul_xfer_cb = mhi_ep_loopback_ul_callback,
+	.id_table = mhi_ep_loopback_id_table,
+	.driver = {
+		.name = "mhi_ep_loopback",
+	},
+};
+
+module_mhi_ep_driver(mhi_ep_loopback_driver);
+
+MODULE_AUTHOR("Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>");
+MODULE_AUTHOR("Sumit Kumar <sumit.kumar@oss.qualcomm.com>");
+MODULE_DESCRIPTION("MHI Endpoint Loopback driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1


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

* Re: [PATCH v5 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface
  2026-08-17 10:14 ` [PATCH v5 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface Sumit Kumar
@ 2026-08-17 20:14   ` Jeff Hugo
  2026-08-19  9:46     ` Sumit Kumar
  2026-08-18  8:50   ` Uwe Kleine-König
  1 sibling, 1 reply; 10+ messages in thread
From: Jeff Hugo @ 2026-08-17 20:14 UTC (permalink / raw)
  To: Sumit Kumar, Manivannan Sadhasivam
  Cc: mhi, linux-arm-msm, linux-kernel, Krishna Chaitanya Chundru

On 8/17/2026 4:14 AM, 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.
> 
> 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     |  39 +++
>   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            | 289 +++++++++++++++++++++
>   7 files changed, 350 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..1622a146110c2069c8b51e5bdf665c882b853303
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-mhi-devices-loopback
> @@ -0,0 +1,39 @@
> +What:		/sys/bus/mhi/devices/mhi<N>_LOOPBACK/tre_size
> +Date:		August 2026
> +KernelVersion:	7.2

This series missed 7.2, and will miss 7.3 since the merge window is 
already open.  Next oppertunity is 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.2
> +Contact:	mhi@lists.linux.dev
> +Description:
> +		(RO) Maximum allowed TRE size in bytes. Reading this file

Entry before this and after this explains the TRE acronym, but this does 
not.  It probably should, as someone may jump directly to this section 
and not read the entire document start to finish.

> +		returns the upper bound for the tre_size attribute.
> +
> +What:		/sys/bus/mhi/devices/mhi<N>_LOOPBACK/num_tre
> +Date:		August 2026
> +KernelVersion:	7.2
> +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 would I know the channel ring capacity?  Should I just try values 
until I get an error?

> +
> +What:		/sys/bus/mhi/devices/mhi<N>_LOOPBACK/start
> +Date:		August 2026
> +KernelVersion:	7.2
> +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

It feels like these kind of interfaces usually give an option to specify 
a byte pattern as well, but I don't see that option here.  Is random 
data considered to cover all usecases?

> +		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 a4b89bd497ad5051c190025838e0e93d257ed061..098b12692140514da52565a3200801f5e24a43b7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17469,6 +17469,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

Commit text says this ABI is stable.  Seems odd its being added to testing.

>   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..312a0b5a8e83b6ad7246453908bd552841a89188
> --- /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

Are modems, WLAN the only devices that implement this, or are those 
intended to be examples?  Current reads as a limitation.

> +	  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..02a952e8e01222cd8cb842c9f9490479df8ea2bd
> --- /dev/null
> +++ b/drivers/bus/mhi/host/clients/loopback.c
> @@ -0,0 +1,289 @@
> +// 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/mod_devicetable.h>

I'm pretty sure this is now incorrect based on cleanups that just 
occured in tree.

> +#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;
> +	atomic_t tre_pending;

I'm not really sure I understand what purpose this is serving.

> +	u32 num_tre;
> +	u32 tre_size;

I'm pretty sure these are the wrong types since they cross kernel 
boundaries.



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

* Re: [PATCH v5 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface
  2026-08-17 10:14 ` [PATCH v5 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface Sumit Kumar
  2026-08-17 20:14   ` Jeff Hugo
@ 2026-08-18  8:50   ` Uwe Kleine-König
  2026-08-19  9:48     ` Sumit Kumar
  1 sibling, 1 reply; 10+ messages in thread
From: Uwe Kleine-König @ 2026-08-18  8:50 UTC (permalink / raw)
  To: Sumit Kumar
  Cc: Manivannan Sadhasivam, Jeff Hugo, mhi, linux-arm-msm,
	linux-kernel, Krishna Chaitanya Chundru

[-- Attachment #1: Type: text/plain, Size: 2199 bytes --]

Hello,

On Mon, Aug 17, 2026 at 03:44:34PM +0530, Sumit Kumar wrote:
> +#include <linux/mhi.h>
> +#include <linux/mod_devicetable.h>

Please rely on <linux/mhi.h> to provide mhi_device_id and drop the
inclusion for <linux/mod_devicetable.h>. The latter header is ugly and
should go away soon.

> [...]
> +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,

Please no , after sentinel entries.

> +};
> +
> +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) {
> +		dev_err(&mhi_dev->dev, "Failed to prepare for transfers: %d\n", ret);

dev_err_probe() please.

> +		return ret;
> +	}
> +
> +	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;
> +	}
> +
> +	return 0;
> +}
> +
> +static void mhi_loopback_remove(struct mhi_device *mhi_dev)
> +{
> +	/* Blocks until any in-progress store() has returned */
> +	sysfs_remove_group(&mhi_dev->dev.kobj, &mhi_loopback_group);
> +	mhi_unprepare_from_transfer(mhi_dev);
> +}
> +
> +static const struct mhi_device_id mhi_loopback_id_table[] = {
> +	{ .chan = "LOOPBACK"},
> +	{}

{ } (i.e. a space between the curly braces) is the more common way to
write that. Please also add a space before the closing } in the line
above.

> +};
> +MODULE_DEVICE_TABLE(mhi, mhi_loopback_id_table);

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v5 3/3] bus: mhi: ep: clients: Add loopback driver for data path testing
  2026-08-17 10:14 ` [PATCH v5 3/3] bus: mhi: ep: clients: Add loopback driver for data path testing Sumit Kumar
@ 2026-08-18  8:53   ` Uwe Kleine-König
  2026-08-19  9:48     ` Sumit Kumar
  0 siblings, 1 reply; 10+ messages in thread
From: Uwe Kleine-König @ 2026-08-18  8:53 UTC (permalink / raw)
  To: Sumit Kumar
  Cc: Manivannan Sadhasivam, Jeff Hugo, mhi, linux-arm-msm,
	linux-kernel, Krishna Chaitanya Chundru

[-- Attachment #1: Type: text/plain, Size: 1101 bytes --]

Hello,

On Mon, Aug 17, 2026 at 03:44:36PM +0530, Sumit Kumar wrote:
> diff --git a/drivers/bus/mhi/ep/clients/loopback.c b/drivers/bus/mhi/ep/clients/loopback.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..18a92d6a40e0f7de277d155cdb5495d14a60c19d
> --- /dev/null
> +++ b/drivers/bus/mhi/ep/clients/loopback.c
> @@ -0,0 +1,130 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include <linux/mhi_ep.h>
> +#include <linux/mod_devicetable.h>

Similar as with my feedback for patch #1, please don't use
<linux/mod_devicetable.h>. Either rely on <linux/mhi_ep.h> ->
<linux/mhi.h> -> <linux/device-id/mhi.h>, or explicitly include one of
the these headers but not <linux/mod_devicetable.h>.

> +#include <linux/module.h>
> +#include <linux/string.h>
> +
> [...]
> +static const struct mhi_device_id mhi_ep_loopback_id_table[] = {
> +	{ .chan = "LOOPBACK"},
> +	{}

Again, please add spaces before the closing }.

> +};
> +MODULE_DEVICE_TABLE(mhi, mhi_ep_loopback_id_table);

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v5 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface
  2026-08-17 20:14   ` Jeff Hugo
@ 2026-08-19  9:46     ` Sumit Kumar
  0 siblings, 0 replies; 10+ messages in thread
From: Sumit Kumar @ 2026-08-19  9:46 UTC (permalink / raw)
  To: Jeff Hugo, Manivannan Sadhasivam
  Cc: mhi, linux-arm-msm, linux-kernel, Krishna Chaitanya Chundru



On 8/18/2026 1:44 AM, Jeff Hugo wrote:
> On 8/17/2026 4:14 AM, 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.
>>
>> 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     |  39 +++
>>   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            | 289 
>> +++++++++++++++++++++
>>   7 files changed, 350 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..1622a146110c2069c8b51e5bdf665c882b853303
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-mhi-devices-loopback
>> @@ -0,0 +1,39 @@
>> +What: /sys/bus/mhi/devices/mhi<N>_LOOPBACK/tre_size
>> +Date:        August 2026
>> +KernelVersion:    7.2
>
> This series missed 7.2, and will miss 7.3 since the merge window is 
> already open.  Next oppertunity is 7.4.
Acknowledged, thanks — targeting 7.4 next.
>
>> +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.2
>> +Contact:    mhi@lists.linux.dev
>> +Description:
>> +        (RO) Maximum allowed TRE size in bytes. Reading this file
>
> Entry before this and after this explains the TRE acronym, but this 
> does not.  It probably should, as someone may jump directly to this 
> section and not read the entire document start to finish.
Will update the description.
>
>> +        returns the upper bound for the tre_size attribute.
>> +
>> +What:        /sys/bus/mhi/devices/mhi<N>_LOOPBACK/num_tre
>> +Date:        August 2026
>> +KernelVersion:    7.2
>> +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 would I know the channel ring capacity?  Should I just try values 
> until I get an error?
if you try to write any value bigger than ring capacity the error will 
report it with the max ring capacity.
Would you prefer having another read only sysfs to find out the ring 
capacity ?
>
>> +
>> +What:        /sys/bus/mhi/devices/mhi<N>_LOOPBACK/start
>> +Date:        August 2026
>> +KernelVersion:    7.2
>> +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
>
> It feels like these kind of interfaces usually give an option to 
> specify a byte pattern as well, but I don't see that option here. Is 
> random data considered to cover all usecases?
Since currently we are checking the data path, random bytes should 
suffice for now.
Happy to add a pattern option later if a concrete use case needs 
deterministic content, but I'd prefer random as the default.
>
>> +        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 
>> a4b89bd497ad5051c190025838e0e93d257ed061..098b12692140514da52565a3200801f5e24a43b7 
>> 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -17469,6 +17469,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
>
> Commit text says this ABI is stable.  Seems odd its being added to 
> testing.
Commit text needs to be fixed for this, Thanks will update it in the 
next revision.
>
>>   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..312a0b5a8e83b6ad7246453908bd552841a89188
>> --- /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
>
> Are modems, WLAN the only devices that implement this, or are those 
> intended to be examples?  Current reads as a limitation.
those are just examples, will reword it.
>
>> +      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..02a952e8e01222cd8cb842c9f9490479df8ea2bd
>> --- /dev/null
>> +++ b/drivers/bus/mhi/host/clients/loopback.c
>> @@ -0,0 +1,289 @@
>> +// 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/mod_devicetable.h>
>
> I'm pretty sure this is now incorrect based on cleanups that just 
> occured in tree.
I missed the recent rebase on mhi-next therefore missed these merges. 
will rebase and fix.
>
>> +#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;
>> +    atomic_t tre_pending;
>
> I'm not really sure I understand what purpose this is serving.
>
tre_pending tracks how many of the tre_count queued DL+UL completions
are still outstanding; comp is signaled once it hits zero, which is what
start_store() waits on. Will add a comment for it.
>> +    u32 num_tre;
>> +    u32 tre_size;
>
> I'm pretty sure these are the wrong types since they cross kernel 
> boundaries.
>
num_tre and tre_size are sysfs attribute values parsed with kstrtou32() 
from a text write,
they don't cross into any MHI wire-protocol struct or UAPI boundary, and 
u32 comfortably covers
the valid range (num_tre is ring-capacity-bounded, tre_size caps at 
MHI_LOOPBACK_MAX_TRE_SIZE = 64K-1)
Could you explain a little bit more about the boundary concern.


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

* Re: [PATCH v5 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface
  2026-08-18  8:50   ` Uwe Kleine-König
@ 2026-08-19  9:48     ` Sumit Kumar
  0 siblings, 0 replies; 10+ messages in thread
From: Sumit Kumar @ 2026-08-19  9:48 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Manivannan Sadhasivam, Jeff Hugo, mhi, linux-arm-msm,
	linux-kernel, Krishna Chaitanya Chundru



On 8/18/2026 2:20 PM, Uwe Kleine-König wrote:
> Hello,
>
> On Mon, Aug 17, 2026 at 03:44:34PM +0530, Sumit Kumar wrote:
>> +#include <linux/mhi.h>
>> +#include <linux/mod_devicetable.h>
> Please rely on <linux/mhi.h> to provide mhi_device_id and drop the
> inclusion for <linux/mod_devicetable.h>. The latter header is ugly and
> should go away soon.
thanks, Will rebase and fix it
>
>> [...]
>> +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,
> Please no , after sentinel entries.
Acknowledged, will remove it.
>
>> +};
>> +
>> +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) {
>> +		dev_err(&mhi_dev->dev, "Failed to prepare for transfers: %d\n", ret);
> dev_err_probe() please.
Sure.
>
>> +		return ret;
>> +	}
>> +
>> +	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;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void mhi_loopback_remove(struct mhi_device *mhi_dev)
>> +{
>> +	/* Blocks until any in-progress store() has returned */
>> +	sysfs_remove_group(&mhi_dev->dev.kobj, &mhi_loopback_group);
>> +	mhi_unprepare_from_transfer(mhi_dev);
>> +}
>> +
>> +static const struct mhi_device_id mhi_loopback_id_table[] = {
>> +	{ .chan = "LOOPBACK"},
>> +	{}
> { } (i.e. a space between the curly braces) is the more common way to
> write that. Please also add a space before the closing } in the line
> above.
Will fix this too.
>
>> +};
>> +MODULE_DEVICE_TABLE(mhi, mhi_loopback_id_table);
> Best regards
> Uwe


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

* Re: [PATCH v5 3/3] bus: mhi: ep: clients: Add loopback driver for data path testing
  2026-08-18  8:53   ` Uwe Kleine-König
@ 2026-08-19  9:48     ` Sumit Kumar
  0 siblings, 0 replies; 10+ messages in thread
From: Sumit Kumar @ 2026-08-19  9:48 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Manivannan Sadhasivam, Jeff Hugo, mhi, linux-arm-msm,
	linux-kernel, Krishna Chaitanya Chundru



On 8/18/2026 2:23 PM, Uwe Kleine-König wrote:
> Hello,
>
> On Mon, Aug 17, 2026 at 03:44:36PM +0530, Sumit Kumar wrote:
>> diff --git a/drivers/bus/mhi/ep/clients/loopback.c b/drivers/bus/mhi/ep/clients/loopback.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..18a92d6a40e0f7de277d155cdb5495d14a60c19d
>> --- /dev/null
>> +++ b/drivers/bus/mhi/ep/clients/loopback.c
>> @@ -0,0 +1,130 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +
>> +#include <linux/mhi_ep.h>
>> +#include <linux/mod_devicetable.h>
> Similar as with my feedback for patch #1, please don't use
> <linux/mod_devicetable.h>. Either rely on <linux/mhi_ep.h> ->
> <linux/mhi.h> -> <linux/device-id/mhi.h>, or explicitly include one of
> the these headers but not <linux/mod_devicetable.h>.
Will fix it after rebase in v6.
>
>> +#include <linux/module.h>
>> +#include <linux/string.h>
>> +
>> [...]
>> +static const struct mhi_device_id mhi_ep_loopback_id_table[] = {
>> +	{ .chan = "LOOPBACK"},
>> +	{}
> Again, please add spaces before the closing }.
sure.
>
>> +};
>> +MODULE_DEVICE_TABLE(mhi, mhi_ep_loopback_id_table);
> Best regards
> Uwe


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

end of thread, other threads:[~2026-08-19  9:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-17 10:14 [PATCH v5 0/3] bus: mhi: Add loopback driver Sumit Kumar
2026-08-17 10:14 ` [PATCH v5 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface Sumit Kumar
2026-08-17 20:14   ` Jeff Hugo
2026-08-19  9:46     ` Sumit Kumar
2026-08-18  8:50   ` Uwe Kleine-König
2026-08-19  9:48     ` Sumit Kumar
2026-08-17 10:14 ` [PATCH v5 2/3] bus: mhi: ep: Add mhi_ep_queue_buf() API for raw buffer queuing Sumit Kumar
2026-08-17 10:14 ` [PATCH v5 3/3] bus: mhi: ep: clients: Add loopback driver for data path testing Sumit Kumar
2026-08-18  8:53   ` Uwe Kleine-König
2026-08-19  9:48     ` Sumit Kumar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®