On Wed, Nov 17, 2021 at 02:41:58PM +0800, jason-jh.lin wrote: > @@ -158,6 +159,7 @@ static void mtk_drm_crtc_destroy(struct drm_crtc *crtc) > mtk_drm_cmdq_pkt_destroy(&mtk_crtc->cmdq_handle); > > if (mtk_crtc->cmdq_client.chan) { > + device_link_remove(mtk_crtc->drm_dev, mtk_crtc->cmdq_client.chan->mbox->dev); > mbox_free_channel(mtk_crtc->cmdq_client.chan); > mtk_crtc->cmdq_client.chan = NULL; > } [...] > @@ -956,6 +959,16 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev, > } > > if (mtk_crtc->cmdq_client.chan) { > + struct device_link *link; > + > + /* add devlink to cmdq dev to make sure suspend/resume order is correct */ > + link = device_link_add(dev, mtk_crtc->cmdq_client.chan->mbox->dev, > + DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS); > + if (!link) { > + dev_err(dev, "Unable to link dev=%s\n", > + dev_name(mtk_crtc->cmdq_client.chan->mbox->dev)); > + } > + If device_link_add() failed, doesn't mtk_drm_crtc_create() need to return an error and exit? OTOH, if device_link_add() failed, won't it bring any side effects to call device_link_remove()?