* [PATCH v4 0/3] bus: mhi: Add loopback driver
@ 2026-06-22 5:09 Sumit Kumar
2026-06-22 5:09 ` [PATCH v4 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-06-22 5:09 UTC (permalink / raw)
To: Manivannan Sadhasivam
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 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 | 51 ++++
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 | 128 ++++++++
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 | 329 +++++++++++++++++++++
include/linux/mhi_ep.h | 15 +
14 files changed, 586 insertions(+), 9 deletions(-)
---
base-commit: e6b9dce0aeeb91dfc0974ab87f02454e24566182
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 v4 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface 2026-06-22 5:09 [PATCH v4 0/3] bus: mhi: Add loopback driver Sumit Kumar @ 2026-06-22 5:09 ` Sumit Kumar 2026-06-23 8:45 ` Manivannan Sadhasivam 2026-06-22 5:09 ` [PATCH v4 2/3] bus: mhi: ep: Add mhi_ep_queue_buf() API for raw buffer queuing Sumit Kumar 2026-06-22 5:09 ` [PATCH v4 3/3] bus: mhi: ep: clients: Add loopback driver for data path testing Sumit Kumar 2 siblings, 1 reply; 10+ messages in thread From: Sumit Kumar @ 2026-06-22 5:09 UTC (permalink / raw) To: Manivannan Sadhasivam 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 | 51 ++++ MAINTAINERS | 1 + drivers/bus/mhi/host/Kconfig | 1 + drivers/bus/mhi/host/Makefile | 1 + drivers/bus/mhi/host/clients/Kconfig | 17 ++ drivers/bus/mhi/host/clients/Makefile | 2 + drivers/bus/mhi/host/clients/loopback.c | 329 +++++++++++++++++++++ 7 files changed, 402 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-bus-mhi-devices-loopback b/Documentation/ABI/testing/sysfs-bus-mhi-devices-loopback new file mode 100644 index 0000000000000000000000000000000000000000..3bd770747799a3341a23903cc1a108e650e915b8 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-mhi-devices-loopback @@ -0,0 +1,51 @@ +What: /sys/bus/mhi/devices/mhi<N>_LOOPBACK/tre_size +Date: April 2026 +KernelVersion: 7.1 +Contact: mhi@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: April 2026 +KernelVersion: 7.1 +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: April 2026 +KernelVersion: 7.1 +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: April 2026 +KernelVersion: 7.1 +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. + +What: /sys/bus/mhi/devices/mhi<N>_LOOPBACK/status +Date: April 2026 +KernelVersion: 7.1 +Contact: mhi@lists.linux.dev +Description: + (RO) Result of the last loopback test. Returns one of: + "pass" - last test completed successfully + "fail" - last test failed + "not started" - no test has been run yet + + Reading this file while a test is in progress will block + until the test completes. diff --git a/MAINTAINERS b/MAINTAINERS index 6dcfbd11efef87927041f5cf58d70633dbb4b18d..ff12a6da48947ac853bc638359a7046fea85fc21 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -16441,6 +16441,7 @@ L: linux-arm-msm@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..d1463c3e0df0da461c815afaec623ba349b51dda --- /dev/null +++ b/drivers/bus/mhi/host/clients/Kconfig @@ -0,0 +1,17 @@ +# SPDX-License-Identifier: GPL-2.0 + +config MHI_BUS_LOOPBACK + tristate "MHI LOOPBACK client driver" + depends on MHI_BUS + help + MHI LOOPBACK client driver that binds to the MHI LOOPBACK channel + as defined in the MHI specification. The LOOPBACK channel is + implemented by MHI-based devices (modems, WLAN) in the field, where + the endpoint firmware echoes back whatever the host sends. + + This driver exposes a sysfs interface for testing MHI data path + integrity between host and endpoint. Users can configure the TRE + size and count, trigger a loopback test, and read the result. + + To compile this driver as a module, choose M here. The module + will be called mhi_loopback. diff --git a/drivers/bus/mhi/host/clients/Makefile b/drivers/bus/mhi/host/clients/Makefile new file mode 100644 index 0000000000000000000000000000000000000000..3811b6928f42b38f94b1167941cf3b0fe512d32b --- /dev/null +++ b/drivers/bus/mhi/host/clients/Makefile @@ -0,0 +1,2 @@ +obj-$(CONFIG_MHI_BUS_LOOPBACK) += mhi_loopback.o +mhi_loopback-y += loopback.o diff --git a/drivers/bus/mhi/host/clients/loopback.c b/drivers/bus/mhi/host/clients/loopback.c new file mode 100644 index 0000000000000000000000000000000000000000..693691fff26dc8fa0d58931b98ce5f287fbd5c3e --- /dev/null +++ b/drivers/bus/mhi/host/clients/loopback.c @@ -0,0 +1,329 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. + */ + +/* + * The MHI LOOPBACK channel is defined in the MHI specification and is + * implemented by MHI-based devices (modems, WLAN) already deployed in the + * field. The endpoint firmware echoes back whatever the host sends on this + * channel. This driver binds to the LOOPBACK channel and exposes a sysfs + * interface for testing MHI data path integrity between host and endpoint. + * The sysfs interface is stable ABI because the wire protocol is fixed by + * the endpoint firmware and cannot be changed. + */ + +#include <linux/atomic.h> +#include <linux/cleanup.h> +#include <linux/completion.h> +#include <linux/errno.h> +#include <linux/mhi.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/random.h> +#include <linux/sizes.h> +#include <linux/slab.h> +#include <linux/string.h> +#include <linux/sysfs.h> +#include <linux/types.h> + +#define MHI_LOOPBACK_DEFAULT_TRE_SIZE 32 +#define MHI_LOOPBACK_DEFAULT_NUM_TRE 1 +#define MHI_LOOPBACK_TIMEOUT_MS 5000 +#define MHI_LOOPBACK_MAX_TRE_SIZE (SZ_64K - 1) + +struct mhi_loopback { + struct mhi_device *mdev; + struct mutex lb_mutex; + struct completion comp; + atomic_t tres_pending; + const char *result; + u32 num_tre; + u32 tre_size; +}; + +static ssize_t tre_size_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct mhi_loopback *loopback = dev_get_drvdata(dev); + + if (!loopback) + return -ENODEV; + + return sysfs_emit(buf, "%u\n", loopback->tre_size); +} + +static ssize_t tre_size_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct mhi_loopback *loopback = dev_get_drvdata(dev); + u32 val; + + if (!loopback) + return -ENODEV; + + if (kstrtou32(buf, 0, &val)) + return -EINVAL; + + if (val == 0 || val > MHI_LOOPBACK_MAX_TRE_SIZE) + return -EINVAL; + + guard(mutex)(&loopback->lb_mutex); + loopback->tre_size = val; + + return count; +} +static DEVICE_ATTR_RW(tre_size); + +static ssize_t max_tre_size_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + return sysfs_emit(buf, "%u\n", MHI_LOOPBACK_MAX_TRE_SIZE); +} +static DEVICE_ATTR_RO(max_tre_size); + +static ssize_t num_tre_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct mhi_loopback *loopback = dev_get_drvdata(dev); + + if (!loopback) + return -ENODEV; + + return sysfs_emit(buf, "%u\n", loopback->num_tre); +} + +static ssize_t num_tre_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct mhi_loopback *loopback = dev_get_drvdata(dev); + u32 val; + int el_num; + + if (!loopback) + return -ENODEV; + + if (kstrtou32(buf, 0, &val)) + return -EINVAL; + + if (val == 0) + return -EINVAL; + + guard(mutex)(&loopback->lb_mutex); + + el_num = mhi_get_free_desc_count(loopback->mdev, DMA_TO_DEVICE); + if (val > el_num) { + dev_err(dev, "num_tre (%u) exceeds ring capacity (%d)\n", val, el_num); + return -EINVAL; + } + + loopback->num_tre = val; + + return count; +} +static DEVICE_ATTR_RW(num_tre); + +static ssize_t start_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct mhi_loopback *loopback = dev_get_drvdata(dev); + u32 total_size, tre_count, tre_size; + int i; + + if (!loopback) + return -ENODEV; + + guard(mutex)(&loopback->lb_mutex); + + tre_size = loopback->tre_size; + tre_count = loopback->num_tre; + total_size = size_mul(tre_count, tre_size); + + if (total_size > KMALLOC_MAX_SIZE) + return -EINVAL; + + void *recv_buf __free(kfree) = kzalloc(total_size, GFP_KERNEL); + if (!recv_buf) + return -ENOMEM; + + void *send_buf __free(kfree) = kzalloc(total_size, GFP_KERNEL); + if (!send_buf) + return -ENOMEM; + + get_random_bytes(send_buf, total_size); + + atomic_set(&loopback->tres_pending, tre_count); + reinit_completion(&loopback->comp); + + for (i = 0; i < tre_count; i++) { + int ret = mhi_queue_buf(loopback->mdev, DMA_FROM_DEVICE, + recv_buf + (i * tre_size), tre_size, MHI_EOT); + if (ret) { + dev_err(dev, "Unable to queue read TRE %d: %d\n", i, ret); + loopback->result = "fail"; + if (atomic_sub_and_test(tre_count - i, &loopback->tres_pending)) + complete(&loopback->comp); + return ret; + } + } + + for (i = 0; i < tre_count - 1; i++) { + int ret = mhi_queue_buf(loopback->mdev, DMA_TO_DEVICE, + send_buf + (i * tre_size), tre_size, MHI_CHAIN); + if (ret) { + dev_err(dev, "Unable to queue send TRE %d: %d\n", i, ret); + loopback->result = "fail"; + return ret; + } + } + + int ret = mhi_queue_buf(loopback->mdev, DMA_TO_DEVICE, + send_buf + (i * tre_size), tre_size, MHI_EOT); + if (ret) { + dev_err(dev, "Unable to queue final TRE: %d\n", ret); + loopback->result = "fail"; + return ret; + } + + if (!wait_for_completion_timeout(&loopback->comp, + msecs_to_jiffies(MHI_LOOPBACK_TIMEOUT_MS))) { + dev_err(dev, "Loopback test timed out\n"); + loopback->result = "fail"; + return -ETIMEDOUT; + } + + if (memcmp(send_buf, recv_buf, total_size)) { + dev_err(dev, "Loopback data mismatch\n"); + loopback->result = "fail"; + return -EIO; + } + + loopback->result = "pass"; + + return count; +} +static DEVICE_ATTR_WO(start); + +static ssize_t status_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct mhi_loopback *loopback = dev_get_drvdata(dev); + + if (!loopback) + return -ENODEV; + + guard(mutex)(&loopback->lb_mutex); + + return sysfs_emit(buf, "%s\n", loopback->result); +} +static DEVICE_ATTR_RO(status); + +static void mhi_loopback_dl_callback(struct mhi_device *mhi_dev, + struct mhi_result *mhi_res) +{ + struct mhi_loopback *loopback = dev_get_drvdata(&mhi_dev->dev); + + if (!loopback) + return; + + if (mhi_res->transaction_status && mhi_res->transaction_status != -ENOTCONN) + dev_err(&mhi_dev->dev, "DL callback error: status %d\n", + mhi_res->transaction_status); + + if (atomic_dec_and_test(&loopback->tres_pending)) + complete(&loopback->comp); +} + +static void mhi_loopback_ul_callback(struct mhi_device *mhi_dev, + struct mhi_result *mhi_res) +{ +} + +static struct attribute *mhi_loopback_attrs[] = { + &dev_attr_tre_size.attr, + &dev_attr_max_tre_size.attr, + &dev_attr_num_tre.attr, + &dev_attr_start.attr, + &dev_attr_status.attr, + NULL, +}; + +static const struct attribute_group mhi_loopback_group = { + .attrs = mhi_loopback_attrs, +}; + +static int mhi_loopback_probe(struct mhi_device *mhi_dev, + const struct mhi_device_id *id) +{ + struct mhi_loopback *loopback; + int rc; + + loopback = devm_kzalloc(&mhi_dev->dev, sizeof(*loopback), GFP_KERNEL); + if (!loopback) + return -ENOMEM; + + loopback->mdev = mhi_dev; + loopback->tre_size = MHI_LOOPBACK_DEFAULT_TRE_SIZE; + loopback->num_tre = MHI_LOOPBACK_DEFAULT_NUM_TRE; + loopback->result = "not started"; + + mutex_init(&loopback->lb_mutex); + init_completion(&loopback->comp); + + dev_set_drvdata(&mhi_dev->dev, loopback); + + rc = mhi_prepare_for_transfer(mhi_dev); + if (rc) { + dev_err(&mhi_dev->dev, "failed to prepare for transfers\n"); + return rc; + } + + rc = devm_device_add_group(&mhi_dev->dev, &mhi_loopback_group); + if (rc) { + dev_err(&mhi_dev->dev, "failed to create sysfs attributes\n"); + mhi_unprepare_from_transfer(mhi_dev); + } + + return rc; +} + +static void mhi_loopback_remove(struct mhi_device *mhi_dev) +{ + struct mhi_loopback *loopback = dev_get_drvdata(&mhi_dev->dev); + + complete(&loopback->comp); + + mutex_lock(&loopback->lb_mutex); + mutex_unlock(&loopback->lb_mutex); + + 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface 2026-06-22 5:09 ` [PATCH v4 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface Sumit Kumar @ 2026-06-23 8:45 ` Manivannan Sadhasivam 2026-06-23 10:28 ` Sumit Kumar 0 siblings, 1 reply; 10+ messages in thread From: Manivannan Sadhasivam @ 2026-06-23 8:45 UTC (permalink / raw) To: Sumit Kumar; +Cc: mhi, linux-arm-msm, linux-kernel, Krishna Chaitanya Chundru On Mon, Jun 22, 2026 at 10:39:15AM +0530, Sumit Kumar wrote: > The MHI specification defines a LOOPBACK channel. The endpoint firmware > echoes back whatever the host sends on this channel. Without a host-side > driver, there is no way to exercise this channel to validate MHI data path > integrity between host and endpoint. > > Add a host-side loopback driver that binds to the LOOPBACK channel and > expose a sysfs interface for data path testing. The sysfs interface allows > users to configure TRE buffer size and count, trigger a loopback test, and > read the result. > > 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 | 51 ++++ > MAINTAINERS | 1 + > drivers/bus/mhi/host/Kconfig | 1 + > drivers/bus/mhi/host/Makefile | 1 + > drivers/bus/mhi/host/clients/Kconfig | 17 ++ > drivers/bus/mhi/host/clients/Makefile | 2 + > drivers/bus/mhi/host/clients/loopback.c | 329 +++++++++++++++++++++ > 7 files changed, 402 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-mhi-devices-loopback b/Documentation/ABI/testing/sysfs-bus-mhi-devices-loopback > new file mode 100644 > index 0000000000000000000000000000000000000000..3bd770747799a3341a23903cc1a108e650e915b8 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-mhi-devices-loopback > @@ -0,0 +1,51 @@ > +What: /sys/bus/mhi/devices/mhi<N>_LOOPBACK/tre_size > +Date: April 2026 > +KernelVersion: 7.1 > +Contact: mhi@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: April 2026 > +KernelVersion: 7.1 > +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: April 2026 > +KernelVersion: 7.1 > +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: April 2026 > +KernelVersion: 7.1 > +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. > + > +What: /sys/bus/mhi/devices/mhi<N>_LOOPBACK/status > +Date: April 2026 > +KernelVersion: 7.1 > +Contact: mhi@lists.linux.dev > +Description: > + (RO) Result of the last loopback test. Returns one of: > + "pass" - last test completed successfully > + "fail" - last test failed > + "not started" - no test has been run yet > + > + Reading this file while a test is in progress will block > + until the test completes. > diff --git a/MAINTAINERS b/MAINTAINERS > index 6dcfbd11efef87927041f5cf58d70633dbb4b18d..ff12a6da48947ac853bc638359a7046fea85fc21 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -16441,6 +16441,7 @@ L: linux-arm-msm@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..d1463c3e0df0da461c815afaec623ba349b51dda > --- /dev/null > +++ b/drivers/bus/mhi/host/clients/Kconfig > @@ -0,0 +1,17 @@ > +# SPDX-License-Identifier: GPL-2.0 > + > +config MHI_BUS_LOOPBACK > + tristate "MHI LOOPBACK client driver" > + depends on MHI_BUS > + help > + MHI LOOPBACK client driver that binds to the MHI LOOPBACK channel > + as defined in the MHI specification. The LOOPBACK channel is > + implemented by MHI-based devices (modems, WLAN) in the field, where > + the endpoint firmware echoes back whatever the host sends. > + > + This driver exposes a sysfs interface for testing MHI data path > + integrity between host and endpoint. Users can configure the TRE > + size and count, trigger a loopback test, and read the result. > + > + To compile this driver as a module, choose M here. The module > + will be called mhi_loopback. > diff --git a/drivers/bus/mhi/host/clients/Makefile b/drivers/bus/mhi/host/clients/Makefile > new file mode 100644 > index 0000000000000000000000000000000000000000..3811b6928f42b38f94b1167941cf3b0fe512d32b > --- /dev/null > +++ b/drivers/bus/mhi/host/clients/Makefile > @@ -0,0 +1,2 @@ > +obj-$(CONFIG_MHI_BUS_LOOPBACK) += mhi_loopback.o > +mhi_loopback-y += loopback.o > diff --git a/drivers/bus/mhi/host/clients/loopback.c b/drivers/bus/mhi/host/clients/loopback.c > new file mode 100644 > index 0000000000000000000000000000000000000000..693691fff26dc8fa0d58931b98ce5f287fbd5c3e > --- /dev/null > +++ b/drivers/bus/mhi/host/clients/loopback.c > @@ -0,0 +1,329 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. > + */ > + > +/* > + * The MHI LOOPBACK channel is defined in the MHI specification and is > + * implemented by MHI-based devices (modems, WLAN) already deployed in the > + * field. The endpoint firmware echoes back whatever the host sends on this > + * channel. This driver binds to the LOOPBACK channel and exposes a sysfs > + * interface for testing MHI data path integrity between host and endpoint. > + * The sysfs interface is stable ABI because the wire protocol is fixed by > + * the endpoint firmware and cannot be changed. > + */ This comment just duplicates Kconfig help text. So drop it. > + > +#include <linux/atomic.h> > +#include <linux/cleanup.h> > +#include <linux/completion.h> > +#include <linux/errno.h> > +#include <linux/mhi.h> > +#include <linux/mod_devicetable.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/random.h> > +#include <linux/sizes.h> > +#include <linux/slab.h> > +#include <linux/string.h> > +#include <linux/sysfs.h> > +#include <linux/types.h> > + > +#define MHI_LOOPBACK_DEFAULT_TRE_SIZE 32 > +#define MHI_LOOPBACK_DEFAULT_NUM_TRE 1 > +#define MHI_LOOPBACK_TIMEOUT_MS 5000 > +#define MHI_LOOPBACK_MAX_TRE_SIZE (SZ_64K - 1) > + > +struct mhi_loopback { > + struct mhi_device *mdev; > + struct mutex lb_mutex; > + struct completion comp; > + atomic_t tres_pending; tre_pending > + const char *result; > + u32 num_tre; > + u32 tre_size; > +}; > + > +static ssize_t tre_size_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct mhi_loopback *loopback = dev_get_drvdata(dev); > + > + if (!loopback) > + return -ENODEV; I think you have this check here to avoid race between sysfs cleanup and driver remove due to the use of devm_device_add_group(). But you can drop these by switching to non-devm helpers and freeing the sysfs entries directly in mhi_loopback_remove(). > + > + return sysfs_emit(buf, "%u\n", loopback->tre_size); > +} > + > +static ssize_t tre_size_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct mhi_loopback *loopback = dev_get_drvdata(dev); > + u32 val; > + > + if (!loopback) > + return -ENODEV; > + > + if (kstrtou32(buf, 0, &val)) > + return -EINVAL; > + > + if (val == 0 || val > MHI_LOOPBACK_MAX_TRE_SIZE) > + return -EINVAL; > + > + guard(mutex)(&loopback->lb_mutex); > + loopback->tre_size = val; > + > + return count; > +} > +static DEVICE_ATTR_RW(tre_size); > + > +static ssize_t max_tre_size_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + return sysfs_emit(buf, "%u\n", MHI_LOOPBACK_MAX_TRE_SIZE); > +} > +static DEVICE_ATTR_RO(max_tre_size); > + > +static ssize_t num_tre_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct mhi_loopback *loopback = dev_get_drvdata(dev); > + > + if (!loopback) > + return -ENODEV; > + > + return sysfs_emit(buf, "%u\n", loopback->num_tre); > +} > + > +static ssize_t num_tre_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct mhi_loopback *loopback = dev_get_drvdata(dev); > + u32 val; > + int el_num; > + > + if (!loopback) > + return -ENODEV; > + > + if (kstrtou32(buf, 0, &val)) > + return -EINVAL; > + > + if (val == 0) > + return -EINVAL; > + > + guard(mutex)(&loopback->lb_mutex); > + > + el_num = mhi_get_free_desc_count(loopback->mdev, DMA_TO_DEVICE); > + if (val > el_num) { > + dev_err(dev, "num_tre (%u) exceeds ring capacity (%d)\n", val, el_num); > + return -EINVAL; > + } > + > + loopback->num_tre = val; > + > + return count; > +} > +static DEVICE_ATTR_RW(num_tre); > + > +static ssize_t start_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct mhi_loopback *loopback = dev_get_drvdata(dev); > + u32 total_size, tre_count, tre_size; > + int i; > + > + if (!loopback) > + return -ENODEV; > + > + guard(mutex)(&loopback->lb_mutex); > + > + tre_size = loopback->tre_size; > + tre_count = loopback->num_tre; > + total_size = size_mul(tre_count, tre_size); > + > + if (total_size > KMALLOC_MAX_SIZE) > + return -EINVAL; > + > + void *recv_buf __free(kfree) = kzalloc(total_size, GFP_KERNEL); > + if (!recv_buf) > + return -ENOMEM; > + > + void *send_buf __free(kfree) = kzalloc(total_size, GFP_KERNEL); > + if (!send_buf) > + return -ENOMEM; > + > + get_random_bytes(send_buf, total_size); > + > + atomic_set(&loopback->tres_pending, tre_count); > + reinit_completion(&loopback->comp); > + > + for (i = 0; i < tre_count; i++) { > + int ret = mhi_queue_buf(loopback->mdev, DMA_FROM_DEVICE, > + recv_buf + (i * tre_size), tre_size, MHI_EOT); > + if (ret) { > + dev_err(dev, "Unable to queue read TRE %d: %d\n", i, ret); > + loopback->result = "fail"; > + if (atomic_sub_and_test(tre_count - i, &loopback->tres_pending)) > + complete(&loopback->comp); > + return ret; > + } > + } > + > + for (i = 0; i < tre_count - 1; i++) { > + int ret = mhi_queue_buf(loopback->mdev, DMA_TO_DEVICE, > + send_buf + (i * tre_size), tre_size, MHI_CHAIN); > + if (ret) { > + dev_err(dev, "Unable to queue send TRE %d: %d\n", i, ret); > + loopback->result = "fail"; > + return ret; > + } > + } > + > + int ret = mhi_queue_buf(loopback->mdev, DMA_TO_DEVICE, > + send_buf + (i * tre_size), tre_size, MHI_EOT); > + if (ret) { > + dev_err(dev, "Unable to queue final TRE: %d\n", ret); > + loopback->result = "fail"; > + return ret; > + } > + > + if (!wait_for_completion_timeout(&loopback->comp, > + msecs_to_jiffies(MHI_LOOPBACK_TIMEOUT_MS))) { > + dev_err(dev, "Loopback test timed out\n"); > + loopback->result = "fail"; > + return -ETIMEDOUT; So once this function exits, both buffers will get freed due to the destructor. But the device may still hold the reference to the buffers in TRE and may read/write to it later. So you need to make sure that you flush the buffers in the error path. But we don't have any explicit APIs to do that, so maybe you can call mhi_unprepare_from_transfer() followed by mhi_prepare_for_transfer() in the error path? > + } > + > + if (memcmp(send_buf, recv_buf, total_size)) { > + dev_err(dev, "Loopback data mismatch\n"); > + loopback->result = "fail"; > + return -EIO; > + } > + > + loopback->result = "pass"; > + > + return count; > +} > +static DEVICE_ATTR_WO(start); > + > +static ssize_t status_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct mhi_loopback *loopback = dev_get_drvdata(dev); > + > + if (!loopback) > + return -ENODEV; > + > + guard(mutex)(&loopback->lb_mutex); > + > + return sysfs_emit(buf, "%s\n", loopback->result); I don't see a need for this separate 'status' attribute. 'start' attribute blocks until the write is completed or timesout, prints an error message with relevant errno and returns the error code. Though the syscall interface converts all error code to (-1), it is sufficient for the userspace to know whether the test has passed or not. > +} > +static DEVICE_ATTR_RO(status); > + > +static void mhi_loopback_dl_callback(struct mhi_device *mhi_dev, > + struct mhi_result *mhi_res) > +{ > + struct mhi_loopback *loopback = dev_get_drvdata(&mhi_dev->dev); > + > + if (!loopback) > + return; > + > + if (mhi_res->transaction_status && mhi_res->transaction_status != -ENOTCONN) > + dev_err(&mhi_dev->dev, "DL callback error: status %d\n", > + mhi_res->transaction_status); > + > + if (atomic_dec_and_test(&loopback->tres_pending)) > + complete(&loopback->comp); > +} > + > +static void mhi_loopback_ul_callback(struct mhi_device *mhi_dev, > + struct mhi_result *mhi_res) > +{ > +} > + > +static struct attribute *mhi_loopback_attrs[] = { > + &dev_attr_tre_size.attr, > + &dev_attr_max_tre_size.attr, > + &dev_attr_num_tre.attr, > + &dev_attr_start.attr, > + &dev_attr_status.attr, > + NULL, > +}; > + > +static const struct attribute_group mhi_loopback_group = { > + .attrs = mhi_loopback_attrs, > +}; > + > +static int mhi_loopback_probe(struct mhi_device *mhi_dev, > + const struct mhi_device_id *id) > +{ > + struct mhi_loopback *loopback; > + int rc; '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; > + loopback->result = "not started"; > + > + mutex_init(&loopback->lb_mutex); > + init_completion(&loopback->comp); > + > + dev_set_drvdata(&mhi_dev->dev, loopback); > + > + rc = mhi_prepare_for_transfer(mhi_dev); > + if (rc) { > + dev_err(&mhi_dev->dev, "failed to prepare for transfers\n"); nit: Capitalize 'Failed' and print the errno. Applies to all error prints. > + return rc; > + } > + > + rc = devm_device_add_group(&mhi_dev->dev, &mhi_loopback_group); > + if (rc) { > + dev_err(&mhi_dev->dev, "failed to create sysfs attributes\n"); > + mhi_unprepare_from_transfer(mhi_dev); > + } > + > + return rc; 'return 0' > +} > + > +static void mhi_loopback_remove(struct mhi_device *mhi_dev) > +{ > + struct mhi_loopback *loopback = dev_get_drvdata(&mhi_dev->dev); > + > + complete(&loopback->comp); > + > + mutex_lock(&loopback->lb_mutex); > + mutex_unlock(&loopback->lb_mutex); What does this locking protect? > + > + mhi_unprepare_from_transfer(mhi_dev); > + dev_set_drvdata(&mhi_dev->dev, NULL); As I mentioned above, once you call sysfs_create_group() in probe() and sysfs_remove_group() before mhi_unprepare_from_transfer(), you can drop setting drvdata to NULL. - Mani -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface 2026-06-23 8:45 ` Manivannan Sadhasivam @ 2026-06-23 10:28 ` Sumit Kumar 2026-06-23 11:20 ` Manivannan Sadhasivam 0 siblings, 1 reply; 10+ messages in thread From: Sumit Kumar @ 2026-06-23 10:28 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: mhi, linux-arm-msm, linux-kernel, Krishna Chaitanya Chundru On 6/23/2026 2:15 PM, Manivannan Sadhasivam wrote: > On Mon, Jun 22, 2026 at 10:39:15AM +0530, Sumit Kumar wrote: >> The MHI specification defines a LOOPBACK channel. The endpoint firmware >> echoes back whatever the host sends on this channel. Without a host-side >> driver, there is no way to exercise this channel to validate MHI data path >> integrity between host and endpoint. >> >> Add a host-side loopback driver that binds to the LOOPBACK channel and >> expose a sysfs interface for data path testing. The sysfs interface allows >> users to configure TRE buffer size and count, trigger a loopback test, and >> read the result. >> >> 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 | 51 ++++ >> MAINTAINERS | 1 + >> drivers/bus/mhi/host/Kconfig | 1 + >> drivers/bus/mhi/host/Makefile | 1 + >> drivers/bus/mhi/host/clients/Kconfig | 17 ++ >> drivers/bus/mhi/host/clients/Makefile | 2 + >> drivers/bus/mhi/host/clients/loopback.c | 329 +++++++++++++++++++++ >> 7 files changed, 402 insertions(+) >> >> diff --git a/Documentation/ABI/testing/sysfs-bus-mhi-devices-loopback b/Documentation/ABI/testing/sysfs-bus-mhi-devices-loopback >> new file mode 100644 >> index 0000000000000000000000000000000000000000..3bd770747799a3341a23903cc1a108e650e915b8 >> --- /dev/null >> +++ b/Documentation/ABI/testing/sysfs-bus-mhi-devices-loopback >> @@ -0,0 +1,51 @@ >> +What: /sys/bus/mhi/devices/mhi<N>_LOOPBACK/tre_size >> +Date: April 2026 >> +KernelVersion: 7.1 >> +Contact: mhi@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: April 2026 >> +KernelVersion: 7.1 >> +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: April 2026 >> +KernelVersion: 7.1 >> +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: April 2026 >> +KernelVersion: 7.1 >> +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. >> + >> +What: /sys/bus/mhi/devices/mhi<N>_LOOPBACK/status >> +Date: April 2026 >> +KernelVersion: 7.1 >> +Contact: mhi@lists.linux.dev >> +Description: >> + (RO) Result of the last loopback test. Returns one of: >> + "pass" - last test completed successfully >> + "fail" - last test failed >> + "not started" - no test has been run yet >> + >> + Reading this file while a test is in progress will block >> + until the test completes. >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 6dcfbd11efef87927041f5cf58d70633dbb4b18d..ff12a6da48947ac853bc638359a7046fea85fc21 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -16441,6 +16441,7 @@ L: linux-arm-msm@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..d1463c3e0df0da461c815afaec623ba349b51dda >> --- /dev/null >> +++ b/drivers/bus/mhi/host/clients/Kconfig >> @@ -0,0 +1,17 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> + >> +config MHI_BUS_LOOPBACK >> + tristate "MHI LOOPBACK client driver" >> + depends on MHI_BUS >> + help >> + MHI LOOPBACK client driver that binds to the MHI LOOPBACK channel >> + as defined in the MHI specification. The LOOPBACK channel is >> + implemented by MHI-based devices (modems, WLAN) in the field, where >> + the endpoint firmware echoes back whatever the host sends. >> + >> + This driver exposes a sysfs interface for testing MHI data path >> + integrity between host and endpoint. Users can configure the TRE >> + size and count, trigger a loopback test, and read the result. >> + >> + To compile this driver as a module, choose M here. The module >> + will be called mhi_loopback. >> diff --git a/drivers/bus/mhi/host/clients/Makefile b/drivers/bus/mhi/host/clients/Makefile >> new file mode 100644 >> index 0000000000000000000000000000000000000000..3811b6928f42b38f94b1167941cf3b0fe512d32b >> --- /dev/null >> +++ b/drivers/bus/mhi/host/clients/Makefile >> @@ -0,0 +1,2 @@ >> +obj-$(CONFIG_MHI_BUS_LOOPBACK) += mhi_loopback.o >> +mhi_loopback-y += loopback.o >> diff --git a/drivers/bus/mhi/host/clients/loopback.c b/drivers/bus/mhi/host/clients/loopback.c >> new file mode 100644 >> index 0000000000000000000000000000000000000000..693691fff26dc8fa0d58931b98ce5f287fbd5c3e >> --- /dev/null >> +++ b/drivers/bus/mhi/host/clients/loopback.c >> @@ -0,0 +1,329 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. >> + */ >> + >> +/* >> + * The MHI LOOPBACK channel is defined in the MHI specification and is >> + * implemented by MHI-based devices (modems, WLAN) already deployed in the >> + * field. The endpoint firmware echoes back whatever the host sends on this >> + * channel. This driver binds to the LOOPBACK channel and exposes a sysfs >> + * interface for testing MHI data path integrity between host and endpoint. >> + * The sysfs interface is stable ABI because the wire protocol is fixed by >> + * the endpoint firmware and cannot be changed. >> + */ > This comment just duplicates Kconfig help text. So drop it. > >> + >> +#include <linux/atomic.h> >> +#include <linux/cleanup.h> >> +#include <linux/completion.h> >> +#include <linux/errno.h> >> +#include <linux/mhi.h> >> +#include <linux/mod_devicetable.h> >> +#include <linux/module.h> >> +#include <linux/mutex.h> >> +#include <linux/random.h> >> +#include <linux/sizes.h> >> +#include <linux/slab.h> >> +#include <linux/string.h> >> +#include <linux/sysfs.h> >> +#include <linux/types.h> >> + >> +#define MHI_LOOPBACK_DEFAULT_TRE_SIZE 32 >> +#define MHI_LOOPBACK_DEFAULT_NUM_TRE 1 >> +#define MHI_LOOPBACK_TIMEOUT_MS 5000 >> +#define MHI_LOOPBACK_MAX_TRE_SIZE (SZ_64K - 1) >> + >> +struct mhi_loopback { >> + struct mhi_device *mdev; >> + struct mutex lb_mutex; >> + struct completion comp; >> + atomic_t tres_pending; > tre_pending > >> + const char *result; >> + u32 num_tre; >> + u32 tre_size; >> +}; >> + >> +static ssize_t tre_size_show(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct mhi_loopback *loopback = dev_get_drvdata(dev); >> + >> + if (!loopback) >> + return -ENODEV; > I think you have this check here to avoid race between sysfs cleanup and driver > remove due to the use of devm_device_add_group(). But you can drop these by > switching to non-devm helpers and freeing the sysfs entries directly in > mhi_loopback_remove(). > >> + >> + return sysfs_emit(buf, "%u\n", loopback->tre_size); >> +} >> + >> +static ssize_t tre_size_store(struct device *dev, >> + struct device_attribute *attr, >> + const char *buf, size_t count) >> +{ >> + struct mhi_loopback *loopback = dev_get_drvdata(dev); >> + u32 val; >> + >> + if (!loopback) >> + return -ENODEV; >> + >> + if (kstrtou32(buf, 0, &val)) >> + return -EINVAL; >> + >> + if (val == 0 || val > MHI_LOOPBACK_MAX_TRE_SIZE) >> + return -EINVAL; >> + >> + guard(mutex)(&loopback->lb_mutex); >> + loopback->tre_size = val; >> + >> + return count; >> +} >> +static DEVICE_ATTR_RW(tre_size); >> + >> +static ssize_t max_tre_size_show(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + return sysfs_emit(buf, "%u\n", MHI_LOOPBACK_MAX_TRE_SIZE); >> +} >> +static DEVICE_ATTR_RO(max_tre_size); >> + >> +static ssize_t num_tre_show(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct mhi_loopback *loopback = dev_get_drvdata(dev); >> + >> + if (!loopback) >> + return -ENODEV; >> + >> + return sysfs_emit(buf, "%u\n", loopback->num_tre); >> +} >> + >> +static ssize_t num_tre_store(struct device *dev, >> + struct device_attribute *attr, >> + const char *buf, size_t count) >> +{ >> + struct mhi_loopback *loopback = dev_get_drvdata(dev); >> + u32 val; >> + int el_num; >> + >> + if (!loopback) >> + return -ENODEV; >> + >> + if (kstrtou32(buf, 0, &val)) >> + return -EINVAL; >> + >> + if (val == 0) >> + return -EINVAL; >> + >> + guard(mutex)(&loopback->lb_mutex); >> + >> + el_num = mhi_get_free_desc_count(loopback->mdev, DMA_TO_DEVICE); >> + if (val > el_num) { >> + dev_err(dev, "num_tre (%u) exceeds ring capacity (%d)\n", val, el_num); >> + return -EINVAL; >> + } >> + >> + loopback->num_tre = val; >> + >> + return count; >> +} >> +static DEVICE_ATTR_RW(num_tre); >> + >> +static ssize_t start_store(struct device *dev, >> + struct device_attribute *attr, >> + const char *buf, size_t count) >> +{ >> + struct mhi_loopback *loopback = dev_get_drvdata(dev); >> + u32 total_size, tre_count, tre_size; >> + int i; >> + >> + if (!loopback) >> + return -ENODEV; >> + >> + guard(mutex)(&loopback->lb_mutex); >> + >> + tre_size = loopback->tre_size; >> + tre_count = loopback->num_tre; >> + total_size = size_mul(tre_count, tre_size); >> + >> + if (total_size > KMALLOC_MAX_SIZE) >> + return -EINVAL; >> + >> + void *recv_buf __free(kfree) = kzalloc(total_size, GFP_KERNEL); >> + if (!recv_buf) >> + return -ENOMEM; >> + >> + void *send_buf __free(kfree) = kzalloc(total_size, GFP_KERNEL); >> + if (!send_buf) >> + return -ENOMEM; >> + >> + get_random_bytes(send_buf, total_size); >> + >> + atomic_set(&loopback->tres_pending, tre_count); >> + reinit_completion(&loopback->comp); >> + >> + for (i = 0; i < tre_count; i++) { >> + int ret = mhi_queue_buf(loopback->mdev, DMA_FROM_DEVICE, >> + recv_buf + (i * tre_size), tre_size, MHI_EOT); >> + if (ret) { >> + dev_err(dev, "Unable to queue read TRE %d: %d\n", i, ret); >> + loopback->result = "fail"; >> + if (atomic_sub_and_test(tre_count - i, &loopback->tres_pending)) >> + complete(&loopback->comp); >> + return ret; >> + } >> + } >> + >> + for (i = 0; i < tre_count - 1; i++) { >> + int ret = mhi_queue_buf(loopback->mdev, DMA_TO_DEVICE, >> + send_buf + (i * tre_size), tre_size, MHI_CHAIN); >> + if (ret) { >> + dev_err(dev, "Unable to queue send TRE %d: %d\n", i, ret); >> + loopback->result = "fail"; >> + return ret; >> + } >> + } >> + >> + int ret = mhi_queue_buf(loopback->mdev, DMA_TO_DEVICE, >> + send_buf + (i * tre_size), tre_size, MHI_EOT); >> + if (ret) { >> + dev_err(dev, "Unable to queue final TRE: %d\n", ret); >> + loopback->result = "fail"; >> + return ret; >> + } >> + >> + if (!wait_for_completion_timeout(&loopback->comp, >> + msecs_to_jiffies(MHI_LOOPBACK_TIMEOUT_MS))) { >> + dev_err(dev, "Loopback test timed out\n"); >> + loopback->result = "fail"; >> + return -ETIMEDOUT; > So once this function exits, both buffers will get freed due to the destructor. > But the device may still hold the reference to the buffers in TRE and may > read/write to it later. So you need to make sure that you flush the buffers in > the error path. But we don't have any explicit APIs to do that, so maybe you can > call mhi_unprepare_from_transfer() followed by mhi_prepare_for_transfer() in the > error path? > >> + } >> + >> + if (memcmp(send_buf, recv_buf, total_size)) { >> + dev_err(dev, "Loopback data mismatch\n"); >> + loopback->result = "fail"; >> + return -EIO; >> + } >> + >> + loopback->result = "pass"; >> + >> + return count; >> +} >> +static DEVICE_ATTR_WO(start); >> + >> +static ssize_t status_show(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct mhi_loopback *loopback = dev_get_drvdata(dev); >> + >> + if (!loopback) >> + return -ENODEV; >> + >> + guard(mutex)(&loopback->lb_mutex); >> + >> + return sysfs_emit(buf, "%s\n", loopback->result); > I don't see a need for this separate 'status' attribute. 'start' attribute > blocks until the write is completed or timesout, prints an error message with > relevant errno and returns the error code. Though the syscall interface converts > all error code to (-1), it is sufficient for the userspace to know whether the > test has passed or not. > >> +} >> +static DEVICE_ATTR_RO(status); >> + >> +static void mhi_loopback_dl_callback(struct mhi_device *mhi_dev, >> + struct mhi_result *mhi_res) >> +{ >> + struct mhi_loopback *loopback = dev_get_drvdata(&mhi_dev->dev); >> + >> + if (!loopback) >> + return; >> + >> + if (mhi_res->transaction_status && mhi_res->transaction_status != -ENOTCONN) >> + dev_err(&mhi_dev->dev, "DL callback error: status %d\n", >> + mhi_res->transaction_status); >> + >> + if (atomic_dec_and_test(&loopback->tres_pending)) >> + complete(&loopback->comp); >> +} >> + >> +static void mhi_loopback_ul_callback(struct mhi_device *mhi_dev, >> + struct mhi_result *mhi_res) >> +{ >> +} >> + >> +static struct attribute *mhi_loopback_attrs[] = { >> + &dev_attr_tre_size.attr, >> + &dev_attr_max_tre_size.attr, >> + &dev_attr_num_tre.attr, >> + &dev_attr_start.attr, >> + &dev_attr_status.attr, >> + NULL, >> +}; >> + >> +static const struct attribute_group mhi_loopback_group = { >> + .attrs = mhi_loopback_attrs, >> +}; >> + >> +static int mhi_loopback_probe(struct mhi_device *mhi_dev, >> + const struct mhi_device_id *id) >> +{ >> + struct mhi_loopback *loopback; >> + int rc; > '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; >> + loopback->result = "not started"; >> + >> + mutex_init(&loopback->lb_mutex); >> + init_completion(&loopback->comp); >> + >> + dev_set_drvdata(&mhi_dev->dev, loopback); >> + >> + rc = mhi_prepare_for_transfer(mhi_dev); >> + if (rc) { >> + dev_err(&mhi_dev->dev, "failed to prepare for transfers\n"); > nit: Capitalize 'Failed' and print the errno. Applies to all error prints. > >> + return rc; >> + } >> + >> + rc = devm_device_add_group(&mhi_dev->dev, &mhi_loopback_group); >> + if (rc) { >> + dev_err(&mhi_dev->dev, "failed to create sysfs attributes\n"); >> + mhi_unprepare_from_transfer(mhi_dev); >> + } >> + >> + return rc; > 'return 0' > >> +} >> + >> +static void mhi_loopback_remove(struct mhi_device *mhi_dev) >> +{ >> + struct mhi_loopback *loopback = dev_get_drvdata(&mhi_dev->dev); >> + >> + complete(&loopback->comp); >> + >> + mutex_lock(&loopback->lb_mutex); >> + mutex_unlock(&loopback->lb_mutex); > What does this locking protect? since start_store() is holding the mutex for its entire duration, this lock will block until any in-progress start_store() has fully completed before teardown, ensuring mhi_unprepare_from_transfer() does not race with an active test still accessing the channel. Will fix other things you have mentioned. - Sumit > >> + >> + mhi_unprepare_from_transfer(mhi_dev); >> + dev_set_drvdata(&mhi_dev->dev, NULL); > As I mentioned above, once you call sysfs_create_group() in probe() and > sysfs_remove_group() before mhi_unprepare_from_transfer(), you can drop setting > drvdata to NULL. > > - Mani > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface 2026-06-23 10:28 ` Sumit Kumar @ 2026-06-23 11:20 ` Manivannan Sadhasivam 0 siblings, 0 replies; 10+ messages in thread From: Manivannan Sadhasivam @ 2026-06-23 11:20 UTC (permalink / raw) To: Sumit Kumar; +Cc: mhi, linux-arm-msm, linux-kernel, Krishna Chaitanya Chundru On Tue, Jun 23, 2026 at 03:58:00PM +0530, Sumit Kumar wrote: > [...] > > > +static void mhi_loopback_remove(struct mhi_device *mhi_dev) > > > +{ > > > + struct mhi_loopback *loopback = dev_get_drvdata(&mhi_dev->dev); > > > + > > > + complete(&loopback->comp); > > > + > > > + mutex_lock(&loopback->lb_mutex); > > > + mutex_unlock(&loopback->lb_mutex); > > What does this locking protect? > since start_store() is holding the mutex for its entire duration, > this lock will block until any in-progress start_store() has fully > completed before teardown, ensuring mhi_unprepare_from_transfer() > does not race with an active test still accessing the channel. > How can you ensure that start_store doesn't race if you unlock the mutex before mhi_unprepare_from_transfer()? Anyway, once you remove the sysfs files here as I suggested, you do not need to worry about the race. - Mani -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 2/3] bus: mhi: ep: Add mhi_ep_queue_buf() API for raw buffer queuing 2026-06-22 5:09 [PATCH v4 0/3] bus: mhi: Add loopback driver Sumit Kumar 2026-06-22 5:09 ` [PATCH v4 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface Sumit Kumar @ 2026-06-22 5:09 ` Sumit Kumar 2026-06-23 8:54 ` Manivannan Sadhasivam 2026-06-22 5:09 ` [PATCH v4 3/3] bus: mhi: ep: clients: Add loopback driver for data path testing Sumit Kumar 2 siblings, 1 reply; 10+ messages in thread From: Sumit Kumar @ 2026-06-22 5:09 UTC (permalink / raw) To: Manivannan Sadhasivam; +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 | 15 +++++++++++++++ 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c index b3eafcf2a2c50d95e3efd3afb27038ecf55552a5..d44e1e54cfb4404b6589aab372e687db7492d3c3 100644 --- a/drivers/bus/mhi/ep/main.c +++ b/drivers/bus/mhi/ep/main.c @@ -516,7 +516,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; @@ -544,22 +544,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); @@ -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 = mhi_ep_buf_completion; + 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..59f796e56207aaf8be09edc9ba4d1f59b665581f 100644 --- a/include/linux/mhi_ep.h +++ b/include/linux/mhi_ep.h @@ -302,4 +302,19 @@ 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. 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 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/3] bus: mhi: ep: Add mhi_ep_queue_buf() API for raw buffer queuing 2026-06-22 5:09 ` [PATCH v4 2/3] bus: mhi: ep: Add mhi_ep_queue_buf() API for raw buffer queuing Sumit Kumar @ 2026-06-23 8:54 ` Manivannan Sadhasivam 0 siblings, 0 replies; 10+ messages in thread From: Manivannan Sadhasivam @ 2026-06-23 8:54 UTC (permalink / raw) To: Sumit Kumar; +Cc: mhi, linux-arm-msm, linux-kernel On Mon, Jun 22, 2026 at 10:39:16AM +0530, Sumit Kumar wrote: > 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 | 15 +++++++++++++++ > 2 files changed, 35 insertions(+), 9 deletions(-) > > diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c > index b3eafcf2a2c50d95e3efd3afb27038ecf55552a5..d44e1e54cfb4404b6589aab372e687db7492d3c3 100644 > --- a/drivers/bus/mhi/ep/main.c > +++ b/drivers/bus/mhi/ep/main.c > @@ -516,7 +516,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; > @@ -544,22 +544,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); > @@ -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 = mhi_ep_buf_completion; > + 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..59f796e56207aaf8be09edc9ba4d1f59b665581f 100644 > --- a/include/linux/mhi_ep.h > +++ b/include/linux/mhi_ep.h > @@ -302,4 +302,19 @@ 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 You are not really 'sending' the buffer, but just 'transferring the buffer contents'. - Mani -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 3/3] bus: mhi: ep: clients: Add loopback driver for data path testing 2026-06-22 5:09 [PATCH v4 0/3] bus: mhi: Add loopback driver Sumit Kumar 2026-06-22 5:09 ` [PATCH v4 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface Sumit Kumar 2026-06-22 5:09 ` [PATCH v4 2/3] bus: mhi: ep: Add mhi_ep_queue_buf() API for raw buffer queuing Sumit Kumar @ 2026-06-22 5:09 ` Sumit Kumar 2026-06-23 9:17 ` Manivannan Sadhasivam 2 siblings, 1 reply; 10+ messages in thread From: Sumit Kumar @ 2026-06-22 5:09 UTC (permalink / raw) To: Manivannan Sadhasivam 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 | 128 ++++++++++++++++++++++++++++++++++ 5 files changed, 149 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..05db91be6ffc5afe5a2022962410c96a7ec19962 --- /dev/null +++ b/drivers/bus/mhi/ep/clients/loopback.c @@ -0,0 +1,128 @@ +// 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 *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_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) + return; + + mhi_ep_lb_work = kmalloc(sizeof(*mhi_ep_lb_work), GFP_KERNEL); + if (!mhi_ep_lb_work) { + 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) +{ + 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->loopback_wq = alloc_ordered_workqueue("mhi_ep_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", + }, +}; + +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 v4 3/3] bus: mhi: ep: clients: Add loopback driver for data path testing 2026-06-22 5:09 ` [PATCH v4 3/3] bus: mhi: ep: clients: Add loopback driver for data path testing Sumit Kumar @ 2026-06-23 9:17 ` Manivannan Sadhasivam 2026-06-24 11:12 ` Sumit Kumar 0 siblings, 1 reply; 10+ messages in thread From: Manivannan Sadhasivam @ 2026-06-23 9:17 UTC (permalink / raw) To: Sumit Kumar; +Cc: mhi, linux-arm-msm, linux-kernel, Krishna Chaitanya Chundru On Mon, Jun 22, 2026 at 10:39:17AM +0530, Sumit Kumar wrote: > 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 | 128 ++++++++++++++++++++++++++++++++++ > 5 files changed, 149 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..05db91be6ffc5afe5a2022962410c96a7ec19962 > --- /dev/null > +++ b/drivers/bus/mhi/ep/clients/loopback.c > @@ -0,0 +1,128 @@ > +// 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 *loopback_wq; s/loopback_wq/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"); 'Failed to queue buffer' > + 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; Hmm. This seems similar to patch 1 race condition. But we do not have a EP specific API to stop a channel and flush the wq like mhi_ep_unprepare_from_transfer(). But you should add one and call it from remove(). > + > + 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) > + return; Error log? > + > + mhi_ep_lb_work = kmalloc(sizeof(*mhi_ep_lb_work), GFP_KERNEL); > + if (!mhi_ep_lb_work) { > + kfree(buf); Same here. > + 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) > +{ > + 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->loopback_wq = alloc_ordered_workqueue("mhi_ep_loopback", WQ_MEM_RECLAIM); > + if (!mhi_ep_lb->loopback_wq) { > + dev_err(&mhi_dev->dev, "Failed to create workqueue.\n"); nit: Remove fullstop at the end of error message. > + 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); As mentioned above, this should be dropped if you can ensure that xfer_cb() won't be called. - Mani -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/3] bus: mhi: ep: clients: Add loopback driver for data path testing 2026-06-23 9:17 ` Manivannan Sadhasivam @ 2026-06-24 11:12 ` Sumit Kumar 0 siblings, 0 replies; 10+ messages in thread From: Sumit Kumar @ 2026-06-24 11:12 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: mhi, linux-arm-msm, linux-kernel, Krishna Chaitanya Chundru On 6/23/2026 2:47 PM, Manivannan Sadhasivam wrote: > On Mon, Jun 22, 2026 at 10:39:17AM +0530, Sumit Kumar wrote: >> 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 | 128 ++++++++++++++++++++++++++++++++++ >> 5 files changed, 149 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..05db91be6ffc5afe5a2022962410c96a7ec19962 >> --- /dev/null >> +++ b/drivers/bus/mhi/ep/clients/loopback.c >> @@ -0,0 +1,128 @@ >> +// 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 *loopback_wq; > s/loopback_wq/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"); > 'Failed to queue buffer' > >> + 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; > Hmm. This seems similar to patch 1 race condition. But we do not have a EP > specific API to stop a channel and flush the wq like > mhi_ep_unprepare_from_transfer(). But you should add one and call it from > remove(). mhi_ep_driver_remove() already does this before calling mhi_drv->remove(): mutex_lock(&mhi_chan->lock); mhi_chan->xfer_cb(mhi_dev, -ENOTCONN); mhi_chan->state = MHI_CH_STATE_DISABLED; mhi_chan->xfer_cb = NULL; mutex_unlock(&mhi_chan->lock); So by the time mhi_ep_loopback_remove() is entered, both channels are already stopped and xfer_cb is NULL. A mhi_ep_unprepare_from_transfer() call from remove() would be redundant. The race with mhi_ep_read_completion() is handled by setting dev_set_drvdata(NULL) before destroy_workqueue(), combined with the NULL guard in ul_callback. Please correct me if i am missing something here. -Sumit > >> + >> + 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) >> + return; > Error log? > >> + >> + mhi_ep_lb_work = kmalloc(sizeof(*mhi_ep_lb_work), GFP_KERNEL); >> + if (!mhi_ep_lb_work) { >> + kfree(buf); > Same here. > >> + 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) >> +{ >> + 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->loopback_wq = alloc_ordered_workqueue("mhi_ep_loopback", WQ_MEM_RECLAIM); >> + if (!mhi_ep_lb->loopback_wq) { >> + dev_err(&mhi_dev->dev, "Failed to create workqueue.\n"); > nit: Remove fullstop at the end of error message. > >> + 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); > As mentioned above, this should be dropped if you can ensure that xfer_cb() > won't be called. > > - Mani > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-06-24 11:12 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-06-22 5:09 [PATCH v4 0/3] bus: mhi: Add loopback driver Sumit Kumar 2026-06-22 5:09 ` [PATCH v4 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface Sumit Kumar 2026-06-23 8:45 ` Manivannan Sadhasivam 2026-06-23 10:28 ` Sumit Kumar 2026-06-23 11:20 ` Manivannan Sadhasivam 2026-06-22 5:09 ` [PATCH v4 2/3] bus: mhi: ep: Add mhi_ep_queue_buf() API for raw buffer queuing Sumit Kumar 2026-06-23 8:54 ` Manivannan Sadhasivam 2026-06-22 5:09 ` [PATCH v4 3/3] bus: mhi: ep: clients: Add loopback driver for data path testing Sumit Kumar 2026-06-23 9:17 ` Manivannan Sadhasivam 2026-06-24 11:12 ` 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®