[v2] timers: Cleanup delay/sleep related mess

[PATCH v2 14/15] media: anysee: Fix link to outdated sleep function documentation

Posted by Anna-Maria Behnsen 2 months, 2 weeks ago

The TODO FIXME comment references the outdated lower limit for msleep()
function of 20ms. As this is not right and the proper documentation of
msleep() is now part of the function description, remove the old stuff and
point to the up to date documentation.

Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: linux-media@vger.kernel.org
Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
---
 drivers/media/usb/dvb-usb-v2/anysee.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/anysee.c b/drivers/media/usb/dvb-usb-v2/anysee.c
index 8699846eb416..b2f5db961245 100644
--- a/drivers/media/usb/dvb-usb-v2/anysee.c
+++ b/drivers/media/usb/dvb-usb-v2/anysee.c
@@ -55,10 +55,8 @@ static int anysee_ctrl_msg(struct dvb_usb_device *d,
 
 	/* TODO FIXME: dvb_usb_generic_rw() fails rarely with error code -32
 	 * (EPIPE, Broken pipe). Function supports currently msleep() as a
-	 * parameter but I would not like to use it, since according to
-	 * Documentation/timers/timers-howto.rst it should not be used such
-	 * short, under < 20ms, sleeps. Repeating failed message would be
-	 * better choice as not to add unwanted delays...
+	 * parameter. Check msleep() for details. Repeating failed message would
+	 * be better choice as not to add unwanted delays...
 	 * Fixing that correctly is one of those or both;
 	 * 1) use repeat if possible
 	 * 2) add suitable delay

-- 
2.39.2

Re: [PATCH v2 14/15] media: anysee: Fix link to outdated sleep function documentation

Posted by Frederic Weisbecker 1 month, 3 weeks ago

Le Wed, Sep 11, 2024 at 07:13:40AM +0200, Anna-Maria Behnsen a écrit :
> The TODO FIXME comment references the outdated lower limit for msleep()
> function of 20ms. As this is not right and the proper documentation of
> msleep() is now part of the function description, remove the old stuff and
> point to the up to date documentation.
> 
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: linux-media@vger.kernel.org
> Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
> ---
>  drivers/media/usb/dvb-usb-v2/anysee.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/usb/dvb-usb-v2/anysee.c b/drivers/media/usb/dvb-usb-v2/anysee.c
> index 8699846eb416..b2f5db961245 100644
> --- a/drivers/media/usb/dvb-usb-v2/anysee.c
> +++ b/drivers/media/usb/dvb-usb-v2/anysee.c
> @@ -55,10 +55,8 @@ static int anysee_ctrl_msg(struct dvb_usb_device *d,
>  
>  	/* TODO FIXME: dvb_usb_generic_rw() fails rarely with error code -32
>  	 * (EPIPE, Broken pipe). Function supports currently msleep() as a
> -	 * parameter but I would not like to use it, since according to
> -	 * Documentation/timers/timers-howto.rst it should not be used such
> -	 * short, under < 20ms, sleeps. Repeating failed message would be
> -	 * better choice as not to add unwanted delays...
> +	 * parameter. Check msleep() for details. Repeating failed message would
> +	 * be better choice as not to add unwanted delays...

Does the comment still matches any up-to-date worry? It passes 2000 ms which is
2 seconds...

Thanks.

>  	 * Fixing that correctly is one of those or both;
>  	 * 1) use repeat if possible
>  	 * 2) add suitable delay
> 
> -- 
> 2.39.2
>

Re: [PATCH v2 14/15] media: anysee: Fix link to outdated sleep function documentation

Posted by Anna-Maria Behnsen 1 month, 2 weeks ago

Frederic Weisbecker <frederic@kernel.org> writes:

> Le Wed, Sep 11, 2024 at 07:13:40AM +0200, Anna-Maria Behnsen a écrit :
>> The TODO FIXME comment references the outdated lower limit for msleep()
>> function of 20ms. As this is not right and the proper documentation of
>> msleep() is now part of the function description, remove the old stuff and
>> point to the up to date documentation.
>> 
>> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
>> Cc: linux-media@vger.kernel.org
>> Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
>> ---
>>  drivers/media/usb/dvb-usb-v2/anysee.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/media/usb/dvb-usb-v2/anysee.c b/drivers/media/usb/dvb-usb-v2/anysee.c
>> index 8699846eb416..b2f5db961245 100644
>> --- a/drivers/media/usb/dvb-usb-v2/anysee.c
>> +++ b/drivers/media/usb/dvb-usb-v2/anysee.c
>> @@ -55,10 +55,8 @@ static int anysee_ctrl_msg(struct dvb_usb_device *d,
>>  
>>  	/* TODO FIXME: dvb_usb_generic_rw() fails rarely with error code -32
>>  	 * (EPIPE, Broken pipe). Function supports currently msleep() as a
>> -	 * parameter but I would not like to use it, since according to
>> -	 * Documentation/timers/timers-howto.rst it should not be used such
>> -	 * short, under < 20ms, sleeps. Repeating failed message would be
>> -	 * better choice as not to add unwanted delays...
>> +	 * parameter. Check msleep() for details. Repeating failed message would
>> +	 * be better choice as not to add unwanted delays...
>
> Does the comment still matches any up-to-date worry? It passes 2000 ms which is
> 2 seconds...

I had a closer look at it: dvb_usb_generic_rw() is no longer used. The
v2 interfaces are used anyway and this uses usleep_range() instead of
msleep().

I updated the patch and also talked to Sebastian. Will send an update
with v3 which removes and updates the comments.

> Thanks.
>
>>  	 * Fixing that correctly is one of those or both;
>>  	 * 1) use repeat if possible
>>  	 * 2) add suitable delay
>> 
>> -- 
>> 2.39.2
>>