>Hi, Kamil
>This is third feedback about watchdog timer. 
>(s5p_mfc.c)

Hi, Peter

Thanks for pointing that out, enabling and disabling watchdog in
open/release is reasonable.

>[...]

>> +    platform_set_drvdata(pdev, dev);
>> +    dev->hw_lock = 0;
>> +    dev->watchdog_workqueue = create_singlethread_workqueue("s5p-mfc");
>> +    INIT_WORK(&dev->watchdog_work, s5p_mfc_watchdog_worker);
>> +    atomic_set(&dev->watchdog_cnt, 0);
>> +    init_timer(&dev->watchdog_timer);
>> +    dev->watchdog_timer.data = 0;
>> +    dev->watchdog_timer.function = s5p_mfc_watchdog;
>> +    dev->watchdog_timer.expires = jiffies +
>> + msecs_to_jiffies(MFC_WATCHDOG_INTERVAL);
>> +    add_timer(&dev->watchdog_timer);

>Watch_dog single thread runs right after probing MFC, but this doesn't look
>like
>nice way in terms of purpose of this timer which is for error handling in
>the 
>middle of decoding. What about moving point running this timer to the
>open().
>And it should be stopped in release time. Of course, dev->num_inst should
>be 
>considered.

Yes, I agree. I've changed that.

> 
>> +
>> +    dev->alloc_ctx = vb2_cma_init_multi(&pdev->dev, 2, s5p_mem_types,
>> +                                                    s5p_mem_alignments);
>> +    if (IS_ERR(dev->alloc_ctx)) {
>> +            mfc_err("Couldn't prepare allocator context.\n");
>> +            ret = PTR_ERR(dev->alloc_ctx);
>> +            goto alloc_ctx_fail;
>> +    }
>> +
>> +    ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
>> +    if (ret) {
>> +            v4l2_err(&dev->v4l2_dev, "Failed to register video
>> device\n");
>> +            video_device_release(vfd);
>> +            goto rel_vdev;
>> +    }
>>

--
Kamil Debski
Linux Platform Group
Samsung Poland R&D Center

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to