On Wed, Oct 15, 2025, at 10:41, AngeloGioacchino Del Regno wrote: > After a reply on the mailing lists [1] it emerged that the DT > property "firmware-name" should not be relied on because of > possible issues with firmware versions. > For MediaTek SCP, there has never been any firmware version vs > driver version desync issue but, regardless, the firmwares are > always using the same name and they're always located in a path > with a specific pattern. > > Instead of unconditionally always relying on the firmware-name > devicetree property to get a path to the SCP FW file, drivers > should construct a name based on what firmware it knows and > what hardware it is running on. > > In order to do that, add a `scp_get_default_fw_path()` function > that constructs the path and filename based on two of the infos > that the driver can get: > 1. The compatible string with the highest priority (so, the > first one at index 0); and > 2. The type of SCP HW - single-core or multi-core. > > This means that the default firmware path is generated as: > - Single core SCP: mediatek/(soc_model)/scp.img > for example: mediatek/mt8183/scp.img; > > - Multi core SCP: mediatek/(soc_model)/scp_c(core_number).img > for example: mediatek/mt8188/scp_c0.img for Core 0, and > mediatek/mt8188/scp_c1.img for Core 1. > > Note that the generated firmware path is being used only if the > "firmware-name" devicetree property is not present in the SCP > node or in the SCP Core node(s). > > [1 - Reply regarding firmware-name property] > Link: > https://lore.kernel.org/all/[email protected]/ > Signed-off-by: AngeloGioacchino Del Regno > <[email protected]>
This looks good to me, thanks for the changes! Reviewed-by: Arnd Bergmann <[email protected]> > + > + /* If the compatible string starts with "mediatek,mt" assume that it's > ok */ > + if (!str_has_prefix(compatible, "mediatek,mt")) > + return ERR_PTR(-EINVAL); > + > + if (core_id >= 0) > + ret = snprintf(scp_fw_file, ARRAY_SIZE(scp_fw_file), > "scp_c%1d", core_id); > + else > + ret = snprintf(scp_fw_file, ARRAY_SIZE(scp_fw_file), "scp"); > + if (ret <= 0) > + return ERR_PTR(ret); > + > + /* Not using strchr here, as strlen of a const gets optimized by > compiler */ > + soc = &compatible[strlen("mediatek,")]; > + > + return devm_kasprintf(dev, GFP_KERNEL, "mediatek/%.*s/%s.img", > + (int)strlen("mtXXXX"), soc, scp_fw_file); > +} This might have to eventually support version numbers, in case there are ever incompatible ABIs where updating the firmware requires a minimum kernel driver version. Until there is a firmware file that needs it, this code is fine. I'm not sure if you need to handle the case where there is a "firmware-name" property in DT, but the filesystem only contains the generated filenames, or when the generated firmware name refers to a newer version. It may be best to try both the generated names and the name from the dts file. Arnd
