| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aPZSzbXDBar3_X9O@p14s>
Date: Mon, 20 Oct 2025 09:18:37 -0600
From: Mathieu Poirier <mathieu.poirier@...aro.org>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Cc: linux-remoteproc@...r.kernel.org, arnd@...db.de, andersson@...nel.org,
matthias.bgg@...il.com, wenst@...omium.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, kernel@...labora.com
Subject: Re: [PATCH v2] remoteproc: mtk_scp: Construct FW path if
firmware-name not present
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/7e8718b0-df78-44a6-a102-89529d6abcce@app.fastmail.com/
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
> ---
>
> 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
>
Powered by blists - more mailing lists