On 2018-05-08 08:51, Jyri Sarha wrote: > On 05/04/18 16:51, Peter Rosin wrote: >> It gets rid of an #ifdef and the .of_node member is going away. >> >> Signed-off-by: Peter Rosin <peda@xxxxxxxxxx> >> --- >> drivers/gpu/drm/bridge/panel.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c >> index 6d99d4a3beb3..f43d77b5ed20 100644 >> --- a/drivers/gpu/drm/bridge/panel.c >> +++ b/drivers/gpu/drm/bridge/panel.c >> @@ -169,10 +169,8 @@ struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel, >> panel_bridge->connector_type = connector_type; >> panel_bridge->panel = panel; >> >> + panel_bridge->bridge.odev = panel->dev; > > I am afraid this approach will eventually conflict with my lately > accepted patch[1]. I don't see how? The links are refcounted. So, if there is one link each for the panel and bridge between the drm device and the panel device that link will simply get two references. If/when the panel device then goes away, the drm device will be brought down because of that link (with two references, but that is irrelevant). When the drm device is brought down, it will (presumably) bring down the bridge as well (which will fix the refcount as the bridge link is killed as part of that). Or have you done some test and seen an actual problem? > The panel already has a device pointer of its own, and technically the > bridge element created here is created by the master drm device itself. Not always, some bridges also call drm_panel_bridge_add for connecting its output to either a panel or another bridge. And by that line of argument, the devm_kzalloc in drm_panel_bridge_add attaches the bridge memory to the wrong device as well. Maybe that should be fixed? Seems orthogonal though, but it would introduce a natural struct device in that function to assign to .odev. I think the device owning the bridge memory should be the same as the .odev device of the bridge. > I suggest assigning odev here to NULL or to master drm device itself. I'd rather not use NULL, since it is nice to be able to rely on the .odev being there, and WARN if it isn't. Cheers, Peter > Best regards, > Jyri > >> panel_bridge->bridge.funcs = &panel_bridge_bridge_funcs; >> -#ifdef CONFIG_OF >> - panel_bridge->bridge.of_node = panel->dev->of_node; >> -#endif >> >> drm_bridge_add(&panel_bridge->bridge); >> >> > > [1] https://lists.freedesktop.org/archives/dri-devel/2018-April/174559.html > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
