Both the inner and outer loops in this code use the "i" iterator.
The inner loop should really use a different iterator.

It doesn't affect things in practice because the data comes from the
device tree.  The "protocol" and "windows" variables are going to be
zero.  That means we're always going to hit the "return &chans[channel];"
statement and we're not going to want to iterate through the outer
loop again.

Still it's worth fixing this for future use cases.

Fixes: 5a6338cce9f4 ("mailbox: arm_mhuv2: Add driver")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 drivers/mailbox/arm_mhuv2.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mailbox/arm_mhuv2.c b/drivers/mailbox/arm_mhuv2.c
index 0ec21dcdbde7..cff7c343ee08 100644
--- a/drivers/mailbox/arm_mhuv2.c
+++ b/drivers/mailbox/arm_mhuv2.c
@@ -500,7 +500,7 @@ static const struct mhuv2_protocol_ops mhuv2_data_transfer_ops = {
 static struct mbox_chan *get_irq_chan_comb(struct mhuv2 *mhu, u32 __iomem *reg)
 {
 	struct mbox_chan *chans = mhu->mbox.chans;
-	int channel = 0, i, offset = 0, windows, protocol, ch_wn;
+	int channel = 0, i, j, offset = 0, windows, protocol, ch_wn;
 	u32 stat;
 
 	for (i = 0; i < MHUV2_CMB_INT_ST_REG_CNT; i++) {
@@ -510,9 +510,9 @@ static struct mbox_chan *get_irq_chan_comb(struct mhuv2 *mhu, u32 __iomem *reg)
 
 		ch_wn = i * MHUV2_STAT_BITS + __builtin_ctz(stat);
 
-		for (i = 0; i < mhu->length; i += 2) {
-			protocol = mhu->protocols[i];
-			windows = mhu->protocols[i + 1];
+		for (j = 0; j < mhu->length; j += 2) {
+			protocol = mhu->protocols[j];
+			windows = mhu->protocols[j + 1];
 
 			if (ch_wn >= offset + windows) {
 				if (protocol == DOORBELL)
-- 
2.45.2

On 14-11-24, 12:00, Dan Carpenter wrote:
> Both the inner and outer loops in this code use the "i" iterator.
> The inner loop should really use a different iterator.
> 
> It doesn't affect things in practice because the data comes from the
> device tree.  The "protocol" and "windows" variables are going to be
> zero.  That means we're always going to hit the "return &chans[channel];"
> statement and we're not going to want to iterate through the outer
> loop again.
> 
> Still it's worth fixing this for future use cases.
> 
> Fixes: 5a6338cce9f4 ("mailbox: arm_mhuv2: Add driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>  drivers/mailbox/arm_mhuv2.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mailbox/arm_mhuv2.c b/drivers/mailbox/arm_mhuv2.c
> index 0ec21dcdbde7..cff7c343ee08 100644
> --- a/drivers/mailbox/arm_mhuv2.c
> +++ b/drivers/mailbox/arm_mhuv2.c
> @@ -500,7 +500,7 @@ static const struct mhuv2_protocol_ops mhuv2_data_transfer_ops = {
>  static struct mbox_chan *get_irq_chan_comb(struct mhuv2 *mhu, u32 __iomem *reg)
>  {
>  	struct mbox_chan *chans = mhu->mbox.chans;
> -	int channel = 0, i, offset = 0, windows, protocol, ch_wn;
> +	int channel = 0, i, j, offset = 0, windows, protocol, ch_wn;
>  	u32 stat;
>  
>  	for (i = 0; i < MHUV2_CMB_INT_ST_REG_CNT; i++) {
> @@ -510,9 +510,9 @@ static struct mbox_chan *get_irq_chan_comb(struct mhuv2 *mhu, u32 __iomem *reg)
>  
>  		ch_wn = i * MHUV2_STAT_BITS + __builtin_ctz(stat);
>  
> -		for (i = 0; i < mhu->length; i += 2) {
> -			protocol = mhu->protocols[i];
> -			windows = mhu->protocols[i + 1];
> +		for (j = 0; j < mhu->length; j += 2) {
> +			protocol = mhu->protocols[j];
> +			windows = mhu->protocols[j + 1];
>  
>  			if (ch_wn >= offset + windows) {
>  				if (protocol == DOORBELL)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh