On Wed, Sep 21, 2016 at 3:36 PM, Sean Paul <seanpaul@xxxxxxxxxxxx> wrote:
> 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.
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@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
