-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-
owner@vger.kernel.org] On Behalf Of Ben Hutchings
Sent: Friday, August 02, 2013 2:09 AM
To: Kirsher, Jeffrey T
Cc: davem@davemloft.net; Tantilov, Emil S;
netdev@vger.kernel.org; gospo@redhat.com;
sassmann@redhat.com
Subject: Re: [net-next 07/15] ixgbe: fix SFF data dumps of
SFP+ modules

On Mon, 2013-07-29 at 05:52 -0700, Jeff Kirsher wrote:

quoted

From: Emil Tantilov <redacted>

This patch fixes several issues with the previous

implementation of the

quoted

SFF data dump of SFP+ modules:

- removed the __IXGBE_READ_I2C flag - I2C access locking

is handled in the

quoted

  HW specific routines

- fixed the read loop to read data from ee->offset to ee-
len

- the reads fail if __IXGBE_IN_SFP_INIT is set in the

process - this is

quoted

  needed because on some HW I2C operations can take long

time and disrupt

quoted

  the SFP and link detection process

Signed-off-by: Emil Tantilov <redacted>
Reported-by: Ben Hutchings <redacted>
Tested-by: Phil Schmitt <redacted>
Signed-off-by: Jeff Kirsher <redacted>

[...]

quoted

@@ -2969,48 +2955,25 @@ static int

ixgbe_get_module_eeprom(struct net_device *dev,

quoted

 	int i = 0;
 	int ret_val = 0;

-	/* ixgbe_get_module_info is called before this function

in all

quoted

-	 * cases, so we do not need any checks we already do

above,

quoted

-	 * and can trust ee->len to be a known value.
-	 */
+	if (ee->len == 0)
+		return -EINVAL;

-	while (test_bit(__IXGBE_IN_SFP_INIT, &adapter->state))
-		msleep(100);
-	set_bit(__IXGBE_READ_I2C, &adapter->state);
+	for (i = ee->offset; i < ee->len; i++) {

                              i < ee->offset + ee->len

quoted

+		/* I2C reads can take long time */
+		if (test_bit(__IXGBE_IN_SFP_INIT, &adapter-
state))
+			return -EBUSY;

-	/* Read the first block, SFF-8079 */
-	for (i = 0; i < ETH_MODULE_SFF_8079_LEN; i++) {
-		status = hw->phy.ops.read_i2c_eeprom(hw, i,

&databyte);

quoted

-		if (status != 0) {
-			/* Error occured while reading module */
+		if (i < ETH_MODULE_SFF_8079_LEN)
+			status  = hw->phy.ops.read_i2c_eeprom(hw, i,

&databyte);

quoted

+		else
+			status = hw->phy.ops.read_i2c_sff8472(hw, i,

&databyte);
[...]
                                                    i -
ETH_MODULE_SFF_8079_LEN ?

But this works anyway because the address is truncated to
u8.

Actually as is the loop will only work when offset is 0, so it looks like it should be:

for (i = ee->offset; i < ee->offset + ee->len; i++) {


Unless you had something else in mind.

I will submit a fix for this.

Thanks,
Emil

Ben.