Hi Tzung-Bi,

Thanks for the reviews.

From: Linux-mediatek linux-mediatek-bounces@lists.infradead.org On Behalf Of Tzung-Bi Shih Sent: Thursday, November 18, 2021 1:55 PM To: Jason-JH Lin (林睿祥) Jason-JH.Lin@mediatek.com Cc: Chun-Kuang Hu chunkuang.hu@kernel.org; Philipp Zabel < p.zabel@pengutronix.de>; Matthias Brugger matthias.bgg@gmail.com; Jassi Brar jassisinghbrar@gmail.com; David Airlie <airlied@linux.ie

...

; Daniel Vetter daniel@ffwll.ch; dri-devel@lists.freedesktop.org;

linux-mediatek@lists.infradead.org; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; hsinyi@chromium.org; fshao@chromium.org; Nancy Lin (林欣螢) < Nancy.Lin@mediatek.com>; Singo Chang (張興國) Singo.Chang@mediatek.com Subject: Re: [PATCH 3/3] drm/mediatek: add devlink to cmdq dev

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?

OK, I'll add the return error at the next verion.

OTOH, if device_link_add() failed, won't it bring any side effects to call device_link_remove()?

Because device_link_remove() will find the device_link of supplier and consumer, then delete the device_link between them. If device_link_add() failed, supplier and consumer won't create the device_link. So calling device_link_remove() won't do anything without deive_link and won't bring any side effects.

Regards, Jason-JH.Lin

---

Linux-mediatek mailing list Linux-mediatek@lists.infradead.org

https://urldefense.com/v3/__http://lists.infradead.org/mailman/listinfo/linu...