Il 15/10/25 15:41, Arnd Bergmann ha scritto:
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]>
Thanks!
+
+ /* 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.
No, not really.
All of the platforms using firmware-name for the SCP are following the same
pattern as path... so this code autogenerates the path based on what it is
set already in DT as firmware-name where present.
The only exception is MT8188, where the firmware was named scp.img regardless
of the fact that it is dual-core and supports dual-firmware... that will need
a rename in linux-firmware. It's necessary evil, but caught just in time ;-)
P.S.: There is no MT8188 DT declaring firmware-name, so the firmware can
really just carelessly be renamed; doing so, no breakage/regression will
happen, and we again don't need to check for existance of both.
Cheers!
Angelo
