2019-06-13 18:52:13

by Nathan Huckleberry

[permalink] [raw]
Subject: [PATCH] thermal: armada: Fix -Wshift-negative-value

Clang produces the following warning

drivers/thermal/armada_thermal.c:270:33: warning: shifting a negative
signed value is undefined [-Wshift-negative-value]
1 warning reg &= ~CONTROL1_TSEN_AVG_MASK <<
CONTROL1_TSEN_AVG_SHIFT; generated
.
~~~~~~~~~~~~~~~~~~~~~~~ ^

CONTROL1_TSEN_AVG_SHIFT is defined to be zero.
Since shifting by zero does nothing this variable can be removed.

Cc: [email protected]
Link: https://github.com/ClangBuiltLinux/linux/issues/532
Signed-off-by: Nathan Huckleberry <[email protected]>
---
drivers/thermal/armada_thermal.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 8c07a393dc2e..709a22f455e9 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -53,7 +53,6 @@
#define CONTROL0_TSEN_MODE_EXTERNAL 0x2
#define CONTROL0_TSEN_MODE_MASK 0x3

-#define CONTROL1_TSEN_AVG_SHIFT 0
#define CONTROL1_TSEN_AVG_MASK 0x7
#define CONTROL1_EXT_TSEN_SW_RESET BIT(7)
#define CONTROL1_EXT_TSEN_HW_RESETn BIT(8)
@@ -267,8 +266,8 @@ static void armada_cp110_init(struct platform_device *pdev,

/* Average the output value over 2^1 = 2 samples */
regmap_read(priv->syscon, data->syscon_control1_off, &reg);
- reg &= ~CONTROL1_TSEN_AVG_MASK << CONTROL1_TSEN_AVG_SHIFT;
- reg |= 1 << CONTROL1_TSEN_AVG_SHIFT;
+ reg &= ~CONTROL1_TSEN_AVG_MASK;
+ reg |= 1;
regmap_write(priv->syscon, data->syscon_control1_off, reg);
}

--
2.22.0.rc2.383.gf4fbbf30c2-goog


2019-06-14 10:13:27

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH] thermal: armada: Fix -Wshift-negative-value

On 13/06/2019 20:49, Nathan Huckleberry wrote:
> Clang produces the following warning
>
> drivers/thermal/armada_thermal.c:270:33: warning: shifting a negative
> signed value is undefined [-Wshift-negative-value]
> 1 warning reg &= ~CONTROL1_TSEN_AVG_MASK <<
> CONTROL1_TSEN_AVG_SHIFT; generated
> .
> ~~~~~~~~~~~~~~~~~~~~~~~ ^
>
> CONTROL1_TSEN_AVG_SHIFT is defined to be zero.
> Since shifting by zero does nothing this variable can be removed.
>
> Cc: [email protected]
> Link: https://github.com/ClangBuiltLinux/linux/issues/532
> Signed-off-by: Nathan Huckleberry <[email protected]>

Reviewed-by: Daniel Lezcano <[email protected]>

> ---
> drivers/thermal/armada_thermal.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index 8c07a393dc2e..709a22f455e9 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -53,7 +53,6 @@
> #define CONTROL0_TSEN_MODE_EXTERNAL 0x2
> #define CONTROL0_TSEN_MODE_MASK 0x3
>
> -#define CONTROL1_TSEN_AVG_SHIFT 0
> #define CONTROL1_TSEN_AVG_MASK 0x7
> #define CONTROL1_EXT_TSEN_SW_RESET BIT(7)
> #define CONTROL1_EXT_TSEN_HW_RESETn BIT(8)
> @@ -267,8 +266,8 @@ static void armada_cp110_init(struct platform_device *pdev,
>
> /* Average the output value over 2^1 = 2 samples */
> regmap_read(priv->syscon, data->syscon_control1_off, &reg);
> - reg &= ~CONTROL1_TSEN_AVG_MASK << CONTROL1_TSEN_AVG_SHIFT;
> - reg |= 1 << CONTROL1_TSEN_AVG_SHIFT;
> + reg &= ~CONTROL1_TSEN_AVG_MASK;
> + reg |= 1;
> regmap_write(priv->syscon, data->syscon_control1_off, reg);
> }
>
>


--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

2019-08-13 18:31:04

by Nathan Huckleberry

[permalink] [raw]
Subject: Re: [PATCH] thermal: armada: Fix -Wshift-negative-value

Following up to see if this patch is going to be accepted.

2019-08-14 22:35:46

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] thermal: armada: Fix -Wshift-negative-value

On Tue, Aug 13, 2019 at 10:28 AM 'Nathan Huckleberry' via Clang Built
Linux <[email protected]> wrote:
>
> Following up to see if this patch is going to be accepted.

Miquel is listed as the maintainer of this file in MAINTAINERS.
Miquel, can you please pick this up? Otherwise Zhang, Eduardo, and
Daniel are listed as maintainers for drivers/thermal/.
--
Thanks,
~Nick Desaulniers

2019-08-14 23:58:09

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH] thermal: armada: Fix -Wshift-negative-value

On 15/08/2019 00:12, Nick Desaulniers wrote:
> On Tue, Aug 13, 2019 at 10:28 AM 'Nathan Huckleberry' via Clang Built
> Linux <[email protected]> wrote:
>>
>> Following up to see if this patch is going to be accepted.
>
> Miquel is listed as the maintainer of this file in MAINTAINERS.
> Miquel, can you please pick this up? Otherwise Zhang, Eduardo, and
> Daniel are listed as maintainers for drivers/thermal/.

I'm listed as reviewer, it is up to Zhang or Eduardo to take the patches.


--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

2019-08-19 08:24:09

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH] thermal: armada: Fix -Wshift-negative-value

Hello,

Daniel Lezcano <[email protected]> wrote on Thu, 15 Aug 2019
01:06:21 +0200:

> On 15/08/2019 00:12, Nick Desaulniers wrote:
> > On Tue, Aug 13, 2019 at 10:28 AM 'Nathan Huckleberry' via Clang Built
> > Linux <[email protected]> wrote:
> >>
> >> Following up to see if this patch is going to be accepted.
> >
> > Miquel is listed as the maintainer of this file in MAINTAINERS.
> > Miquel, can you please pick this up? Otherwise Zhang, Eduardo, and
> > Daniel are listed as maintainers for drivers/thermal/.
>
> I'm listed as reviewer, it is up to Zhang or Eduardo to take the patches.
>
>

Sorry for the delay, I don't manage a tree for this driver, I'll let
Zhang or Eduardo take the patch with my

Acked-by: Miquel Raynal <[email protected]>


Thanks,
Miquèl

2019-08-28 08:55:31

by Zhang, Rui

[permalink] [raw]
Subject: Re: [PATCH] thermal: armada: Fix -Wshift-negative-value

On Mon, 2019-08-19 at 10:21 +0200, Miquel Raynal wrote:
> Hello,
>
> Daniel Lezcano <[email protected]> wrote on Thu, 15 Aug 2019
> 01:06:21 +0200:
>
> > On 15/08/2019 00:12, Nick Desaulniers wrote:
> > > On Tue, Aug 13, 2019 at 10:28 AM 'Nathan Huckleberry' via Clang
> > > Built
> > > Linux <[email protected]> wrote:
> > > >
> > > > Following up to see if this patch is going to be accepted.
> > >
> > > Miquel is listed as the maintainer of this file in MAINTAINERS.
> > > Miquel, can you please pick this up? Otherwise Zhang, Eduardo,
> > > and
> > > Daniel are listed as maintainers for drivers/thermal/.
> >
> > I'm listed as reviewer, it is up to Zhang or Eduardo to take the
> > patches.
> >
> >
>
> Sorry for the delay, I don't manage a tree for this driver, I'll let
> Zhang or Eduardo take the patch with my
>
> Acked-by: Miquel Raynal <[email protected]>
>

