[PATCH v2 0/3] bus: mhi: Add loopback driver

[PATCH v2 0/3] bus: mhi: Add loopback driver

[Sumit Kumar]

This series adds MHI loopback drivers to enable comprehensive testing and
validation of MHI data path functionality between host controllers and
endpoint devices. The drivers support configurable transfer sizes, TRE
chaining, and provide real-time status reporting for thorough testing.

Testing is initiated with sysfs entries from the host side and results are
reflected back in the sysfs entry.
The device side driver echoes back the data recieved.

Signed-off-by: Sumit Kumar <sumit.kumar@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: Add loopback driver with sysfs interface
      bus: mhi: ep: Create mhi_ep_queue_buf API for raw buffer queuing
      bus: mhi: ep: Add loopback driver for data path testing

 drivers/bus/mhi/ep/Kconfig           |   8 +
 drivers/bus/mhi/ep/Makefile          |   1 +
 drivers/bus/mhi/ep/main.c            |  23 ++-
 drivers/bus/mhi/ep/mhi_ep_loopback.c | 134 ++++++++++++++
 drivers/bus/mhi/host/Kconfig         |   7 +
 drivers/bus/mhi/host/Makefile        |   1 +
 drivers/bus/mhi/host/mhi_loopback.c  | 347 +++++++++++++++++++++++++++++++++++
 include/linux/mhi_ep.h               |  10 +
 8 files changed, 525 insertions(+), 6 deletions(-)
---
base-commit: e6b9dce0aeeb91dfc0974ab87f02454e24566182
change-id: 20250903-loopback_mhi-dee55ff0d462

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

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

[Sumit Kumar]

Add loopback driver for MHI host controllers that provides sysfs based
testing interface for data path validation. The driver supports the
"LOOPBACK" channel and offers configurable test parameters.

Sysfs interface provides:
- size: Configure TRE size
- num_tre: Set number of TREs for chained transfers
- start: Initiate loopback test
- status: Read test results

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/host/Kconfig        |   7 +
 drivers/bus/mhi/host/Makefile       |   1 +
 drivers/bus/mhi/host/mhi_loopback.c | 347 ++++++++++++++++++++++++++++++++++++
 3 files changed, 355 insertions(+)

diff --git a/drivers/bus/mhi/host/Kconfig b/drivers/bus/mhi/host/Kconfig
index da5cd0c9fc620ab595e742c422f1a22a2a84c7b9..08a39ecb47f585bf39721c101ed5e2ff44bdd5f8 100644
--- a/drivers/bus/mhi/host/Kconfig
+++ b/drivers/bus/mhi/host/Kconfig
@@ -29,3 +29,10 @@ config MHI_BUS_PCI_GENERIC
 	  This driver provides MHI PCI controller driver for devices such as
 	  Qualcomm SDX55 based PCIe modems.
 
+config MHI_BUS_LOOPBACK
+	tristate "MHI loopback driver"
+	depends on MHI_BUS
+	help
+	  MHI loopback driver for data path testing. This driver
+	  provides a mechanism to test MHI data transfer functionality
+	  by implementing an echo service between host and endpoint.
diff --git a/drivers/bus/mhi/host/Makefile b/drivers/bus/mhi/host/Makefile
index 859c2f38451c669b3d3014c374b2b957c99a1cfe..e5d6dccf5a976eaeb827c47924ad0614c9958f8b 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-$(CONFIG_MHI_BUS_LOOPBACK) += mhi_loopback.o
diff --git a/drivers/bus/mhi/host/mhi_loopback.c b/drivers/bus/mhi/host/mhi_loopback.c
new file mode 100644
index 0000000000000000000000000000000000000000..980ace675718a79c97d9b2968ccef04c992a6c20
--- /dev/null
+++ b/drivers/bus/mhi/host/mhi_loopback.c
@@ -0,0 +1,347 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/mhi.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/completion.h>
+#include <linux/string.h>
+#include <linux/random.h>
+#include <linux/kernel.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/mutex.h>
+#include <linux/atomic.h>
+#include <linux/cleanup.h>
+#include <linux/sizes.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
+
+struct mhi_loopback {
+	struct mhi_device *mdev;
+	struct mutex lb_mutex;
+	struct completion comp;
+	atomic_t num_completions_received;
+	char result[32];
+	u32 num_tre;
+	u32 size;
+	bool loopback_in_progress;
+};
+
+static ssize_t size_show(struct device *dev,
+			 struct device_attribute *attr, char *buf)
+{
+	struct mhi_loopback *mhi_lb = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%u\n", mhi_lb->size);
+}
+
+static ssize_t size_store(struct device *dev,
+			  struct device_attribute *attr,
+			  const char *buf, size_t count)
+{
+	struct mhi_loopback *mhi_lb = dev_get_drvdata(dev);
+	u32 val;
+
+	if (kstrtou32(buf, 0, &val)) {
+		dev_err(dev, "Invalid size value\n");
+		return -EINVAL;
+	}
+
+	if (val == 0 || val > MHI_LOOPBACK_MAX_TRE_SIZE) {
+		dev_err(dev, "Size must be between 1 and %u bytes\n",
+			MHI_LOOPBACK_MAX_TRE_SIZE);
+		return -EINVAL;
+	}
+
+	guard(mutex)(&mhi_lb->lb_mutex);
+	if (mhi_lb->loopback_in_progress)
+		return -EBUSY;
+
+	mhi_lb->size = val;
+	return count;
+}
+static DEVICE_ATTR_RW(size);
+
+static ssize_t num_tre_show(struct device *dev,
+			    struct device_attribute *attr, char *buf)
+{
+	struct mhi_loopback *mhi_lb = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%u\n", mhi_lb->num_tre);
+}
+
+static ssize_t num_tre_store(struct device *dev,
+			     struct device_attribute *attr,
+			     const char *buf, size_t count)
+{
+	struct mhi_loopback *mhi_lb = dev_get_drvdata(dev);
+	u32 val;
+	int el_num;
+
+	if (kstrtou32(buf, 0, &val)) {
+		dev_err(dev, "Invalid num_tre value\n");
+		return -EINVAL;
+	}
+
+	if (val == 0) {
+		dev_err(dev, "Number of TREs cannot be zero\n");
+		return -EINVAL;
+	}
+
+	guard(mutex)(&mhi_lb->lb_mutex);
+	if (mhi_lb->loopback_in_progress)
+		return -EBUSY;
+
+	el_num = mhi_get_free_desc_count(mhi_lb->mdev, DMA_TO_DEVICE);
+	if (val > el_num) {
+		dev_err(dev, "num_tre (%u) exceeds ring capacity (%d)\n", val, el_num);
+		return -EINVAL;
+	}
+
+	mhi_lb->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 *mhi_lb = dev_get_drvdata(dev);
+	void *send_buf __free(kfree) = NULL;
+	void *recv_buf __free(kfree) = NULL;
+	u32 total_size, tre_count, tre_size;
+	int ret, i;
+
+	guard(mutex)(&mhi_lb->lb_mutex);
+
+	if (mhi_lb->loopback_in_progress)
+		return -EBUSY;
+
+	atomic_set(&mhi_lb->num_completions_received, 0);
+	mhi_lb->loopback_in_progress = true;
+
+	tre_size = mhi_lb->size;
+	tre_count = mhi_lb->num_tre;
+
+	strscpy(mhi_lb->result, "Loopback started", sizeof(mhi_lb->result));
+
+	total_size = tre_count * tre_size;
+
+	recv_buf = kzalloc(total_size, GFP_KERNEL);
+	if (!recv_buf) {
+		strscpy(mhi_lb->result, "Memory allocation failed", sizeof(mhi_lb->result));
+		mhi_lb->loopback_in_progress = false;
+		return -ENOMEM;
+	}
+
+	send_buf = kzalloc(total_size, GFP_KERNEL);
+	if (!send_buf) {
+		strscpy(mhi_lb->result, "Memory allocation failed", sizeof(mhi_lb->result));
+		mhi_lb->loopback_in_progress = false;
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < tre_count; i++) {
+		ret = mhi_queue_buf(mhi_lb->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);
+			strscpy(mhi_lb->result, "Queue tre failed", sizeof(mhi_lb->result));
+			mhi_lb->loopback_in_progress = false;
+			return ret;
+		}
+	}
+
+	get_random_bytes(send_buf, total_size);
+
+	reinit_completion(&mhi_lb->comp);
+
+	for (i = 0; i < tre_count - 1; i++) {
+		ret = mhi_queue_buf(mhi_lb->mdev, DMA_TO_DEVICE, send_buf + (i * tre_size),
+				    tre_size, MHI_CHAIN);
+		if (ret) {
+			dev_err(dev, "Unable to queue send TRE %d (chained): %d\n", i, ret);
+			strscpy(mhi_lb->result, "Queue send failed", sizeof(mhi_lb->result));
+			mhi_lb->loopback_in_progress = false;
+			return ret;
+		}
+	}
+
+	ret = mhi_queue_buf(mhi_lb->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);
+		strscpy(mhi_lb->result, "Queue final tre failed", sizeof(mhi_lb->result));
+		mhi_lb->loopback_in_progress = false;
+		return ret;
+	}
+
+	if (!wait_for_completion_timeout(&mhi_lb->comp,
+					 msecs_to_jiffies(MHI_LOOPBACK_TIMEOUT_MS))) {
+		strscpy(mhi_lb->result, "Loopback timeout", sizeof(mhi_lb->result));
+		dev_err(dev, "Loopback test timed out\n");
+		mhi_lb->loopback_in_progress = false;
+		return -ETIMEDOUT;
+	}
+
+	ret = memcmp(send_buf, recv_buf, total_size);
+	if (!ret) {
+		strscpy(mhi_lb->result, "Loopback successful", sizeof(mhi_lb->result));
+		dev_info(dev, "Loopback test passed\n");
+	} else {
+		strscpy(mhi_lb->result, "Loopback data mismatch", sizeof(mhi_lb->result));
+		dev_err(dev, "Loopback test failed\n");
+		ret = -EIO;
+	}
+
+	mhi_lb->loopback_in_progress = false;
+	return ret;
+}
+
+static DEVICE_ATTR_WO(start);
+
+static ssize_t status_show(struct device *dev,
+			   struct device_attribute *attr, char *buf)
+{
+	struct mhi_loopback *mhi_lb = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%s\n", mhi_lb->result);
+}
+static DEVICE_ATTR_RO(status);
+
+static void mhi_loopback_dl_callback(struct mhi_device *mhi_dev,
+				     struct mhi_result *mhi_res)
+{
+	struct mhi_loopback *mhi_lb = dev_get_drvdata(&mhi_dev->dev);
+
+	if (!mhi_res->transaction_status) {
+		if (atomic_inc_return(&mhi_lb->num_completions_received) >= mhi_lb->num_tre) {
+			atomic_set(&mhi_lb->num_completions_received, 0);
+			complete(&mhi_lb->comp);
+		}
+	} else {
+		dev_err(&mhi_dev->dev, "DL callback error: status %d\n",
+			mhi_res->transaction_status);
+		atomic_set(&mhi_lb->num_completions_received, 0);
+		complete(&mhi_lb->comp);
+	}
+}
+
+static void mhi_loopback_ul_callback(struct mhi_device *mhi_dev,
+				     struct mhi_result *mhi_res)
+{
+}
+
+static int mhi_loopback_probe(struct mhi_device *mhi_dev,
+			      const struct mhi_device_id *id)
+{
+	struct mhi_loopback *mhi_lb;
+	int rc;
+
+	mhi_lb = devm_kzalloc(&mhi_dev->dev, sizeof(*mhi_lb), GFP_KERNEL);
+	if (!mhi_lb)
+		return -ENOMEM;
+
+	mhi_lb->mdev = mhi_dev;
+
+	dev_set_drvdata(&mhi_dev->dev, mhi_lb);
+
+	mhi_lb->size = MHI_LOOPBACK_DEFAULT_TRE_SIZE;
+	mhi_lb->num_tre = MHI_LOOPBACK_DEFAULT_NUM_TRE;
+	mhi_lb->loopback_in_progress = false;
+
+	mutex_init(&mhi_lb->lb_mutex);
+	strscpy(mhi_lb->result, "Loopback not started", sizeof(mhi_lb->result));
+
+	rc = sysfs_create_file(&mhi_dev->dev.kobj, &dev_attr_size.attr);
+	if (rc) {
+		dev_err(&mhi_dev->dev, "failed to create size sysfs file\n");
+		goto out;
+	}
+
+	rc = sysfs_create_file(&mhi_dev->dev.kobj, &dev_attr_num_tre.attr);
+	if (rc) {
+		dev_err(&mhi_dev->dev, "failed to create num_tre sysfs file\n");
+		goto del_size_sysfs;
+	}
+
+	rc = sysfs_create_file(&mhi_dev->dev.kobj, &dev_attr_start.attr);
+	if (rc) {
+		dev_err(&mhi_dev->dev, "failed to create start sysfs file\n");
+		goto del_num_tre_sysfs;
+	}
+
+	rc = sysfs_create_file(&mhi_dev->dev.kobj, &dev_attr_status.attr);
+	if (rc) {
+		dev_err(&mhi_dev->dev, "failed to create status sysfs file\n");
+		goto del_start_sysfs;
+	}
+
+	rc = mhi_prepare_for_transfer(mhi_lb->mdev);
+	if (rc) {
+		dev_err(&mhi_dev->dev, "failed to prepare for transfers\n");
+		goto del_status_sysfs;
+	}
+
+	init_completion(&mhi_lb->comp);
+
+	return 0;
+
+del_status_sysfs:
+	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_status.attr);
+del_start_sysfs:
+	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_start.attr);
+del_num_tre_sysfs:
+	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_num_tre.attr);
+del_size_sysfs:
+	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_size.attr);
+out:
+	return rc;
+}
+
+static void mhi_loopback_remove(struct mhi_device *mhi_dev)
+{
+	struct mhi_loopback *mhi_lb = dev_get_drvdata(&mhi_dev->dev);
+
+	if (mhi_lb)
+		complete(&mhi_lb->comp);
+
+	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_status.attr);
+	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_start.attr);
+	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_num_tre.attr);
+	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_size.attr);
+	mhi_unprepare_from_transfer(mhi_dev);
+	dev_set_drvdata(&mhi_dev->dev, NULL);
+}
+
+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

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

