RE: [PATCH] firmware: imx: Skip return value check for some special SCU firmware APIs

From: Anson Huang <anson.huang@nxp.com>
To: Marco Felsch <m.felsch@pengutronix.de>
Cc: "shawnguo@kernel.org" <shawnguo@kernel.org>,
	"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>,
	"festevam@gmail.com" <festevam@gmail.com>,
	Aisheng Dong <aisheng.dong@nxp.com>,
	Leonard Crestez <leonard.crestez@nxp.com>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	dl-linux-imx <linux-imx@nxp.com>
Subject: RE: [PATCH] firmware: imx: Skip return value check for some special SCU firmware APIs
Date: Thu, 26 Sep 2019 08:03:01 +0000	[thread overview]
Message-ID: <DB3PR0402MB391683202692BEAE4D2CD9C1F5860@DB3PR0402MB3916.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20190926075914.i7tsd3cbpitrqe4q@pengutronix.de>

Hi, Marco

> On 19-09-25 18:07, Anson Huang wrote:
> > The SCU firmware does NOT always have return value stored in message
> > header's function element even the API has response data, those
> > special APIs are defined as void function in SCU firmware, so they
> > should be treated as return success always.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > 	- This patch is based on the patch of
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> >
> hwork.kernel.org%2Fpatch%2F11129553%2F&amp;data=02%7C01%7Canson.
> huang%
> >
> 40nxp.com%7C1f4108cc25eb4618f43c08d742576fa3%7C686ea1d3bc2b4c6fa
> 92cd99
> >
> c5c301635%7C0%7C0%7C637050815608963707&amp;sdata=BZBg4cOR2rP%2
> BRBNn15i
> > Qq3%2FXBYwhuCLkgYzFRbfEgVU%3D&amp;reserved=0
> > ---
> >  drivers/firmware/imx/imx-scu.c | 34
> > ++++++++++++++++++++++++++++++++--
> >  1 file changed, 32 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/firmware/imx/imx-scu.c
> > b/drivers/firmware/imx/imx-scu.c index 869be7a..ced5b12 100644
> > --- a/drivers/firmware/imx/imx-scu.c
> > +++ b/drivers/firmware/imx/imx-scu.c
> > @@ -78,6 +78,11 @@ static int imx_sc_linux_errmap[IMX_SC_ERR_LAST] =
> {
> >  	-EIO,	 /* IMX_SC_ERR_FAIL */
> >  };
> >
> > +static const struct imx_sc_rpc_msg whitelist[] = {
> > +	{ .svc = IMX_SC_RPC_SVC_MISC, .func =
> IMX_SC_MISC_FUNC_UNIQUE_ID },
> > +	{ .svc = IMX_SC_RPC_SVC_MISC, .func =
> > +IMX_SC_MISC_FUNC_GET_BUTTON_STATUS }, };
> 
> Is this going to be extended in the near future? I see some upcoming
> problems here if someone uses a different scu-fw<->kernel combination as
> nxp would suggest.

Could be, but I checked the current APIs, ONLY these 2 will be used in Linux kernel, so
I ONLY add these 2 APIs for now.

However, after rethink, maybe we should add another imx_sc_rpc API for those special
APIs? To avoid checking it for all the APIs called which may impact some performance.
Still under discussion, if you have better idea, please advise, thanks!

Anson