* [PATCH] iio: adc: stmpe-adc: Shuffle an if statement around in stmpe_adc_isr
@ 2019-03-07 17:16 Nathan Chancellor
2019-03-07 18:31 ` Nick Desaulniers
0 siblings, 1 reply; 3+ messages in thread
From: Nathan Chancellor @ 2019-03-07 17:16 UTC (permalink / raw)
To: Jonathan Cameron, Lee Jones
Cc: Max Krummenacher, Lars-Peter Clausen, Alexandre Torgue,
clang-built-linux, linux-iio, Peter Meerwald-Stadler,
Nick Desaulniers, linux-kernel, Stefan Agner, Philippe Schenker,
Maxime Coquelin, Hartmut Knaack, Nathan Chancellor, linux-stm32,
linux-arm-kernel
When building with -Wsometimes-uninitialized, Clang warns:
drivers/iio/adc/stmpe-adc.c:204:13: warning: variable 'data' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
Clang can't tell that data will never be used uninitialized because the
two if statements take care of all cases. Remove the first if statement
and make it the else branch of the second one so that it is apparent to
Clang that all cases are covered.
Link: https://github.com/ClangBuiltLinux/linux/issues/387
Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
drivers/iio/adc/stmpe-adc.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/adc/stmpe-adc.c b/drivers/iio/adc/stmpe-adc.c
index 37f4b74a5d32..7921f827c6ec 100644
--- a/drivers/iio/adc/stmpe-adc.c
+++ b/drivers/iio/adc/stmpe-adc.c
@@ -184,9 +184,6 @@ static irqreturn_t stmpe_adc_isr(int irq, void *dev_id)
struct stmpe_adc *info = (struct stmpe_adc *)dev_id;
u16 data;
- if (info->channel > STMPE_TEMP_CHANNEL)
- return IRQ_NONE;
-
if (info->channel <= STMPE_ADC_LAST_NR) {
int int_sta;
@@ -205,6 +202,8 @@ static irqreturn_t stmpe_adc_isr(int irq, void *dev_id)
/* Read value */
stmpe_block_read(info->stmpe, STMPE_REG_TEMP_DATA, 2,
(u8 *) &data);
+ } else {
+ return IRQ_NONE;
}
info->value = (u32) be16_to_cpu(data);
--
2.21.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: adc: stmpe-adc: Shuffle an if statement around in stmpe_adc_isr
2019-03-07 17:16 [PATCH] iio: adc: stmpe-adc: Shuffle an if statement around in stmpe_adc_isr Nathan Chancellor
@ 2019-03-07 18:31 ` Nick Desaulniers
2019-03-10 9:45 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Nick Desaulniers @ 2019-03-07 18:31 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Max Krummenacher, Lars-Peter Clausen, Peter Meerwald-Stadler,
clang-built-linux, linux-iio, LKML, Stefan Agner,
Philippe Schenker, Linux ARM, Maxime Coquelin, Hartmut Knaack,
Lee Jones, linux-stm32, Jonathan Cameron, Alexandre Torgue
On Thu, Mar 7, 2019 at 9:16 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> When building with -Wsometimes-uninitialized, Clang warns:
>
> drivers/iio/adc/stmpe-adc.c:204:13: warning: variable 'data' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
>
> Clang can't tell that data will never be used uninitialized because the
> two if statements take care of all cases. Remove the first if statement
> and make it the else branch of the second one so that it is apparent to
> Clang that all cases are covered.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/387
> Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
LGTM, thanks Nathan.
Reviewed-by: NIck Desaulniers <ndesaulniers@google.com>
> ---
> drivers/iio/adc/stmpe-adc.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/stmpe-adc.c b/drivers/iio/adc/stmpe-adc.c
> index 37f4b74a5d32..7921f827c6ec 100644
> --- a/drivers/iio/adc/stmpe-adc.c
> +++ b/drivers/iio/adc/stmpe-adc.c
> @@ -184,9 +184,6 @@ static irqreturn_t stmpe_adc_isr(int irq, void *dev_id)
> struct stmpe_adc *info = (struct stmpe_adc *)dev_id;
> u16 data;
>
> - if (info->channel > STMPE_TEMP_CHANNEL)
> - return IRQ_NONE;
> -
> if (info->channel <= STMPE_ADC_LAST_NR) {
> int int_sta;
>
> @@ -205,6 +202,8 @@ static irqreturn_t stmpe_adc_isr(int irq, void *dev_id)
> /* Read value */
> stmpe_block_read(info->stmpe, STMPE_REG_TEMP_DATA, 2,
> (u8 *) &data);
> + } else {
> + return IRQ_NONE;
> }
>
> info->value = (u32) be16_to_cpu(data);
> --
> 2.21.0
>
--
Thanks,
~Nick Desaulniers
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: adc: stmpe-adc: Shuffle an if statement around in stmpe_adc_isr
2019-03-07 18:31 ` Nick Desaulniers
@ 2019-03-10 9:45 ` Jonathan Cameron
0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2019-03-10 9:45 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Max Krummenacher, Lars-Peter Clausen, Peter Meerwald-Stadler,
clang-built-linux, linux-iio, LKML, Stefan Agner,
Philippe Schenker, Maxime Coquelin, Hartmut Knaack,
Nathan Chancellor, Lee Jones, linux-stm32, Linux ARM,
Alexandre Torgue
On Thu, 7 Mar 2019 10:31:55 -0800
Nick Desaulniers <ndesaulniers@google.com> wrote:
> On Thu, Mar 7, 2019 at 9:16 AM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > When building with -Wsometimes-uninitialized, Clang warns:
> >
> > drivers/iio/adc/stmpe-adc.c:204:13: warning: variable 'data' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
> >
> > Clang can't tell that data will never be used uninitialized because the
> > two if statements take care of all cases. Remove the first if statement
> > and make it the else branch of the second one so that it is apparent to
> > Clang that all cases are covered.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/387
> > Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
>
> LGTM, thanks Nathan.
> Reviewed-by: NIck Desaulniers <ndesaulniers@google.com>
Agreed. Seems obviously correct. Stefan, I'm only pushing this out as
testing for now so happy to rebase if you have comments.
Thanks,
Jonathan
>
> > ---
> > drivers/iio/adc/stmpe-adc.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/iio/adc/stmpe-adc.c b/drivers/iio/adc/stmpe-adc.c
> > index 37f4b74a5d32..7921f827c6ec 100644
> > --- a/drivers/iio/adc/stmpe-adc.c
> > +++ b/drivers/iio/adc/stmpe-adc.c
> > @@ -184,9 +184,6 @@ static irqreturn_t stmpe_adc_isr(int irq, void *dev_id)
> > struct stmpe_adc *info = (struct stmpe_adc *)dev_id;
> > u16 data;
> >
> > - if (info->channel > STMPE_TEMP_CHANNEL)
> > - return IRQ_NONE;
> > -
> > if (info->channel <= STMPE_ADC_LAST_NR) {
> > int int_sta;
> >
> > @@ -205,6 +202,8 @@ static irqreturn_t stmpe_adc_isr(int irq, void *dev_id)
> > /* Read value */
> > stmpe_block_read(info->stmpe, STMPE_REG_TEMP_DATA, 2,
> > (u8 *) &data);
> > + } else {
> > + return IRQ_NONE;
> > }
> >
> > info->value = (u32) be16_to_cpu(data);
> > --
> > 2.21.0
> >
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-03-10 9:46 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-07 17:16 [PATCH] iio: adc: stmpe-adc: Shuffle an if statement around in stmpe_adc_isr Nathan Chancellor
2019-03-07 18:31 ` Nick Desaulniers
2019-03-10 9:45 ` 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).