Re: [PATCH v9 02/19] media: iris: Add Gen2 firmware autodetect and fallback

[Vikash Garodia <vikash.garodia@oss.qualcomm.com>]

On 9/24/2026 6:06 PM, Bryan O'Donoghue wrote:
> On 31/07/2026 00:48, Dmitry Baryshkov wrote:
>> +
>> +/*
>> + * Detect the firmware generation and publish the descriptor. Run 
>> only after
>> + * qcom_mdt_load() has succeeded, so the driver commits to a HFI 
>> generation
>> + * only for a firmware image that has actually been loaded.
>> + *
>> + * The generation is detected from the loaded image (@data / @size 
>> point at the
>> + * reserved memory region populated by qcom_mdt_load()) rather than 
>> from the
>> + * request_firmware() blob: for a split .mdt the latter holds only 
>> the ELF
>> + * headers, while QC_IMAGE_VERSION_STRING lives in the .bNN data 
>> segments.
>> + *
>> + * The descriptor and firmware data are published exactly once, 
>> before any
>> + * session exists, so the lockless readers in the ioctl paths never 
>> observe a
>> + * reassignment. Later bring-ups reuse the already published descriptor.
>> + */
>> +static void iris_detect_firmware(struct iris_core *core, const char 
>> *fw_name,
>> +                 const u8 *data, size_t size,
>> +                 const struct iris_firmware_desc *desc)
>> +{
>> +    if (core->iris_firmware_desc)
>> +        return;
>> +
>> +    /*
>> +     * With a DT firmware-name override on a dual-generation platform 
>> the
>> +     * image on disk decides the generation, so inspect it and switch 
>> to the
>> +     * Gen1 descriptor when a Gen1 image was loaded.
>> +     */
>> +    if (desc == core->iris_platform_data->firmware_desc_gen2 &&
>> +        core->iris_platform_data->firmware_desc_gen1 &&
>> +        of_property_present(dev_of_node(core->dev), "firmware-name") &&
>> +        !iris_detect_gen2_from_fwdata(data, size)) {
>> +        dev_info(core->dev, "Gen1 FW detected in %s\n", fw_name);
>> +        desc = core->iris_platform_data->firmware_desc_gen1;
>> +    }
>> +
>> +    /* Publish iris_firmware_data first, then iris_firmware_desc (the 
>> guard). */
>> +    core->iris_firmware_data = desc->firmware_data;
>> +    core->iris_firmware_desc = desc;
>> +}
>> +
> 
> In function ‘iris_detect_firmware’,
>      inlined from ‘iris_load_fw_to_memory’ at drivers/media/platform/ 
> qcom/iris/iris_firmware.c:216:3:
> drivers/media/platform/qcom/iris/iris_firmware.c:161:12: warning: ‘desc’ 
> may be used uninitialized [-Wmaybe-uninitialized]
>    161 |         if (desc == core->iris_platform_data- 
>  >firmware_desc_gen2 &&
>        |            ^
> drivers/media/platform/qcom/iris/iris_firmware.c: In function 
> ‘iris_load_fw_to_memory’:
> drivers/media/platform/qcom/iris/iris_firmware.c:176:42: note: ‘desc’ 
> was declared here
>    176 |         const struct iris_firmware_desc *desc;
>        |                                          ^~~~
> In function ‘iris_detect_firmware’,
>      inlined from ‘iris_load_fw_to_memory’ at drivers/media/platform/ 
> qcom/iris/iris_firmware.c:216:3:
> drivers/media/platform/qcom/iris/iris_firmware.c:170:40: warning: ‘desc’ 
> may be used uninitialized [-Wmaybe-uninitialized]
>    170 |         core->iris_firmware_data = desc->firmware_data;
>        |                                    ~~~~^~~~~~~~~~~~~~~
> drivers/media/platform/qcom/iris/iris_firmware.c: In function 
> ‘iris_load_fw_to_memory’:
> drivers/media/platform/qcom/iris/iris_firmware.c:176:42: note: ‘desc’ 
> was declared here
>    176 |         const struct iris_firmware_desc *desc;
>        |                                          ^~~~
> . "$TS_DIR"/lib/common-end.sh
> #!/bin/sh
> # Copyright 2024 Google LLC
> 
> Proposed fix
> 
> ╰─$ git diff
> diff --git a/drivers/media/platform/qcom/iris/iris_firmware.c b/drivers/ 
> media/platform/qcom/iris/iris_firmware.c
> index dfc4c184646c6..39558a300d38c 100644
> --- a/drivers/media/platform/qcom/iris/iris_firmware.c
> +++ b/drivers/media/platform/qcom/iris/iris_firmware.c
> @@ -153,6 +153,9 @@ static void iris_detect_firmware(struct iris_core 
> *core, const char *fw_name,
>         if (core->iris_firmware_desc)
>                 return;
> 
> +      if (!desc)
> +              return;
> +

Reviewed-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>