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.