public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Lars-Peter Clausen <lars@metafoo.de>
To: Doug Anderson <dianders@chromium.org>
Cc: Naveen Krishna Chatradhi <ch.naveen@samsung.com>,
	linux-iio <linux-iio@vger.kernel.org>,
	dan.carpenter@oracle.com,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	linux-samsung-soc@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Naveen Krishna <naveenkrishna.ch@gmail.com>
Subject: Re: [PATCH v3] iio: adc: exynos5_adc: fix compilation warnings
Date: Wed, 13 Mar 2013 19:40:22 +0100	[thread overview]
Message-ID: <5140C816.3090008@metafoo.de> (raw)
In-Reply-To: <5140C6EE.9080802@metafoo.de>

On 03/13/2013 07:35 PM, Lars-Peter Clausen wrote:
> On 03/13/2013 07:23 PM, Doug Anderson wrote:
>> Lars,
>>
>> On Wed, Mar 13, 2013 at 11:11 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>>> Agreed. Adding the dependency on OF in Kconfig should be all that is needed.
>>
>> I think changing the timeout from 'unsigned long' to 'long' is also
>> legit (to match the actual type returned) and a good idea.
>>
>> -Doug
> 
> Yes, but that's a different issue and to be honest I didn't even realize
> that the patch was trying to fix this as well. In my opinion it's best to
> split this up into two patches one which fixes the OF dependency. The other
> fixing the timeout issue. Cause there is more to it than just changing the type.
> 
> wait_for_completion_interruptible_timeout() may return
> 	1) 0, if there was a timeout, waiting for the completion
> 	2) > 0, if the completion was completeted, the returned value
> 	   the  remaining time.
> 	3) < 0, If it was interrupt while waiting for the completion
> 
> The code currently only handles 1) and 2), but it also needs to handle 3).
> Since the completion has not been completed in case 3.
> 
> E.g. something like this should work:
> 
> 	if (timeout == 0)
> 		return -ETIMEDOUT;
> 	else if(timeout < 0)
> 		return timeout;
> 	return 0;
> 

I just saw, there is another issue related to this. The driver should call
INIT_COMPLETION(&info->completion) before starting the conversion. Otherwise
there may be a problem if we got interrupted while waiting for the
interrupt. E.g. imagine the following sequence.

1) Start conversion
2) Wait for completion
3) Abort waiting
4) Interrupt kicks in and marks the completion as done

Now if another conversion is started the completion will already be
completed and wait_for_completion_interruptible_timeout() will return right
away without waiting for the conversion to be finished.

- Lars

  reply	other threads:[~2013-03-13 18:38 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-07  3:09 [PATCH v2] iio: adc: exynos5_adc: fix compilation warnings Naveen Krishna Chatradhi
2013-03-12 16:14 ` Doug Anderson
2013-03-13  4:48 ` [PATCH v3] " Naveen Krishna Chatradhi
2013-03-13 18:01   ` Doug Anderson
2013-03-13 18:11     ` Lars-Peter Clausen
2013-03-13 18:23       ` Doug Anderson
2013-03-13 18:35         ` Lars-Peter Clausen
2013-03-13 18:40           ` Lars-Peter Clausen [this message]
2013-03-13 20:12             ` Doug Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5140C816.3090008@metafoo.de \
    --to=lars@metafoo.de \
    --cc=ch.naveen@samsung.com \
    --cc=dan.carpenter@oracle.com \
    --cc=dianders@chromium.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=naveenkrishna.ch@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox