* [PATCH v2] remoteproc: mtk_scp: Construct FW path if firmware-name not present
@ 2025-10-15 8:41 AngeloGioacchino Del Regno
2025-10-15 13:41 ` Arnd Bergmann
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: AngeloGioacchino Del Regno @ 2025-10-15 8:41 UTC (permalink / raw)
To: linux-remoteproc
Cc: arnd, andersson, mathieu.poirier, matthias.bgg,
angelogioacchino.delregno, wenst, linux-kernel, linux-arm-kernel,
linux-mediatek, kernel
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@collabora.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(-)
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] remoteproc: mtk_scp: Construct FW path if firmware-name not present
2025-10-15 8:41 [PATCH v2] remoteproc: mtk_scp: Construct FW path if firmware-name not present AngeloGioacchino Del Regno
@ 2025-10-15 13:41 ` Arnd Bergmann
2025-10-15 13:49 ` AngeloGioacchino Del Regno
2025-10-20 15:18 ` Mathieu Poirier
2025-10-29 9:14 ` Chen-Yu Tsai
2 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2025-10-15 13:41 UTC (permalink / raw)
To: AngeloGioacchino Del Regno, linux-remoteproc
Cc: Bjorn Andersson, Mathieu Poirier, Matthias Brugger, Chen-Yu Tsai,
linux-kernel, linux-arm-kernel, linux-mediatek, kernel
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/7e8718b0-df78-44a6-a102-89529d6abcce@app.fastmail.com/
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
This looks good to me, thanks for the changes!
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> +
> + /* 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] remoteproc: mtk_scp: Construct FW path if firmware-name not present
2025-10-15 13:41 ` Arnd Bergmann
@ 2025-10-15 13:49 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 10+ messages in thread
From: AngeloGioacchino Del Regno @ 2025-10-15 13:49 UTC (permalink / raw)
To: Arnd Bergmann, linux-remoteproc
Cc: Bjorn Andersson, Mathieu Poirier, Matthias Brugger, Chen-Yu Tsai,
linux-kernel, linux-arm-kernel, linux-mediatek, kernel
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/7e8718b0-df78-44a6-a102-89529d6abcce@app.fastmail.com/
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> This looks good to me, thanks for the changes!
>
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
>
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] remoteproc: mtk_scp: Construct FW path if firmware-name not present
2025-10-15 8:41 [PATCH v2] remoteproc: mtk_scp: Construct FW path if firmware-name not present AngeloGioacchino Del Regno
2025-10-15 13:41 ` Arnd Bergmann
@ 2025-10-20 15:18 ` Mathieu Poirier
2025-10-29 9:14 ` Chen-Yu Tsai
2 siblings, 0 replies; 10+ messages in thread
From: Mathieu Poirier @ 2025-10-20 15:18 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: linux-remoteproc, arnd, andersson, matthias.bgg, wenst,
linux-kernel, linux-arm-kernel, linux-mediatek, kernel
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@collabora.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
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] remoteproc: mtk_scp: Construct FW path if firmware-name not present
2025-10-15 8:41 [PATCH v2] remoteproc: mtk_scp: Construct FW path if firmware-name not present AngeloGioacchino Del Regno
2025-10-15 13:41 ` Arnd Bergmann
2025-10-20 15:18 ` Mathieu Poirier
@ 2025-10-29 9:14 ` Chen-Yu Tsai
2025-10-29 11:05 ` AngeloGioacchino Del Regno
2 siblings, 1 reply; 10+ messages in thread
From: Chen-Yu Tsai @ 2025-10-29 9:14 UTC (permalink / raw)
To: AngeloGioacchino Del Regno, mathieu.poirier
Cc: linux-remoteproc, arnd, andersson, matthias.bgg, linux-kernel,
linux-arm-kernel, linux-mediatek, kernel
On Wed, Oct 15, 2025 at 4:41 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> 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.
I know this patch has been applied, but this scheme doesn't actually
follow what is already in the linux-firmware repository.
For all the supported platforms, the first core, even for multi core SCP,
already have their firmware uploaded as just "scp.img". Multicore SCP
is seen in MT8195 and MT8188.
I guess I can send a followup patch?
ChenYu
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] remoteproc: mtk_scp: Construct FW path if firmware-name not present
2025-10-29 9:14 ` Chen-Yu Tsai
@ 2025-10-29 11:05 ` AngeloGioacchino Del Regno
2025-10-30 8:21 ` Chen-Yu Tsai
0 siblings, 1 reply; 10+ messages in thread
From: AngeloGioacchino Del Regno @ 2025-10-29 11:05 UTC (permalink / raw)
To: Chen-Yu Tsai, mathieu.poirier
Cc: linux-remoteproc, arnd, andersson, matthias.bgg, linux-kernel,
linux-arm-kernel, linux-mediatek, kernel
Il 29/10/25 10:14, Chen-Yu Tsai ha scritto:
> On Wed, Oct 15, 2025 at 4:41 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> 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.
>
> I know this patch has been applied, but this scheme doesn't actually
> follow what is already in the linux-firmware repository.
>
> For all the supported platforms, the first core, even for multi core SCP,
> already have their firmware uploaded as just "scp.img". Multicore SCP
> is seen in MT8195 and MT8188.
The only one that is affected is MT8188, which needs a rename or a symlink in
linux-firmware.
MT8195 is not affected by this change, because the SCP is used as single-core,
hence this code will look for scp.img and not for scp_c0.img.
>
> I guess I can send a followup patch?
The only followup patch that I deem to be necessary is one adding a symlink
or renaming for MT8188's SCP and nothing else.
Please remember that some of those SoCs (including MT8195) allow the SCP to be
configured as *either* single-core *or* dual-core - and usually firmwares for
single-core configurations are not compatible with dual-core ones, because of
the SRAM carveout/usage.
Cheers,
Angelo
>
>
> ChenYu
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] remoteproc: mtk_scp: Construct FW path if firmware-name not present
2025-10-29 11:05 ` AngeloGioacchino Del Regno
@ 2025-10-30 8:21 ` Chen-Yu Tsai
2025-10-30 9:10 ` Arnd Bergmann
2025-10-30 9:33 ` AngeloGioacchino Del Regno
0 siblings, 2 replies; 10+ messages in thread
From: Chen-Yu Tsai @ 2025-10-30 8:21 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: mathieu.poirier, linux-remoteproc, arnd, andersson, matthias.bgg,
linux-kernel, linux-arm-kernel, linux-mediatek, kernel
On Wed, Oct 29, 2025 at 7:05 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 29/10/25 10:14, Chen-Yu Tsai ha scritto:
> > On Wed, Oct 15, 2025 at 4:41 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com> 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.
> >
> > I know this patch has been applied, but this scheme doesn't actually
> > follow what is already in the linux-firmware repository.
> >
> > For all the supported platforms, the first core, even for multi core SCP,
> > already have their firmware uploaded as just "scp.img". Multicore SCP
> > is seen in MT8195 and MT8188.
>
> The only one that is affected is MT8188, which needs a rename or a symlink in
> linux-firmware.
>
> MT8195 is not affected by this change, because the SCP is used as single-core,
> hence this code will look for scp.img and not for scp_c0.img.
>
> >
> > I guess I can send a followup patch?
>
> The only followup patch that I deem to be necessary is one adding a symlink
> or renaming for MT8188's SCP and nothing else.
The firmware was uploaded in March of 2025, and is packaged in Debian
Trixie, and was also backported to Bookworm. Either adding a symlink or
renaming it won't trickle down to users for some time. So this seems
like a possible ABI break, where the ABI is the file name.
Or maybe you don't consider it as such because SCP hasn't been enabled
in the kernel in any release yet?
> Please remember that some of those SoCs (including MT8195) allow the SCP to be
> configured as *either* single-core *or* dual-core - and usually firmwares for
> single-core configurations are not compatible with dual-core ones, because of
> the SRAM carveout/usage.
For MT8188, whether the current blob is dual core compatible needs to be
checked. The blob upstreamed to linux-firmware was built locally by
MediaTek engineers and is not what we ship with ChromeOS devices.
ChenYu
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] remoteproc: mtk_scp: Construct FW path if firmware-name not present
2025-10-30 8:21 ` Chen-Yu Tsai
@ 2025-10-30 9:10 ` Arnd Bergmann
2025-10-30 9:29 ` AngeloGioacchino Del Regno
2025-10-30 9:33 ` AngeloGioacchino Del Regno
1 sibling, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2025-10-30 9:10 UTC (permalink / raw)
To: Chen-Yu Tsai, AngeloGioacchino Del Regno
Cc: Mathieu Poirier, linux-remoteproc, Bjorn Andersson,
Matthias Brugger, linux-kernel, linux-arm-kernel, linux-mediatek,
kernel
On Thu, Oct 30, 2025, at 09:21, Chen-Yu Tsai wrote:
> On Wed, Oct 29, 2025 at 7:05 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> >
>> > I guess I can send a followup patch?
>>
>> The only followup patch that I deem to be necessary is one adding a symlink
>> or renaming for MT8188's SCP and nothing else.
>
> The firmware was uploaded in March of 2025, and is packaged in Debian
> Trixie, and was also backported to Bookworm. Either adding a symlink or
> renaming it won't trickle down to users for some time. So this seems
> like a possible ABI break, where the ABI is the file name.
>
> Or maybe you don't consider it as such because SCP hasn't been enabled
> in the kernel in any release yet?
It's normally up to the kernel driver to know about the firmware
file names and the order of trying the possible fallbacks, which
is exactly why I originally asked to not rely on the property
from dtb.
If you want a symlink in linux-firmware, that would go the other
way, pointing the old filename to the new location.
Arnd
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] remoteproc: mtk_scp: Construct FW path if firmware-name not present
2025-10-30 9:10 ` Arnd Bergmann
@ 2025-10-30 9:29 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 10+ messages in thread
From: AngeloGioacchino Del Regno @ 2025-10-30 9:29 UTC (permalink / raw)
To: Arnd Bergmann, Chen-Yu Tsai
Cc: Mathieu Poirier, linux-remoteproc, Bjorn Andersson,
Matthias Brugger, linux-kernel, linux-arm-kernel, linux-mediatek,
kernel
Il 30/10/25 10:10, Arnd Bergmann ha scritto:
> On Thu, Oct 30, 2025, at 09:21, Chen-Yu Tsai wrote:
>> On Wed, Oct 29, 2025 at 7:05 PM AngeloGioacchino Del Regno
>> <angelogioacchino.delregno@collabora.com> wrote:
>>>
>>>>
>>>> I guess I can send a followup patch?
>>>
>>> The only followup patch that I deem to be necessary is one adding a symlink
>>> or renaming for MT8188's SCP and nothing else.
>>
>> The firmware was uploaded in March of 2025, and is packaged in Debian
>> Trixie, and was also backported to Bookworm. Either adding a symlink or
>> renaming it won't trickle down to users for some time. So this seems
>> like a possible ABI break, where the ABI is the file name.
>>
>> Or maybe you don't consider it as such because SCP hasn't been enabled
>> in the kernel in any release yet?
Just as a clarification:
Exactly, SCP hasn't been enabled in the kernel in any release in the specific
case of MT8188, so this is not breaking anything, and it's not creating any
regression.
>
> It's normally up to the kernel driver to know about the firmware
> file names and the order of trying the possible fallbacks, which
> is exactly why I originally asked to not rely on the property
> from dtb.
>
> If you want a symlink in linux-firmware, that would go the other
> way, pointing the old filename to the new location.
>
> Arnd
Cheers!
Angelo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] remoteproc: mtk_scp: Construct FW path if firmware-name not present
2025-10-30 8:21 ` Chen-Yu Tsai
2025-10-30 9:10 ` Arnd Bergmann
@ 2025-10-30 9:33 ` AngeloGioacchino Del Regno
1 sibling, 0 replies; 10+ messages in thread
From: AngeloGioacchino Del Regno @ 2025-10-30 9:33 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: mathieu.poirier, linux-remoteproc, arnd, andersson, matthias.bgg,
linux-kernel, linux-arm-kernel, linux-mediatek, kernel
Il 30/10/25 09:21, Chen-Yu Tsai ha scritto:
> On Wed, Oct 29, 2025 at 7:05 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Il 29/10/25 10:14, Chen-Yu Tsai ha scritto:
>>> On Wed, Oct 15, 2025 at 4:41 PM AngeloGioacchino Del Regno
>>> <angelogioacchino.delregno@collabora.com> 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.
>>>
>>> I know this patch has been applied, but this scheme doesn't actually
>>> follow what is already in the linux-firmware repository.
>>>
>>> For all the supported platforms, the first core, even for multi core SCP,
>>> already have their firmware uploaded as just "scp.img". Multicore SCP
>>> is seen in MT8195 and MT8188.
>>
>> The only one that is affected is MT8188, which needs a rename or a symlink in
>> linux-firmware.
>>
>> MT8195 is not affected by this change, because the SCP is used as single-core,
>> hence this code will look for scp.img and not for scp_c0.img.
>>
>>>
>>> I guess I can send a followup patch?
>>
>> The only followup patch that I deem to be necessary is one adding a symlink
>> or renaming for MT8188's SCP and nothing else.
>
> The firmware was uploaded in March of 2025, and is packaged in Debian
> Trixie, and was also backported to Bookworm. Either adding a symlink or
> renaming it won't trickle down to users for some time. So this seems
> like a possible ABI break, where the ABI is the file name.
>
> Or maybe you don't consider it as such because SCP hasn't been enabled
> in the kernel in any release yet?
>
>> Please remember that some of those SoCs (including MT8195) allow the SCP to be
>> configured as *either* single-core *or* dual-core - and usually firmwares for
>> single-core configurations are not compatible with dual-core ones, because of
>> the SRAM carveout/usage.
>
> For MT8188, whether the current blob is dual core compatible needs to be
> checked. The blob upstreamed to linux-firmware was built locally by
> MediaTek engineers and is not what we ship with ChromeOS devices.
The current blob is dual core compatible.
We assisted MediaTek (Moudy) in choosing and validating the SCP firmware for
MT8188, and made sure that it uses only one core, and that doesn't clash with
the second core SRAM.
Cheers,
Angelo
>
>
> ChenYu
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-10-30 9:33 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-15 8:41 [PATCH v2] remoteproc: mtk_scp: Construct FW path if firmware-name not present AngeloGioacchino Del Regno
2025-10-15 13:41 ` Arnd Bergmann
2025-10-15 13:49 ` AngeloGioacchino Del Regno
2025-10-20 15:18 ` Mathieu Poirier
2025-10-29 9:14 ` Chen-Yu Tsai
2025-10-29 11:05 ` AngeloGioacchino Del Regno
2025-10-30 8:21 ` Chen-Yu Tsai
2025-10-30 9:10 ` Arnd Bergmann
2025-10-30 9:29 ` AngeloGioacchino Del Regno
2025-10-30 9:33 ` AngeloGioacchino Del Regno
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).