* [PATCH] drm/rockchip: Cleanup dangling devm pointers @ 2016-09-16 18:22 Sean Paul 2016-09-19 4:14 ` Daniel Kurtz 0 siblings, 1 reply; 4+ messages in thread From: Sean Paul @ 2016-09-16 18:22 UTC (permalink / raw) To: mark.yao, heiko, dri-devel; +Cc: linux-rockchip Instead of assigning device managed resources to local variables, keep track of them in the vop struct. Signed-off-by: Sean Paul <seanpaul@chromium.org> --- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index 131ae0f..bed782e 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -142,6 +142,7 @@ struct vop { /* vop dclk reset */ struct reset_control *dclk_rst; + struct reset_control *ahb_rst; struct vop_win win[]; }; @@ -1333,7 +1334,6 @@ static int vop_initial(struct vop *vop) { const struct vop_data *vop_data = vop->data; const struct vop_reg_data *init_table = vop_data->init_table; - struct reset_control *ahb_rst; int i, ret; vop->hclk = devm_clk_get(vop->dev, "hclk_vop"); @@ -1374,15 +1374,15 @@ static int vop_initial(struct vop *vop) /* * do hclk_reset, reset all vop registers. */ - ahb_rst = devm_reset_control_get(vop->dev, "ahb"); - if (IS_ERR(ahb_rst)) { + vop->ahb_rst = devm_reset_control_get(vop->dev, "ahb"); + if (IS_ERR(vop->ahb_rst)) { dev_err(vop->dev, "failed to get ahb reset\n"); - ret = PTR_ERR(ahb_rst); + ret = PTR_ERR(vop->ahb_rst); goto err_disable_aclk; } - reset_control_assert(ahb_rst); + reset_control_assert(vop->ahb_rst); usleep_range(10, 20); - reset_control_deassert(ahb_rst); + reset_control_deassert(vop->ahb_rst); memcpy(vop->regsbak, vop->regs, vop->len); -- 2.8.0.rc3.226.g39d4020 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/rockchip: Cleanup dangling devm pointers 2016-09-16 18:22 [PATCH] drm/rockchip: Cleanup dangling devm pointers Sean Paul @ 2016-09-19 4:14 ` Daniel Kurtz 2016-09-21 7:36 ` Sean Paul 0 siblings, 1 reply; 4+ messages in thread From: Daniel Kurtz @ 2016-09-19 4:14 UTC (permalink / raw) To: Sean Paul; +Cc: open list:ARM/Rockchip SoC..., dri-devel Hi Sean, On Sat, Sep 17, 2016 at 2:22 AM, Sean Paul <seanpaul@chromium.org> wrote: > > Instead of assigning device managed resources to local variables, > keep track of them in the vop struct. Why this patch? Is it fixing an issue? Or, is it preparing for some future use of ahb_rst outside of vop_initial? I thought that one of the nice features of using devm is you do not need to carry around pointers to devm allocated resources in the driver local device struct. -Dan > > Signed-off-by: Sean Paul <seanpaul@chromium.org> > --- > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index 131ae0f..bed782e 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -142,6 +142,7 @@ struct vop { > > /* vop dclk reset */ > struct reset_control *dclk_rst; > + struct reset_control *ahb_rst; > > struct vop_win win[]; > }; > @@ -1333,7 +1334,6 @@ static int vop_initial(struct vop *vop) > { > const struct vop_data *vop_data = vop->data; > const struct vop_reg_data *init_table = vop_data->init_table; > - struct reset_control *ahb_rst; > int i, ret; > > vop->hclk = devm_clk_get(vop->dev, "hclk_vop"); > @@ -1374,15 +1374,15 @@ static int vop_initial(struct vop *vop) > /* > * do hclk_reset, reset all vop registers. > */ > - ahb_rst = devm_reset_control_get(vop->dev, "ahb"); > - if (IS_ERR(ahb_rst)) { > + vop->ahb_rst = devm_reset_control_get(vop->dev, "ahb"); > + if (IS_ERR(vop->ahb_rst)) { > dev_err(vop->dev, "failed to get ahb reset\n"); > - ret = PTR_ERR(ahb_rst); > + ret = PTR_ERR(vop->ahb_rst); > goto err_disable_aclk; > } > - reset_control_assert(ahb_rst); > + reset_control_assert(vop->ahb_rst); > usleep_range(10, 20); > - reset_control_deassert(ahb_rst); > + reset_control_deassert(vop->ahb_rst); > > memcpy(vop->regsbak, vop->regs, vop->len); > > -- > 2.8.0.rc3.226.g39d4020 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/rockchip: Cleanup dangling devm pointers 2016-09-19 4:14 ` Daniel Kurtz @ 2016-09-21 7:36 ` Sean Paul 2016-09-21 8:55 ` Daniel Kurtz 0 siblings, 1 reply; 4+ messages in thread From: Sean Paul @ 2016-09-21 7:36 UTC (permalink / raw) To: Daniel Kurtz; +Cc: open list:ARM/Rockchip SoC..., dri-devel On Mon, Sep 19, 2016 at 7:14 AM, Daniel Kurtz <djkurtz@chromium.org> wrote: > Hi Sean, > > On Sat, Sep 17, 2016 at 2:22 AM, Sean Paul <seanpaul@chromium.org> wrote: >> >> Instead of assigning device managed resources to local variables, >> keep track of them in the vop struct. > > Why this patch? > Is it fixing an issue? > Or, is it preparing for some future use of ahb_rst outside of vop_initial? > Nah, this is just me being pedantic. > I thought that one of the nice features of using devm is you do not > need to carry around pointers to devm allocated resources in the > driver local device struct. > True, but it feels a bit weird to allocate something on driver load and intentionally abandon it for the lifetime of the driver. I'd feel better either keeping it around, or perhaps it should be put once we're done using it in vop_initial. Sean > -Dan > >> >> Signed-off-by: Sean Paul <seanpaul@chromium.org> >> --- >> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >> index 131ae0f..bed782e 100644 >> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >> @@ -142,6 +142,7 @@ struct vop { >> >> /* vop dclk reset */ >> struct reset_control *dclk_rst; >> + struct reset_control *ahb_rst; >> >> struct vop_win win[]; >> }; >> @@ -1333,7 +1334,6 @@ static int vop_initial(struct vop *vop) >> { >> const struct vop_data *vop_data = vop->data; >> const struct vop_reg_data *init_table = vop_data->init_table; >> - struct reset_control *ahb_rst; >> int i, ret; >> >> vop->hclk = devm_clk_get(vop->dev, "hclk_vop"); >> @@ -1374,15 +1374,15 @@ static int vop_initial(struct vop *vop) >> /* >> * do hclk_reset, reset all vop registers. >> */ >> - ahb_rst = devm_reset_control_get(vop->dev, "ahb"); >> - if (IS_ERR(ahb_rst)) { >> + vop->ahb_rst = devm_reset_control_get(vop->dev, "ahb"); >> + if (IS_ERR(vop->ahb_rst)) { >> dev_err(vop->dev, "failed to get ahb reset\n"); >> - ret = PTR_ERR(ahb_rst); >> + ret = PTR_ERR(vop->ahb_rst); >> goto err_disable_aclk; >> } >> - reset_control_assert(ahb_rst); >> + reset_control_assert(vop->ahb_rst); >> usleep_range(10, 20); >> - reset_control_deassert(ahb_rst); >> + reset_control_deassert(vop->ahb_rst); >> >> memcpy(vop->regsbak, vop->regs, vop->len); >> >> -- >> 2.8.0.rc3.226.g39d4020 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/rockchip: Cleanup dangling devm pointers 2016-09-21 7:36 ` Sean Paul @ 2016-09-21 8:55 ` Daniel Kurtz 0 siblings, 0 replies; 4+ messages in thread From: Daniel Kurtz @ 2016-09-21 8:55 UTC (permalink / raw) To: Sean Paul; +Cc: open list:ARM/Rockchip SoC..., dri-devel On Wed, Sep 21, 2016 at 3:36 PM, Sean Paul <seanpaul@chromium.org> wrote: > On Mon, Sep 19, 2016 at 7:14 AM, Daniel Kurtz <djkurtz@chromium.org> wrote: >> Hi Sean, >> >> On Sat, Sep 17, 2016 at 2:22 AM, Sean Paul <seanpaul@chromium.org> wrote: >>> >>> Instead of assigning device managed resources to local variables, >>> keep track of them in the vop struct. >> >> Why this patch? >> Is it fixing an issue? >> Or, is it preparing for some future use of ahb_rst outside of vop_initial? >> > > Nah, this is just me being pedantic. > >> I thought that one of the nice features of using devm is you do not >> need to carry around pointers to devm allocated resources in the >> driver local device struct. >> > > True, but it feels a bit weird to allocate something on driver load > and intentionally abandon it for the lifetime of the driver. I'd feel > better either keeping it around, or perhaps it should be put once > we're done using it in vop_initial. Yeah... if we don't ever use it again, I agree - no reason to devm it, just use reset_control_get / _put here in vop_initial. > > Sean > > >> -Dan >> >>> >>> Signed-off-by: Sean Paul <seanpaul@chromium.org> >>> --- >>> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 12 ++++++------ >>> 1 file changed, 6 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >>> index 131ae0f..bed782e 100644 >>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >>> @@ -142,6 +142,7 @@ struct vop { >>> >>> /* vop dclk reset */ >>> struct reset_control *dclk_rst; >>> + struct reset_control *ahb_rst; >>> >>> struct vop_win win[]; >>> }; >>> @@ -1333,7 +1334,6 @@ static int vop_initial(struct vop *vop) >>> { >>> const struct vop_data *vop_data = vop->data; >>> const struct vop_reg_data *init_table = vop_data->init_table; >>> - struct reset_control *ahb_rst; >>> int i, ret; >>> >>> vop->hclk = devm_clk_get(vop->dev, "hclk_vop"); >>> @@ -1374,15 +1374,15 @@ static int vop_initial(struct vop *vop) >>> /* >>> * do hclk_reset, reset all vop registers. >>> */ >>> - ahb_rst = devm_reset_control_get(vop->dev, "ahb"); >>> - if (IS_ERR(ahb_rst)) { >>> + vop->ahb_rst = devm_reset_control_get(vop->dev, "ahb"); >>> + if (IS_ERR(vop->ahb_rst)) { >>> dev_err(vop->dev, "failed to get ahb reset\n"); >>> - ret = PTR_ERR(ahb_rst); >>> + ret = PTR_ERR(vop->ahb_rst); >>> goto err_disable_aclk; >>> } >>> - reset_control_assert(ahb_rst); >>> + reset_control_assert(vop->ahb_rst); >>> usleep_range(10, 20); >>> - reset_control_deassert(ahb_rst); >>> + reset_control_deassert(vop->ahb_rst); >>> >>> memcpy(vop->regsbak, vop->regs, vop->len); >>> >>> -- >>> 2.8.0.rc3.226.g39d4020 >>> >>> _______________________________________________ >>> dri-devel mailing list >>> dri-devel@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-09-21 8:55 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-09-16 18:22 [PATCH] drm/rockchip: Cleanup dangling devm pointers Sean Paul 2016-09-19 4:14 ` Daniel Kurtz 2016-09-21 7:36 ` Sean Paul 2016-09-21 8:55 ` Daniel Kurtz
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.