On Mon, Aug 25, 2025 at 03:49:18PM +0530, Kishore Batta wrote:
> Register Qualcomm AIC-specific image tables with the Sahara protocol.
> The Sahara protocol provides a method for client drivers to register
> device-specific image tables, which is mandatory for firmware transfer.
> During QAIC device initialization, the QAIC driver must register the
> image table information with the Sahara protocol for firmware transfer
> to occur. Once the device is probed, it sends the required Sahara packets
> to the host. Based on the connected device, Sahara selects the appropriate
> image table and sends the firmware image data back to the device.
This does describe things that is happening. But it doesn't describe the
purpose of this patch.
>
> Signed-off-by: Kishore Batta <kishore.batta@xxxxxxxxxxxxxxxx>
> ---
> drivers/accel/qaic/mhi_controller.c | 57 +++++++++++++++++++++++++++--
> drivers/accel/qaic/mhi_controller.h | 2 +
> drivers/accel/qaic/qaic_drv.c | 7 ++++
> drivers/accel/qaic/sahara.c | 17 +++++----
> drivers/accel/qaic/sahara.h | 6 ---
> 5 files changed, 73 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/accel/qaic/mhi_controller.c b/drivers/accel/qaic/mhi_controller.c
> index 5cc7994f4809..16c346e0e3b5 100644
> --- a/drivers/accel/qaic/mhi_controller.c
> +++ b/drivers/accel/qaic/mhi_controller.c
> @@ -13,6 +13,7 @@
>
> #include "mhi_controller.h"
> #include "qaic.h"
> +#include "sahara_image_table_ops.h"
>
> #define MAX_RESET_TIME_SEC 25
>
> @@ -801,8 +802,6 @@ const char * const aic100_image_table[] = {
> [10] = "qcom/aic100/fw10.bin",
> };
>
> -const size_t aic100_image_table_size = ARRAY_SIZE(aic100_image_table);
> -
> const char * const aic200_image_table[] = {
> [5] = "qcom/aic200/uefi.elf",
> [12] = "qcom/aic200/aic200-nsp.bin",
> @@ -831,7 +830,59 @@ const char * const aic200_image_table[] = {
> [75] = "qcom/aic200/pvs.bin",
> };
>
> -const size_t aic200_image_table_size = ARRAY_SIZE(aic200_image_table);
> +static struct sahara_image_table_provider aic100_provider = {
> + .image_table = aic100_image_table,
> + .image_table_size = ARRAY_SIZE(aic100_image_table),
> + .dev_name = "AIC100",
> + .fw_folder_name = "aic100",
> + .list = LIST_HEAD_INIT(aic100_provider.list)
> +};
> +
> +static struct sahara_image_table_provider aic200_provider = {
> + .image_table = aic200_image_table,
> + .image_table_size = ARRAY_SIZE(aic200_image_table),
> + .dev_name = "AIC200",
> + .fw_folder_name = "aic200",
> + .list = LIST_HEAD_INIT(aic200_provider.list)
> +};
> +
> +static struct sahara_image_table_provider *aic_providers[] = {
> + &aic100_provider,
> + &aic200_provider,
> +};
> +
> +int qaic_sahara_register_image_tables(void)
> +{
> + int ret;
> +
> + for (int i = 0; i < ARRAY_SIZE(aic_providers); i++) {
> + ret = sahara_register_image_table_provider(aic_providers[i]);
> + if (ret) {
> + pr_err("qaic: Failed to register image table %d\n",
> + ret);
> +
> + /* Rollback previously registered providers */
> + while (--i >= 0)
> + sahara_unregister_image_table_provider(aic_providers[i]);
> +
> + return ret;
> + }
> + }
> + return 0;
> +}
> +
> +void qaic_sahara_unregister_image_tables(void)
> +{
> + int ret;
> +
> + for (int i = 0; i < ARRAY_SIZE(aic_providers); i++) {
> + ret = sahara_unregister_image_table_provider(aic_providers[i]);
> + if (ret)
> + pr_err("qaic: Failed to unregister image table %d\n",
> + ret);
> + }
> +}
> +
>
> static int mhi_read_reg(struct mhi_controller *mhi_cntrl, void __iomem *addr, u32 *out)
> {
> diff --git a/drivers/accel/qaic/mhi_controller.h b/drivers/accel/qaic/mhi_controller.h
> index 8939f6ae185e..90c0f07cbdf6 100644
> --- a/drivers/accel/qaic/mhi_controller.h
> +++ b/drivers/accel/qaic/mhi_controller.h
> @@ -12,5 +12,7 @@ struct mhi_controller *qaic_mhi_register_controller(struct pci_dev *pci_dev, voi
> void qaic_mhi_free_controller(struct mhi_controller *mhi_cntrl, bool link_up);
> void qaic_mhi_start_reset(struct mhi_controller *mhi_cntrl);
> void qaic_mhi_reset_done(struct mhi_controller *mhi_cntrl);
> +int qaic_sahara_register_image_tables(void);
> +void qaic_sahara_unregister_image_tables(void);
>
> #endif /* MHICONTROLLERQAIC_H_ */
> diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c
> index e31bcb0ecfc9..5c4fab328003 100644
> --- a/drivers/accel/qaic/qaic_drv.c
> +++ b/drivers/accel/qaic/qaic_drv.c
> @@ -688,6 +688,12 @@ static int __init qaic_init(void)
> goto free_mhi;
> }
>
> + ret = qaic_sahara_register_image_tables();
Now that you're doing this on a per-device basis (but actually per
driver), could this somehow be done from qaic_mhi_register_controller()
instead. So we don't run this code unless you actually have a QAIC
attached?
> + if (ret) {
> + pr_debug("qaic: Image table registration failed %d\n", ret);
That's not a debug print...which is also the reason why you pr_err()
inside the function. I.e. this is at best spamming the log.
> + goto free_mhi;
> + }
> +
Regards,
Bjorn
> ret = qaic_timesync_init();
> if (ret)
> pr_debug("qaic: qaic_timesync_init failed %d\n", ret);
> @@ -727,6 +733,7 @@ static void __exit qaic_exit(void)
> * reinitializing the link_up state after the cleanup is done.
> */
> link_up = true;
> + qaic_sahara_unregister_image_tables();
> qaic_ras_unregister();
> qaic_bootlog_unregister();
> qaic_timesync_deinit();
> diff --git a/drivers/accel/qaic/sahara.c b/drivers/accel/qaic/sahara.c
> index cf8f8b585223..7eae329396be 100644
> --- a/drivers/accel/qaic/sahara.c
> +++ b/drivers/accel/qaic/sahara.c
> @@ -14,6 +14,7 @@
> #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 */
> @@ -738,13 +739,15 @@ static int sahara_mhi_probe(struct mhi_device *mhi_dev, const struct mhi_device_
> INIT_WORK(&context->fw_work, sahara_processing);
> INIT_WORK(&context->dump_work, sahara_dump_processing);
>
> - if (!strcmp(mhi_dev->mhi_cntrl->name, "AIC200")) {
> - context->image_table = aic200_image_table;
> - context->table_size = aic200_image_table_size;
> - } else {
> - context->image_table = aic100_image_table;
> - context->table_size = aic100_image_table_size;
> - }
> + /* Get the image table for a given device name */
> + context->image_table = sahara_get_image_table(mhi_dev->mhi_cntrl->name);
> + if (!context->image_table)
> + return -EINVAL;
> +
> + /* Get the image table size for a given device name */
> + context->table_size = sahara_get_image_table_size(mhi_dev->mhi_cntrl->name);
> + if (!context->table_size)
> + return -EINVAL;
>
> context->active_image_id = SAHARA_IMAGE_ID_NONE;
> dev_set_drvdata(&mhi_dev->dev, context);
> diff --git a/drivers/accel/qaic/sahara.h b/drivers/accel/qaic/sahara.h
> index d7fd447ca85b..dde8c736d29e 100644
> --- a/drivers/accel/qaic/sahara.h
> +++ b/drivers/accel/qaic/sahara.h
> @@ -8,10 +8,4 @@
> int sahara_register(void);
> void sahara_unregister(void);
>
> -extern const char * const aic200_image_table[];
> -extern const size_t aic200_image_table_size;
> -
> -extern const char * const aic100_image_table[];
> -extern const size_t aic100_image_table_size;
> -
> #endif /* __SAHARA_H__ */
> --
> 2.34.1
>
