On Thu, 2024-10-31 at 01:52 -0700, Zhi Wang wrote:
> @@ -336,59 +336,60 @@ static struct nvfw_gsp_rpc *
>  r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32 gsp_rpc_len)
>  {
>       struct nvkm_subdev *subdev = &gsp->subdev;
> -     struct nvfw_gsp_rpc *msg;
> +     struct nvfw_gsp_rpc *rpc;
>       int time = 4000000, i;
>       u32 size;
>  
>  retry:
> -     msg = r535_gsp_msgq_wait(gsp, sizeof(*msg), &size, &time);
> -     if (IS_ERR_OR_NULL(msg))
> -             return msg;
> +     rpc = r535_gsp_msgq_wait(gsp, sizeof(*rpc), &size, &time);
> +     if (IS_ERR_OR_NULL(rpc))
> +             return rpc;

I know this change is supposed to be non-functional, but I did notice a
pattern here.

This function:

        rpc = r535_gsp_msgq_wait(gsp, sizeof(*rpc), &size, &time);
        if (IS_ERR_OR_NULL(rpc))
                return rpc;

Function r535_gsp_rpc_poll, which calls this function:

        repv = r535_gsp_msg_recv(gsp, fn, 0);
        mutex_unlock(&gsp->cmdq.mutex);
        if (IS_ERR(repv))
                return PTR_ERR(repv);

So if rpc is NULL, r535_gsp_msg_recv() will return NULL, but r535_gsp_rpc_poll
expects an error code instead.  Since it technically doesn't get one, it
returns 0 (success).

To be fair, it does not appear that r535_gsp_msgq_wait() can return NULL, but
that is obscured by the code.


Reply via email to