[Intel-wired-lan] [next-queue 1/8] e1000: clean up the useless 'continue' statement

[=?unknown-8bit?q?J=CE=B5an?= Sacren <sakiwit@gmail.com>]

From: "Rustad, Mark D" <mark.d.rustad@intel.com>
Date: Tue, 15 Sep 2015 18:24:39 +0000
>
> > On Sep 14, 2015, at 10:35 PM, J?an Sacren <sakiwit@gmail.com> wrote:

[...]

> > The goto statement might be executed if and only if at the last loop out
> > of a total of 32. I think we need to rewrite the checking logic here:
> > 
> > 			...
> > 			if (tmp != 0 && tmp != 0xFF)
> > 				break;
> > 
> > 			if (i == 31)
> > 				goto err_eeprom;
> > 			...
> > 
> > What do you think?
> 
> This is ok too, but this makes it look to me like the if and goto
> should really be outside the loop, so after the loop one would have:
> 
> 		if (i == 32)
> 			goto err_eeprom;

I think moving the above check out of the loop is a good idea, yet it
still retains the same issue as doing the unnecessary checking.

How about the following patch:

diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 74dc15055971..4c3ca7c9ce49 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -1199,15 +1199,14 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		for (i = 0; i < 32; i++) {
 			hw->phy_addr = i;
 			e1000_read_phy_reg(hw, PHY_ID2, &tmp);
-			if (tmp == 0 || tmp == 0xFF) {
-				if (i == 31)
-					goto err_eeprom;
-				continue;
-			} else
-				break;
+
+			if (tmp != 0 && tmp != 0xFF)
+				goto no_err;
 		}
-	}
 
+		goto err_eeprom;
+	}
+no_err:
 	/* reset the hardware with the new settings */
 	e1000_reset(adapter);