Re: [PATCH v4 2/9] bus: mhi: Move sahara protocol driver under drivers/bus/mhi

From: Jeff Hugo

Date: Thu Apr 09 2026 - 16:24:57 EST

On 3/19/2026 12:31 AM, Kishore Batta wrote:

The Sahara protocol driver is currently located under the QAIC
accelerator subsystem even though protocol itself is transported over the
MHI bus and is used by multiple Qualcomm flashless devices.

Relocate the Sahara protocol driver to drivers/bus/mhi and register it as
an independent MHI protocol driver. This avoids treating Sahara as QAIC
specific and makes it available for reuse by other MHI based devices.

As part of this move, introduce a dedicated Kconfig and Makefile under the
MHI subsystem and expose the sahara interface via a common header.

I don't think this belongs under MHI. Mani needs to confirm that he agrees with the concept of moving this there.

The Sahara protocol as defined by the spec does not require MHI. We know that there are Sahara implementations over USB. I don't see a dependency or relationship to MHI other than the current in-kernel implementation uses MHI, but there are plenty of things that use MHI (qaic, mhi-net, ath12k, etc) which are not a part of the MHI bus.

The implementation presented in this series is not well integrated into MHI, which also suggests to me that it doesn't belong there. The Documentation is not integrated with MHI (which I mentioned over on that patch) and I see the header file (sahara.h) is also not integrated.

diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c
index 63fb8c7b4abcbe4f1b76c32106f4e8b9ea5e2c8e..76cc8086825e7949ed756d51fcb56a08f392d228 100644
--- a/drivers/accel/qaic/qaic_drv.c
+++ b/drivers/accel/qaic/qaic_drv.c
@@ -15,6 +15,7 @@
#include <linux/msi.h>
#include <linux/mutex.h>
#include <linux/pci.h>
+#include <linux/sahara.h>

What do we need this for? register()/unregister() get removed.