[Bjorn Andersson]

On Tue, Nov 04, 2025 at 11:09:05AM +0530, Sumit Kumar wrote:
> Add loopback driver for MHI host controllers that provides sysfs based
  ^--- Here would be e good place to explain why we want this driver. Per
https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
start your commit message with a description of the problem you're
solving.

> testing interface for data path validation. The driver supports the
> "LOOPBACK" channel and offers configurable test parameters.
> 
> Sysfs interface provides:
> - size: Configure TRE size
> - num_tre: Set number of TREs for chained transfers
> - start: Initiate loopback test
> - status: Read test results

The words "loopback" and "testing" gives clear indications that this
should live in debugfs and not sysfs.

Also, sysfs attribute should be documented in Documentation/ABI/testing/

> 
> 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/host/Kconfig        |   7 +
>  drivers/bus/mhi/host/Makefile       |   1 +
>  drivers/bus/mhi/host/mhi_loopback.c | 347 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 355 insertions(+)
> 
> diff --git a/drivers/bus/mhi/host/Kconfig b/drivers/bus/mhi/host/Kconfig
> index da5cd0c9fc620ab595e742c422f1a22a2a84c7b9..08a39ecb47f585bf39721c101ed5e2ff44bdd5f8 100644
> --- a/drivers/bus/mhi/host/Kconfig
> +++ b/drivers/bus/mhi/host/Kconfig
> @@ -29,3 +29,10 @@ config MHI_BUS_PCI_GENERIC
>  	  This driver provides MHI PCI controller driver for devices such as
>  	  Qualcomm SDX55 based PCIe modems.
>  
> +config MHI_BUS_LOOPBACK
> +	tristate "MHI loopback driver"
> +	depends on MHI_BUS
> +	help
> +	  MHI loopback driver for data path testing. This driver
> +	  provides a mechanism to test MHI data transfer functionality
> +	  by implementing an echo service between host and endpoint.
> diff --git a/drivers/bus/mhi/host/Makefile b/drivers/bus/mhi/host/Makefile
> index 859c2f38451c669b3d3014c374b2b957c99a1cfe..e5d6dccf5a976eaeb827c47924ad0614c9958f8b 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-$(CONFIG_MHI_BUS_LOOPBACK) += mhi_loopback.o
> diff --git a/drivers/bus/mhi/host/mhi_loopback.c b/drivers/bus/mhi/host/mhi_loopback.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..980ace675718a79c97d9b2968ccef04c992a6c20
> --- /dev/null
> +++ b/drivers/bus/mhi/host/mhi_loopback.c
> @@ -0,0 +1,347 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include <linux/mhi.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/completion.h>
> +#include <linux/string.h>
> +#include <linux/random.h>
> +#include <linux/kernel.h>
> +#include <linux/sysfs.h>
> +#include <linux/types.h>
> +#include <linux/errno.h>
> +#include <linux/mutex.h>
> +#include <linux/atomic.h>
> +#include <linux/cleanup.h>
> +#include <linux/sizes.h>

Keep them sorted, and make sure you need all of them.

