| Message ID | 20170228093016.5624-1-jbrunet@baylibre.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
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))
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?
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
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.
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))
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(-)