Hi,
Static analysis on linux-next with Coverity has detected an issue in
rz_ssi_stop, sound/soc/sh/rz-ssi.c with the following commit:
commit 03e786bd43410fa93e5d2459f7a43e90ff0ae801
Author: Biju Das <[email protected]>
Date: Fri Aug 13 10:11:54 2021 +0100
ASoC: sh: Add RZ/G2L SSIF-2 driver
The analysis is as follows:
367
368 /* Wait for idle */
369 timeout = 100;
370 while (--timeout) {
Wrong operator used (CONSTANT_EXPRESSION_RESULT)
operator_confusion: rz_ssi_reg_readl(ssi, 4) | (33554432UL /* 1UL << 25
*/) is always 1/true regardless of the values of its operand. This
occurs as the logical operand of if.
371 if (rz_ssi_reg_readl(ssi, SSISR) | SSISR_IIRQ)
372 break;
373 udelay(1);
374 }
I suspect the & operator was intended to mask the specific bit. However
I'm not sure if the intent is to detect if the SSISR_IIRQ bit should be
zero or one at the point where the break should occur, hence I'm
reporting this issue.
Colin
Hi Colin,
Thanks for the report.
> Subject: re: ASoC: sh: Add RZ/G2L SSIF-2 driver
>
> Hi,
>
> Static analysis on linux-next with Coverity has detected an issue in
> rz_ssi_stop, sound/soc/sh/rz-ssi.c with the following commit:
>
> commit 03e786bd43410fa93e5d2459f7a43e90ff0ae801
> Author: Biju Das <[email protected]>
> Date: Fri Aug 13 10:11:54 2021 +0100
>
> ASoC: sh: Add RZ/G2L SSIF-2 driver
>
>
> The analysis is as follows:
>
> 367
> 368 /* Wait for idle */
> 369 timeout = 100;
> 370 while (--timeout) {
>
> Wrong operator used (CONSTANT_EXPRESSION_RESULT)
>
> operator_confusion: rz_ssi_reg_readl(ssi, 4) | (33554432UL /* 1UL << 25
> */) is always 1/true regardless of the values of its operand. This occurs
> as the logical operand of if.
>
> 371 if (rz_ssi_reg_readl(ssi, SSISR) | SSISR_IIRQ)
> 372 break;
> 373 udelay(1);
> 374 }
>
> I suspect the & operator was intended to mask the specific bit.
However
> I'm not sure if the intent is to detect if the SSISR_IIRQ bit should be
> zero or one at the point where the break should occur, hence I'm reporting
> this issue.
You are correct, it is supposed to be &.
The idle condition is set by hw on the BIT(25) of this register.
So it should be &.
I will add your Reported-by tag, when I send the patch.
Thanks,
Biju