All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iio: adc: stmpe-adc: Shuffle an if statement around in stmpe_adc_isr
@ 2019-03-07 17:16 ` Nathan Chancellor
  0 siblings, 0 replies; 6+ messages in thread
From: Nathan Chancellor @ 2019-03-07 17:16 UTC (permalink / raw)
  To: Jonathan Cameron, Lee Jones
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Maxime Coquelin, Alexandre Torgue, Stefan Agner,
	Max Krummenacher, Philippe Schenker, linux-iio, linux-stm32,
	linux-arm-kernel, linux-kernel, clang-built-linux,
	Nick Desaulniers, Nathan Chancellor

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] iio: adc: stmpe-adc: Shuffle an if statement around in stmpe_adc_isr
@ 2019-03-07 17:16 ` Nathan Chancellor
  0 siblings, 0 replies; 6+ 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] 6+ messages in thread

* Re: [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
  -1 siblings, 0 replies; 6+ messages in thread
From: Nick Desaulniers @ 2019-03-07 18:31 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Jonathan Cameron, Lee Jones, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Maxime Coquelin, Alexandre Torgue,
	Stefan Agner, Max Krummenacher, Philippe Schenker, linux-iio,
	linux-stm32, Linux ARM, LKML, clang-built-linux

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

^ permalink raw reply	[flat|nested] 6+ 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
  0 siblings, 0 replies; 6+ 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] 6+ 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
  -1 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2019-03-10  9:45 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Nathan Chancellor, Lee Jones, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Maxime Coquelin, Alexandre Torgue,
	Stefan Agner, Max Krummenacher, Philippe Schenker, linux-iio,
	linux-stm32, Linux ARM, LKML, clang-built-linux

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
> >  
> 
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] iio: adc: stmpe-adc: Shuffle an if statement around in stmpe_adc_isr
@ 2019-03-10  9:45     ` Jonathan Cameron
  0 siblings, 0 replies; 6+ 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] 6+ messages in thread

end of thread, other threads:[~2019-03-10  9:46 UTC | newest]

Thread overview: 6+ 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 17:16 ` Nathan Chancellor
2019-03-07 18:31 ` Nick Desaulniers
2019-03-07 18:31   ` Nick Desaulniers
2019-03-10  9:45   ` Jonathan Cameron
2019-03-10  9:45     ` Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.