> +
> +#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
> +
> +struct mhi_loopback {
> +	struct mhi_device *mdev;
> +	struct mutex lb_mutex;
> +	struct completion comp;
> +	atomic_t num_completions_received;
> +	char result[32];
> +	u32 num_tre;
> +	u32 size;
> +	bool loopback_in_progress;
> +};
> +
> +static ssize_t size_show(struct device *dev,
> +			 struct device_attribute *attr, char *buf)
> +{
> +	struct mhi_loopback *mhi_lb = dev_get_drvdata(dev);
> +
> +	return sysfs_emit(buf, "%u\n", mhi_lb->size);
> +}
> +
> +static ssize_t size_store(struct device *dev,
> +			  struct device_attribute *attr,
> +			  const char *buf, size_t count)
> +{
> +	struct mhi_loopback *mhi_lb = dev_get_drvdata(dev);
> +	u32 val;
> +
> +	if (kstrtou32(buf, 0, &val)) {
> +		dev_err(dev, "Invalid size value\n");
> +		return -EINVAL;
> +	}
> +
> +	if (val == 0 || val > MHI_LOOPBACK_MAX_TRE_SIZE) {
> +		dev_err(dev, "Size must be between 1 and %u bytes\n",
> +			MHI_LOOPBACK_MAX_TRE_SIZE);
> +		return -EINVAL;
> +	}
> +
> +	guard(mutex)(&mhi_lb->lb_mutex);
> +	if (mhi_lb->loopback_in_progress)

The only time loopback_in_progress is true is between the beginning and
end of start_store(), and that entire function is under guard(lb_mutex),
just as here and in num_tre_store().

So at all times loopback_in_progress is true, any other context will
block on getting the mutex, and then it will be reset to false before
the mutex is let go.

In other words, loopback_in_progress is unnecessary.

> +		return -EBUSY;
> +
> +	mhi_lb->size = val;
> +	return count;
> +}
> +static DEVICE_ATTR_RW(size);
> +
> +static ssize_t num_tre_show(struct device *dev,
> +			    struct device_attribute *attr, char *buf)
> +{
> +	struct mhi_loopback *mhi_lb = dev_get_drvdata(dev);
> +
> +	return sysfs_emit(buf, "%u\n", mhi_lb->num_tre);
> +}
> +
> +static ssize_t num_tre_store(struct device *dev,
> +			     struct device_attribute *attr,
> +			     const char *buf, size_t count)
> +{
> +	struct mhi_loopback *mhi_lb = dev_get_drvdata(dev);
> +	u32 val;
> +	int el_num;
> +
> +	if (kstrtou32(buf, 0, &val)) {
> +		dev_err(dev, "Invalid num_tre value\n");
> +		return -EINVAL;
> +	}
> +
> +	if (val == 0) {
> +		dev_err(dev, "Number of TREs cannot be zero\n");
> +		return -EINVAL;
> +	}
> +
> +	guard(mutex)(&mhi_lb->lb_mutex);
> +	if (mhi_lb->loopback_in_progress)
> +		return -EBUSY;
> +
> +	el_num = mhi_get_free_desc_count(mhi_lb->mdev, DMA_TO_DEVICE);

Aren't the descs per-channel in MHI? Given that you have a mutex around
start_store() is this actually a dynamic value?

> +	if (val > el_num) {
> +		dev_err(dev, "num_tre (%u) exceeds ring capacity (%d)\n", val, el_num);
> +		return -EINVAL;
> +	}
> +
> +	mhi_lb->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 *mhi_lb = dev_get_drvdata(dev);
> +	void *send_buf __free(kfree) = NULL;
> +	void *recv_buf __free(kfree) = NULL;
> +	u32 total_size, tre_count, tre_size;
> +	int ret, i;
> +
> +	guard(mutex)(&mhi_lb->lb_mutex);
> +
> +	if (mhi_lb->loopback_in_progress)
> +		return -EBUSY;
> +
> +	atomic_set(&mhi_lb->num_completions_received, 0);
> +	mhi_lb->loopback_in_progress = true;
> +
> +	tre_size = mhi_lb->size;
> +	tre_count = mhi_lb->num_tre;
> +
> +	strscpy(mhi_lb->result, "Loopback started", sizeof(mhi_lb->result));

All assignments to result are static const strings being strscpy'ed into
the buffer, if you made result a const char * instead, you could just
assign the string.

> +
> +	total_size = tre_count * tre_size;
> +
> +	recv_buf = kzalloc(total_size, GFP_KERNEL);
> +	if (!recv_buf) {
> +		strscpy(mhi_lb->result, "Memory allocation failed", sizeof(mhi_lb->result));
> +		mhi_lb->loopback_in_progress = false;
> +		return -ENOMEM;

You're setting loopback_in_progress to false and returning in 7
different places in this function. There seems to be some room for
improvement here.

That said, as I said above, I don't think your code can ever find
loopback_in_progress to be true...

> +	}
> +
> +	send_buf = kzalloc(total_size, GFP_KERNEL);
> +	if (!send_buf) {
> +		strscpy(mhi_lb->result, "Memory allocation failed", sizeof(mhi_lb->result));
> +		mhi_lb->loopback_in_progress = false;
> +		return -ENOMEM;
> +	}
> +
> +	for (i = 0; i < tre_count; i++) {
> +		ret = mhi_queue_buf(mhi_lb->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);
> +			strscpy(mhi_lb->result, "Queue tre failed", sizeof(mhi_lb->result));
> +			mhi_lb->loopback_in_progress = false;
> +			return ret;
> +		}
> +	}
> +
> +	get_random_bytes(send_buf, total_size);
> +
> +	reinit_completion(&mhi_lb->comp);
> +
> +	for (i = 0; i < tre_count - 1; i++) {
> +		ret = mhi_queue_buf(mhi_lb->mdev, DMA_TO_DEVICE, send_buf + (i * tre_size),
> +				    tre_size, MHI_CHAIN);
> +		if (ret) {
> +			dev_err(dev, "Unable to queue send TRE %d (chained): %d\n", i, ret);
> +			strscpy(mhi_lb->result, "Queue send failed", sizeof(mhi_lb->result));
> +			mhi_lb->loopback_in_progress = false;
> +			return ret;
> +		}
> +	}
> +
> +	ret = mhi_queue_buf(mhi_lb->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);
> +		strscpy(mhi_lb->result, "Queue final tre failed", sizeof(mhi_lb->result));
> +		mhi_lb->loopback_in_progress = false;
> +		return ret;
> +	}
> +
> +	if (!wait_for_completion_timeout(&mhi_lb->comp,
> +					 msecs_to_jiffies(MHI_LOOPBACK_TIMEOUT_MS))) {
> +		strscpy(mhi_lb->result, "Loopback timeout", sizeof(mhi_lb->result));
> +		dev_err(dev, "Loopback test timed out\n");
> +		mhi_lb->loopback_in_progress = false;
> +		return -ETIMEDOUT;
> +	}
> +
> +	ret = memcmp(send_buf, recv_buf, total_size);
> +	if (!ret) {
> +		strscpy(mhi_lb->result, "Loopback successful", sizeof(mhi_lb->result));
> +		dev_info(dev, "Loopback test passed\n");

Why both print the test status and log it to the result? Less is more...

> +	} else {
> +		strscpy(mhi_lb->result, "Loopback data mismatch", sizeof(mhi_lb->result));
> +		dev_err(dev, "Loopback test failed\n");
> +		ret = -EIO;
> +	}
> +
> +	mhi_lb->loopback_in_progress = false;
> +	return ret;
> +}
> +
> +static DEVICE_ATTR_WO(start);
> +
> +static ssize_t status_show(struct device *dev,
> +			   struct device_attribute *attr, char *buf)
> +{
> +	struct mhi_loopback *mhi_lb = dev_get_drvdata(dev);
> +
> +	return sysfs_emit(buf, "%s\n", mhi_lb->result);
> +}
> +static DEVICE_ATTR_RO(status);
> +
> +static void mhi_loopback_dl_callback(struct mhi_device *mhi_dev,
> +				     struct mhi_result *mhi_res)
> +{
> +	struct mhi_loopback *mhi_lb = dev_get_drvdata(&mhi_dev->dev);
> +
> +	if (!mhi_res->transaction_status) {
> +		if (atomic_inc_return(&mhi_lb->num_completions_received) >= mhi_lb->num_tre) {
> +			atomic_set(&mhi_lb->num_completions_received, 0);
> +			complete(&mhi_lb->comp);
> +		}
> +	} else {
> +		dev_err(&mhi_dev->dev, "DL callback error: status %d\n",
> +			mhi_res->transaction_status);
> +		atomic_set(&mhi_lb->num_completions_received, 0);
> +		complete(&mhi_lb->comp);
> +	}
> +}
> +
> +static void mhi_loopback_ul_callback(struct mhi_device *mhi_dev,
> +				     struct mhi_result *mhi_res)
> +{
> +}
> +
> +static int mhi_loopback_probe(struct mhi_device *mhi_dev,
> +			      const struct mhi_device_id *id)
> +{
> +	struct mhi_loopback *mhi_lb;
> +	int rc;
> +
> +	mhi_lb = devm_kzalloc(&mhi_dev->dev, sizeof(*mhi_lb), GFP_KERNEL);
> +	if (!mhi_lb)
> +		return -ENOMEM;
> +
> +	mhi_lb->mdev = mhi_dev;
> +
> +	dev_set_drvdata(&mhi_dev->dev, mhi_lb);
> +
> +	mhi_lb->size = MHI_LOOPBACK_DEFAULT_TRE_SIZE;
> +	mhi_lb->num_tre = MHI_LOOPBACK_DEFAULT_NUM_TRE;
> +	mhi_lb->loopback_in_progress = false;

kzalloc() already did that for you.

> +
> +	mutex_init(&mhi_lb->lb_mutex);
> +	strscpy(mhi_lb->result, "Loopback not started", sizeof(mhi_lb->result));
> +
> +	rc = sysfs_create_file(&mhi_dev->dev.kobj, &dev_attr_size.attr);
> +	if (rc) {
> +		dev_err(&mhi_dev->dev, "failed to create size sysfs file\n");
> +		goto out;
> +	}
> +
> +	rc = sysfs_create_file(&mhi_dev->dev.kobj, &dev_attr_num_tre.attr);
> +	if (rc) {
> +		dev_err(&mhi_dev->dev, "failed to create num_tre sysfs file\n");
> +		goto del_size_sysfs;

This is ugly, devm_device_add_group() seems more appropriate. Then
again, I don't think this belongs in sysfs in the first place.

Regards,
Bjorn

> +	}
> +
> +	rc = sysfs_create_file(&mhi_dev->dev.kobj, &dev_attr_start.attr);
> +	if (rc) {
> +		dev_err(&mhi_dev->dev, "failed to create start sysfs file\n");
> +		goto del_num_tre_sysfs;
> +	}
> +
> +	rc = sysfs_create_file(&mhi_dev->dev.kobj, &dev_attr_status.attr);
> +	if (rc) {
> +		dev_err(&mhi_dev->dev, "failed to create status sysfs file\n");
> +		goto del_start_sysfs;
> +	}
> +
> +	rc = mhi_prepare_for_transfer(mhi_lb->mdev);
> +	if (rc) {
> +		dev_err(&mhi_dev->dev, "failed to prepare for transfers\n");
> +		goto del_status_sysfs;
> +	}
> +
> +	init_completion(&mhi_lb->comp);
> +
> +	return 0;
> +
> +del_status_sysfs:
> +	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_status.attr);
> +del_start_sysfs:
> +	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_start.attr);
> +del_num_tre_sysfs:
> +	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_num_tre.attr);
> +del_size_sysfs:
> +	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_size.attr);
> +out:
> +	return rc;
> +}
> +
> +static void mhi_loopback_remove(struct mhi_device *mhi_dev)
> +{
> +	struct mhi_loopback *mhi_lb = dev_get_drvdata(&mhi_dev->dev);
> +
> +	if (mhi_lb)
> +		complete(&mhi_lb->comp);
> +
> +	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_status.attr);
> +	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_start.attr);
> +	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_num_tre.attr);
> +	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_size.attr);
> +	mhi_unprepare_from_transfer(mhi_dev);
> +	dev_set_drvdata(&mhi_dev->dev, NULL);
> +}
> +
> +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
> 
> 

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

