On Mon, Aug 16, 2021 at 06:59:30PM +0800, Irui Wang wrote: > -void mtk_vcodec_enc_clock_on(struct mtk_vcodec_pm *pm) > +void mtk_vcodec_enc_clock_on(struct mtk_vcodec_dev *dev, int core_id) > { > - struct mtk_vcodec_clk *enc_clk = &pm->venc_clk; > - int ret, i = 0; > + struct mtk_venc_comp_dev *venc; > + struct mtk_vcodec_pm *enc_pm; > + struct mtk_vcodec_clk *enc_clk; > + struct clk *clk; To be neat, remove the extra spaces. > - ret = mtk_smi_larb_get(pm->larbvenc); > - if (ret) { > - mtk_v4l2_err("mtk_smi_larb_get larb3 fail %d", ret); > - goto clkerr; I may miss the context but why does it remove mtk_smi_larb_get()? > -void mtk_vcodec_enc_clock_off(struct mtk_vcodec_pm *pm) > +void mtk_vcodec_enc_clock_off(struct mtk_vcodec_dev *dev, int core_id) > { > - struct mtk_vcodec_clk *enc_clk = &pm->venc_clk; > - int i = 0; > + struct mtk_venc_comp_dev *venc; > + struct mtk_vcodec_pm *enc_pm; > + struct mtk_vcodec_clk *enc_clk; > + int i; > > - mtk_smi_larb_put(pm->larbvenc); Same here. Why does it remove mtk_smi_larb_put()? > int mtk_venc_enable_comp_hw(struct mtk_vcodec_dev *dev) > { > int i, ret; > struct mtk_venc_comp_dev *venc_comp; > + struct mtk_vcodec_clk *enc_clk; > + int j = 0; It doesn't need to be initialized. Can inline to "int i, ret;".