Hi Tzung-Bi,
Thanks for the reviews.
> From: Linux-mediatek <linux-mediatek-boun...@lists.infradead.org> On
> Behalf Of Tzung-Bi Shih
> Sent: Thursday, November 18, 2021 1:55 PM
> To: Jason-JH Lin (林睿祥) <jason-jh....@mediatek.com>
> Cc: Chun-Kuang Hu <chunkuang...@kernel.org>; Philipp Zabel <
> p.za...@pengutronix.de>; Matthias Brugger <matthias....@gmail.com>;
> Jassi Brar <jassisinghb...@gmail.com>; David Airlie <airl...@linux.ie
> linux-media...@lists.infradead.org;
> linux-arm-ker...@lists.infradead.org; linux-ker...@vger.kernel.org;
> hsi...@chromium.org; fs...@chromium.org; Nancy Lin (林欣螢) <
> nancy....@mediatek.com>; Singo Chang (張興國)singo.ch...@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-media...@lists.infradead.org
>
>
>
>
https://urldefense.com/v3/__http://lists.infradead.org/mailman/listinfo/linux-mediatek__;!!CTRNKA9wMg0ARbw!125dmOZ7xCLkwbIrzZf1VqVLQBbdT4RFHoMjR38U1GeCxaTM6xvvB-mSr7sTc6BuY5IH$
>
https://urldefense.com/v3/__http://lists.infradead.org/mailman/listinfo/linux-mediatek__;!!CTRNKA9wMg0ARbw!125dmOZ7xCLkwbIrzZf1VqVLQBbdT4RFHoMjR38U1GeCxaTM6xvvB-mSr7sTc6BuY5IH$
>
https://urldefense.com/v3/__http://lists.infradead.org/mailman/listinfo/linux-mediatek__;!!CTRNKA9wMg0ARbw!125dmOZ7xCLkwbIrzZf1VqVLQBbdT4RFHoMjR38U1GeCxaTM6xvvB-mSr7sTc6BuY5IH$
>
--
Jason-JH Lin <jason-jh....@mediatek.com>
