Re: [PATCH v4 4/9] bus: mhi: Centralize firmware image table selection at probe time

[Manivannan Sadhasivam]

On Thu, Mar 19, 2026 at 12:01:44PM +0530, Kishore Batta wrote:
> The Sahara driver currently selects firmware image tables using
> scattered, device specific conditionals in the probe path, making the
> logic harder to  follow and extend.
> 
> Refactor firmware image table selection into a single, explicit probe-time
> mechanism by introducing a variant table that captures device matching,
> firmware image tables, firmware folder names, and streaming behavior in
> one place.
> 
> This centralizes device specific decisions, simplifies the probe logic,
> and avoids ad-hoc conditionals while preserving the existing behavior for
> all supported AIC devices.
> 
> Signed-off-by: Kishore Batta <kishore.batta@xxxxxxxxxxxxxxxx>
> ---
>  drivers/bus/mhi/sahara/sahara.c | 66 ++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 58 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/bus/mhi/sahara/sahara.c b/drivers/bus/mhi/sahara/sahara.c
> index e3499977e7c6b53bc624a8eb00d0636f2ea63307..8f1c0d72066c0cf80c09d78bfc51df2e482133b9 100644
> --- a/drivers/bus/mhi/sahara/sahara.c
> +++ b/drivers/bus/mhi/sahara/sahara.c
> @@ -180,6 +180,16 @@ struct sahara_context {
>  	u32				read_data_length;
>  	bool				is_mem_dump_mode;
>  	bool				non_streaming;
> +	const char			*fw_folder;
> +};
> +
> +struct sahara_variant {
> +	const char *match;
> +	bool match_is_chan;

This name makes no sense.

- Mani

> +	const char * const *image_table;
> +	size_t table_size;
> +	const char *fw_folder;
> +	bool non_streaming;
>  };
>  
>  static const char * const aic100_image_table[] = {
> @@ -224,11 +234,50 @@ static const char * const aic200_image_table[] = {
>  	[78] = "qcom/aic200/pvs.bin",
>  };
>  
> +static const struct sahara_variant sahara_variants[] = {
> +	{
> +		.match = "AIC100",
> +		.match_is_chan = false,
> +		.image_table = aic100_image_table,
> +		.table_size = ARRAY_SIZE(aic100_image_table),
> +		.fw_folder = "aic100",
> +		.non_streaming = true,
> +	},
> +	{
> +		.match = "AIC200",
> +		.match_is_chan = false,
> +		.image_table = aic200_image_table,
> +		.table_size = ARRAY_SIZE(aic200_image_table),
> +		.fw_folder = "aic200",
> +		.non_streaming = false,
> +	}
> +};
> +
>  static bool is_streaming(struct sahara_context *context)
>  {
>  	return !context->non_streaming;
>  }
>  
> +static const struct sahara_variant *sahara_select_variant(struct mhi_device *mhi_dev,
> +							  const struct mhi_device_id *id)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(sahara_variants); i++) {
> +		const struct sahara_variant *v = &sahara_variants[i];
> +
> +		if (v->match_is_chan) {
> +			if (id && id->chan && !strcmp(id->chan, v->match))
> +				return v;
> +		} else {
> +			if (mhi_dev->mhi_cntrl && mhi_dev->mhi_cntrl->name &&
> +			    !strcmp(mhi_dev->mhi_cntrl->name, v->match))
> +				return v;
> +		}
> +	}
> +	return NULL;
> +}
> +
>  static int sahara_find_image(struct sahara_context *context, u32 image_id)
>  {
>  	int ret;
> @@ -797,6 +846,7 @@ static void sahara_read_data_processing(struct work_struct *work)
>  
>  static int sahara_mhi_probe(struct mhi_device *mhi_dev, const struct mhi_device_id *id)
>  {
> +	const struct sahara_variant *variant;
>  	struct sahara_context *context;
>  	int ret;
>  	int i;
> @@ -809,14 +859,14 @@ static int sahara_mhi_probe(struct mhi_device *mhi_dev, const struct mhi_device_
>  	if (!context->rx)
>  		return -ENOMEM;
>  
> -	if (!strcmp(mhi_dev->mhi_cntrl->name, "AIC200")) {
> -		context->image_table = aic200_image_table;
> -		context->table_size = ARRAY_SIZE(aic200_image_table);
> -	} else {
> -		context->image_table = aic100_image_table;
> -		context->table_size = ARRAY_SIZE(aic100_image_table);
> -		context->non_streaming = true;
> -	}
> +	variant = sahara_select_variant(mhi_dev, id);
> +	if (!variant)
> +		return -ENODEV;
> +
> +	context->image_table = variant->image_table;
> +	context->table_size = variant->table_size;
> +	context->non_streaming = variant->non_streaming;
> +	context->fw_folder = variant->fw_folder;
>  
>  	/*
>  	 * There are two firmware implementations for READ_DATA handling.
> 
> -- 
> 2.34.1
> 

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