Re: [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845

From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Will Deacon <will@kernel.org>
Cc: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>,
	Robin Murphy <robin.murphy@arm.com>,
	Joerg Roedel <joro@8bytes.org>,
	iommu@lists.linux-foundation.org,
	Stephen Boyd <swboyd@chromium.org>,
	Vivek Gautam <vivek.gautam@codeaurora.org>,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Rajendra Nayak <rnayak@codeaurora.org>,
	linux-arm-msm-owner@vger.kernel.org
Subject: Re: [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845
Date: Mon, 4 Nov 2019 09:41:30 -0800	[thread overview]
Message-ID: <20191104174130.GA586@tuxbook-pro> (raw)
In-Reply-To: <20191104162339.GD24909@willie-the-truck>

On Mon 04 Nov 08:23 PST 2019, Will Deacon wrote:

> On Mon, Nov 04, 2019 at 03:15:06PM +0000, Will Deacon wrote:
> > On Sun, Nov 03, 2019 at 11:19:25PM -0600, Andy Gross wrote:
> > > On Fri, Nov 01, 2019 at 11:01:59PM +0530, Sai Prakash Ranjan wrote:
> > > > >>> What's the plan for getting this merged? I'm not happy taking the
> > > > >>> firmware
> > > > >>> bits without Andy's ack, but I also think the SMMU changes should go via
> > > > >>> the IOMMU tree to avoid conflicts.
> > > > >>>
> > > > >>> Andy?
> > > > >>>
> > > > >>
> > > > >>Bjorn maintains QCOM stuff now if I am not wrong and he has already
> > > > >>reviewed
> > > > >>the firmware bits. So I'm hoping you could take all these through IOMMU
> > > > >>tree.
> > > > >
> > > > >Oh, I didn't realise that. Is there a MAINTAINERS update someplace? If I
> > > > >run:
> > > > >
> > > > >$ ./scripts/get_maintainer.pl -f drivers/firmware/qcom_scm-64.c
> > > > >
> > > > >in linux-next, I get:
> > > > >
> > > > >Andy Gross <agross@kernel.org> (maintainer:ARM/QUALCOMM SUPPORT)
> > > > >linux-arm-msm@vger.kernel.org (open list:ARM/QUALCOMM SUPPORT)
> > > > >linux-kernel@vger.kernel.org (open list)
> > > > >
> > > > 
> > > > It hasn't been updated yet then. I will leave it to Bjorn or Andy to comment
> > > > on this.
> > > 
> > > The rumors of my demise have been greatly exaggerated.  All kidding aside, I
> > > ack'ed both.  Bjorn will indeed be coming on as a co-maintener at some point.
> > > He has already done a lot of yeomans work in helping me out the past 3 months.
> > 
> > Cheers Andy, and I'm pleased to hear that you're still with us! I've queued
> > this lot for 5.5 and I'll send to Joerg this week.
> 
> Bah, in doing so I spotted that the existing code doesn't handle error codes
> properly because 'a0' is unsigned. I'll queue the patch below at the start
> of the series.
> 

Thanks, I've hit this a few times but missed this and assumed it was a
firmware issue...

In case you haven't published your branch:
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> Will
> 
> --->8
> 
> From a9a1047f08de0eff249fb65e2d5d6f6f8b2a87f0 Mon Sep 17 00:00:00 2001
> From: Will Deacon <will@kernel.org>
> Date: Mon, 4 Nov 2019 15:58:15 +0000
> Subject: [PATCH] firmware: qcom: scm: Ensure 'a0' status code is treated as
>  signed
> 
> The 'a0' member of 'struct arm_smccc_res' is declared as 'unsigned long',
> however the Qualcomm SCM firmware interface driver expects to receive
> negative error codes via this field, so ensure that it's cast to 'long'
> before comparing to see if it is less than 0.
> 
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
>  drivers/firmware/qcom_scm-64.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
> index 91d5ad7cf58b..25e0f60c759a 100644
> --- a/drivers/firmware/qcom_scm-64.c
> +++ b/drivers/firmware/qcom_scm-64.c
> @@ -150,7 +150,7 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
>  		kfree(args_virt);
>  	}
>  
> -	if (res->a0 < 0)
> +	if ((long)res->a0 < 0)
>  		return qcom_scm_remap_error(res->a0);
>  
>  	return 0;
> -- 
> 2.24.0.rc1.363.gb1bccd3e3d-goog
> 