On Mon, Sep 19, 2016 at 7:14 AM, Daniel Kurtz <djkurtz@xxxxxxxxxxxx> wrote:
> Hi Sean,
>
> On Sat, Sep 17, 2016 at 2:22 AM, Sean Paul <seanpaul@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
