[PATCH 1/2] ASOC: rochchip: Simplify the condition logic in rockchip_sai_xfer_stop

[Nicolas Frattaroli]

On Wednesday, 4 June 2025 05:13:29 Central European Summer Time Pei Xiao wrote:
> cocci warning:
> ./sound/soc/rockchip/rockchip_sai.c:387:8-10:
> 	WARNING: possible condition with no effect (if == else)
> 
> Simplify the condition logic in rockchip_sai_xfer_stop() by removing the
> redundant SNDRV_PCM_STREAM_PLAYBACK branch. The modified logic now:
> 1. For stream < 0: handles both playback and capture
> 2. For all other cases (both PLAYBACK and CAPTURE):
>    sets playback = true and capture = false
> 
> Signed-off-by: Pei Xiao <xiaopei01 at kylinos.cn>
> ---
>  sound/soc/rockchip/rockchip_sai.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/sound/soc/rockchip/rockchip_sai.c b/sound/soc/rockchip/rockchip_sai.c
> index 602f1ddfad00..79b04770da1c 100644
> --- a/sound/soc/rockchip/rockchip_sai.c
> +++ b/sound/soc/rockchip/rockchip_sai.c
> @@ -384,9 +384,6 @@ static void rockchip_sai_xfer_stop(struct rk_sai_dev *sai, int stream)
>  	if (stream < 0) {
>  		playback = true;
>  		capture = true;
> -	} else if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
> -		playback = true;
> -		capture = false;
>  	} else {
>  		playback = true;
>  		capture = false;
> 

You can probably get rid of the locals playback and capture altogether:

    static void rockchip_sai_xfer_stop(struct rk_sai_dev *sai, int stream)
    {
        unsigned int msk, val, clr;

        msk = SAI_XFER_TXS_MASK;
        val = SAI_XFER_TXS_DIS;
        clr = SAI_CLR_TXC;
        
        if (stream < 0) {
            msk |= SAI_XFER_RXS_MASK;
            val |= SAI_XFER_RXS_DIS;
            clr |= SAI_CLR_RXC;
        }

        regmap_update_bits(sai->regmap, SAI_XFER, msk, val);
        rockchip_sai_poll_stream_idle(sai, true, stream < 0);

        rockchip_sai_clear(sai, clr);
    }

but this in general makes me suspicious of the intent of the code in
the first place. Playback always being true and capture only being
true if playback is also true seems odd. Checking the callsites of
this function confirms my suspicions that while this fixes the cocci
warning, it just means the code is now intentionally broken.

This here may be closer to the original intent:

    static void rockchip_sai_xfer_stop(struct rk_sai_dev *sai, int stream)
    {
            unsigned int msk = 0, val = 0, clr = 0;
            bool capture = stream == SNDRV_PCM_STREAM_CAPTURE || stream < 0;
            /* could be <= 0 but we don't want to depend on enum values */
            bool playback = stream == SNDRV_PCM_STREAM_PLAYBACK || stream < 0;

            if (playback) {
                    msk |= SAI_XFER_TXS_MASK;
                    val |= SAI_XFER_TXS_DIS;
                    clr |= SAI_CLR_TXC;
            }

            if (capture) {
                    msk |= SAI_XFER_RXS_MASK;
                    val |= SAI_XFER_RXS_DIS;
                    clr |= SAI_CLR_RXC;
            }

            regmap_update_bits(sai->regmap, SAI_XFER, msk, val);
            rockchip_sai_poll_stream_idle(sai, playback, capture);

            rockchip_sai_clear(sai, clr);
    }


Please let me know whether this looks right to you.

Kind regards,
Nicolas Frattaroli