[PATCH] OPP: Fix -Wunsequenced in _of_add_opp_table_v1()

[PATCH] OPP: Fix -Wunsequenced in _of_add_opp_table_v1()

[Nathan Chancellor]

Clang warns (or errors with CONFIG_WERROR=y):

  drivers/opp/of.c:1081:28: error: multiple unsequenced modifications to 'val' [-Werror,-Wunsequenced]
   1081 |                         .freq = be32_to_cpup(val++) * 1000,
        |                                                 ^
   1082 |                         .u_volt = be32_to_cpup(val++),
        |                                                   ~~
  1 error generated.

There is no sequence point in a designated initializer. Move back to
separate variables for the creation of the values, so that there are
sequence points between each evaluation and increment of val.

Closes: https://github.com/ClangBuiltLinux/linux/issues/1940
Fixes: 75bbc92c09d8 ("OPP: Add dev_pm_opp_add_dynamic() to allow more flexibility")
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 drivers/opp/of.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index ade6d42cae46..ae5c405bbf9a 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -1077,9 +1077,11 @@ static int _of_add_opp_table_v1(struct device *dev, struct opp_table *opp_table)
 
 	val = prop->value;
 	while (nr) {
+		unsigned long freq = be32_to_cpup(val++) * 1000;
+		unsigned long volt = be32_to_cpup(val++);
 		struct dev_pm_opp_data data = {
-			.freq = be32_to_cpup(val++) * 1000,
-			.u_volt = be32_to_cpup(val++),
+			.freq = freq,
+			.u_volt = volt,
 		};
 
 		ret = _opp_add_v1(opp_table, dev, &data, false);

---
base-commit: 3c4746e28af13fdd134d3c4312169fda0a8abef0
change-id: 20231005-opp-of-wunsequenced-a594ea4c053d

Best regards,
-- 
Nathan Chancellor <nathan@kernel.org>

Re: [PATCH] OPP: Fix -Wunsequenced in _of_add_opp_table_v1()

[Nick Desaulniers]

On Thu, Oct 5, 2023 at 10:25 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> Clang warns (or errors with CONFIG_WERROR=y):
>
>   drivers/opp/of.c:1081:28: error: multiple unsequenced modifications to 'val' [-Werror,-Wunsequenced]
>    1081 |                         .freq = be32_to_cpup(val++) * 1000,
>         |                                                 ^
>    1082 |                         .u_volt = be32_to_cpup(val++),
>         |                                                   ~~
>   1 error generated.
>
> There is no sequence point in a designated initializer. Move back to
> separate variables for the creation of the values, so that there are
> sequence points between each evaluation and increment of val.
>
> Closes: https://github.com/ClangBuiltLinux/linux/issues/1940
> Fixes: 75bbc92c09d8 ("OPP: Add dev_pm_opp_add_dynamic() to allow more flexibility")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>  drivers/opp/of.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/opp/of.c b/drivers/opp/of.c
> index ade6d42cae46..ae5c405bbf9a 100644
> --- a/drivers/opp/of.c
> +++ b/drivers/opp/of.c
> @@ -1077,9 +1077,11 @@ static int _of_add_opp_table_v1(struct device *dev, struct opp_table *opp_table)
>
>         val = prop->value;
>         while (nr) {
> +               unsigned long freq = be32_to_cpup(val++) * 1000;
> +               unsigned long volt = be32_to_cpup(val++);
>                 struct dev_pm_opp_data data = {
> -                       .freq = be32_to_cpup(val++) * 1000,
> -                       .u_volt = be32_to_cpup(val++),
> +                       .freq = freq,
> +                       .u_volt = volt,
>                 };
>
>                 ret = _opp_add_v1(opp_table, dev, &data, false);
>
> ---
> base-commit: 3c4746e28af13fdd134d3c4312169fda0a8abef0
> change-id: 20231005-opp-of-wunsequenced-a594ea4c053d
>
> Best regards,
> --
> Nathan Chancellor <nathan@kernel.org>
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] OPP: Fix -Wunsequenced in _of_add_opp_table_v1()

[Viresh Kumar]

On 05-10-23, 11:31, Nick Desaulniers wrote:
> On Thu, Oct 5, 2023 at 10:25 AM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > Clang warns (or errors with CONFIG_WERROR=y):
> >
> >   drivers/opp/of.c:1081:28: error: multiple unsequenced modifications to 'val' [-Werror,-Wunsequenced]
> >    1081 |                         .freq = be32_to_cpup(val++) * 1000,
> >         |                                                 ^
> >    1082 |                         .u_volt = be32_to_cpup(val++),
> >         |                                                   ~~
> >   1 error generated.
> >
> > There is no sequence point in a designated initializer. Move back to
> > separate variables for the creation of the values, so that there are
> > sequence points between each evaluation and increment of val.
> >
> > Closes: https://github.com/ClangBuiltLinux/linux/issues/1940
> > Fixes: 75bbc92c09d8 ("OPP: Add dev_pm_opp_add_dynamic() to allow more flexibility")
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> 
> Thanks for the patch!
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Applied. Thanks.

-- 
viresh