On Mon, Aug 25, 2025 at 03:49:19PM +0530, Kishore Batta wrote:
> Move the Sahara protocol driver from the "drivers/accel" directory
> to the "drivers/soc/qcom" directory. This change makes the Sahara
> driver applicable to all Qualcomm devices, not just Qualcomm Accelerator
> devices. It also facilitates adding support for new devices. Client drivers
> can use the registration and deregistration functionalities of the Sahara
> driver as needed.
>
> Signed-off-by: Kishore Batta <kishore.batta@xxxxxxxxxxxxxxxx>
> ---
> drivers/accel/qaic/Kconfig | 1 +
> drivers/accel/qaic/Makefile | 4 +---
> drivers/accel/qaic/mhi_controller.c | 2 +-
> drivers/accel/qaic/qaic_drv.c | 9 +--------
> drivers/soc/qcom/Kconfig | 6 ++++--
> drivers/soc/qcom/Makefile | 1 +
> drivers/soc/qcom/sahara/Kconfig | 17 +++++++++++++++++
No other drivers under drivers/soc/qcom/ has their own directory, I'm
not sure I see a reason for Sahara to be different.
> drivers/soc/qcom/sahara/Makefile | 6 ++++++
> .../{accel/qaic => soc/qcom/sahara}/sahara.c | 11 ++++++++---
> .../qcom/sahara}/sahara_image_table.c | 7 ++++++-
> {drivers/accel/qaic => include/linux}/sahara.h | 0
> .../linux}/sahara_image_table_ops.h | 0
> 12 files changed, 46 insertions(+), 18 deletions(-)
> create mode 100644 drivers/soc/qcom/sahara/Kconfig
> create mode 100644 drivers/soc/qcom/sahara/Makefile
> rename drivers/{accel/qaic => soc/qcom/sahara}/sahara.c (99%)
> rename drivers/{accel/qaic => soc/qcom/sahara}/sahara_image_table.c (94%)
> rename {drivers/accel/qaic => include/linux}/sahara.h (100%)
> rename {drivers/accel/qaic => include/linux}/sahara_image_table_ops.h (100%)
I was going to say something about other soc drivers living in
include/linux/soc/qcom/...
But that does touch upon another topic...drivers/soc/qcom is for
Qualcomm SoC drivers; and at least in the case of qaic, this driver
doesn't have anything to do with Qualcomm SoCs...
Given that this implementation only support, and is only ever going to
be used with, MHI. Perhaps drivers/bus/mhi would be a better home?
>
> diff --git a/drivers/accel/qaic/Kconfig b/drivers/accel/qaic/Kconfig
> index 5e405a19c157..5e2ac1ecede3 100644
> --- a/drivers/accel/qaic/Kconfig
> +++ b/drivers/accel/qaic/Kconfig
> @@ -8,6 +8,7 @@ config DRM_ACCEL_QAIC
> depends on DRM_ACCEL
> depends on PCI && HAS_IOMEM
> depends on MHI_BUS
> + select QCOM_SAHARA_PROTOCOL
> select CRC32
> help
> Enables driver for Qualcomm's Cloud AI accelerator PCIe cards that are
> diff --git a/drivers/accel/qaic/Makefile b/drivers/accel/qaic/Makefile
> index 586a6877e568..4ad84f7e2162 100644
> --- a/drivers/accel/qaic/Makefile
> +++ b/drivers/accel/qaic/Makefile
> @@ -11,8 +11,6 @@ qaic-y := \
> qaic_data.o \
> qaic_drv.o \
> qaic_ras.o \
> - qaic_timesync.o \
> - sahara.o \
> - sahara_image_table.o
> + qaic_timesync.o
>
> qaic-$(CONFIG_DEBUG_FS) += qaic_debugfs.o
> diff --git a/drivers/accel/qaic/mhi_controller.c b/drivers/accel/qaic/mhi_controller.c
> index 16c346e0e3b5..76beef6018a7 100644
> --- a/drivers/accel/qaic/mhi_controller.c
> +++ b/drivers/accel/qaic/mhi_controller.c
> @@ -9,11 +9,11 @@
> #include <linux/mhi.h>
> #include <linux/moduleparam.h>
> #include <linux/pci.h>
> +#include <linux/sahara_image_table_ops.h>
> #include <linux/sizes.h>
>
> #include "mhi_controller.h"
> #include "qaic.h"
> -#include "sahara_image_table_ops.h"
>
> #define MAX_RESET_TIME_SEC 25
>
> diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c
> index 5c4fab328003..a55e279411c3 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>
> #include <linux/spinlock.h>
> #include <linux/workqueue.h>
> #include <linux/wait.h>
> @@ -31,7 +32,6 @@
> #include "qaic_debugfs.h"
> #include "qaic_ras.h"
> #include "qaic_timesync.h"
> -#include "sahara.h"
>
> MODULE_IMPORT_NS("DMA_BUF");
>
> @@ -682,12 +682,6 @@ static int __init qaic_init(void)
> goto free_pci;
> }
>
> - ret = sahara_register();
> - if (ret) {
> - pr_debug("qaic: sahara_register failed %d\n", ret);
> - goto free_mhi;
> - }
> -
> ret = qaic_sahara_register_image_tables();
> if (ret) {
> pr_debug("qaic: Image table registration failed %d\n", ret);
> @@ -737,7 +731,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/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index 2caadbbcf830..7ea4cff9a679 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -295,8 +295,6 @@ config QCOM_PBS
> This module provides the APIs to the client drivers that wants to send the
> PBS trigger event to the PBS RAM.
>
> -endmenu
> -
> config QCOM_UBWC_CONFIG
> tristate
> help
> @@ -304,3 +302,7 @@ config QCOM_UBWC_CONFIG
> (UBWC) engines across various IP blocks, which need to be initialized
> with coherent configuration data. This module functions as a single
> source of truth for that information.
> +
> +source "drivers/soc/qcom/sahara/Kconfig"
> +
> +endmenu
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index b7f1d2a57367..99e490e3174e 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -40,3 +40,4 @@ qcom_ice-objs += ice.o
> obj-$(CONFIG_QCOM_INLINE_CRYPTO_ENGINE) += qcom_ice.o
> obj-$(CONFIG_QCOM_PBS) += qcom-pbs.o
> obj-$(CONFIG_QCOM_UBWC_CONFIG) += ubwc_config.o
> +obj-$(CONFIG_QCOM_SAHARA_PROTOCOL) += sahara/
> diff --git a/drivers/soc/qcom/sahara/Kconfig b/drivers/soc/qcom/sahara/Kconfig
> new file mode 100644
> index 000000000000..4be90959736e
> --- /dev/null
> +++ b/drivers/soc/qcom/sahara/Kconfig
> @@ -0,0 +1,17 @@
> +config QCOM_SAHARA_PROTOCOL
> + tristate "Qualcomm Sahara protocol"
It's bad practice to mix "select" and human selectable options. Drop the
"Qualcomm Sahara Protocol" and rely on the select to enable the driver.
> + depends on MHI_BUS
> + select FW_LOADER_COMPRESS
> + select FW_LOADER_COMPRESS_XZ
> + select FW_LOADER_COMPRESS_ZSTD
> + help
> + The sahara protocol is primarily designed for transferring software
> + images from a host device to a target device using a simplified data
> + transfer mechanism over any physical link. However, the sahara
> + protocol does not support any authentication/validation of data sent
> + between devices. Such mechanism is beyond the scope of protocol.
> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the module will be
> + called qcom_sahara.
> diff --git a/drivers/soc/qcom/sahara/Makefile b/drivers/soc/qcom/sahara/Makefile
> new file mode 100644
> index 000000000000..ad3922b30a31
> --- /dev/null
> +++ b/drivers/soc/qcom/sahara/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +obj-$(CONFIG_QCOM_SAHARA_PROTOCOL) := qcom_sahara.o
> +
> +qcom_sahara-y := sahara.o \
> + sahara_image_table.o
> diff --git a/drivers/accel/qaic/sahara.c b/drivers/soc/qcom/sahara/sahara.c
> similarity index 99%
> rename from drivers/accel/qaic/sahara.c
> rename to drivers/soc/qcom/sahara/sahara.c
> index 7eae329396be..5e17d71a2d34 100644
> --- a/drivers/accel/qaic/sahara.c
> +++ b/drivers/soc/qcom/sahara/sahara.c
> @@ -9,13 +9,12 @@
Make sure the style of the copyright comment matches the subsystem where
you move this driver to.
> #include <linux/minmax.h>
> #include <linux/mod_devicetable.h>
> #include <linux/overflow.h>
> +#include <linux/sahara.h>
> +#include <linux/sahara_image_table_ops.h>
> #include <linux/types.h>
> #include <linux/vmalloc.h>
> #include <linux/workqueue.h>
>
> -#include "sahara.h"
> -#include "sahara_image_table_ops.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 */
> @@ -814,8 +813,14 @@ 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_AUTHOR("Qualcomm Innovation Center, Inc");
Skip MODULE_AUTHOR, or correct it.
> +MODULE_DESCRIPTION("Sahara driver");
There's already another driver by the name "sahara", so be more
specific.
Regards,
Bjorn
> diff --git a/drivers/accel/qaic/sahara_image_table.c b/drivers/soc/qcom/sahara/sahara_image_table.c
> similarity index 94%
> rename from drivers/accel/qaic/sahara_image_table.c
> rename to drivers/soc/qcom/sahara/sahara_image_table.c
> index dd0793a33727..18f9b7a59f25 100644
> --- a/drivers/accel/qaic/sahara_image_table.c
> +++ b/drivers/soc/qcom/sahara/sahara_image_table.c
> @@ -5,8 +5,8 @@
> #include <linux/device.h>
> #include <linux/list.h>
> #include <linux/mutex.h>
> +#include <linux/sahara_image_table_ops.h>
>
> -#include "sahara_image_table_ops.h"
>
> struct sahara_image_table_context {
> struct list_head provider_list;
> @@ -49,6 +49,7 @@ int sahara_register_image_table_provider(struct sahara_image_table_provider
>
> return 0;
> }
> +EXPORT_SYMBOL_GPL(sahara_register_image_table_provider);
>
> /**
> * sahara_get_image_table - Get the image table for a given device name
> @@ -78,6 +79,7 @@ const char * const *sahara_get_image_table(const char *dev_name)
>
> return NULL;
> }
> +EXPORT_SYMBOL_GPL(sahara_get_image_table);
>
> /**
> * sahara_get_image_table_size - Get the size of the image table for a given
> @@ -109,6 +111,7 @@ size_t sahara_get_image_table_size(const char *dev_name)
>
> return 0;
> }
> +EXPORT_SYMBOL_GPL(sahara_get_image_table_size);
>
> /**
> * sahara_unregister_image_table_provider - Unregister an image table provider.
> @@ -139,6 +142,7 @@ int sahara_unregister_image_table_provider(struct sahara_image_table_provider
>
> return 0;
> }
> +EXPORT_SYMBOL_GPL(sahara_unregister_image_table_provider);
>
> /**
> * sahara_get_fw_folder_name - Retrieve the firmware folder name for a given
> @@ -171,3 +175,4 @@ char *sahara_get_fw_folder_name(const char *dev_name)
>
> return NULL;
> }
> +EXPORT_SYMBOL_GPL(sahara_get_fw_folder_name);
> 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
> diff --git a/drivers/accel/qaic/sahara_image_table_ops.h b/include/linux/sahara_image_table_ops.h
> similarity index 100%
> rename from drivers/accel/qaic/sahara_image_table_ops.h
> rename to include/linux/sahara_image_table_ops.h
> --
> 2.34.1
>
