On Tue, Aug 19, 2025 at 01:08:04PM +0200, Stephan Gerhold wrote:
> The ADSP firmware on X1E has separate firmware binaries for the main
> firmware and the DTB. The same applies for the "lite" firmware loaded by
> the boot firmware.
>
> When preparing to load the new ADSP firmware we shutdown the lite_pas_id
> for the main firmware, but we don't shutdown the corresponding lite pas_id
> for the DTB. The fact that we're leaving it "running" forever becomes
> obvious if you try to reuse (or just access) the memory region used by the
> "lite" firmware: The &adsp_boot_mem is accessible, but accessing the
> &adsp_boot_dtb_mem results in a crash.
>
> We don't support reusing the memory regions currently, but nevertheless we
> should not keep part of the lite firmware running. Fix this by adding the
> lite_dtb_pas_id and shutting it down as well.
>
> We don't have a way to detect if the lite firmware is actually running yet,
> so ignore the return status of qcom_scm_pas_shutdown() for now. This was
> already the case before, the assignment to "ret" was not used anywhere.
>
> Fixes: 62210f7509e1 ("remoteproc: qcom_q6v5_pas: Unload lite firmware on
> ADSP")
> Signed-off-by: Stephan Gerhold <stephan.gerh...@linaro.org>
> ---
> drivers/remoteproc/qcom_q6v5_pas.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c
> b/drivers/remoteproc/qcom_q6v5_pas.c
> index
> 02e29171cbbee2d305827365ef7d2241b6eb786b..6faedae8d32ef6c3c2071975f2f1e37a9ffd8abe
> 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -42,6 +42,7 @@ struct qcom_pas_data {
> int pas_id;
> int dtb_pas_id;
> int lite_pas_id;
> + int lite_dtb_pas_id;
> unsigned int minidump_id;
> bool auto_boot;
> bool decrypt_shutdown;
> @@ -80,6 +81,7 @@ struct qcom_pas {
> int pas_id;
> int dtb_pas_id;
> int lite_pas_id;
> + int lite_dtb_pas_id;
> unsigned int minidump_id;
> int crash_reason_smem;
> unsigned int smem_host_id;
> @@ -225,7 +227,9 @@ static int qcom_pas_load(struct rproc *rproc, const
> struct firmware *fw)
> pas->firmware = fw;
>
> if (pas->lite_pas_id)
> - ret = qcom_scm_pas_shutdown(pas->lite_pas_id);
> + qcom_scm_pas_shutdown(pas->lite_pas_id);
Unrelated change. With it being split to a separate commit:
Reviewed-by: Dmitry Baryshkov <dmitry.barysh...@oss.qualcomm.com>
> + if (pas->lite_dtb_pas_id)
> + qcom_scm_pas_shutdown(pas->lite_dtb_pas_id);
>
> if (pas->dtb_pas_id) {
> ret = request_firmware(&pas->dtb_firmware,
> pas->dtb_firmware_name, pas->dev);
> @@ -722,6 +726,7 @@ static int qcom_pas_probe(struct platform_device *pdev)
> pas->minidump_id = desc->minidump_id;
> pas->pas_id = desc->pas_id;
> pas->lite_pas_id = desc->lite_pas_id;
> + pas->lite_dtb_pas_id = desc->lite_dtb_pas_id;
> pas->info_name = desc->sysmon_name;
> pas->smem_host_id = desc->smem_host_id;
> pas->decrypt_shutdown = desc->decrypt_shutdown;
> @@ -1085,6 +1090,7 @@ static const struct qcom_pas_data
> x1e80100_adsp_resource = {
> .pas_id = 1,
> .dtb_pas_id = 0x24,
> .lite_pas_id = 0x1f,
> + .lite_dtb_pas_id = 0x29,
> .minidump_id = 5,
> .auto_boot = true,
> .proxy_pd_names = (char*[]){
>
> --
> 2.50.1
>
--
With best wishes
Dmitry
