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

[David Laight <david.laight.linux@gmail.com>]

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;