Re: [PATCH] venus: firmware: Ignore secure call error on first resume

From: Bjorn Andersson
Date: Sun Mar 08 2020 - 18:01:02 EST


On Wed 04 Mar 07:58 PST 2020, Stanimir Varbanov wrote:

> With the latest cleanup in qcom scm driver

I've seen similar issues in other drivers, but I think the culprit is
the following bug fix 'ff34f3cce278 ("firmware: qcom: scm: Ensure 'a0'
status code is treated as signed")', rather than the recent refactoring.

As this is a bug fix it's quite likely to show up in linux-stable as
well and regress older versions, so you should probably Cc stable as
well.

> the secure monitor
> call for setting the remote processor state returns EINVAL when
> it is called for the first time and after another scm call
> auth_and_reset. The error returned from scm call could be ignored
> because the state transition is already done in auth_and_reset.
>

I'm not sure what the appropriate mechanism is here, but your workaround
seems reasonable.

Acked-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>

Regards,
Bjorn

> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@xxxxxxxxxx>
> ---
> drivers/media/platform/qcom/venus/firmware.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
> index de6812fb55f4..8801a6a7543d 100644
> --- a/drivers/media/platform/qcom/venus/firmware.c
> +++ b/drivers/media/platform/qcom/venus/firmware.c
> @@ -44,8 +44,14 @@ static void venus_reset_cpu(struct venus_core *core)
>
> int venus_set_hw_state(struct venus_core *core, bool resume)
> {
> - if (core->use_tz)
> - return qcom_scm_set_remote_state(resume, 0);
> + int ret;
> +
> + if (core->use_tz) {
> + ret = qcom_scm_set_remote_state(resume, 0);
> + if (resume && ret == -EINVAL)
> + ret = 0;
> + return ret;
> + }
>
> if (resume)
> venus_reset_cpu(core);
> --
> 2.17.1
>