On Fri, Feb 21, 2020 at 10:04 PM Daniel Vetter <daniel.vetter@xxxxxxxx> wrote: > > It's (almost, there's some iommu stuff without significance) right > above the drm_dev_put(). > > This is made possible by a preceeding patch which added a drmm_ > cleanup action to drm_mode_config_init(), hence all we need to do to > ensure that drm_mode_config_cleanup() is run on final drm_device > cleanup is check the new error code for _init(). > > Aside: Another driver with a bit much devm_kzalloc, which should > probably use drmm_kzalloc instead ... > > v2: Explain why this cleanup is possible (Laurent). > > Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > Cc: Sandy Huang <hjc@xxxxxxxxxxxxxx> > Cc: "Heiko Stübner" <heiko@xxxxxxxxx> > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > Cc: linux-rockchip@xxxxxxxxxxxxxxxxxxx > --- > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > index 20ecb1508a22..d0eba21eebc9 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > @@ -135,7 +135,9 @@ static int rockchip_drm_bind(struct device *dev) > if (ret) > goto err_free; > > - drm_mode_config_init(drm_dev); > + ret = drm_mode_config_init(drm_dev); > + if (ret) > + goto err_free; Shouldn't the goto label be err_mode_config_cleanup here? Otherwise this error path misses the call to rockchip_iommu_cleanup(). > > rockchip_drm_mode_config_init(drm_dev); > > @@ -174,11 +176,8 @@ static int rockchip_drm_bind(struct device *dev) > err_unbind_all: > component_unbind_all(dev, drm_dev); > err_mode_config_cleanup: > - drm_mode_config_cleanup(drm_dev); > rockchip_iommu_cleanup(drm_dev); > err_free: > - drm_dev->dev_private = NULL; > - dev_set_drvdata(dev, NULL); > drm_dev_put(drm_dev); > return ret; > } On Fri, Feb 21, 2020 at 10:04 PM Daniel Vetter <daniel.vetter@xxxxxxxx> wrote: > > It's (almost, there's some iommu stuff without significance) right > above the drm_dev_put(). > > This is made possible by a preceeding patch which added a drmm_ > cleanup action to drm_mode_config_init(), hence all we need to do to > ensure that drm_mode_config_cleanup() is run on final drm_device > cleanup is check the new error code for _init(). > > Aside: Another driver with a bit much devm_kzalloc, which should > probably use drmm_kzalloc instead ... > > v2: Explain why this cleanup is possible (Laurent). > > Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > Cc: Sandy Huang <hjc@xxxxxxxxxxxxxx> > Cc: "Heiko Stübner" <heiko@xxxxxxxxx> > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > Cc: linux-rockchip@xxxxxxxxxxxxxxxxxxx > --- > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > index 20ecb1508a22..d0eba21eebc9 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > @@ -135,7 +135,9 @@ static int rockchip_drm_bind(struct device *dev) > if (ret) > goto err_free; > > - drm_mode_config_init(drm_dev); > + ret = drm_mode_config_init(drm_dev); > + if (ret) > + goto err_free; > > rockchip_drm_mode_config_init(drm_dev); > > @@ -174,11 +176,8 @@ static int rockchip_drm_bind(struct device *dev) > err_unbind_all: > component_unbind_all(dev, drm_dev); > err_mode_config_cleanup: > - drm_mode_config_cleanup(drm_dev); > rockchip_iommu_cleanup(drm_dev); > err_free: > - drm_dev->dev_private = NULL; > - dev_set_drvdata(dev, NULL); > drm_dev_put(drm_dev); > return ret; > } > @@ -194,11 +193,8 @@ static void rockchip_drm_unbind(struct device *dev) > > drm_atomic_helper_shutdown(drm_dev); > component_unbind_all(dev, drm_dev); > - drm_mode_config_cleanup(drm_dev); > rockchip_iommu_cleanup(drm_dev); > > - drm_dev->dev_private = NULL; > - dev_set_drvdata(dev, NULL); > drm_dev_put(drm_dev); > } > > -- > 2.24.1 > > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/linux-rockchip _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
