From mboxrd@z Thu Jan 1 00:00:00 1970 From: daniel@ffwll.ch (Daniel Vetter) Date: Mon, 30 Apr 2018 17:32:36 +0200 Subject: [PATCH 24/24] drm/bridge: establish a link between the bridge supplier and consumer In-Reply-To: <20180426223139.16740-25-peda@axentia.se> References: <20180426223139.16740-1-peda@axentia.se> <20180426223139.16740-25-peda@axentia.se> Message-ID: <20180430153236.GP12521@phenom.ffwll.local> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Fri, Apr 27, 2018 at 12:31:39AM +0200, Peter Rosin wrote: > If the bridge supplier is unbound, this will bring the bridge consumer > down along with the bridge. Thus, there will no longer linger any > dangling pointers from the bridge consumer (the drm_device) to some > non-existent bridge supplier. > > Signed-off-by: Peter Rosin Minus the ->owner bikeshed I brought up in the previous patch I agree with this approach as the best way to move forward for now. Acked-by: Daniel Vetter One small suggestion below, for merging I'd say pls get Jyri's review/tested-by too, since you're both working on the same problem it seems. Aside: Do you want commit rights to drm-misc to be able to push work like this? Cheers, Daniel > --- > drivers/gpu/drm/drm_bridge.c | 18 ++++++++++++++++++ > include/drm/drm_bridge.h | 2 ++ > 2 files changed, 20 insertions(+) > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > index a038da696802..f0c79043ec43 100644 > --- a/drivers/gpu/drm/drm_bridge.c > +++ b/drivers/gpu/drm/drm_bridge.c > @@ -26,6 +26,7 @@ > #include > > #include > +#include > #include > > #include "drm_crtc_internal.h" > @@ -124,12 +125,25 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge, > if (bridge->dev) > return -EBUSY; > > + if (encoder->dev->dev != bridge->owner) { You might end up with a NULL encoder->dev->dev. Perhaps check that and bail with a WARN_ON? > + bridge->link = device_link_add(encoder->dev->dev, > + bridge->owner, 0); > + if (!bridge->link) { > + dev_err(bridge->owner, "failed to link bridge to %s\n", > + dev_name(encoder->dev->dev)); > + return -EINVAL; > + } > + } > + > bridge->dev = encoder->dev; > bridge->encoder = encoder; > > if (bridge->funcs->attach) { > ret = bridge->funcs->attach(bridge); > if (ret < 0) { > + if (bridge->link) > + device_link_del(bridge->link); > + bridge->link = NULL; > bridge->dev = NULL; > bridge->encoder = NULL; > return ret; > @@ -156,6 +170,10 @@ void drm_bridge_detach(struct drm_bridge *bridge) > if (bridge->funcs->detach) > bridge->funcs->detach(bridge); > > + if (bridge->link) > + device_link_del(bridge->link); > + bridge->link = NULL; > + > bridge->dev = NULL; > } > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > index 3bc659f3e7d2..9a386559a41a 100644 > --- a/include/drm/drm_bridge.h > +++ b/include/drm/drm_bridge.h > @@ -261,6 +261,7 @@ struct drm_bridge_timings { > * @list: to keep track of all added bridges > * @timings: the timing specification for the bridge, if any (may > * be NULL) > + * @link: drm consumer <-> bridge supplier > * @funcs: control functions > * @driver_private: pointer to the bridge driver's internal context > */ > @@ -271,6 +272,7 @@ struct drm_bridge { > struct drm_bridge *next; > struct list_head list; > const struct drm_bridge_timings *timings; > + struct device_link *link; > > const struct drm_bridge_funcs *funcs; > void *driver_private; > -- > 2.11.0 > > _______________________________________________ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch