On Mon, Feb 24, 2020 at 8:13 PM Francesco Lavra <francescolavra.fl@xxxxxxxxx> wrote: > > 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(). Indeed. I'll also rename the label to have a more meaningful name while at it. -Daniel > > > > > 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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
