lkml.org 
[lkml]   [2019]   [Sep]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH] firmware: imx: Skip return value check for some special SCU firmware APIs
    Date
    On 25.09.2019 13:09, 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%2Fpatchwork.kernel.org%2Fpatch%2F11129553%2F&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7Cc0ced6cd07f04023977008d741a07367%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637050029712216472&amp;sdata=Ccq%2Fb2RJdMqmnL7VXrl8YhOlUwC7bWiUG%2BNmiw4OsSM%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 },
    > +};

    Until now this low level IPC code didn't treat any svc/func specially
    and this seems good.

    The imx_scu_call_rpc function already has an have_resp argument and
    callers are responsible to fill it. Can't we deal with this by adding an
    additional err_ret flag passed by the caller?

    We can add wrapper functions to avoid tree-wide changes for all callers.

    > +
    > static struct imx_sc_ipc *imx_sc_ipc_handle;
    >
    > static inline int imx_sc_to_linux_errno(int errno)
    > @@ -157,11 +162,24 @@ static int imx_scu_ipc_write(struct imx_sc_ipc *sc_ipc, void *msg)
    > return 0;
    > }
    >
    > +static bool imx_scu_call_skip_return_value_check(uint8_t svc, uint8_t func)
    > +{
    > + int i;
    > +
    > + for (i = 0; i < ARRAY_SIZE(whitelist); i++)
    > + if (svc == whitelist[i].svc &&
    > + func == whitelist[i].func)
    > + return true;
    > +
    > + return false;
    > +}
    > +
    > /*
    > * RPC command/response
    > */
    > 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 +189,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;
    > + saved_func = ((struct imx_sc_rpc_msg *)msg)->func;
    > + }
    > sc_ipc->count = 0;
    > ret = imx_scu_ipc_write(sc_ipc, msg);
    > if (ret < 0) {
    > @@ -190,7 +211,16 @@ int imx_scu_call_rpc(struct imx_sc_ipc *sc_ipc, void *msg, bool have_resp)
    >
    > /* response status is stored in hdr->func field */
    > hdr = msg;
    > - ret = hdr->func;
    > + /*
    > + * Some special SCU firmware APIs do NOT have return value
    > + * in hdr->func, but they do have response data, those special
    > + * APIs are defined as void function in SCU firmware, so they
    > + * should be treated as return success always.
    > + */
    > + if (!imx_scu_call_skip_return_value_check(saved_svc, saved_func))
    > + ret = hdr->func;
    > + else
    > + ret = 0;
    > }
    >
    > out:
    >


    \
     
     \ /
      Last update: 2019-09-25 15:13    [W:4.375 / U:1.096 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site