[Manivannan Sadhasivam]

On Wed, Nov 05, 2025 at 04:17:41PM -0600, Bjorn Andersson wrote:
> On Tue, Nov 04, 2025 at 11:09:05AM +0530, Sumit Kumar wrote:
> > Add loopback driver for MHI host controllers that provides sysfs based
>   ^--- Here would be e good place to explain why we want this driver. Per
> https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> start your commit message with a description of the problem you're
> solving.
> 
> > testing interface for data path validation. The driver supports the
> > "LOOPBACK" channel and offers configurable test parameters.
> > 
> > Sysfs interface provides:
> > - size: Configure TRE size
> > - num_tre: Set number of TREs for chained transfers
> > - start: Initiate loopback test
> > - status: Read test results
> 
> The words "loopback" and "testing" gives clear indications that this
> should live in debugfs and not sysfs.
> 

Though the wording gives an impression like that, this interface is for a MHI
channel that is defined in the MHI spec, so it is perfectly fine to have it
exposed as a sysfs interface to the users. This channel is intented to be used
for MHI protocol testing.

> Also, sysfs attribute should be documented in Documentation/ABI/testing/
> 

Yes, this is mandatory.

> > 
> > 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/host/Kconfig        |   7 +
> >  drivers/bus/mhi/host/Makefile       |   1 +
> >  drivers/bus/mhi/host/mhi_loopback.c | 347 ++++++++++++++++++++++++++++++++++++

Create a separate sub-directory 'clients' and move this driver there. Also,
rename it to just 'loopback', but keep the module name as 'mhi_loopback'.

> >  3 files changed, 355 insertions(+)
> > 
> > diff --git a/drivers/bus/mhi/host/Kconfig b/drivers/bus/mhi/host/Kconfig
> > index da5cd0c9fc620ab595e742c422f1a22a2a84c7b9..08a39ecb47f585bf39721c101ed5e2ff44bdd5f8 100644
> > --- a/drivers/bus/mhi/host/Kconfig
> > +++ b/drivers/bus/mhi/host/Kconfig
> > @@ -29,3 +29,10 @@ config MHI_BUS_PCI_GENERIC
> >  	  This driver provides MHI PCI controller driver for devices such as
> >  	  Qualcomm SDX55 based PCIe modems.
> >  
> > +config MHI_BUS_LOOPBACK
> > +	tristate "MHI loopback driver"

'MHI LOOPBACK client driver'

Use 'LOOPBACK' everywhere.

> > +	depends on MHI_BUS
> > +	help
> > +	  MHI loopback driver for data path testing. This driver
> > +	  provides a mechanism to test MHI data transfer functionality
> > +	  by implementing an echo service between host and endpoint.

Again, this just sounds like this driver exposes a random interface for testing.
You need to properly describe that this driver binds to the MHI LOOPBACK channel
and offers the testing interface to the users.

> > diff --git a/drivers/bus/mhi/host/Makefile b/drivers/bus/mhi/host/Makefile
> > index 859c2f38451c669b3d3014c374b2b957c99a1cfe..e5d6dccf5a976eaeb827c47924ad0614c9958f8b 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-$(CONFIG_MHI_BUS_LOOPBACK) += mhi_loopback.o
> > diff --git a/drivers/bus/mhi/host/mhi_loopback.c b/drivers/bus/mhi/host/mhi_loopback.c
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..980ace675718a79c97d9b2968ccef04c992a6c20
> > --- /dev/null
> > +++ b/drivers/bus/mhi/host/mhi_loopback.c
> > @@ -0,0 +1,347 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > + */
> > +
> > +#include <linux/mhi.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/module.h>
> > +#include <linux/completion.h>
> > +#include <linux/string.h>
> > +#include <linux/random.h>
> > +#include <linux/kernel.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/types.h>
> > +#include <linux/errno.h>
> > +#include <linux/mutex.h>
> > +#include <linux/atomic.h>
> > +#include <linux/cleanup.h>
> > +#include <linux/sizes.h>
> 
> Keep them sorted, and make sure you need all of them.
> 
> > +
> > +#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
> > +
> > +struct mhi_loopback {
> > +	struct mhi_device *mdev;
> > +	struct mutex lb_mutex;
> > +	struct completion comp;
> > +	atomic_t num_completions_received;
> > +	char result[32];
> > +	u32 num_tre;
> > +	u32 size;
> > +	bool loopback_in_progress;
> > +};
> > +
> > +static ssize_t size_show(struct device *dev,
> > +			 struct device_attribute *attr, char *buf)
> > +{
> > +	struct mhi_loopback *mhi_lb = dev_get_drvdata(dev);

s/mhi_lb/loopback

> > +
> > +	return sysfs_emit(buf, "%u\n", mhi_lb->size);
> > +}
> > +
> > +static ssize_t size_store(struct device *dev,
> > +			  struct device_attribute *attr,
> > +			  const char *buf, size_t count)
> > +{
> > +	struct mhi_loopback *mhi_lb = dev_get_drvdata(dev);
> > +	u32 val;
> > +
> > +	if (kstrtou32(buf, 0, &val)) {
> > +		dev_err(dev, "Invalid size value\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (val == 0 || val > MHI_LOOPBACK_MAX_TRE_SIZE) {
> > +		dev_err(dev, "Size must be between 1 and %u bytes\n",
> > +			MHI_LOOPBACK_MAX_TRE_SIZE);

How the user is supposed to know the size limit if it is just hardcoded in the
driver? You need to expose it as a RO attribute.

Also, 'size' here is the TRE size, not the size of the buffer. So name it as
such.

> > +		return -EINVAL;
> > +	}
> > +
> > +	guard(mutex)(&mhi_lb->lb_mutex);
> > +	if (mhi_lb->loopback_in_progress)
> 
> The only time loopback_in_progress is true is between the beginning and
> end of start_store(), and that entire function is under guard(lb_mutex),
> just as here and in num_tre_store().
> 
> So at all times loopback_in_progress is true, any other context will
> block on getting the mutex, and then it will be reset to false before
> the mutex is let go.
> 
> In other words, loopback_in_progress is unnecessary.
> 
> > +		return -EBUSY;
> > +
> > +	mhi_lb->size = val;
> > +	return count;
> > +}
> > +static DEVICE_ATTR_RW(size);
> > +
> > +static ssize_t num_tre_show(struct device *dev,
> > +			    struct device_attribute *attr, char *buf)
> > +{
> > +	struct mhi_loopback *mhi_lb = dev_get_drvdata(dev);
> > +
> > +	return sysfs_emit(buf, "%u\n", mhi_lb->num_tre);
> > +}
> > +
> > +static ssize_t num_tre_store(struct device *dev,
> > +			     struct device_attribute *attr,
> > +			     const char *buf, size_t count)
> > +{
> > +	struct mhi_loopback *mhi_lb = dev_get_drvdata(dev);
> > +	u32 val;
> > +	int el_num;
> > +
> > +	if (kstrtou32(buf, 0, &val)) {
> > +		dev_err(dev, "Invalid num_tre value\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (val == 0) {
> > +		dev_err(dev, "Number of TREs cannot be zero\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	guard(mutex)(&mhi_lb->lb_mutex);
> > +	if (mhi_lb->loopback_in_progress)
> > +		return -EBUSY;
> > +
> > +	el_num = mhi_get_free_desc_count(mhi_lb->mdev, DMA_TO_DEVICE);
> 
> Aren't the descs per-channel in MHI? Given that you have a mutex around
> start_store() is this actually a dynamic value?
> 
> > +	if (val > el_num) {
> > +		dev_err(dev, "num_tre (%u) exceeds ring capacity (%d)\n", val, el_num);
> > +		return -EINVAL;
> > +	}
> > +
> > +	mhi_lb->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 *mhi_lb = dev_get_drvdata(dev);
> > +	void *send_buf __free(kfree) = NULL;
> > +	void *recv_buf __free(kfree) = NULL;
> > +	u32 total_size, tre_count, tre_size;
> > +	int ret, i;
> > +
> > +	guard(mutex)(&mhi_lb->lb_mutex);
> > +
> > +	if (mhi_lb->loopback_in_progress)
> > +		return -EBUSY;
> > +
> > +	atomic_set(&mhi_lb->num_completions_received, 0);
> > +	mhi_lb->loopback_in_progress = true;
> > +
> > +	tre_size = mhi_lb->size;
> > +	tre_count = mhi_lb->num_tre;
> > +
> > +	strscpy(mhi_lb->result, "Loopback started", sizeof(mhi_lb->result));
> 

Do not print these progress reports in the 'status', just print the end result.
It can be a blocking read.

> All assignments to result are static const strings being strscpy'ed into
> the buffer, if you made result a const char * instead, you could just
> assign the string.
> 
> > +
> > +	total_size = tre_count * tre_size;
> > +
> > +	recv_buf = kzalloc(total_size, GFP_KERNEL);
> > +	if (!recv_buf) {
> > +		strscpy(mhi_lb->result, "Memory allocation failed", sizeof(mhi_lb->result));

When kzalloc() fails, it will itself print the error log.

> > +		mhi_lb->loopback_in_progress = false;
> > +		return -ENOMEM;
> 
> You're setting loopback_in_progress to false and returning in 7
> different places in this function. There seems to be some room for
> improvement here.
> 
> That said, as I said above, I don't think your code can ever find
> loopback_in_progress to be true...
> 
> > +	}
> > +
> > +	send_buf = kzalloc(total_size, GFP_KERNEL);
> > +	if (!send_buf) {
> > +		strscpy(mhi_lb->result, "Memory allocation failed", sizeof(mhi_lb->result));
> > +		mhi_lb->loopback_in_progress = false;
> > +		return -ENOMEM;
> > +	}
> > +
> > +	for (i = 0; i < tre_count; i++) {
> > +		ret = mhi_queue_buf(mhi_lb->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);
> > +			strscpy(mhi_lb->result, "Queue tre failed", sizeof(mhi_lb->result));
> > +			mhi_lb->loopback_in_progress = false;
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	get_random_bytes(send_buf, total_size);
> > +
> > +	reinit_completion(&mhi_lb->comp);
> > +
> > +	for (i = 0; i < tre_count - 1; i++) {
> > +		ret = mhi_queue_buf(mhi_lb->mdev, DMA_TO_DEVICE, send_buf + (i * tre_size),
> > +				    tre_size, MHI_CHAIN);
> > +		if (ret) {
> > +			dev_err(dev, "Unable to queue send TRE %d (chained): %d\n", i, ret);
> > +			strscpy(mhi_lb->result, "Queue send failed", sizeof(mhi_lb->result));
> > +			mhi_lb->loopback_in_progress = false;
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	ret = mhi_queue_buf(mhi_lb->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);
> > +		strscpy(mhi_lb->result, "Queue final tre failed", sizeof(mhi_lb->result));
> > +		mhi_lb->loopback_in_progress = false;
> > +		return ret;
> > +	}
> > +
> > +	if (!wait_for_completion_timeout(&mhi_lb->comp,
> > +					 msecs_to_jiffies(MHI_LOOPBACK_TIMEOUT_MS))) {
> > +		strscpy(mhi_lb->result, "Loopback timeout", sizeof(mhi_lb->result));
> > +		dev_err(dev, "Loopback test timed out\n");
> > +		mhi_lb->loopback_in_progress = false;
> > +		return -ETIMEDOUT;
> > +	}
> > +
> > +	ret = memcmp(send_buf, recv_buf, total_size);
> > +	if (!ret) {
> > +		strscpy(mhi_lb->result, "Loopback successful", sizeof(mhi_lb->result));
> > +		dev_info(dev, "Loopback test passed\n");
> 
> Why both print the test status and log it to the result? Less is more...
> 

Yes, I do find it quite annoying to see both getting passed on. Use dev_info()
to print the detailed error logs and just return the 'pass' or 'fail' status to
the user via the buffer.

> > +	} else {
> > +		strscpy(mhi_lb->result, "Loopback data mismatch", sizeof(mhi_lb->result));
> > +		dev_err(dev, "Loopback test failed\n");
> > +		ret = -EIO;
> > +	}
> > +
> > +	mhi_lb->loopback_in_progress = false;
> > +	return ret;
> > +}
> > +
> > +static DEVICE_ATTR_WO(start);
> > +
> > +static ssize_t status_show(struct device *dev,
> > +			   struct device_attribute *attr, char *buf)
> > +{
> > +	struct mhi_loopback *mhi_lb = dev_get_drvdata(dev);
> > +
> > +	return sysfs_emit(buf, "%s\n", mhi_lb->result);

Don't you need to have some lock or sync here?

> > +}
> > +static DEVICE_ATTR_RO(status);
> > +
> > +static void mhi_loopback_dl_callback(struct mhi_device *mhi_dev,
> > +				     struct mhi_result *mhi_res)
> > +{
> > +	struct mhi_loopback *mhi_lb = dev_get_drvdata(&mhi_dev->dev);
> > +
> > +	if (!mhi_res->transaction_status) {
> > +		if (atomic_inc_return(&mhi_lb->num_completions_received) >= mhi_lb->num_tre) {
> > +			atomic_set(&mhi_lb->num_completions_received, 0);
> > +			complete(&mhi_lb->comp);
> > +		}
> > +	} else {
> > +		dev_err(&mhi_dev->dev, "DL callback error: status %d\n",
> > +			mhi_res->transaction_status);
> > +		atomic_set(&mhi_lb->num_completions_received, 0);
> > +		complete(&mhi_lb->comp);
> > +	}
> > +}
> > +
> > +static void mhi_loopback_ul_callback(struct mhi_device *mhi_dev,
> > +				     struct mhi_result *mhi_res)
> > +{
> > +}
> > +
> > +static int mhi_loopback_probe(struct mhi_device *mhi_dev,
> > +			      const struct mhi_device_id *id)
> > +{
> > +	struct mhi_loopback *mhi_lb;
> > +	int rc;
> > +
> > +	mhi_lb = devm_kzalloc(&mhi_dev->dev, sizeof(*mhi_lb), GFP_KERNEL);
> > +	if (!mhi_lb)
> > +		return -ENOMEM;
> > +
> > +	mhi_lb->mdev = mhi_dev;
> > +
> > +	dev_set_drvdata(&mhi_dev->dev, mhi_lb);
> > +
> > +	mhi_lb->size = MHI_LOOPBACK_DEFAULT_TRE_SIZE;
> > +	mhi_lb->num_tre = MHI_LOOPBACK_DEFAULT_NUM_TRE;
> > +	mhi_lb->loopback_in_progress = false;
> 
> kzalloc() already did that for you.
> 
> > +
> > +	mutex_init(&mhi_lb->lb_mutex);
> > +	strscpy(mhi_lb->result, "Loopback not started", sizeof(mhi_lb->result));
> > +
> > +	rc = sysfs_create_file(&mhi_dev->dev.kobj, &dev_attr_size.attr);
> > +	if (rc) {
> > +		dev_err(&mhi_dev->dev, "failed to create size sysfs file\n");
> > +		goto out;
> > +	}
> > +
> > +	rc = sysfs_create_file(&mhi_dev->dev.kobj, &dev_attr_num_tre.attr);
> > +	if (rc) {
> > +		dev_err(&mhi_dev->dev, "failed to create num_tre sysfs file\n");
> > +		goto del_size_sysfs;
> 
> This is ugly, devm_device_add_group() seems more appropriate. Then
> again, I don't think this belongs in sysfs in the first place.
> 
> Regards,
> Bjorn
> 
> > +	}
> > +
> > +	rc = sysfs_create_file(&mhi_dev->dev.kobj, &dev_attr_start.attr);
> > +	if (rc) {
> > +		dev_err(&mhi_dev->dev, "failed to create start sysfs file\n");
> > +		goto del_num_tre_sysfs;
> > +	}
> > +
> > +	rc = sysfs_create_file(&mhi_dev->dev.kobj, &dev_attr_status.attr);
> > +	if (rc) {
> > +		dev_err(&mhi_dev->dev, "failed to create status sysfs file\n");
> > +		goto del_start_sysfs;
> > +	}
> > +
> > +	rc = mhi_prepare_for_transfer(mhi_lb->mdev);
> > +	if (rc) {
> > +		dev_err(&mhi_dev->dev, "failed to prepare for transfers\n");
> > +		goto del_status_sysfs;
> > +	}
> > +
> > +	init_completion(&mhi_lb->comp);
> > +
> > +	return 0;
> > +
> > +del_status_sysfs:
> > +	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_status.attr);
> > +del_start_sysfs:
> > +	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_start.attr);
> > +del_num_tre_sysfs:
> > +	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_num_tre.attr);
> > +del_size_sysfs:
> > +	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_size.attr);
> > +out:
> > +	return rc;
> > +}
> > +
> > +static void mhi_loopback_remove(struct mhi_device *mhi_dev)
> > +{
> > +	struct mhi_loopback *mhi_lb = dev_get_drvdata(&mhi_dev->dev);
> > +
> > +	if (mhi_lb)

Does this check make sense?

> > +		complete(&mhi_lb->comp);
> > +
> > +	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_status.attr);
> > +	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_start.attr);
> > +	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_num_tre.attr);
> > +	sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_size.attr);

