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

[Anson Huang]

Hi, Marco

> On 19-10-07 09:15, 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@xxxxxxx>
> > ---
> > Changes since V1:
> > 	- Use direct API check instead of calling another function to check.
> > 	- This patch is based on
> > 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%7Cbefd2849a124462caa4a08d74c972dc9%7C686ea1d3bc2b4c6f
> a92cd99
> >
> c5c301635%7C0%7C0%7C637062084506889431&amp;sdata=7fW8hZB4AaUK
> 9QTKTJQR7
> > LuV2nGo6e%2Fqb%2Fqmn4ykquk%3D&amp;reserved=0
> > ---
> >  drivers/firmware/imx/imx-scu.c | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/firmware/imx/imx-scu.c
> > b/drivers/firmware/imx/imx-scu.c index 869be7a..03b43b7 100644
> > --- a/drivers/firmware/imx/imx-scu.c
> > +++ b/drivers/firmware/imx/imx-scu.c
> > @@ -162,6 +162,7 @@ static int imx_scu_ipc_write(struct imx_sc_ipc
> *sc_ipc, void *msg)
> >   */
> >  int imx_scu_call_rpc(struct imx_sc_ipc *sc_ipc, void *msg, bool
> > have_resp)  {
> > +	uint8_t saved_svc, saved_func;
> >  	struct imx_sc_rpc_msg *hdr;
> >  	int ret;
> >
> > @@ -171,8 +172,11 @@ int imx_scu_call_rpc(struct imx_sc_ipc *sc_ipc,
> void *msg, bool have_resp)
> >  	mutex_lock(&sc_ipc->lock);
> >  	reinit_completion(&sc_ipc->done);
> >
> > -	if (have_resp)
> > +	if (have_resp) {
> >  		sc_ipc->msg = msg;
> > +		saved_svc = ((struct imx_sc_rpc_msg *)msg)->svc;
> 
> Why do we need to check the svc too?

It is because the SCU firmware API has many different category called SVC, each category has
many different function, and the function value could be same in each category,
for example, there are IRQ, PM, MISC etc. SVC category, and each of them could have function
type defined as 0, 1, 2 .... That is why I need to save both SVC and FUNC to identify the SCU FW
API. See below: 

PM SVC:
#define PM_FUNC_SET_PARTITION_POWER_MODE 1U /* Index for pm_set_partition_power_mode() RPC call */
#define PM_FUNC_GET_SYS_POWER_MODE 2U /* Index for pm_get_sys_power_mode() RPC call */
#define PM_FUNC_SET_RESOURCE_POWER_MODE 3U /* Index for pm_set_resource_power_mode() RPC call */

MISC SVC:
#define MISC_FUNC_SET_CONTROL 1U /* Index for misc_set_control() RPC call */
#define MISC_FUNC_GET_CONTROL 2U /* Index for misc_get_control() RPC call */
#define MISC_FUNC_SET_MAX_DMA_GROUP 4U /* Index for misc_set_max_dma_group() RPC call */

> 
> > +		saved_func = ((struct imx_sc_rpc_msg *)msg)->func;
> 
> Nitpick, should we call it requested_func/req_func?

OK, I will change them If I have to sent out a new version, otherwise, I think the saved_func and saved_svc
should also be fine.

Thanks,
Anson