linux-kernel - Re: FW: [PATCH 3/3] drm/mediatek: add devlink to cmdq dev

Open Source and information security mailing list archives

Message-ID: <ae29424d8121f8951cfb5fece114b6fcba90d351.camel@mediatek.com>
Date:   Sun, 28 Nov 2021 23:57:33 +0800
From:   Jason-JH Lin <jason-jh.lin@...iatek.com>
To:     Tzung-Bi Shih <tzungbi@...gle.com>
CC:     Chun-Kuang Hu <chunkuang.hu@...nel.org>,
        "; Philipp Zabel" <p.zabel@...gutronix.de>,
        "; Matthias Brugger" <matthias.bgg@...il.com>,
        "; Jassi Brar" <jassisinghbrar@...il.com>,
        "; David Airlie" <airlied@...ux.ie>,
        "; Daniel Vetter" <daniel@...ll.ch>,
        "; dri-devel@...ts.freedesktop.org; linux-mediatek@...ts.infradead.org; 
        linux-arm-kernel@...ts.infradead.org; linux-kernel@...r.kernel.org; 
        hsinyi@...omium.org; fshao@...omium.org; Nancy Lin
        (林欣螢)" 
        <Nancy.Lin@...iatek.com>,
        "; Singo Chang (張興國)" 
        <Singo.Chang@...iatek.com>
Subject: Re: FW: [PATCH 3/3] drm/mediatek: add devlink to cmdq dev

Hi Tzung-Bi,

Thanks for the reviews.

> From: Linux-mediatek <linux-mediatek-bounces@...ts.infradead.org> On
> Behalf Of Tzung-Bi Shih
> Sent: Thursday, November 18, 2021 1:55 PM
> To: Jason-JH Lin (林睿祥) <Jason-JH.Lin@...iatek.com>
> Cc: Chun-Kuang Hu <chunkuang.hu@...nel.org>; Philipp Zabel <
> p.zabel@...gutronix.de>; Matthias Brugger <matthias.bgg@...il.com>;
> Jassi Brar <jassisinghbrar@...il.com>; David Airlie <airlied@...ux.ie
> >; Daniel Vetter <daniel@...ll.ch>; dri-devel@...ts.freedesktop.org; 
> linux-mediatek@...ts.infradead.org; 
> linux-arm-kernel@...ts.infradead.org; linux-kernel@...r.kernel.org; 
> hsinyi@...omium.org; fshao@...omium.org; Nancy Lin (林欣螢) <
> Nancy.Lin@...iatek.com>; Singo Chang (張興國) <Singo.Chang@...iatek.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@...ts.infradead.org
> 
https://urldefense.com/v3/__http://lists.infradead.org/mailman/listinfo/linux-mediatek__;!!CTRNKA9wMg0ARbw!125dmOZ7xCLkwbIrzZf1VqVLQBbdT4RFHoMjR38U1GeCxaTM6xvvB-mSr7sTc6BuY5IH$
>  
-- 
Jason-JH Lin <jason-jh.lin@...iatek.com>

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives