Skip to contentSkip to conversation

e1000: fix sw fw sync on igb i210/i211

PATCH In review Marcel Ziswiler 3 participants 5 messages Oct 21, 2014 – Oct 27, 2014 1 message not in the archive Network drivers

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): bd71d1eb1cf3ba594ba7492f4f311fb1bf58dfd8.1413893734.git.marcel@ziswiler.com

#1 lore raw

PATCH 1/1 e1000: fix sw fw sync on igb i210/i211

I finally had a look at the datasheet and spotted an additional register address difference between regular E1000 and i210/i211 chips. This patch fixes this and now successfully works on programmed i210/i211 as well as unprogrammed i211.

---
Please note that unprogrammed i210 seem to behave slightly different and do not assert the CFG_DONE bit in the EEMNGCTL register upon PHY reset. However if this error condition is ignored then successful data transfer is possible. Due to the rather hacky nature thereof this is not addressed by my patch.

BTW: What about my previous patch this one kind of bases on?

[PATCH] e1000: add i211 and unprogrammed i210/i211 support

 drivers/net/e1000.c | 6 ++++--
 drivers/net/e1000.h | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)
drivers/net/e1000.c +4 -2
index b092867..798c8aa 100644
@@ -1112,7 +1112,10 @@ e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask)
 		if (e1000_get_hw_eeprom_semaphore(hw))
 			return -E1000_ERR_SWFW_SYNC;
 
-		swfw_sync = E1000_READ_REG(hw, SW_FW_SYNC);
+		if (hw->mac_type == e1000_igb)
+			swfw_sync = E1000_READ_REG(hw, I210_SW_FW_SYNC);
+		else
+			swfw_sync = E1000_READ_REG(hw, SW_FW_SYNC);
 		if (!(swfw_sync & (fwmask | swmask)))
 			break;
 
@@ -4429,7 +4432,6 @@ e1000_phy_hw_reset(struct e1000_hw *hw)
 
 		if (hw->mac_type >= e1000_82571)
 			mdelay(10);
-
 	} else {
 		/* Read the Extended Device Control Register, assert the PHY_RESET_DIR
 		 * bit to put the PHY into reset. Then, take it out of reset.
drivers/net/e1000.h +1 -0
index b025ecc..6d110eb 100644
@@ -2497,6 +2497,7 @@ struct e1000_hw {
 #define ICH_GFPREG_BASE_MASK       0x1FFF
 #define ICH_FLASH_LINEAR_ADDR_MASK 0x00FFFFFF
 
+#define E1000_I210_SW_FW_SYNC 0x5B50 /* Software-Firmware Synchronization - RW */
 #define E1000_SW_FW_SYNC 0x05B5C /* Software-Firmware Synchronization - RW */
 
 /* SPI EEPROM Status Register */
Signature · 1 line
1.9.3
#2 lore raw

PATCH 1/1 e1000: fix sw fw sync on igb i210/i211

Quoted 5 lines · Marcel Ziswiler

I finally had a look at the datasheet and spotted an additional register address difference between regular E1000 and i210/i211 chips. This patch fixes this and now successfully works on programmed i210/i211 as well as unprogrammed i211.

Quoted 7 lines

---
Please note that unprogrammed i210 seem to behave slightly different and do not assert the CFG_DONE bit in the EEMNGCTL register upon PHY reset. However if this error condition is ignored then successful data transfer is possible. Due to the rather hacky nature thereof this is not addressed by my patch.

Hi Marcel,

I've never been able to get e1000_swfw_sync_acquire() to not timeout on the boards with i210 devices that I have (IMX6 with 2x i210 programmed as 8086:1533 using int phys). In my case with your patch below as well as York's patch
(https://patchwork.ozlabs.org/patch/400628/) I still see failures. The I210_SW_FW_SYNC reg returns swfw_sync=0x03, fwmask=0x20000, and swmask=0x2 which results in:

Driver can't access resource, SW_FW_SYNC timeout. Unable to acquire swfw sync
Error Resetting the PHY
e1000: e1000#0: ERROR: Hardware Initialization Failed

I've always had to ignore the return value from e1000_reset() to avoid this issue and use the devices.

Any ideas?

Quoted 4 lines

BTW: What about my previous patch this one kind of bases on?

[PATCH] e1000: add i211 and unprogrammed i210/i211 support

I don't have any i211's but I'll see if I can get hold of an unprogrammed i210 to test with.

Tim

Quoted 44 lines
 drivers/net/e1000.c | 6 ++++--
 drivers/net/e1000.h | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)
drivers/net/e1000.c +4 -2
index b092867..798c8aa 100644
@@ -1112,7 +1112,10 @@ e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask)
                if (e1000_get_hw_eeprom_semaphore(hw))
                        return -E1000_ERR_SWFW_SYNC;
 
