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

On 4/13/2026 4:56 PM, Manivannan Sadhasivam wrote:
> 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 <[email protected]>
>> ---
>>   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
I will drop this in the next version.
>> +	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
>>