* [PATCH] clk: meson: fix SET_PARM macro
@ 2017-02-28 9:30 ` Jerome Brunet
0 siblings, 0 replies; 10+ messages in thread
From: Jerome Brunet @ 2017-02-28 9:30 UTC (permalink / raw)
To: linus-amlogic
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] clk: meson: fix SET_PARM macro
@ 2017-02-28 9:30 ` Jerome Brunet
0 siblings, 0 replies; 10+ messages in thread
From: Jerome Brunet @ 2017-02-28 9:30 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Kevin Hilman, Carlo Caione
Cc: Jerome Brunet, linux-clk, linux-amlogic, linux-kernel
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] clk: meson: fix SET_PARM macro
2017-02-28 9:30 ` Jerome Brunet
@ 2017-02-28 18:04 ` Kevin Hilman
-1 siblings, 0 replies; 10+ messages in thread
From: Kevin Hilman @ 2017-02-28 18:04 UTC (permalink / raw)
To: linus-amlogic
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))
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] clk: meson: fix SET_PARM macro
@ 2017-02-28 18:04 ` Kevin Hilman
0 siblings, 0 replies; 10+ messages in thread
From: Kevin Hilman @ 2017-02-28 18:04 UTC (permalink / raw)
To: Jerome Brunet
Cc: Michael Turquette, Stephen Boyd, Carlo Caione, linux-clk,
linux-amlogic, linux-kernel
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))
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] clk: meson: fix SET_PARM macro
2017-02-28 9:30 ` Jerome Brunet
@ 2017-02-28 21:26 ` Stephen Boyd
-1 siblings, 0 replies; 10+ messages in thread
From: Stephen Boyd @ 2017-02-28 21:26 UTC (permalink / raw)
To: linus-amlogic
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] clk: meson: fix SET_PARM macro
@ 2017-02-28 21:26 ` Stephen Boyd
0 siblings, 0 replies; 10+ messages in thread
From: Stephen Boyd @ 2017-02-28 21:26 UTC (permalink / raw)
To: Jerome Brunet
Cc: Michael Turquette, Kevin Hilman, Carlo Caione, linux-clk,
linux-amlogic, linux-kernel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] clk: meson: fix SET_PARM macro
2017-02-28 21:26 ` Stephen Boyd
@ 2017-02-28 21:41 ` Jerome Brunet
-1 siblings, 0 replies; 10+ messages in thread
From: Jerome Brunet @ 2017-02-28 21:41 UTC (permalink / raw)
To: linus-amlogic
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] clk: meson: fix SET_PARM macro
@ 2017-02-28 21:41 ` Jerome Brunet
0 siblings, 0 replies; 10+ messages in thread
From: Jerome Brunet @ 2017-02-28 21:41 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, Kevin Hilman, Carlo Caione, linux-clk,
linux-amlogic, linux-kernel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] clk: meson: fix SET_PARM macro
2017-02-28 21:41 ` Jerome Brunet
@ 2017-03-01 19:03 ` Stephen Boyd
-1 siblings, 0 replies; 10+ messages in thread
From: Stephen Boyd @ 2017-03-01 19:03 UTC (permalink / raw)
To: linus-amlogic
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] clk: meson: fix SET_PARM macro
@ 2017-03-01 19:03 ` Stephen Boyd
0 siblings, 0 replies; 10+ messages in thread
From: Stephen Boyd @ 2017-03-01 19:03 UTC (permalink / raw)
To: Jerome Brunet
Cc: Michael Turquette, Kevin Hilman, Carlo Caione, linux-clk,
linux-amlogic, linux-kernel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-03-01 19:03 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-28 9:30 [PATCH] clk: meson: fix SET_PARM macro Jerome Brunet
2017-02-28 9:30 ` Jerome Brunet
2017-02-28 18:04 ` Kevin Hilman
2017-02-28 18:04 ` Kevin Hilman
2017-02-28 21:26 ` Stephen Boyd
2017-02-28 21:26 ` Stephen Boyd
2017-02-28 21:41 ` Jerome Brunet
2017-02-28 21:41 ` Jerome Brunet
2017-03-01 19:03 ` Stephen Boyd
2017-03-01 19:03 ` Stephen Boyd
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.