Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] mhi: sahara: patch changes behavior for AIC100/AIC200 despite commit message claims --
commit 95400c2cb3bcfef8a4adb0c664417375ff8137c7 Author: Kishore Batta <[email protected]> bus: mhi: Add QDU100 Sahara variant and firmware fallback Extend the probe time variant selection to match the SAHARA MHI channel and associate it with the QDU100 firmware folder. Add a firmware lookup fallback for cases where an image does not have an explicit entry in the device's firmware table. > This change only affects devices matched on the SAHARA MHI channel and > does not change behavior for existing AIC100 and AIC200 devices. Is this description accurate? The qaic controller driver configures aic100_sahara_fw and aic200_sahara_fw to populate the fw_folder field as "aic100" and "aic200". With the new fallback logic in sahara_find_image(), if an existing AIC device requests an unknown image ID, it appears it will now trigger the new fallback path and perform synchronous VFS lookups for "qcom/aic.../<id>", whereas previously it would immediately fail with -EINVAL. > diff --git a/drivers/bus/mhi/host/clients/sahara/sahara.c > b/drivers/bus/mhi/host/clients/sahara/sahara.c > index ce4fcbd40baa1..a4b3ef197b303 100644 > --- a/drivers/bus/mhi/host/clients/sahara/sahara.c > +++ b/drivers/bus/mhi/host/clients/sahara/sahara.c [ ... ] > @@ -200,8 +201,28 @@ static int sahara_find_image(struct sahara_context > *context, u32 image_id) > } > > if (image_id >= context->table_size || !context->image_table[image_id]) > { > - dev_err(&context->mhi_dev->dev, "request for unknown image: > %d\n", image_id); > - return -EINVAL; > + if (!context->fw_folder) { > + dev_err(&context->mhi_dev->dev, > + "Request for unknown image: %u (no fw > folder)\n", image_id); > + return -EINVAL; > + } > + > + fw_path = kasprintf(GFP_KERNEL, "qcom/%s/%u", > + context->fw_folder, image_id); > + if (!fw_path) > + return -ENOMEM; > + > + ret = firmware_request_nowarn(&context->firmware, > + fw_path, > + &context->mhi_dev->dev); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [Severity: Medium] Could this fallback path introduce a regression for AIC100 and AIC200 devices? It looks like those devices have a fw_folder configured, so they would execute this filesystem lookup for unknown IDs rather than immediately failing with -EINVAL as they did before this patch. > + kfree(fw_path); > + if (ret) { > + dev_err(&context->mhi_dev->dev, > + "request for unknown image: %d\n", image_id); > + return -EINVAL; > + } > + context->active_image_id = image_id; > + return 0; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20261013-sahara_protocol_new_v2-v9-0-dc9f24357...@oss.qualcomm.com?part=4
