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

[Kishore Batta <kishore.batta@oss.qualcomm.com>]

On 4/10/2026 2:22 AM, Jeff Hugo wrote:
> On 3/19/2026 12:31 AM, 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.
>
> "to follow" (remove extra space)


ACK.

>
>> 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.
>
> It would probably be useful to mention this is in preparation for 
> adding QDU100 support, otherwise this reads like it is just change for 
> the sake of change.


Sure. I will mention in next version.

>
>> Signed-off-by: Kishore Batta <kishore.batta@oss.qualcomm.com>
>> ---
>>   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 is dead code, add it later on when it gets used.


ACK.

>
>> +    const char * const *image_table;
>> +    size_t table_size;
>> +    const char *fw_folder;
>
> This is dead code, add it later on when it gets used.

ACK.
>
>> +    bool non_streaming;
>
> Please run pahole on this structure. With the interleaving of types 
> for every other member, I'm expecting there would be quite a bit of 
> compiler added padding.


Sure. I had ran pahole on this structure and 8 bytes can be saved. I 
will rearrange the members of the structure in the next version.

>
>