On 02-06-18, 01:56, Vikash Garodia wrote: > +int venus_set_hw_state(enum tzbsp_video_state state, struct venus_core *core) > +{ > + int ret; this should be init to 0 ... > + struct device *dev = core->dev; > + void __iomem *reg_base = core->base; > + > + switch (state) { > + case TZBSP_VIDEO_SUSPEND: > + if (qcom_scm_is_available()) > + ret = qcom_scm_set_remote_state(TZBSP_VIDEO_SUSPEND, 0); > + else > + writel_relaxed(1, reg_base + WRAPPER_A9SS_SW_RESET); > + break; > + case TZBSP_VIDEO_RESUME: > + if (qcom_scm_is_available()) > + ret = qcom_scm_set_remote_state(TZBSP_VIDEO_RESUME, 0); > + else > + venus_reset_hw(core); > + break; > + default: if it is default, ret contains garbage > + dev_err(dev, "invalid state\n"); > + break; > + } > + return ret; and that is returned. Compiler should complain about these ... > -enum tzbsp_video_state { > - TZBSP_VIDEO_STATE_SUSPEND = 0, > - TZBSP_VIDEO_STATE_RESUME > -}; ah you are moving existing defines, please mention this in changelog. Till this line I was expecting additions... -- ~Vinod