Hi, Leonard > On 2019-09-27 4:20 AM, Anson Huang wrote: > >> On 2019-09-26 1:06 PM, Marco Felsch wrote: > >>> On 19-09-26 08:03, Anson Huang wrote: > >>>>> 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. > >>>>>> > >>>>>> +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. > >>> > >>> Okay. > >>> > >>>> 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! > >> > >> My suggestion is to refactor the code and add a new API for the this > >> "no error value" convention. Internally they can call a common > >> function with flags. > > > > If I understand your point correctly, that means the loop check of > > whether the API is with "no error value" for every API still NOT be > > skipped, it is just refactoring the code, right? > > There would be no "loop" anywhere: the responsibility would fall on the call > to call the right RPC function. In the current layering scheme (drivers -> > RPC -> > mailbox) the RPC layer treats all calls the same and it's up the the caller to > provide information about calling convention. > > An example implementation: > * Rename imx_sc_rpc_call to __imx_sc_rpc_call_flags > * Make a tiny imx_sc_rpc_call wrapper which just converts resp/noresp to a > flag > * Make get button status call __imx_sc_rpc_call_flags with the > _IMX_SC_RPC_NOERROR flag > > Hope this makes my suggestion clearer? Pushing this to the caller is a bit > ugly > but I think it's worth preserving the fact that the imx rpc core treats > services > in an uniform way.
It is clear now, so essentially it is same as 2 separate APIs, still need to change the button driver and uid driver to use the special flag, meanwhile, need to change the third parament of imx_sc_rpc_call() from bool to u32. If no one opposes this approach, I will redo the patch together with the button driver and uid driver after holiday. Anson