-               swfw_sync = E1000_READ_REG(hw, SW_FW_SYNC);
+               if (hw->mac_type == e1000_igb)
+                       swfw_sync = E1000_READ_REG(hw, I210_SW_FW_SYNC);
+               else
+                       swfw_sync = E1000_READ_REG(hw, SW_FW_SYNC);
                if (!(swfw_sync & (fwmask | swmask)))
                        break;
 
@@ -4429,7 +4432,6 @@ e1000_phy_hw_reset(struct e1000_hw *hw)
 
                if (hw->mac_type >= e1000_82571)
                        mdelay(10);
-
        } else {
                /* Read the Extended Device Control Register, assert the PHY_RESET_DIR
                 * bit to put the PHY into reset. Then, take it out of reset.
drivers/net/e1000.h +1 -1
index b025ecc..6d110eb 100644
@@ -2497,6 +2497,7 @@ struct e1000_hw {
 #define ICH_GFPREG_BASE_MASK       0x1FFF
 #define ICH_FLASH_LINEAR_ADDR_MASK 0x00FFFFFF
 
+#define E1000_I210_SW_FW_SYNC 0x5B50 /* Software-Firmware Synchronization - RW */
 #define E1000_SW_FW_SYNC 0x05B5C /* Software-Firmware Synchronization - RW */
 
 /* SPI EEPROM Status Register */
--

1.9.3

#3 lore raw

PATCH 1/1 e1000: fix sw fw sync on igb i210/i211

Quoted 3 lines · Tim Harvey

I've never been able to get e1000_swfw_sync_acquire() to not timeout on the boards with i210 devices that I have (IMX6 with 2x i210 programmed as 8086:1533 using int phys).

Ah, OK. I see. We do use them in flash less aka iNVM only mode as 8086:157b.

Quoted 10 lines

In my case with your patch
below as well as York's patch
(https://patchwork.ozlabs.org/patch/400628/) I still see failures. The I210_SW_FW_SYNC reg returns swfw_sync=0x03, fwmask=0x20000, and swmask=0x2 which results in:

Driver can't access resource, SW_FW_SYNC timeout. Unable to acquire swfw sync
Error Resetting the PHY
e1000: e1000#0: ERROR: Hardware Initialization Failed

As follows my results:

unprogrammed i210
01:00.0 - 8086:1531 - Network controller
swfw_sync=0x0, fwmask=0x20000, swmask=0x2
e1000: e1000#0: ERROR: Hardware Initialization Failed

programmed i210
01:00.0 - 8086:157b - Network controller
swfw_sync=0x0, fwmask=0x20000, swmask=0x2
working

unprogrammed i211
01:00.0 - 8086:1532 - Network controller
swfw_sync=0x0, fwmask=0x20000, swmask=0x2
working

programmed i211
01:00.0 - 8086:1539 - Network controller
swfw_sync=0x0, fwmask=0x20000, swmask=0x2
working

Quoted 2 lines

I've always had to ignore the return value from e1000_reset() to avoid this issue and use the devices.

As mentioned before for me only the unprogrammed i210 gives me grief as it does not assert the CFG_DONE bit in the EEMNGCTL register upon PHY reset. However the e1000_swfw_sync_acquire() always works just fine.

Quoted 1 line

Any ideas?

Maybe the flash less aka iNVM only i210 behaves differently.

Let me enquire Intel about this as well, I anyway need to ask our rep about the latest eepromARM programming tool as we have an upcoming production lot of a few thousand units arrive any minute now.

Quoted 2 lines

I don't have any i211's but I'll see if I can get hold of an unprogrammed i210 to test with.

Sure, would be interesting to see whether yours behave equally.

#4 lore raw

PATCH 1/1 e1000: fix sw fw sync on igb i210/i211

Quoted 36 lines · Marcel Ziswiler
Quoted 3 lines · Tim Harvey

I've never been able to get e1000_swfw_sync_acquire() to not timeout on the boards with i210 devices that I have (IMX6 with 2x i210 programmed as 8086:1533 using int phys).

Ah, OK. I see. We do use them in flash less aka iNVM only mode as 8086:157b.

Quoted 10 lines

In my case with your patch
below as well as York's patch
(https://patchwork.ozlabs.org/patch/400628/) I still see failures. The I210_SW_FW_SYNC reg returns swfw_sync=0x03, fwmask=0x20000, and swmask=0x2 which results in:

Driver can't access resource, SW_FW_SYNC timeout. Unable to acquire swfw sync
Error Resetting the PHY
e1000: e1000#0: ERROR: Hardware Initialization Failed

As follows my results:

unprogrammed i210
01:00.0 - 8086:1531 - Network controller
swfw_sync=0x0, fwmask=0x20000, swmask=0x2
e1000: e1000#0: ERROR: Hardware Initialization Failed

programmed i210
01:00.0 - 8086:157b - Network controller
swfw_sync=0x0, fwmask=0x20000, swmask=0x2
working

unprogrammed i211
01:00.0 - 8086:1532 - Network controller
swfw_sync=0x0, fwmask=0x20000, swmask=0x2
working

Quoted 5 lines

programmed i211
01:00.0 - 8086:1539 - Network controller
swfw_sync=0x0, fwmask=0x20000, swmask=0x2
working

Marcel,

What is the configuration of your i210 - is it using an external PHY or internal?

I noticed in e1000.c SWSM is 0x5B50 which is the same reg your adding in your patch here. I think at least you should fix that (use the register definition already created).

It looks like bit0 of SWSM which is set in my case and clear in yours is used as a flag in the driver in a few places and perhaps has nothing to do with hardware state. I'll have to add some debugging to see whats going on.

Tim

Quoted 20 lines
Quoted 2 lines

I've always had to ignore the return value from e1000_reset() to avoid this issue and use the devices.

As mentioned before for me only the unprogrammed i210 gives me grief as it does not assert the CFG_DONE bit in the EEMNGCTL register upon PHY reset. However the e1000_swfw_sync_acquire() always works just fine.

Quoted 1 line

Any ideas?

Maybe the flash less aka iNVM only i210 behaves differently.

Let me enquire Intel about this as well, I anyway need to ask our rep about the latest eepromARM programming tool as we have an upcoming production lot of a few thousand units arrive any minute now.

Quoted 2 lines

I don't have any i211's but I'll see if I can get hold of an unprogrammed i210 to test with.

Sure, would be interesting to see whether yours behave equally.

#5 lore raw
In reply to Marcel Ziswiler
Quoted 6 lines · Marcel Ziswiler

I finally had a look at the datasheet and spotted an additional register address difference between regular E1000 and i210/i211 chips. This patch fixes this and now successfully works on programmed i210/i211 as well as unprogrammed i211.

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141027/c90ec28e/attachment.pgp>