Move to attribute group as Bjorn suggested.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

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

[Bjorn Andersson]

On Fri, Nov 07, 2025 at 05:58:18PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Nov 05, 2025 at 04:17:41PM -0600, Bjorn Andersson wrote:
> > On Tue, Nov 04, 2025 at 11:09:05AM +0530, Sumit Kumar wrote:
> > > Add loopback driver for MHI host controllers that provides sysfs based
> >   ^--- Here would be e good place to explain why we want this driver. Per
> > https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> > start your commit message with a description of the problem you're
> > solving.
> > 
> > > testing interface for data path validation. The driver supports the
> > > "LOOPBACK" channel and offers configurable test parameters.
> > > 
> > > Sysfs interface provides:
> > > - size: Configure TRE size
> > > - num_tre: Set number of TREs for chained transfers
> > > - start: Initiate loopback test
> > > - status: Read test results
> > 
> > The words "loopback" and "testing" gives clear indications that this
> > should live in debugfs and not sysfs.
> > 
> 
> Though the wording gives an impression like that, this interface is for a MHI
> channel that is defined in the MHI spec, so it is perfectly fine to have it
> exposed as a sysfs interface to the users. This channel is intented to be used
> for MHI protocol testing.
> 

The fact that the protocol is defined in the specification doesn't imply
that it's intended to be used by the typical user.

Also, the specification defines the LOOPBACK channel, it doesn't define
an interface where the user can request that a certain number of packets
of random data is sent and if those come back we can learn about that
from a "results" file. Downstream has a completely different
implementation of the same specification.

I could imagine that at some point one would want extend this test
interface by altering the behavior of the packet generator, inject
timestamps etc - to measure raw throughput, latency, jitter etc. Good
reasons for not turning this into an ABI.


Thinking more about the use case, I also presume most MHI devices has a
LOOPBACK channel, so every user is going to have this .ko auto-loaded,
just so that they can poke sysfs to send some random data... So perhaps
we should omit MODULE_DEVICE_TABLE()?


That said, these are merely my humble opinions. If you think it's
broadly useful in this form, please proceed.

Regards,
Bjorn

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

[Manivannan Sadhasivam]

On Sun, Nov 09, 2025 at 10:00:26PM -0600, Bjorn Andersson wrote:
> On Fri, Nov 07, 2025 at 05:58:18PM +0530, Manivannan Sadhasivam wrote:
> > On Wed, Nov 05, 2025 at 04:17:41PM -0600, Bjorn Andersson wrote:
> > > On Tue, Nov 04, 2025 at 11:09:05AM +0530, Sumit Kumar wrote:
> > > > Add loopback driver for MHI host controllers that provides sysfs based
> > >   ^--- Here would be e good place to explain why we want this driver. Per
> > > https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> > > start your commit message with a description of the problem you're
> > > solving.
> > > 
> > > > testing interface for data path validation. The driver supports the
> > > > "LOOPBACK" channel and offers configurable test parameters.
> > > > 
> > > > Sysfs interface provides:
> > > > - size: Configure TRE size
> > > > - num_tre: Set number of TREs for chained transfers
> > > > - start: Initiate loopback test
> > > > - status: Read test results
> > > 
> > > The words "loopback" and "testing" gives clear indications that this
> > > should live in debugfs and not sysfs.
> > > 
> > 
> > Though the wording gives an impression like that, this interface is for a MHI
> > channel that is defined in the MHI spec, so it is perfectly fine to have it
> > exposed as a sysfs interface to the users. This channel is intented to be used
> > for MHI protocol testing.
> > 
> 
> The fact that the protocol is defined in the specification doesn't imply
> that it's intended to be used by the typical user.
> 
> Also, the specification defines the LOOPBACK channel, it doesn't define
> an interface where the user can request that a certain number of packets
> of random data is sent and if those come back we can learn about that
> from a "results" file. Downstream has a completely different
> implementation of the same specification.
> 
> I could imagine that at some point one would want extend this test
> interface by altering the behavior of the packet generator, inject
> timestamps etc - to measure raw throughput, latency, jitter etc. Good
> reasons for not turning this into an ABI.
> 

I missed one important point while replying. This channel has an existing
interface between the host and endpoint i.e., the MHI based WLAN and modems out
there in the field already support an *interface* on top of this channel. We
cannot control the interface unless we change the endpoint firmware (which is
not feasible). And all the future extensions could only be possible if we could
modify the endpoint fw.

So for this specific reason, going by the sysfs ABI made sense to me. But to
admit, this point was never mentioned in the cover letter or in the patch
description and need to be fixed.

I also asked the Qcom PCIe team to verify that this interface, just works with
all kind of MHI devices if the devices support the LOOPBACK channel.

> 
> Thinking more about the use case, I also presume most MHI devices has a
> LOOPBACK channel, so every user is going to have this .ko auto-loaded,
> just so that they can poke sysfs to send some random data... So perhaps
> we should omit MODULE_DEVICE_TABLE()?
> 

I don't want to omit the MODULE_DEVICE_TABLE() since that will require users to
load the driver manually to use it. If users do not want this interface, then
they can always decide not to enable the driver while building the kernel :)

- Mani

-- 
மணிவண்ணன் சதாசிவம்

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

[Bjorn Andersson]

On Mon, Nov 10, 2025 at 11:56:11AM +0530, Manivannan Sadhasivam wrote:
> On Sun, Nov 09, 2025 at 10:00:26PM -0600, Bjorn Andersson wrote:
> > On Fri, Nov 07, 2025 at 05:58:18PM +0530, Manivannan Sadhasivam wrote:
> > > On Wed, Nov 05, 2025 at 04:17:41PM -0600, Bjorn Andersson wrote:
> > > > On Tue, Nov 04, 2025 at 11:09:05AM +0530, Sumit Kumar wrote:
> > > > > Add loopback driver for MHI host controllers that provides sysfs based
> > > >   ^--- Here would be e good place to explain why we want this driver. Per
> > > > https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> > > > start your commit message with a description of the problem you're
> > > > solving.
> > > > 
> > > > > testing interface for data path validation. The driver supports the
> > > > > "LOOPBACK" channel and offers configurable test parameters.
> > > > > 
> > > > > Sysfs interface provides:
> > > > > - size: Configure TRE size
> > > > > - num_tre: Set number of TREs for chained transfers
> > > > > - start: Initiate loopback test
> > > > > - status: Read test results
> > > > 
> > > > The words "loopback" and "testing" gives clear indications that this
> > > > should live in debugfs and not sysfs.
> > > > 
> > > 
> > > Though the wording gives an impression like that, this interface is for a MHI
> > > channel that is defined in the MHI spec, so it is perfectly fine to have it
> > > exposed as a sysfs interface to the users. This channel is intented to be used
> > > for MHI protocol testing.
> > > 
> > 
> > The fact that the protocol is defined in the specification doesn't imply
> > that it's intended to be used by the typical user.
> > 
> > Also, the specification defines the LOOPBACK channel, it doesn't define
> > an interface where the user can request that a certain number of packets
> > of random data is sent and if those come back we can learn about that
> > from a "results" file. Downstream has a completely different
> > implementation of the same specification.
> > 
> > I could imagine that at some point one would want extend this test
> > interface by altering the behavior of the packet generator, inject
> > timestamps etc - to measure raw throughput, latency, jitter etc. Good
> > reasons for not turning this into an ABI.
> > 
> 
> I missed one important point while replying. This channel has an existing
> interface between the host and endpoint i.e., the MHI based WLAN and modems out
> there in the field already support an *interface* on top of this channel.

