[PATCH] ASoC: ux500: avoid -Wparentheses warning

[PATCH] ASoC: ux500: avoid -Wparentheses warning

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

gcc warns about a slightly confusing expression in two lines
of this driver:

In file included from sound/soc/ux500/ux500_msp_i2s.c:20:
sound/soc/ux500/ux500_msp_i2s.c: In function 'configure_protocol':
sound/soc/ux500/ux500_msp_i2s.h:151:38: error: suggest parentheses around arithmetic in operand of '^' [-Werror=parentheses]
  151 | #define MSP_TX_CLKPOL_BIT(n)     ((n & TCKPOL_MASK) << TCKPOL_SHIFT)
sound/soc/ux500/ux500_msp_i2s.c:204:21: note: in expansion of macro 'MSP_TX_CLKPOL_BIT'
  204 |         temp_reg |= MSP_TX_CLKPOL_BIT(!protdesc->tx_clk_pol ^
      |                     ^~~~~~~~~~~~~~~~~

config->bclk_inverted is a boolean variable, while protdesc->tx_clk_pol
is a 32-bit unsigned integer that can only be zero or one in order to
be passed into MSP_RX_CLKPOL_BIT().

Move the negation out of the inner expression to make this easier
to understand by the compiler.

Fixes: 9ccbacf5a012 ("ASoC: ux500: Validate MSP DAI configuration")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
I still find the new version equally confusing, if anyone has a
better idea to make this more readable, let's do that instead.
---
 sound/soc/ux500/ux500_msp_i2s.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/ux500/ux500_msp_i2s.c b/sound/soc/ux500/ux500_msp_i2s.c
index 683b485fb570..588d381230b6 100644
--- a/sound/soc/ux500/ux500_msp_i2s.c
+++ b/sound/soc/ux500/ux500_msp_i2s.c
@@ -201,12 +201,12 @@ static int configure_protocol(struct ux500_msp *msp,
 
 	/* The code below should not be separated. */
 	temp_reg = readl(msp->registers + MSP_GCR) & ~TX_CLK_POL_RISING;
-	temp_reg |= MSP_TX_CLKPOL_BIT(!protdesc->tx_clk_pol ^
-					  config->bclk_inverted);
+	temp_reg |= MSP_TX_CLKPOL_BIT(!(protdesc->tx_clk_pol ^
+					  config->bclk_inverted));
 	writel(temp_reg, msp->registers + MSP_GCR);
 	temp_reg = readl(msp->registers + MSP_GCR) & ~RX_CLK_POL_RISING;
-	temp_reg |= MSP_RX_CLKPOL_BIT(protdesc->rx_clk_pol ^
-					  config->bclk_inverted);
+	temp_reg |= MSP_RX_CLKPOL_BIT(!!(protdesc->rx_clk_pol ^
+					  config->bclk_inverted));
 	writel(temp_reg, msp->registers + MSP_GCR);
 
 	return 0;
-- 
2.53.0

Re: [PATCH] ASoC: ux500: avoid -Wparentheses warning

[Linus Walleij]

On Tue, Sep 15, 2026 at 10:20 PM Arnd Bergmann <arnd@kernel.org> wrote:

> From: Arnd Bergmann <arnd@arndb.de>
>
> gcc warns about a slightly confusing expression in two lines
> of this driver:
>
> In file included from sound/soc/ux500/ux500_msp_i2s.c:20:
> sound/soc/ux500/ux500_msp_i2s.c: In function 'configure_protocol':
> sound/soc/ux500/ux500_msp_i2s.h:151:38: error: suggest parentheses around arithmetic in operand of '^' [-Werror=parentheses]
>   151 | #define MSP_TX_CLKPOL_BIT(n)     ((n & TCKPOL_MASK) << TCKPOL_SHIFT)
> sound/soc/ux500/ux500_msp_i2s.c:204:21: note: in expansion of macro 'MSP_TX_CLKPOL_BIT'
>   204 |         temp_reg |= MSP_TX_CLKPOL_BIT(!protdesc->tx_clk_pol ^
>       |                     ^~~~~~~~~~~~~~~~~
>
> config->bclk_inverted is a boolean variable, while protdesc->tx_clk_pol
> is a 32-bit unsigned integer that can only be zero or one in order to
> be passed into MSP_RX_CLKPOL_BIT().
>
> Move the negation out of the inner expression to make this easier
> to understand by the compiler.
>
> Fixes: 9ccbacf5a012 ("ASoC: ux500: Validate MSP DAI configuration")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

OK fair eniugh,
Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

Re: [PATCH] ASoC: ux500: avoid -Wparentheses warning

[David Laight]

On Tue, 15 Sep 2026 22:20:31 +0200
Arnd Bergmann <arnd@kernel.org> wrote:

> From: Arnd Bergmann <arnd@arndb.de>
> 
> gcc warns about a slightly confusing expression in two lines
> of this driver:
> 
> In file included from sound/soc/ux500/ux500_msp_i2s.c:20:
> sound/soc/ux500/ux500_msp_i2s.c: In function 'configure_protocol':
> sound/soc/ux500/ux500_msp_i2s.h:151:38: error: suggest parentheses around arithmetic in operand of '^' [-Werror=parentheses]
>   151 | #define MSP_TX_CLKPOL_BIT(n)     ((n & TCKPOL_MASK) << TCKPOL_SHIFT)
> sound/soc/ux500/ux500_msp_i2s.c:204:21: note: in expansion of macro 'MSP_TX_CLKPOL_BIT'
>   204 |         temp_reg |= MSP_TX_CLKPOL_BIT(!protdesc->tx_clk_pol ^
>       |                     ^~~~~~~~~~~~~~~~~
> 
> config->bclk_inverted is a boolean variable, while protdesc->tx_clk_pol
> is a 32-bit unsigned integer that can only be zero or one in order to
> be passed into MSP_RX_CLKPOL_BIT().
> 
> Move the negation out of the inner expression to make this easier
> to understand by the compiler.
> 
> Fixes: 9ccbacf5a012 ("ASoC: ux500: Validate MSP DAI configuration")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> I still find the new version equally confusing, if anyone has a
> better idea to make this more readable, let's do that instead.

How about:
	temp_reg = readl(msp->registers + MSP_GCR);
	if (protdesc->tx_clk_pol ^ config->bclk_inverted)
		temp_req &= ~TX_CLK_POL_RISING;
	else
		temp_req |= TX_CLK_POL_RISING;
	if (protdesc->rx_clk_pol ^ config->bclk_inverted)
		temp_req |= RX_CLK_POL_RISING;
	else
		temp_req &= ~RX_CLK_POL_RISING;
	writel(temp_reg, msp->registers + MSP_GCR);
Assuming that [rt]x_clk_pol is only ever 0 or 1,
if invalid if will default to 'normal polarity; clocks.
Completely open-coding it removes the negation.

You could try replacing the ^ with !=

David

> ---
>  sound/soc/ux500/ux500_msp_i2s.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/ux500/ux500_msp_i2s.c b/sound/soc/ux500/ux500_msp_i2s.c
> index 683b485fb570..588d381230b6 100644
> --- a/sound/soc/ux500/ux500_msp_i2s.c
> +++ b/sound/soc/ux500/ux500_msp_i2s.c
> @@ -201,12 +201,12 @@ static int configure_protocol(struct ux500_msp *msp,
>  
>  	/* The code below should not be separated. */
>  	temp_reg = readl(msp->registers + MSP_GCR) & ~TX_CLK_POL_RISING;
> -	temp_reg |= MSP_TX_CLKPOL_BIT(!protdesc->tx_clk_pol ^
> -					  config->bclk_inverted);
> +	temp_reg |= MSP_TX_CLKPOL_BIT(!(protdesc->tx_clk_pol ^
> +					  config->bclk_inverted));
>  	writel(temp_reg, msp->registers + MSP_GCR);
>  	temp_reg = readl(msp->registers + MSP_GCR) & ~RX_CLK_POL_RISING;
> -	temp_reg |= MSP_RX_CLKPOL_BIT(protdesc->rx_clk_pol ^
> -					  config->bclk_inverted);
> +	temp_reg |= MSP_RX_CLKPOL_BIT(!!(protdesc->rx_clk_pol ^
> +					  config->bclk_inverted));
>  	writel(temp_reg, msp->registers + MSP_GCR);
>  
>  	return 0;

Re: [PATCH] ASoC: ux500: avoid -Wparentheses warning

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 272 bytes --]

On Tue, Sep 15, 2026 at 10:20:31PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> gcc warns about a slightly confusing expression in two lines
> of this driver:

Sasha already posted a fix for this which I'd been hoping Linus would
review.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: ux500: avoid -Wparentheses warning

[Linus Walleij]

On Wed, Sep 16, 2026 at 2:36 PM Mark Brown <broonie@kernel.org> wrote:
> On Tue, Sep 15, 2026 at 10:20:31PM +0200, Arnd Bergmann wrote:

> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > gcc warns about a slightly confusing expression in two lines
> > of this driver:
>
> Sasha already posted a fix for this which I'd been hoping Linus would
> review.

Hm I must have missed it, I'll go and dig it out.

Yours,
Linus Walleij