#include <linux/spinlock.h>
#include <linux/workqueue.h>
#include <linux/wait.h>
@@ -32,7 +33,6 @@
#include "qaic_ras.h"
#include "qaic_ssr.h"
#include "qaic_timesync.h"
-#include "sahara.h"
MODULE_IMPORT_NS("DMA_BUF");
@@ -782,18 +782,12 @@ static int __init qaic_init(void)
ret = pci_register_driver(&qaic_pci_driver);
if (ret) {
pr_debug("qaic: pci_register_driver failed %d\n", ret);
- return ret;
+ goto free_pci;

This is wrong, and there should not be a change here.

ret = mhi_driver_register(&qaic_mhi_driver);
if (ret) {
pr_debug("qaic: mhi_driver_register failed %d\n", ret);
- goto free_pci;
- }
-
- ret = sahara_register();
- if (ret) {
- pr_debug("qaic: sahara_register failed %d\n", ret);
goto free_mhi;

This is also wrong

@@ -847,7 +841,6 @@ static void __exit qaic_exit(void)
qaic_ras_unregister();
qaic_bootlog_unregister();
qaic_timesync_deinit();
- sahara_unregister();
mhi_driver_unregister(&qaic_mhi_driver);
pci_unregister_driver(&qaic_pci_driver);
}
diff --git a/drivers/bus/mhi/Kconfig b/drivers/bus/mhi/Kconfig
index b39a11e6c624ba00349cca22d74bd876020590ab..4acedb886adccc6f76f69c241d53106da59b491f 100644
--- a/drivers/bus/mhi/Kconfig
+++ b/drivers/bus/mhi/Kconfig
@@ -7,3 +7,4 @@
source "drivers/bus/mhi/host/Kconfig"
source "drivers/bus/mhi/ep/Kconfig"
+source "drivers/bus/mhi/sahara/Kconfig"
diff --git a/drivers/bus/mhi/Makefile b/drivers/bus/mhi/Makefile
index 354204b0ef3ae4030469a24a659f32429d592aef..e4af535e1bb1bc9481fae60d7eb347700d2e874c 100644
--- a/drivers/bus/mhi/Makefile
+++ b/drivers/bus/mhi/Makefile
@@ -3,3 +3,6 @@ obj-$(CONFIG_MHI_BUS) += host/
# Endpoint MHI stack
obj-$(CONFIG_MHI_BUS_EP) += ep/
+
+# Sahara MHI protocol
+obj-$(CONFIG_MHI_SAHARA) += sahara/
diff --git a/drivers/bus/mhi/sahara/Kconfig b/drivers/bus/mhi/sahara/Kconfig
new file mode 100644
index 0000000000000000000000000000000000000000..3f1caf6acd979a4af68aaf0e250aa54762e8cda5
--- /dev/null
+++ b/drivers/bus/mhi/sahara/Kconfig
@@ -0,0 +1,18 @@
+config MHI_SAHARA
+ tristate
+ depends on MHI_BUS
+ select FW_LOADER_COMPRESS

Why are we selecting this? I don't see anyone else doing this. Sahara should work with and without firmware compression.

+ select FW_LOADER_COMPRESS_XZ
+ select FW_LOADER_COMPRESS_ZSTD
+ help
+ Enable support for the Sahara protocol transported over the MHI bus.
+
+ The Sahara protocol is used to transfer firmware images, retrieve
+ memory dumps and exchange command mode DDR calibration data between
+ host and device. This driver is not tied to a specific SoC and may be
+ used by multiple MHI based devices.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the module will be
+ called mhi_sahara.
diff --git a/drivers/bus/mhi/sahara/Makefile b/drivers/bus/mhi/sahara/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..fc02a25935011cbd7138ea8f24b88cf5b032a4ce
--- /dev/null
+++ b/drivers/bus/mhi/sahara/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_MHI_SAHARA) += mhi_sahara.o
+mhi_sahara-y := sahara.o
diff --git a/drivers/accel/qaic/sahara.c b/drivers/bus/mhi/sahara/sahara.c
similarity index 99%
rename from drivers/accel/qaic/sahara.c
rename to drivers/bus/mhi/sahara/sahara.c
index fd3c3b2d1fd3bb698809e6ca669128e2dce06613..8ff7b6425ac5423ef8f32117151dca10397686a8 100644
--- a/drivers/accel/qaic/sahara.c
+++ b/drivers/bus/mhi/sahara/sahara.c
@@ -1,6 +1,8 @@
-// SPDX-License-Identifier: GPL-2.0-only
-
-/* Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. */
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.
+ *
+ */

What makes you think that changing the copyright markings is appropiate when moving a file?

Furthermore, I wrote this code from scratch based on the spec document and therefore know beyond a doubt that this file did not exist prior to 2024, so what you are changing the markings to is completely invalid.

Also the SPDX marking you are using is long deprecated and should not be used.

#include <linux/devcoredump.h>
#include <linux/firmware.h>
@@ -9,12 +11,11 @@
#include <linux/minmax.h>
#include <linux/mod_devicetable.h>
#include <linux/overflow.h>
+#include <linux/sahara.h>
#include <linux/types.h>
#include <linux/vmalloc.h>
#include <linux/workqueue.h>
-#include "sahara.h"
-
#define SAHARA_HELLO_CMD 0x1 /* Min protocol version 1.0 */
#define SAHARA_HELLO_RESP_CMD 0x2 /* Min protocol version 1.0 */
#define SAHARA_READ_DATA_CMD 0x3 /* Min protocol version 1.0 */
@@ -928,8 +929,13 @@ int sahara_register(void)
{
return mhi_driver_register(&sahara_mhi_driver);
}
+module_init(sahara_register);
void sahara_unregister(void)
{
mhi_driver_unregister(&sahara_mhi_driver);
}
+module_exit(sahara_unregister);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Qualcomm Sahara MHI protocol driver");
diff --git a/drivers/accel/qaic/sahara.h b/include/linux/sahara.h
similarity index 100%
rename from drivers/accel/qaic/sahara.h
rename to include/linux/sahara.h