But, that defines what happens "on the wire", right?

The MHI specification doesn't state that you have to expose this to the
user in the form of taking a "size" and a "num_tre" number and then
expose a "start" knob which performs a loopback test, of which result
you can then find in a "status" file?

> We
> cannot control the interface unless we change the endpoint firmware (which is
> not feasible).

You're already changing the user-facing interface, because downstream
this is implemented using UCI...and I bet it's not sysfs files in
Windows...

> And all the future extensions could only be possible if we could
> modify the endpoint fw.
> 

I'm not talking about extensions to the protocol, or how the endpoint is
supposed to react.

I'm saying that it should be perfectly fine to have a test that just
fills the out-pipe with packets containing say the current timestamp,
and when they return I compare the timestamp in the payload with the
clock to see how long time the particular message took to loop back.
Same LOOPBACK protocol, different user-interface.

> So for this specific reason, going by the sysfs ABI made sense to me. But to
> admit, this point was never mentioned in the cover letter or in the patch
> description and need to be fixed.
> 

I think you should include a comment in the driver as well, stating that
the user-facing interface is ABI and the MHI LOOPBACK protocol defines
that the payload has to be random etc. If that is the case...

> I also asked the Qcom PCIe team to verify that this interface, just works with
> all kind of MHI devices if the devices support the LOOPBACK channel.
> 
> > 
> > Thinking more about the use case, I also presume most MHI devices has a
> > LOOPBACK channel, so every user is going to have this .ko auto-loaded,
> > just so that they can poke sysfs to send some random data... So perhaps
> > we should omit MODULE_DEVICE_TABLE()?
> > 
> 
> I don't want to omit the MODULE_DEVICE_TABLE() since that will require users to
> load the driver manually to use it.

Fair enough.

> If users do not want this interface, then
> they can always decide not to enable the driver while building the kernel :)
> 

I expect even you will use this interface approximately 0% of the times
you boot the kernel. _Users_ will never use it.

So, I guess per your suggestion, we leave this disabled everywhere
(defconfig, distros etc), and then those few _developers_ who want it
can enable it?

Regards,
Bjorn

> - Mani
> 
> -- 
> மணிவண்ணன் சதாசிவம்

[PATCH v2 2/3] bus: mhi: ep: Create mhi_ep_queue_buf API for raw buffer queuing

[Sumit Kumar]

Create and export a new mhi_ep_queue_buf() API that allows raw buffer
queuing for client not using skb.

Extract core logic for queuing buffers into a new internal mhi_ep_queue()
function that provides a unified implementation for both mhi_ep_queue_skb()
and mhi_ep_queue_buf(). This internal function uses a cb_buf parameter to
handle both socket buffers and raw buffers through the same code path.
---
 drivers/bus/mhi/ep/main.c | 23 +++++++++++++++++------
 include/linux/mhi_ep.h    | 10 ++++++++++
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
index b3eafcf2a2c50d95e3efd3afb27038ecf55552a5..f4b119a8dca2dbfb3ffc24b04c85743fb57088fd 100644
--- a/drivers/bus/mhi/ep/main.c
+++ b/drivers/bus/mhi/ep/main.c
@@ -544,9 +544,9 @@ 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;
@@ -559,7 +559,7 @@ int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
 	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);
@@ -582,13 +582,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_buf = cb_buf;
 		buf_info.mhi_dev = mhi_dev;
 
 		/*
@@ -627,8 +627,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 7b40fc8cbe77ab8419d167e89264b69a817b9fb1..7186eb667b081009927af48513519084fb0be3a6 100644
--- a/include/linux/mhi_ep.h
+++ b/include/linux/mhi_ep.h
@@ -302,4 +302,14 @@ 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 - Send buffer to host over MHI Endpoint
+ * @mhi_dev: Device associated with the DL channel
+ * @buf: Buffer to be queued
+ * @len: Size of the buffer
+ *
+ * Return: 0 if the buffer has been sent 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

Re: [PATCH v2 2/3] bus: mhi: ep: Create mhi_ep_queue_buf API for raw buffer queuing

[Bjorn Andersson]

On Tue, Nov 04, 2025 at 11:09:06AM +0530, Sumit Kumar wrote:
> Create and export a new mhi_ep_queue_buf() API that allows raw buffer
> queuing for client not using skb.

Start with make it clear why this patch is desired. Why would such
clients exist, can't they just allocate some skbs?

> 
> Extract core logic for queuing buffers into a new internal mhi_ep_queue()
> function that provides a unified implementation for both mhi_ep_queue_skb()
> and mhi_ep_queue_buf(). This internal function uses a cb_buf parameter to
> handle both socket buffers and raw buffers through the same code path.

No signed-off-by?

> ---
>  drivers/bus/mhi/ep/main.c | 23 +++++++++++++++++------
>  include/linux/mhi_ep.h    | 10 ++++++++++
>  2 files changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
> index b3eafcf2a2c50d95e3efd3afb27038ecf55552a5..f4b119a8dca2dbfb3ffc24b04c85743fb57088fd 100644
> --- a/drivers/bus/mhi/ep/main.c
> +++ b/drivers/bus/mhi/ep/main.c
> @@ -544,9 +544,9 @@ static void mhi_ep_skb_completion(struct mhi_ep_buf_info *buf_info)
>  
>  	mhi_ep_ring_inc_index(ring);
>  }
> -

I'm pretty sure we want that line.

>  /* 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;
> @@ -559,7 +559,7 @@ int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, struct sk_buff *skb)
>  	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);
> @@ -582,13 +582,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_buf = cb_buf;
>  		buf_info.mhi_dev = mhi_dev;
>  
>  		/*
> @@ -627,8 +627,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 7b40fc8cbe77ab8419d167e89264b69a817b9fb1..7186eb667b081009927af48513519084fb0be3a6 100644
> --- a/include/linux/mhi_ep.h
> +++ b/include/linux/mhi_ep.h
> @@ -302,4 +302,14 @@ 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 - Send buffer to host over MHI Endpoint
> + * @mhi_dev: Device associated with the DL channel
> + * @buf: Buffer to be queued
> + * @len: Size of the buffer
> + *
> + * Return: 0 if the buffer has been sent successfully, a negative error code otherwise.

Sent or queued?

Regards,
Bjorn

> + */
> +int mhi_ep_queue_buf(struct mhi_ep_device *mhi_dev, void *buf, size_t len);
> +
>  #endif
> 
> -- 
> 2.34.1
> 
> 

[PATCH v2 3/3] bus: mhi: ep: Add loopback driver for data path testing

[Sumit Kumar]

Add loopback driver for MHI endpoint devices. The driver receives
data on the uplink channel and echoes it back on the downlink
channel using a workqueue for asynchronous processing.

The driver is useful for testing MHI endpoint data path functionality
and debugging communication issues.

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           |   8 +++
 drivers/bus/mhi/ep/Makefile          |   1 +
 drivers/bus/mhi/ep/mhi_ep_loopback.c | 134 +++++++++++++++++++++++++++++++++++
 3 files changed, 143 insertions(+)

diff --git a/drivers/bus/mhi/ep/Kconfig b/drivers/bus/mhi/ep/Kconfig
index 90ab3b040672e0f04181d4802e3062afcc7cf782..ce7b63c2da82a6ca49528517687f4910552c35bb 100644
--- a/drivers/bus/mhi/ep/Kconfig
+++ b/drivers/bus/mhi/ep/Kconfig
@@ -8,3 +8,11 @@ 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.
+
+config MHI_BUS_EP_LOOPBACK
+	tristate "MHI Endpoint loopback driver"
+	depends on MHI_BUS_EP
+	help
+	  MHI endpoint loopback driver for data path testing.
+	  This driver receives data on the uplink channel and echoes
+	  it back on the downlink channel for testing purposes.
diff --git a/drivers/bus/mhi/ep/Makefile b/drivers/bus/mhi/ep/Makefile
index aad85f180b707fb997fcb541837eda9bbbb67437..02e4700e8dc3f860d40290476b0a852286683f8f 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-$(CONFIG_MHI_BUS_EP_LOOPBACK) += mhi_ep_loopback.o
diff --git a/drivers/bus/mhi/ep/mhi_ep_loopback.c b/drivers/bus/mhi/ep/mhi_ep_loopback.c
new file mode 100644
index 0000000000000000000000000000000000000000..ba6154dd9b785f051043c10a980ab340012ba986
--- /dev/null
+++ b/drivers/bus/mhi/ep/mhi_ep_loopback.c
@@ -0,0 +1,134 @@
+// 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>
+
+struct mhi_ep_loopback {
+	struct workqueue_struct *loopback_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 send the packet\n");
+		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_res->transaction_status)) {
+		buf = kmalloc(mhi_res->bytes_xferd, GFP_KERNEL);
+		if (!buf) {
+			dev_err(&mhi_dev->dev, "Failed to allocate buffer\n");
+			return;
+		}
+
+		memcpy(buf, mhi_res->buf_addr, mhi_res->bytes_xferd);
+
+		mhi_ep_lb_work = kmalloc(sizeof(*mhi_ep_lb_work), GFP_KERNEL);
+		if (!mhi_ep_lb_work) {
+			dev_err(&mhi_dev->dev, "Unable to allocate the work structure\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->loopback_wq, &mhi_ep_lb_work->work);
+	}
+}
+
+static void mhi_ep_loopback_dl_callback(struct mhi_ep_device *mhi_dev,
+					struct mhi_result *mhi_res)
+{
+	void *buf;
+
+	if (mhi_res->transaction_status)
+		return;
+
+	buf = mhi_res->buf_addr;
+	if (buf)
+		kfree(buf);
+}
+
+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(struct mhi_ep_loopback), GFP_KERNEL);
+	if (!mhi_ep_lb)
+		return -ENOMEM;
+
+	mhi_ep_lb->loopback_wq = alloc_ordered_workqueue("mhi_loopback", WQ_MEM_RECLAIM);
+	if (!mhi_ep_lb->loopback_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->loopback_wq);
+	dev_set_drvdata(&mhi_dev->dev, NULL);
+}
+
+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",
+		.owner = THIS_MODULE,
+	},
+};
+
+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

