On 2019-09-26 1:06 PM, Marco Felsch wrote:

On 19-09-26 08:03, Anson Huang wrote:

quoted

quoted

On 19-09-25 18:07, Anson Huang wrote:

quoted

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 },

quoted

+	{ .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.

quoted

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.

Adding a special api shouldn't be the right fix. Imagine if someone (not
a nxp-developer) wants to add a new driver. How could he be expected to
know which api he should use. The better abbroach would be to fix the
scu-fw instead of adding quirks..

Right now developers who want to make SCFW calls in upstream need to 
define the message struct in their driver based on protocol 
documentation. This includes:

* Binary layout of the message (a packed struct)
* If the message has a response (already a bool flag)
* If an error code is returned (this patch adds support for it)

Since callers are already exposed to the binary protocol exposing them 
to minor quirks of the calling convention also seems reasonable. Having 
the low-level IPC code peek at message IDs seems like a hack; this 
belong at a slightly higher level.

In older internal trees we use a very large amount of generated wrapper 
functions. This hides calling convention details from callers but is 
extremely ugly and verbose.

--
Regards,
Leonard

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel