* [PATCH] iio: adc: sc27xx: Add ADC data conversion timeout
@ 2018-11-09 3:25 Baolin Wang
2018-11-11 12:40 ` Jonathan Cameron
0 siblings, 1 reply; 4+ messages in thread
From: Baolin Wang @ 2018-11-09 3:25 UTC (permalink / raw)
To: jic23
Cc: knaack.h, lars, pmeerw, freeman.liu, broonie, linux-iio,
linux-kernel, baolin.wang
From: Freeman Liu <freeman.liu@unisoc.com>
Sometimes the ADC controller met some problems, and it will not complete
the data conversion, that will can not wake up the read process any more
to block users. So we should add one maximum conversion time to avoid
this issue.
Signed-off-by: Freeman Liu <freeman.liu@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/iio/adc/sc27xx_adc.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
index 7940b23..f7f7a189 100644
--- a/drivers/iio/adc/sc27xx_adc.c
+++ b/drivers/iio/adc/sc27xx_adc.c
@@ -52,6 +52,9 @@
/* Timeout (ms) for the trylock of hardware spinlocks */
#define SC27XX_ADC_HWLOCK_TIMEOUT 5000
+/* Timeout (ms) for ADC data conversion according to ADC datasheet */
+#define SC27XX_ADC_RDY_TIMEOUT 100
+
/* Maximum ADC channel number */
#define SC27XX_ADC_CHANNEL_MAX 32
@@ -223,7 +226,14 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
if (ret)
goto disable_adc;
- wait_for_completion(&data->completion);
+ ret = wait_for_completion_timeout(&data->completion,
+ msecs_to_jiffies(SC27XX_ADC_RDY_TIMEOUT));
+ if (!ret) {
+ dev_err(data->dev, "read ADC data timeout\n");
+ ret = -ETIMEDOUT;
+ } else {
+ ret = 0;
+ }
disable_adc:
regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: adc: sc27xx: Add ADC data conversion timeout
2018-11-09 3:25 [PATCH] iio: adc: sc27xx: Add ADC data conversion timeout Baolin Wang
@ 2018-11-11 12:40 ` Jonathan Cameron
2018-11-11 15:00 ` Baolin Wang
0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Cameron @ 2018-11-11 12:40 UTC (permalink / raw)
To: Baolin Wang
Cc: knaack.h, lars, pmeerw, freeman.liu, broonie, linux-iio, linux-kernel
On Fri, 9 Nov 2018 11:25:31 +0800
Baolin Wang <baolin.wang@linaro.org> wrote:
> From: Freeman Liu <freeman.liu@unisoc.com>
>
> Sometimes the ADC controller met some problems, and it will not complete
> the data conversion, that will can not wake up the read process any more
> to block users. So we should add one maximum conversion time to avoid
> this issue.
>
> Signed-off-by: Freeman Liu <freeman.liu@unisoc.com>
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
Hi.
Patch looks good, but sounds like this is a fix so could I have a fixes
tag and a patch title that makes that clear?
I think we will want this one back ported to stable by the sound of it.
If it is just a theoretical issue then perhaps we don't need to bother.
Thanks,
Jonathan
> ---
> drivers/iio/adc/sc27xx_adc.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index 7940b23..f7f7a189 100644
> --- a/drivers/iio/adc/sc27xx_adc.c
> +++ b/drivers/iio/adc/sc27xx_adc.c
> @@ -52,6 +52,9 @@
> /* Timeout (ms) for the trylock of hardware spinlocks */
> #define SC27XX_ADC_HWLOCK_TIMEOUT 5000
>
> +/* Timeout (ms) for ADC data conversion according to ADC datasheet */
> +#define SC27XX_ADC_RDY_TIMEOUT 100
> +
> /* Maximum ADC channel number */
> #define SC27XX_ADC_CHANNEL_MAX 32
>
> @@ -223,7 +226,14 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
> if (ret)
> goto disable_adc;
>
> - wait_for_completion(&data->completion);
> + ret = wait_for_completion_timeout(&data->completion,
> + msecs_to_jiffies(SC27XX_ADC_RDY_TIMEOUT));
> + if (!ret) {
> + dev_err(data->dev, "read ADC data timeout\n");
> + ret = -ETIMEDOUT;
> + } else {
> + ret = 0;
> + }
>
> disable_adc:
> regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: adc: sc27xx: Add ADC data conversion timeout
2018-11-11 12:40 ` Jonathan Cameron
@ 2018-11-11 15:00 ` Baolin Wang
2018-11-11 16:03 ` Jonathan Cameron
0 siblings, 1 reply; 4+ messages in thread
From: Baolin Wang @ 2018-11-11 15:00 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
freeman.liu, Mark Brown, linux-iio, LKML
Hi Jonathan,
On 11 November 2018 at 20:40, Jonathan Cameron <jic23@kernel.org> wrote:
> On Fri, 9 Nov 2018 11:25:31 +0800
> Baolin Wang <baolin.wang@linaro.org> wrote:
>
>> From: Freeman Liu <freeman.liu@unisoc.com>
>>
>> Sometimes the ADC controller met some problems, and it will not complete
>> the data conversion, that will can not wake up the read process any more
>> to block users. So we should add one maximum conversion time to avoid
>> this issue.
>>
>> Signed-off-by: Freeman Liu <freeman.liu@unisoc.com>
>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> Hi.
>
> Patch looks good, but sounds like this is a fix so could I have a fixes
> tag and a patch title that makes that clear?
>
> I think we will want this one back ported to stable by the sound of it.
>
> If it is just a theoretical issue then perhaps we don't need to bother.
Thanks for your review. This patch is not one bug fix, just make the
code more robust in case of some bad cases. So I think we do not need
the stable tag.
>
> Thanks,
>
> Jonathan
>
>> ---
>> drivers/iio/adc/sc27xx_adc.c | 12 +++++++++++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
>> index 7940b23..f7f7a189 100644
>> --- a/drivers/iio/adc/sc27xx_adc.c
>> +++ b/drivers/iio/adc/sc27xx_adc.c
>> @@ -52,6 +52,9 @@
>> /* Timeout (ms) for the trylock of hardware spinlocks */
>> #define SC27XX_ADC_HWLOCK_TIMEOUT 5000
>>
>> +/* Timeout (ms) for ADC data conversion according to ADC datasheet */
>> +#define SC27XX_ADC_RDY_TIMEOUT 100
>> +
>> /* Maximum ADC channel number */
>> #define SC27XX_ADC_CHANNEL_MAX 32
>>
>> @@ -223,7 +226,14 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
>> if (ret)
>> goto disable_adc;
>>
>> - wait_for_completion(&data->completion);
>> + ret = wait_for_completion_timeout(&data->completion,
>> + msecs_to_jiffies(SC27XX_ADC_RDY_TIMEOUT));
>> + if (!ret) {
>> + dev_err(data->dev, "read ADC data timeout\n");
>> + ret = -ETIMEDOUT;
>> + } else {
>> + ret = 0;
>> + }
>>
>> disable_adc:
>> regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
>
--
Baolin Wang
Best Regards
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: adc: sc27xx: Add ADC data conversion timeout
2018-11-11 15:00 ` Baolin Wang
@ 2018-11-11 16:03 ` Jonathan Cameron
0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2018-11-11 16:03 UTC (permalink / raw)
To: Baolin Wang
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
freeman.liu, Mark Brown, linux-iio, LKML
On Sun, 11 Nov 2018 23:00:18 +0800
Baolin Wang <baolin.wang@linaro.org> wrote:
> Hi Jonathan,
>
> On 11 November 2018 at 20:40, Jonathan Cameron <jic23@kernel.org> wrote:
> > On Fri, 9 Nov 2018 11:25:31 +0800
> > Baolin Wang <baolin.wang@linaro.org> wrote:
> >
> >> From: Freeman Liu <freeman.liu@unisoc.com>
> >>
> >> Sometimes the ADC controller met some problems, and it will not complete
> >> the data conversion, that will can not wake up the read process any more
> >> to block users. So we should add one maximum conversion time to avoid
> >> this issue.
> >>
> >> Signed-off-by: Freeman Liu <freeman.liu@unisoc.com>
> >> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> > Hi.
> >
> > Patch looks good, but sounds like this is a fix so could I have a fixes
> > tag and a patch title that makes that clear?
> >
> > I think we will want this one back ported to stable by the sound of it.
> >
> > If it is just a theoretical issue then perhaps we don't need to bother.
>
> Thanks for your review. This patch is not one bug fix, just make the
> code more robust in case of some bad cases. So I think we do not need
> the stable tag.
Thanks.
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.
Jonathan
>
> >
> > Thanks,
> >
> > Jonathan
> >
> >> ---
> >> drivers/iio/adc/sc27xx_adc.c | 12 +++++++++++-
> >> 1 file changed, 11 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> >> index 7940b23..f7f7a189 100644
> >> --- a/drivers/iio/adc/sc27xx_adc.c
> >> +++ b/drivers/iio/adc/sc27xx_adc.c
> >> @@ -52,6 +52,9 @@
> >> /* Timeout (ms) for the trylock of hardware spinlocks */
> >> #define SC27XX_ADC_HWLOCK_TIMEOUT 5000
> >>
> >> +/* Timeout (ms) for ADC data conversion according to ADC datasheet */
> >> +#define SC27XX_ADC_RDY_TIMEOUT 100
> >> +
> >> /* Maximum ADC channel number */
> >> #define SC27XX_ADC_CHANNEL_MAX 32
> >>
> >> @@ -223,7 +226,14 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
> >> if (ret)
> >> goto disable_adc;
> >>
> >> - wait_for_completion(&data->completion);
> >> + ret = wait_for_completion_timeout(&data->completion,
> >> + msecs_to_jiffies(SC27XX_ADC_RDY_TIMEOUT));
> >> + if (!ret) {
> >> + dev_err(data->dev, "read ADC data timeout\n");
> >> + ret = -ETIMEDOUT;
> >> + } else {
> >> + ret = 0;
> >> + }
> >>
> >> disable_adc:
> >> regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
> >
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-11-11 16:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-09 3:25 [PATCH] iio: adc: sc27xx: Add ADC data conversion timeout Baolin Wang
2018-11-11 12:40 ` Jonathan Cameron
2018-11-11 15:00 ` Baolin Wang
2018-11-11 16:03 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).