Patch applied.

thanks,
rui
>
> Thanks,
> Miquèl

2019-08-28 18:50:55

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] thermal: armada: Fix -Wshift-negative-value

On Wed, Aug 28, 2019 at 1:53 AM Zhang Rui <[email protected]> wrote:
>
> On Mon, 2019-08-19 at 10:21 +0200, Miquel Raynal wrote:
> > Hello,
> >
> > Daniel Lezcano <[email protected]> wrote on Thu, 15 Aug 2019
> > 01:06:21 +0200:
> >
> > > On 15/08/2019 00:12, Nick Desaulniers wrote:
> > > > On Tue, Aug 13, 2019 at 10:28 AM 'Nathan Huckleberry' via Clang
> > > > Built
> > > > Linux <[email protected]> wrote:
> > > > >
> > > > > Following up to see if this patch is going to be accepted.
> > > >
> > > > Miquel is listed as the maintainer of this file in MAINTAINERS.
> > > > Miquel, can you please pick this up? Otherwise Zhang, Eduardo,
> > > > and
> > > > Daniel are listed as maintainers for drivers/thermal/.
> > >
> > > I'm listed as reviewer, it is up to Zhang or Eduardo to take the
> > > patches.
> > >
> > >
> >
> > Sorry for the delay, I don't manage a tree for this driver, I'll let
> > Zhang or Eduardo take the patch with my
> >
> > Acked-by: Miquel Raynal <[email protected]>
> >
>
> Patch applied.
>
> thanks,
> rui

Thanks Rui, did you push the branch? I guess I would have expected it
in https://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux.git/log/?h=next?
I'm trying to track where this lands in
https://github.com/ClangBuiltLinux/linux/issues/532.

--
Thanks,
~Nick Desaulniers

2019-08-29 01:43:38

by Zhang, Rui

[permalink] [raw]
Subject: Re: [PATCH] thermal: armada: Fix -Wshift-negative-value

On Wed, 2019-08-28 at 11:49 -0700, Nick Desaulniers wrote:
> On Wed, Aug 28, 2019 at 1:53 AM Zhang Rui <[email protected]>
> wrote:
> >
> > On Mon, 2019-08-19 at 10:21 +0200, Miquel Raynal wrote:
> > > Hello,
> > >
> > > Daniel Lezcano <[email protected]> wrote on Thu, 15 Aug
> > > 2019
> > > 01:06:21 +0200:
> > >
> > > > On 15/08/2019 00:12, Nick Desaulniers wrote:
> > > > > On Tue, Aug 13, 2019 at 10:28 AM 'Nathan Huckleberry' via
> > > > > Clang
> > > > > Built
> > > > > Linux <[email protected]> wrote:
> > > > > >
> > > > > > Following up to see if this patch is going to be accepted.
> > > > >
> > > > > Miquel is listed as the maintainer of this file in
> > > > > MAINTAINERS.
> > > > > Miquel, can you please pick this up? Otherwise Zhang,
> > > > > Eduardo,
> > > > > and
> > > > > Daniel are listed as maintainers for drivers/thermal/.
> > > >
> > > > I'm listed as reviewer, it is up to Zhang or Eduardo to take
> > > > the
> > > > patches.
> > > >
> > > >
> > >
> > > Sorry for the delay, I don't manage a tree for this driver, I'll
> > > let
> > > Zhang or Eduardo take the patch with my
> > >
> > > Acked-by: Miquel Raynal <[email protected]>
> > >
> >
> > Patch applied.
> >
> > thanks,
> > rui
>
> Thanks Rui, did you push the branch? I guess I would have expected
> it
> in
> https://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux.git/log/?h=next
> ?
> I'm trying to track where this lands in
> https://github.com/ClangBuiltLinux/linux/issues/532.

Not yet. I will push it to kernel.org after I finish my internal build
test.

thanks,
rui
>