Re: [PATCH v2 3/3] bus: mhi: ep: Add loopback driver for data path testing

[Bjorn Andersson]

On Tue, Nov 04, 2025 at 11:09:07AM +0530, Sumit Kumar wrote:
> Add loopback driver for MHI endpoint devices. The driver receives

Start by establishing why we want this.

> data on the uplink channel and echoes it back on the downlink
> channel using a workqueue for asynchronous processing.
> 
> The driver is useful for testing MHI endpoint data path functionality
> and debugging communication issues.
> 
> 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           |   8 +++
>  drivers/bus/mhi/ep/Makefile          |   1 +
>  drivers/bus/mhi/ep/mhi_ep_loopback.c | 134 +++++++++++++++++++++++++++++++++++
>  3 files changed, 143 insertions(+)
> 
> diff --git a/drivers/bus/mhi/ep/Kconfig b/drivers/bus/mhi/ep/Kconfig
> index 90ab3b040672e0f04181d4802e3062afcc7cf782..ce7b63c2da82a6ca49528517687f4910552c35bb 100644
> --- a/drivers/bus/mhi/ep/Kconfig
> +++ b/drivers/bus/mhi/ep/Kconfig
> @@ -8,3 +8,11 @@ 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.
> +
> +config MHI_BUS_EP_LOOPBACK
> +	tristate "MHI Endpoint loopback driver"
> +	depends on MHI_BUS_EP
> +	help
> +	  MHI endpoint loopback driver for data path testing.
> +	  This driver receives data on the uplink channel and echoes
> +	  it back on the downlink channel for testing purposes.
> diff --git a/drivers/bus/mhi/ep/Makefile b/drivers/bus/mhi/ep/Makefile
> index aad85f180b707fb997fcb541837eda9bbbb67437..02e4700e8dc3f860d40290476b0a852286683f8f 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-$(CONFIG_MHI_BUS_EP_LOOPBACK) += mhi_ep_loopback.o
> diff --git a/drivers/bus/mhi/ep/mhi_ep_loopback.c b/drivers/bus/mhi/ep/mhi_ep_loopback.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..ba6154dd9b785f051043c10a980ab340012ba986
> --- /dev/null
> +++ b/drivers/bus/mhi/ep/mhi_ep_loopback.c
> @@ -0,0 +1,134 @@
> +// 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>
> +
> +struct mhi_ep_loopback {
> +	struct workqueue_struct *loopback_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 you didn't use the "pile of abbreviations" naming scheme for your
local variables, you wouldn't have to line break this.

> +	if (ret) {
> +		dev_err(&mhi_ep_lb_work->mdev->dev, "Failed to send the packet\n");
> +		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_res->transaction_status)) {

Unnecessary parenthesis.

> +		buf = kmalloc(mhi_res->bytes_xferd, GFP_KERNEL);
> +		if (!buf) {
> +			dev_err(&mhi_dev->dev, "Failed to allocate buffer\n");

No error prints on alloc failures, the kernel has already printed for
you.

> +			return;
> +		}
> +
> +		memcpy(buf, mhi_res->buf_addr, mhi_res->bytes_xferd);

kmalloc + memcpy == kmemdup()

> +
> +		mhi_ep_lb_work = kmalloc(sizeof(*mhi_ep_lb_work), GFP_KERNEL);
> +		if (!mhi_ep_lb_work) {
> +			dev_err(&mhi_dev->dev, "Unable to allocate the work structure\n");

Ditto.

> +			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->loopback_wq, &mhi_ep_lb_work->work);
> +	}
> +}
> +
> +static void mhi_ep_loopback_dl_callback(struct mhi_ep_device *mhi_dev,
> +					struct mhi_result *mhi_res)
> +{
> +	void *buf;
> +
> +	if (mhi_res->transaction_status)
> +		return;
> +
> +	buf = mhi_res->buf_addr;
> +	if (buf)
> +		kfree(buf);

No need to check for NULL, or have a local variable.

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(struct mhi_ep_loopback), GFP_KERNEL);
> +	if (!mhi_ep_lb)
> +		return -ENOMEM;
> +
> +	mhi_ep_lb->loopback_wq = alloc_ordered_workqueue("mhi_loopback", WQ_MEM_RECLAIM);
> +	if (!mhi_ep_lb->loopback_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->loopback_wq);
> +	dev_set_drvdata(&mhi_dev->dev, NULL);

Shouldn't be necessary to clear drvdata here.

> +}
> +
> +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",
> +		.owner = THIS_MODULE,

module_mhi_ep_driver() assigns owner for you...

Regards,
Bjorn

> +	},
> +};
> +
> +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
> 
> 

Re: [PATCH v2 3/3] bus: mhi: ep: Add loopback driver for data path testing

[kernel test robot]

Hi Sumit,

kernel test robot noticed the following build warnings:

[auto build test WARNING on e6b9dce0aeeb91dfc0974ab87f02454e24566182]

url:    https://github.com/intel-lab-lkp/linux/commits/Sumit-Kumar/bus-mhi-host-Add-loopback-driver-with-sysfs-interface/20251104-174320
base:   e6b9dce0aeeb91dfc0974ab87f02454e24566182
patch link:    https://lore.kernel.org/r/20251104-loopback_mhi-v2-3-727a3fd9aa74%40oss.qualcomm.com
patch subject: [PATCH v2 3/3] bus: mhi: ep: Add loopback driver for data path testing
config: csky-randconfig-r061-20251110 (https://download.01.org/0day-ci/archive/20251110/202511100649.KfikwcaY-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 15.1.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511100649.KfikwcaY-lkp@intel.com/

cocci warnings: (new ones prefixed by >>)
>> drivers/bus/mhi/ep/mhi_ep_loopback.c:80:2-7: WARNING: NULL check before some freeing functions is not needed.
--
>> drivers/bus/mhi/ep/mhi_ep_loopback.c:46:8-15: WARNING opportunity for kmemdup

vim +80 drivers/bus/mhi/ep/mhi_ep_loopback.c

    37	
    38	static void mhi_ep_loopback_ul_callback(struct mhi_ep_device *mhi_dev,
    39						struct mhi_result *mhi_res)
    40	{
    41		struct mhi_ep_loopback *mhi_ep_lb = dev_get_drvdata(&mhi_dev->dev);
    42		struct mhi_ep_loopback_work *mhi_ep_lb_work;
    43		void *buf;
    44	
    45		if (!(mhi_res->transaction_status)) {
  > 46			buf = kmalloc(mhi_res->bytes_xferd, GFP_KERNEL);
    47			if (!buf) {
    48				dev_err(&mhi_dev->dev, "Failed to allocate buffer\n");
    49				return;
    50			}
    51	
    52			memcpy(buf, mhi_res->buf_addr, mhi_res->bytes_xferd);
    53	
    54			mhi_ep_lb_work = kmalloc(sizeof(*mhi_ep_lb_work), GFP_KERNEL);
    55			if (!mhi_ep_lb_work) {
    56				dev_err(&mhi_dev->dev, "Unable to allocate the work structure\n");
    57				kfree(buf);
    58				return;
    59			}
    60	
    61			INIT_WORK(&mhi_ep_lb_work->work, mhi_ep_loopback_work_handler);
    62			mhi_ep_lb_work->mdev = mhi_dev;
    63			mhi_ep_lb_work->buf = buf;
    64			mhi_ep_lb_work->len = mhi_res->bytes_xferd;
    65	
    66			queue_work(mhi_ep_lb->loopback_wq, &mhi_ep_lb_work->work);
    67		}
    68	}
    69	
    70	static void mhi_ep_loopback_dl_callback(struct mhi_ep_device *mhi_dev,
    71						struct mhi_result *mhi_res)
    72	{
    73		void *buf;
    74	
    75		if (mhi_res->transaction_status)
    76			return;
    77	
    78		buf = mhi_res->buf_addr;
    79		if (buf)
  > 80			kfree(buf);
    81	}
    82	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki