Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1589313 > unrolled thread

[PATCH] clk: meson: fix SET_PARM macro

Started byJerome Brunet <jbrunet@baylibre.com>
First post2017-02-28 10:40 +0100
Last post2017-03-01 21:20 +0100
Articles 5 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] clk: meson: fix SET_PARM macro Jerome Brunet <jbrunet@baylibre.com> - 2017-02-28 10:40 +0100
    Re: [PATCH] clk: meson: fix SET_PARM macro Kevin Hilman <khilman@baylibre.com> - 2017-02-28 19:20 +0100
    Re: [PATCH] clk: meson: fix SET_PARM macro Stephen Boyd <sboyd@codeaurora.org> - 2017-02-28 22:40 +0100
      Re: [PATCH] clk: meson: fix SET_PARM macro Jerome Brunet <jbrunet@baylibre.com> - 2017-02-28 22:50 +0100
        Re: [PATCH] clk: meson: fix SET_PARM macro Stephen Boyd <sboyd@codeaurora.org> - 2017-03-01 21:20 +0100

#1589313 — [PATCH] clk: meson: fix SET_PARM macro

FromJerome Brunet <jbrunet@baylibre.com>
Date2017-02-28 10:40 +0100
Subject[PATCH] clk: meson: fix SET_PARM macro
Message-ID<tfMqe-2wl-15@gated-at.bofh.it>
parameter val is not enclosed in parenthesis which is buggy when given an
expression instead of a simple value

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/meson/clkc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/meson/clkc.h b/drivers/clk/meson/clkc.h
index 9bb70e7a7d6a..c6be77dd8694 100644
--- a/drivers/clk/meson/clkc.h
+++ b/drivers/clk/meson/clkc.h
@@ -25,7 +25,7 @@
 #define PARM_GET(width, shift, reg)					\
 	(((reg) & SETPMASK(width, shift)) >> (shift))
 #define PARM_SET(width, shift, reg, val)				\
-	(((reg) & CLRPMASK(width, shift)) | (val << (shift)))
+	(((reg) & CLRPMASK(width, shift)) | ((val) << (shift)))
 
 #define MESON_PARM_APPLICABLE(p)		(!!((p)->width))
 
-- 
2.9.3

[toc] | [next] | [standalone]


#1589667

FromKevin Hilman <khilman@baylibre.com>
Date2017-02-28 19:20 +0100
Message-ID<tfUxr-897-5@gated-at.bofh.it>
In reply to#1589313
Jerome Brunet <jbrunet@baylibre.com> writes:

> parameter val is not enclosed in parenthesis which is buggy when given an
> expression instead of a simple value
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

Reviewed-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/clk/meson/clkc.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/meson/clkc.h b/drivers/clk/meson/clkc.h
> index 9bb70e7a7d6a..c6be77dd8694 100644
> --- a/drivers/clk/meson/clkc.h
> +++ b/drivers/clk/meson/clkc.h
> @@ -25,7 +25,7 @@
>  #define PARM_GET(width, shift, reg)					\
>  	(((reg) & SETPMASK(width, shift)) >> (shift))
>  #define PARM_SET(width, shift, reg, val)				\
> -	(((reg) & CLRPMASK(width, shift)) | (val << (shift)))
> +	(((reg) & CLRPMASK(width, shift)) | ((val) << (shift)))
>  
>  #define MESON_PARM_APPLICABLE(p)		(!!((p)->width))

[toc] | [prev] | [next] | [standalone]


#1589812

FromStephen Boyd <sboyd@codeaurora.org>
Date2017-02-28 22:40 +0100
Message-ID<tfXF0-1ys-9@gated-at.bofh.it>
In reply to#1589313
On 02/28, Jerome Brunet wrote:
> parameter val is not enclosed in parenthesis which is buggy when given an
> expression instead of a simple value
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

Fixes tag? Is there a place in the code that is using a complex
expression for val right now?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]


#1589828

FromJerome Brunet <jbrunet@baylibre.com>
Date2017-02-28 22:50 +0100
Message-ID<tfXOG-1BX-17@gated-at.bofh.it>
In reply to#1589812
On Tue, 2017-02-28 at 13:26 -0800, Stephen Boyd wrote:
> On 02/28, Jerome Brunet wrote:
> > parameter val is not enclosed in parenthesis which is buggy when
> > given an
> > expression instead of a simple value
> > 
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> 
> Fixes tag? Is there a place in the code that is using a complex
> expression for val right now?
> 

Not if with what's already in, afaik. However "clk: meson: mpll: add rw
operation" I sent earlier today uses a ternary operator for val.
That's  how I found this issue. Instead of running the test, it would
always use the "else" clause.

I realize I should sent these patches in the same series.
Would you prefer me to do so ?

Jerome

[toc] | [prev] | [next] | [standalone]


#1590611

FromStephen Boyd <sboyd@codeaurora.org>
Date2017-03-01 21:20 +0100
Message-ID<tgiT7-89b-19@gated-at.bofh.it>
In reply to#1589828
On 02/28, Jerome Brunet wrote:
> On Tue, 2017-02-28 at 13:26 -0800, Stephen Boyd wrote:
> > On 02/28, Jerome Brunet wrote:
> > > parameter val is not enclosed in parenthesis which is buggy when
> > > given an
> > > expression instead of a simple value
> > > 
> > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > 
> > Fixes tag? Is there a place in the code that is using a complex
> > expression for val right now?
> > 
> 
> Not if with what's already in, afaik. However "clk: meson: mpll: add rw
> operation" I sent earlier today uses a ternary operator for val.
> That's  how I found this issue. Instead of running the test, it would
> always use the "else" clause.
> 
> I realize I should sent these patches in the same series.
> Would you prefer me to do so ?

Sure, but wait a little while for any review comments on the
series first please. For now I'll drop this from my queue because
it isn't an urgent fix and wait for it to come back as part of
the larger series.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web