On Wed, Oct 15, 2025 at 10:41:03AM +0200, 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]>
> ---
>
> Changes in v2:
> - Removed initialization of scp_fw_file[7] char array (or string if you
> prefer)
>
> drivers/remoteproc/mtk_scp.c | 65 ++++++++++++++++++++++++++++++++----
> 1 file changed, 59 insertions(+), 6 deletions(-)
>
Applied.
Thanks,
Mathieu
> diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> index 8206a1766481..10e3f9eb8cd2 100644
> --- a/drivers/remoteproc/mtk_scp.c
> +++ b/drivers/remoteproc/mtk_scp.c
> @@ -16,6 +16,7 @@
> #include <linux/remoteproc.h>
> #include <linux/remoteproc/mtk_scp.h>
> #include <linux/rpmsg/mtk_rpmsg.h>
> +#include <linux/string.h>
>
> #include "mtk_common.h"
> #include "remoteproc_internal.h"
> @@ -1093,22 +1094,74 @@ static void scp_remove_rpmsg_subdev(struct mtk_scp
> *scp)
> }
> }
>
> +/**
> + * scp_get_default_fw_path() - Get default SCP firmware path
> + * @dev: SCP Device
> + * @core_id: SCP Core number
> + *
> + * This function generates a path based on the following format:
> + * mediatek/(soc_model)/scp(_cX).img; for multi-core or
> + * mediatek/(soc_model)/scp.img for single core SCP HW
> + *
> + * Return: A devm allocated string containing the full path to
> + * a SCP firmware or an error pointer
> + */
> +static const char *scp_get_default_fw_path(struct device *dev, int core_id)
> +{
> + struct device_node *np = core_id < 0 ? dev->of_node :
> dev->parent->of_node;
> + const char *compatible, *soc;
> + char scp_fw_file[7];
> + int ret;
> +
> + /* Use only the first compatible string */
> + ret = of_property_read_string_index(np, "compatible", 0, &compatible);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + /* If the compatible string's length is implausible bail out early */
> + if (strlen(compatible) < strlen("mediatek,mtXXXX-scp"))
> + return ERR_PTR(-EINVAL);
> +
> + /* 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);
> +}
> +
> static struct mtk_scp *scp_rproc_init(struct platform_device *pdev,
> struct mtk_scp_of_cluster *scp_cluster,
> - const struct mtk_scp_of_data *of_data)
> + const struct mtk_scp_of_data *of_data,
> + int core_id)
> {
> struct device *dev = &pdev->dev;
> struct device_node *np = dev->of_node;
> struct mtk_scp *scp;
> struct rproc *rproc;
> struct resource *res;
> - const char *fw_name = "scp.img";
> + const char *fw_name;
> int ret, i;
> const struct mtk_scp_sizes_data *scp_sizes;
>
> ret = rproc_of_parse_firmware(dev, 0, &fw_name);
> - if (ret < 0 && ret != -EINVAL)
> - return ERR_PTR(ret);
> + if (ret) {
> + fw_name = scp_get_default_fw_path(dev, core_id);
> + if (IS_ERR(fw_name)) {
> + dev_err(dev, "Cannot get firmware path: %ld\n",
> PTR_ERR(fw_name));
> + return ERR_CAST(fw_name);
> + }
> + }
>
> rproc = devm_rproc_alloc(dev, np->name, &scp_ops, fw_name,
> sizeof(*scp));
> if (!rproc) {
> @@ -1212,7 +1265,7 @@ static int scp_add_single_core(struct platform_device
> *pdev,
> struct mtk_scp *scp;
> int ret;
>
> - scp = scp_rproc_init(pdev, scp_cluster, of_device_get_match_data(dev));
> + scp = scp_rproc_init(pdev, scp_cluster, of_device_get_match_data(dev),
> -1);
> if (IS_ERR(scp))
> return PTR_ERR(scp);
>
> @@ -1259,7 +1312,7 @@ static int scp_add_multi_core(struct platform_device
> *pdev,
> goto init_fail;
> }
>
> - scp = scp_rproc_init(cpdev, scp_cluster,
> cluster_of_data[core_id]);
> + scp = scp_rproc_init(cpdev, scp_cluster,
> cluster_of_data[core_id], core_id);
> put_device(&cpdev->dev);
> if (IS_ERR(scp)) {
> ret = PTR_ERR(scp);
> --
> 2.51.0
>
