Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] sahara: unintended firmware fallback behavior for AIC100/AIC200 --
commit 02c795f9adf45b209ef5da56b858e0490ac4667d Author: Kishore Batta <[email protected]> bus: mhi: Add QDU100 Sahara variant and firmware fallback This commit extends the Sahara driver to recognize QDU100 devices and adds a firmware lookup fallback for unspecified image IDs. It allows QDU100 devices to request required images to be provisioned by the platform. > bus: mhi: Add QDU100 Sahara variant and firmware fallback > > The Sahara driver currently selects a firmware image table based on the > attached device, but it does not recognize QDU100 devices that expose the > protocol on the SAHARA MHI channel. As a result, the host cannot associate > QDU100 devices with the correct firmware namespace during image transfer. > > 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 allows required images to be provisioned by > the platform. > > This change only affects devices matched on the SAHARA MHI channel and > does not change behavior for existing AIC100 and AIC200 devices. [Severity: Medium] Is this statement accurate? Since fw_folder is populated for existing AIC100 devices (for example, in drivers/accel/qaic/mhi_controller.c where aic100_sahara_fw sets fw_folder = "aic100"), the new fallback lookup in sahara_find_image() will also apply to them. If an AIC100 target requests an unspecified image ID, it will now trigger a filesystem lookup instead of being immediately rejected. Does this unintentionally change the behavior for existing devices? > Signed-off-by: Kishore Batta <[email protected]> > diff --git a/drivers/bus/mhi/host/clients/sahara/sahara.c > b/drivers/bus/mhi/host/clients/sahara/sahara.c > index e339c67e236af..7fe5158c17e98 100644 > --- a/drivers/bus/mhi/host/clients/sahara/sahara.c > +++ b/drivers/bus/mhi/host/clients/sahara/sahara.c [ ... ] > @@ -201,8 +202,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); [Severity: Medium] Could this unintentionally execute for AIC100 and AIC200 devices? Because aic100_sahara_fw sets .fw_folder = "aic100", context->fw_folder will be valid for these devices. An unknown image_id request from an AIC100 device will fall through to this path and perform a filesystem lookup instead of returning -EINVAL immediately. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20261008-sahara_protocol_new_v2-v7-0-90d278bd7...@oss.qualcomm.com?part=4
