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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <3f23850e-f01f-4a2e-a7dc-82c04ab5926c@app.fastmail.com>
Date: Wed, 15 Oct 2025 15:41:39 +0200
From: "Arnd Bergmann" <arnd@...db.de>
To: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@...labora.com>,
 linux-remoteproc@...r.kernel.org
Cc: "Bjorn Andersson" <andersson@...nel.org>,
 "Mathieu Poirier" <mathieu.poirier@...aro.org>,
 "Matthias Brugger" <matthias.bgg@...il.com>,
 "Chen-Yu Tsai" <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, 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>

This looks good to me, thanks for the changes!

Reviewed-by: Arnd Bergmann <arnd@...db.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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