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

[Bjorn Andersson <bjorn.andersson@linaro.org>]

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@linaro.org>

Regards,
Bjorn

> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> ---
>  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
>