| Message ID | 20200910132354.692397-1-yukuai3@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | drm: rcar-du: add missing put_device() call in rcar_du_vsp_init() | expand |
Hi Yu, Thank you for the patch. On Thu, Sep 10, 2020 at 09:23:54PM +0800, Yu Kuai wrote: > if of_find_device_by_node() succeed, rcar_du_vsp_init() doesn't have > a corresponding put_device(). Thus add a jump target to fix the exception > handling for this function implementation. > > Fixes: 6d62ef3ac30b ("drm: rcar-du: Expose the VSP1 compositor through KMS planes") > Signed-off-by: Yu Kuai <yukuai3@huawei.com> > --- > drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 19 +++++++++++++------ > 1 file changed, 13 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c > index f1a81c9b184d..172ee3f3b21c 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c > @@ -352,14 +352,16 @@ int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np, > > /* Find the VSP device and initialize it. */ > pdev = of_find_device_by_node(np); > - if (!pdev) > - return -ENXIO; > + if (!pdev) { > + ret = -ENXIO; > + goto put_device; > + } This change isn't needed, and will actually cause a crash, as pdev is NULL. > > vsp->vsp = &pdev->dev; > > ret = vsp1_du_init(vsp->vsp); > if (ret < 0) > - return ret; > + goto put_device; > > /* > * The VSP2D (Gen3) has 5 RPFs, but the VSP1D (Gen2) is limited to > @@ -369,8 +371,10 @@ int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np, > > vsp->planes = devm_kcalloc(rcdu->dev, vsp->num_planes, > sizeof(*vsp->planes), GFP_KERNEL); > - if (!vsp->planes) > - return -ENOMEM; > + if (!vsp->planes) { > + ret = -ENOMEM; > + goto put_device; > + } > > for (i = 0; i < vsp->num_planes; ++i) { > enum drm_plane_type type = i < num_crtcs > @@ -387,7 +391,7 @@ int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np, > ARRAY_SIZE(rcar_du_vsp_formats), > NULL, type, NULL); > if (ret < 0) > - return ret; > + goto put_device; > > drm_plane_helper_add(&plane->plane, > &rcar_du_vsp_plane_helper_funcs); > @@ -403,4 +407,7 @@ int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np, > } > > return 0; I would add a blank line here. > +put_device: And maybe name the label "error" ? > + put_device(&pdev->dev); > + return ret; > } We need more than this, we also need to call put_device() when the driver is unloaded. The way to handle cleanup in DRM is through drmm_add_action() nowadays, and I think we could thus simply replace the change above with a cleanup action that is run both in the error path and at driver remove. I'll post a proposal in a reply to this e-mail.
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index f1a81c9b184d..172ee3f3b21c 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c @@ -352,14 +352,16 @@ int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np, /* Find the VSP device and initialize it. */ pdev = of_find_device_by_node(np); - if (!pdev) - return -ENXIO; + if (!pdev) { + ret = -ENXIO; + goto put_device; + } vsp->vsp = &pdev->dev; ret = vsp1_du_init(vsp->vsp); if (ret < 0) - return ret; + goto put_device; /* * The VSP2D (Gen3) has 5 RPFs, but the VSP1D (Gen2) is limited to @@ -369,8 +371,10 @@ int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np, vsp->planes = devm_kcalloc(rcdu->dev, vsp->num_planes, sizeof(*vsp->planes), GFP_KERNEL); - if (!vsp->planes) - return -ENOMEM; + if (!vsp->planes) { + ret = -ENOMEM; + goto put_device; + } for (i = 0; i < vsp->num_planes; ++i) { enum drm_plane_type type = i < num_crtcs @@ -387,7 +391,7 @@ int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np, ARRAY_SIZE(rcar_du_vsp_formats), NULL, type, NULL); if (ret < 0) - return ret; + goto put_device; drm_plane_helper_add(&plane->plane, &rcar_du_vsp_plane_helper_funcs); @@ -403,4 +407,7 @@ int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np, } return 0; +put_device: + put_device(&pdev->dev); + return ret; }
if of_find_device_by_node() succeed, rcar_du_vsp_init() doesn't have a corresponding put_device(). Thus add a jump target to fix the exception handling for this function implementation. Fixes: 6d62ef3ac30b ("drm: rcar-du: Expose the VSP1 compositor through KMS planes") Signed-off-by: Yu Kuai <yukuai3@huawei.com> --- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)