[U-Boot] [PATCH 0/3] e1000: fix semaphore sync issues
The following patchset resolves semaphore sync issues I've encountered on a board with an i210 (programmed, copper mode) using the internal phy.
The first patch adds releasing of the semaphore once they are no longer needed, and the other two patches revert logic that I believe was working around the fact that they were previsouly not released.
This will need testing on the various configurations of e1000 so I've taken care to Cc the maintainers that were listed by boards using CONFIG_E1000 as well as the authors of the patches I'm reverting sections from.
Cc: Marcel Ziswiler marcel@ziswiler.com Cc: Marek Vasut marex@denx.de Cc: Aneesh Bansal aneesh.bansal@freescale.com Cc: Naveen Burmi NaveenBurmi@freescale.com Cc: Po Liu po.liu@freescale.com Cc: Bin Meng bmeng.cn@gmail.com Cc: Alison Wang alison.wang@freescale.com Cc: Reinhard Arlt reinhard.arlt@esd-electronics.com Cc: Shengzhou Liu Shengzhou.Liu@freescale.com Cc: York Sun yorksun@freescale.com Signed-off-by: Tim Harvey tharvey@gateworks.com
Tim Harvey (3): e1000: fix eeprom sync issues by releasing semaphore once no longer needed Revert "e1000: fix sw fw sync on igb i210/i211" e1000: remove unnecessary clearing of SWSM.SWSM_SMBI
drivers/net/e1000.c | 32 ++++++++++++++++++++++++-------- drivers/net/e1000.h | 1 - 2 files changed, 24 insertions(+), 9 deletions(-)
Once the hwsw semaphore is acquired, it must be released when access to the hw is completed. Without this subsequent calls to acquire will timeout obtaining the semaphore.
Cc: Marcel Ziswiler marcel@ziswiler.com Cc: Marek Vasut marex@denx.de Cc: Aneesh Bansal aneesh.bansal@freescale.com Cc: Naveen Burmi NaveenBurmi@freescale.com Cc: Po Liu po.liu@freescale.com Cc: Bin Meng bmeng.cn@gmail.com Cc: Alison Wang alison.wang@freescale.com Cc: Reinhard Arlt reinhard.arlt@esd-electronics.com Cc: Shengzhou Liu Shengzhou.Liu@freescale.com Cc: York Sun yorksun@freescale.com Signed-off-by: Tim Harvey tharvey@gateworks.com --- drivers/net/e1000.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/drivers/net/e1000.c b/drivers/net/e1000.c index cd44222..a64bc7b 100644 --- a/drivers/net/e1000.c +++ b/drivers/net/e1000.c @@ -126,6 +126,7 @@ static int e1000_detect_gig_phy(struct e1000_hw *hw); static void e1000_set_media_type(struct e1000_hw *hw);
static int32_t e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask); +static void e1000_swfw_sync_release(struct e1000_hw *hw, uint16_t mask); static int32_t e1000_check_phy_reset_block(struct e1000_hw *hw);
#ifndef CONFIG_E1000_NO_NVM @@ -729,7 +730,10 @@ void e1000_release_eeprom(struct e1000_hw *hw) eecd &= ~E1000_EECD_REQ; E1000_WRITE_REG(hw, EECD, eecd); } + + e1000_swfw_sync_release(hw, E1000_SWFW_EEP_SM); } + /****************************************************************************** * Reads a 16 bit word from the EEPROM. * @@ -1102,6 +1106,7 @@ e1000_get_hw_eeprom_semaphore(struct e1000_hw *hw) return E1000_SUCCESS; }
+/* Take ownership of the PHY */ static int32_t e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask) { @@ -1141,6 +1146,21 @@ e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask) return E1000_SUCCESS; }
+static void e1000_swfw_sync_release(struct e1000_hw *hw, uint16_t mask) +{ + uint32_t swfw_sync = 0; + + DEBUGFUNC(); + while (e1000_get_hw_eeprom_semaphore(hw)) + ; /* Empty */ + + swfw_sync = E1000_READ_REG(hw, SW_FW_SYNC); + swfw_sync &= ~mask; + E1000_WRITE_REG(hw, SW_FW_SYNC, swfw_sync); + + e1000_put_hw_eeprom_semaphore(hw); +} + static bool e1000_is_second_port(struct e1000_hw *hw) { switch (hw->mac_type) { @@ -4462,6 +4482,8 @@ e1000_phy_hw_reset(struct e1000_hw *hw) E1000_WRITE_REG(hw, LEDCTL, led_ctrl); }
+ e1000_swfw_sync_release(hw, swfw); + /* Wait for FW to finish PHY configuration. */ ret_val = e1000_get_phy_cfg_done(hw); if (ret_val != E1000_SUCCESS)
On Wed, May 20, 2015 at 1:01 AM, Tim Harvey tharvey@gateworks.com wrote:
Once the hwsw semaphore is acquired, it must be released when access to the hw is completed. Without this subsequent calls to acquire will timeout obtaining the semaphore.
Cc: Marcel Ziswiler marcel@ziswiler.com Cc: Marek Vasut marex@denx.de Cc: Aneesh Bansal aneesh.bansal@freescale.com Cc: Naveen Burmi NaveenBurmi@freescale.com Cc: Po Liu po.liu@freescale.com Cc: Bin Meng bmeng.cn@gmail.com Cc: Alison Wang alison.wang@freescale.com Cc: Reinhard Arlt reinhard.arlt@esd-electronics.com Cc: Shengzhou Liu Shengzhou.Liu@freescale.com Cc: York Sun yorksun@freescale.com Signed-off-by: Tim Harvey tharvey@gateworks.com
drivers/net/e1000.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/drivers/net/e1000.c b/drivers/net/e1000.c index cd44222..a64bc7b 100644 --- a/drivers/net/e1000.c +++ b/drivers/net/e1000.c @@ -126,6 +126,7 @@ static int e1000_detect_gig_phy(struct e1000_hw *hw); static void e1000_set_media_type(struct e1000_hw *hw);
static int32_t e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask); +static void e1000_swfw_sync_release(struct e1000_hw *hw, uint16_t mask); static int32_t e1000_check_phy_reset_block(struct e1000_hw *hw);
#ifndef CONFIG_E1000_NO_NVM @@ -729,7 +730,10 @@ void e1000_release_eeprom(struct e1000_hw *hw) eecd &= ~E1000_EECD_REQ; E1000_WRITE_REG(hw, EECD, eecd); }
e1000_swfw_sync_release(hw, E1000_SWFW_EEP_SM);}
/******************************************************************************
- Reads a 16 bit word from the EEPROM.
@@ -1102,6 +1106,7 @@ e1000_get_hw_eeprom_semaphore(struct e1000_hw *hw) return E1000_SUCCESS; }
+/* Take ownership of the PHY */ static int32_t e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask) { @@ -1141,6 +1146,21 @@ e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask) return E1000_SUCCESS; }
+static void e1000_swfw_sync_release(struct e1000_hw *hw, uint16_t mask) +{
uint32_t swfw_sync = 0;DEBUGFUNC();while (e1000_get_hw_eeprom_semaphore(hw)); /* Empty */swfw_sync = E1000_READ_REG(hw, SW_FW_SYNC);swfw_sync &= ~mask;E1000_WRITE_REG(hw, SW_FW_SYNC, swfw_sync);e1000_put_hw_eeprom_semaphore(hw);+}
static bool e1000_is_second_port(struct e1000_hw *hw) { switch (hw->mac_type) { @@ -4462,6 +4482,8 @@ e1000_phy_hw_reset(struct e1000_hw *hw) E1000_WRITE_REG(hw, LEDCTL, led_ctrl); }
e1000_swfw_sync_release(hw, swfw);/* Wait for FW to finish PHY configuration. */ ret_val = e1000_get_phy_cfg_done(hw); if (ret_val != E1000_SUCCESS)--
Tested on QEMU v2.3.0
Tested-by: Bin Meng bmeng.cn@gmail.com
On Tue, 2015-05-19 at 10:01 -0700, Tim Harvey wrote:
Once the hwsw semaphore is acquired, it must be released when access to the hw is completed. Without this subsequent calls to acquire will timeout obtaining the semaphore.
Cc: Marcel Ziswiler marcel@ziswiler.com Cc: Marek Vasut marex@denx.de Cc: Aneesh Bansal aneesh.bansal@freescale.com Cc: Naveen Burmi NaveenBurmi@freescale.com Cc: Po Liu po.liu@freescale.com Cc: Bin Meng bmeng.cn@gmail.com Cc: Alison Wang alison.wang@freescale.com Cc: Reinhard Arlt reinhard.arlt@esd-electronics.com Cc: Shengzhou Liu Shengzhou.Liu@freescale.com Cc: York Sun yorksun@freescale.com Signed-off-by: Tim Harvey tharvey@gateworks.com
drivers/net/e1000.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/drivers/net/e1000.c b/drivers/net/e1000.c index cd44222..a64bc7b 100644 --- a/drivers/net/e1000.c +++ b/drivers/net/e1000.c @@ -126,6 +126,7 @@ static int e1000_detect_gig_phy(struct e1000_hw *hw); static void e1000_set_media_type(struct e1000_hw *hw);
static int32_t e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask); +static void e1000_swfw_sync_release(struct e1000_hw *hw, uint16_t mask); static int32_t e1000_check_phy_reset_block(struct e1000_hw *hw);
#ifndef CONFIG_E1000_NO_NVM @@ -729,7 +730,10 @@ void e1000_release_eeprom(struct e1000_hw *hw) eecd &= ~E1000_EECD_REQ; E1000_WRITE_REG(hw, EECD, eecd); }
- e1000_swfw_sync_release(hw, E1000_SWFW_EEP_SM);
}
/******************************************************************************
- Reads a 16 bit word from the EEPROM.
@@ -1102,6 +1106,7 @@ e1000_get_hw_eeprom_semaphore(struct e1000_hw *hw) return E1000_SUCCESS; }
+/* Take ownership of the PHY */ static int32_t e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask) { @@ -1141,6 +1146,21 @@ e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask) return E1000_SUCCESS; }
+static void e1000_swfw_sync_release(struct e1000_hw *hw, uint16_t mask) +{
- uint32_t swfw_sync = 0;
- DEBUGFUNC();
- while (e1000_get_hw_eeprom_semaphore(hw))
; /* Empty */- swfw_sync = E1000_READ_REG(hw, SW_FW_SYNC);
- swfw_sync &= ~mask;
- E1000_WRITE_REG(hw, SW_FW_SYNC, swfw_sync);
- e1000_put_hw_eeprom_semaphore(hw);
+}
static bool e1000_is_second_port(struct e1000_hw *hw) { switch (hw->mac_type) { @@ -4462,6 +4482,8 @@ e1000_phy_hw_reset(struct e1000_hw *hw) E1000_WRITE_REG(hw, LEDCTL, led_ctrl); }
- e1000_swfw_sync_release(hw, swfw);
- /* Wait for FW to finish PHY configuration. */ ret_val = e1000_get_phy_cfg_done(hw); if (ret_val != E1000_SUCCESS)
Tested on Apalis T30 1GB V1.1A with properly fused i211 Tested on Apalis T30 2GB V1.1A with iNVM fused i210 Tested on Apalis T30 1GB V1.0A with tools only aka non fused i211 Tested-by: Marcel Ziswiler marcel.ziswiler@toradex.com --- BTW: Still fails on Apalis T30 2GB V1.0E with tools only aka non fused i210 as follows: e1000: e1000#0: ERROR: Hardware Initialization Failed In our downstream production U-Boot we temporarily hacked this as follows for now: http://git.toradex.com/cgit/u-boot-toradex.git/commit/?h=2015.04-toradex&...
Hi Tim,
On Tue, May 19, 2015 at 12:01 PM, Tim Harvey tharvey@gateworks.com wrote:
Once the hwsw semaphore is acquired, it must be released when access to the hw is completed. Without this subsequent calls to acquire will timeout obtaining the semaphore.
Cc: Marcel Ziswiler marcel@ziswiler.com Cc: Marek Vasut marex@denx.de Cc: Aneesh Bansal aneesh.bansal@freescale.com Cc: Naveen Burmi NaveenBurmi@freescale.com Cc: Po Liu po.liu@freescale.com Cc: Bin Meng bmeng.cn@gmail.com Cc: Alison Wang alison.wang@freescale.com Cc: Reinhard Arlt reinhard.arlt@esd-electronics.com Cc: Shengzhou Liu Shengzhou.Liu@freescale.com Cc: York Sun yorksun@freescale.com Signed-off-by: Tim Harvey tharvey@gateworks.com
Applied to u-boot-net, thanks! -Joe
This reverts commit 17da7120249bfdef877f46be5bbcb3cc01212eb9.
The i210/i211 do have the SW_FW_SYNC (0x5b5c) register and this is what should be used when acquiring the semaphore.
I believe the issue that this patch was trying to resolve is now resolved by properly releasing the semaphore once no longer needed.
Cc: Marcel Ziswiler marcel@ziswiler.com Cc: Marek Vasut marex@denx.de Cc: Aneesh Bansal aneesh.bansal@freescale.com Cc: Naveen Burmi NaveenBurmi@freescale.com Cc: Po Liu po.liu@freescale.com Cc: Bin Meng bmeng.cn@gmail.com Cc: Alison Wang alison.wang@freescale.com Cc: Reinhard Arlt reinhard.arlt@esd-electronics.com Cc: Shengzhou Liu Shengzhou.Liu@freescale.com Cc: York Sun yorksun@freescale.com Signed-off-by: Tim Harvey tharvey@gateworks.com --- drivers/net/e1000.c | 6 ++---- drivers/net/e1000.h | 1 - 2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/net/e1000.c b/drivers/net/e1000.c index a64bc7b..f960024 100644 --- a/drivers/net/e1000.c +++ b/drivers/net/e1000.c @@ -1120,10 +1120,7 @@ e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask) if (e1000_get_hw_eeprom_semaphore(hw)) return -E1000_ERR_SWFW_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); + swfw_sync = E1000_READ_REG(hw, SW_FW_SYNC); if (!(swfw_sync & (fwmask | swmask))) break;
@@ -4458,6 +4455,7 @@ 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. diff --git a/drivers/net/e1000.h b/drivers/net/e1000.h index f3b77b1..232c95d 100644 --- a/drivers/net/e1000.h +++ b/drivers/net/e1000.h @@ -2496,7 +2496,6 @@ 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 */
On Wed, May 20, 2015 at 1:01 AM, Tim Harvey tharvey@gateworks.com wrote:
This reverts commit 17da7120249bfdef877f46be5bbcb3cc01212eb9.
The i210/i211 do have the SW_FW_SYNC (0x5b5c) register and this is what should be used when acquiring the semaphore.
I believe the issue that this patch was trying to resolve is now resolved by properly releasing the semaphore once no longer needed.
Cc: Marcel Ziswiler marcel@ziswiler.com Cc: Marek Vasut marex@denx.de Cc: Aneesh Bansal aneesh.bansal@freescale.com Cc: Naveen Burmi NaveenBurmi@freescale.com Cc: Po Liu po.liu@freescale.com Cc: Bin Meng bmeng.cn@gmail.com Cc: Alison Wang alison.wang@freescale.com Cc: Reinhard Arlt reinhard.arlt@esd-electronics.com Cc: Shengzhou Liu Shengzhou.Liu@freescale.com Cc: York Sun yorksun@freescale.com Signed-off-by: Tim Harvey tharvey@gateworks.com
drivers/net/e1000.c | 6 ++---- drivers/net/e1000.h | 1 - 2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/net/e1000.c b/drivers/net/e1000.c index a64bc7b..f960024 100644 --- a/drivers/net/e1000.c +++ b/drivers/net/e1000.c @@ -1120,10 +1120,7 @@ e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask) if (e1000_get_hw_eeprom_semaphore(hw)) return -E1000_ERR_SWFW_SYNC;
if (hw->mac_type == e1000_igb)swfw_sync = E1000_READ_REG(hw, I210_SW_FW_SYNC);elseswfw_sync = E1000_READ_REG(hw, SW_FW_SYNC);
swfw_sync = E1000_READ_REG(hw, SW_FW_SYNC); if (!(swfw_sync & (fwmask | swmask))) break;@@ -4458,6 +4455,7 @@ 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.diff --git a/drivers/net/e1000.h b/drivers/net/e1000.h index f3b77b1..232c95d 100644 --- a/drivers/net/e1000.h +++ b/drivers/net/e1000.h @@ -2496,7 +2496,6 @@ 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 */
Tested on QEMU v2.3.0
Tested-by: Bin Meng bmeng.cn@gmail.com
On Tue, 2015-05-19 at 10:01 -0700, Tim Harvey wrote:
This reverts commit 17da7120249bfdef877f46be5bbcb3cc01212eb9.
The i210/i211 do have the SW_FW_SYNC (0x5b5c) register and this is what should be used when acquiring the semaphore.
I believe the issue that this patch was trying to resolve is now resolved by properly releasing the semaphore once no longer needed.
Cc: Marcel Ziswiler marcel@ziswiler.com Cc: Marek Vasut marex@denx.de Cc: Aneesh Bansal aneesh.bansal@freescale.com Cc: Naveen Burmi NaveenBurmi@freescale.com Cc: Po Liu po.liu@freescale.com Cc: Bin Meng bmeng.cn@gmail.com Cc: Alison Wang alison.wang@freescale.com Cc: Reinhard Arlt reinhard.arlt@esd-electronics.com Cc: Shengzhou Liu Shengzhou.Liu@freescale.com Cc: York Sun yorksun@freescale.com Signed-off-by: Tim Harvey tharvey@gateworks.com
drivers/net/e1000.c | 6 ++---- drivers/net/e1000.h | 1 - 2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/net/e1000.c b/drivers/net/e1000.c index a64bc7b..f960024 100644 --- a/drivers/net/e1000.c +++ b/drivers/net/e1000.c @@ -1120,10 +1120,7 @@ e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask) if (e1000_get_hw_eeprom_semaphore(hw)) return -E1000_ERR_SWFW_SYNC;
if (hw->mac_type == e1000_igb)swfw_sync = E1000_READ_REG(hw, I210_SW_FW_SYNC);elseswfw_sync = E1000_READ_REG(hw, SW_FW_SYNC);
swfw_sync = E1000_READ_REG(hw, SW_FW_SYNC);@@ -4458,6 +4455,7 @@ 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.
diff --git a/drivers/net/e1000.h b/drivers/net/e1000.h index f3b77b1..232c95d 100644 --- a/drivers/net/e1000.h +++ b/drivers/net/e1000.h @@ -2496,7 +2496,6 @@ 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 */
Tested on Apalis T30 1GB V1.1A with properly fused i211 Tested on Apalis T30 2GB V1.1A with iNVM fused i210 Tested on Apalis T30 1GB V1.0A with tools only aka non fused i211 Tested-by: Marcel Ziswiler marcel.ziswiler@toradex.com --- BTW: Still fails on Apalis T30 2GB V1.0E with tools only aka non fused i210 as follows: e1000: e1000#0: ERROR: Hardware Initialization Failed In our downstream production U-Boot we temporarily hacked this as follows for now: http://git.toradex.com/cgit/u-boot-toradex.git/commit/?h=2015.04-toradex&...
On Wed, May 20, 2015 at 4:27 AM, Marcel Ziswiler marcel@ziswiler.com wrote:
On Tue, 2015-05-19 at 10:01 -0700, Tim Harvey wrote:
This reverts commit 17da7120249bfdef877f46be5bbcb3cc01212eb9.
The i210/i211 do have the SW_FW_SYNC (0x5b5c) register and this is what should be used when acquiring the semaphore.
I believe the issue that this patch was trying to resolve is now resolved by properly releasing the semaphore once no longer needed.
<snip>
Tested on Apalis T30 1GB V1.1A with properly fused i211 Tested on Apalis T30 2GB V1.1A with iNVM fused i210 Tested on Apalis T30 1GB V1.0A with tools only aka non fused i211 Tested-by: Marcel Ziswiler marcel.ziswiler@toradex.com
BTW: Still fails on Apalis T30 2GB V1.0E with tools only aka non fused i210 as follows: e1000: e1000#0: ERROR: Hardware Initialization Failed In our downstream production U-Boot we temporarily hacked this as follows for now: http://git.toradex.com/cgit/u-boot-toradex.git/commit/?h=2015.04-toradex&...
Marcel,
I don't understand your results above. What I'm most interested in is if this patch series (adding the proper semaphore release and removing your patch that uses the wrong register for i210) resolves the need for you having added this particular patch for whatever board you needed it for. Is the configuration that was failing for you requiring 17da7120249bfdef877f46be5bbcb3cc01212eb9 resolved with this series applied?
When you say it 'still fails on Apalis T30 2GB V1.0E' does that mean you have that particular failure both before and after this patch series? That would indicate to me there is something more needed specifically for that configuration.
I'm also not really clear what you mean by 'properly fused i211' vs 'iNVM fused i210' vis 'tools only aka non fused i211'. I believe you are referring to if they are programmed parts or not but I'm not clear if all of your configurations are using internal phy or an external phy.
Your downstream patch indicates that in your non-working configuration the EEMNGCTL.CFG_DONE_P0 bit is not getting set indicating (from the datasheet) that the configuration cycle (configuration of SerDes, PHY, PCIe and PLLs) is not done for port 0 which very well may be the expected behavior on a non-programmed part.
The configuration I required this series for was for an i210 with internal phy in copper mode. Without this series it would error out with 'ERROR: Hardware Initialization Failed' because e1000_swfw_sync_acquire() would timeout and return -E1000_ERR_SWFW_SYNC.
Regards,
Tim
On Wed, 2015-05-20 at 06:15 -0700, Tim Harvey wrote: <snip>
Tested on Apalis T30 1GB V1.1A with properly fused i211 Tested on Apalis T30 2GB V1.1A with iNVM fused i210 Tested on Apalis T30 1GB V1.0A with tools only aka non fused i211 Tested-by: Marcel Ziswiler marcel.ziswiler@toradex.com
BTW: Still fails on Apalis T30 2GB V1.0E with tools only aka non fused i210 as follows: e1000: e1000#0: ERROR: Hardware Initialization Failed In our downstream production U-Boot we temporarily hacked this as follows for now: http://git.toradex.com/cgit/u-boot-toradex.git/commit/?h=2015.04-toradex&...
I don't understand your results above. What I'm most interested in is if this patch series (adding the proper semaphore release and removing your patch that uses the wrong register for i210) resolves the need for you having added this particular patch for whatever board you needed it for. Is the configuration that was failing for you requiring 17da7120249bfdef877f46be5bbcb3cc01212eb9 resolved with this series applied?
Yes, exactly.
When you say it 'still fails on Apalis T30 2GB V1.0E' does that mean you have that particular failure both before and after this patch series? That would indicate to me there is something more needed specifically for that configuration.
Yes, exactly. As once mentioned before Intel actually claims tools only mode anyway not being operational at all on the other hand the Linux driver worked just fine for us with each and every such combination. Unfortunately so far I did not get to tracking this any further.
I'm also not really clear what you mean by 'properly fused i211' vs 'iNVM fused i210' vis 'tools only aka non fused i211'. I believe you are referring to if they are programmed parts or not
Yes, exactly. By 'properly fused i211' I mean its iNVM being programmed as it albeit features no other possibility. The i210 on the other hand can be used with an external EEPROM or the iNVM so by 'iNVM fused i210' I mean that I only tested programming the iNVM. I did NOT do any tests with an external EEPROM and therefore also NOT with any activated management stuff or the like requiring external firmware therein.
but I'm not clear if all of your configurations are using internal phy or an external phy.
Yes, sorry. I forgot to mention this: Our hardware only ever uses the internal copper PHY.
Your downstream patch indicates that in your non-working configuration the EEMNGCTL.CFG_DONE_P0 bit is not getting set indicating (from the datasheet) that the configuration cycle (configuration of SerDes, PHY, PCIe and PLLs) is not done for port 0 which very well may be the expected behavior on a non-programmed part.
Yes, maybe the Linux driver just ignores this condition resp. I actually patched it to ignore failing NVM validation: http://git.toradex.com/cgit/linux-toradex.git/commit/?h=tegra&id=c4c3c74...
Plus the tools only PCI IDs are anyway missing in Linux: http://git.toradex.com/cgit/linux-toradex.git/commit/?h=tegra&id=2c71234...
The configuration I required this series for was for an i210 with internal phy in copper mode. Without this series it would error out with 'ERROR: Hardware Initialization Failed' because e1000_swfw_sync_acquire() would timeout and return -E1000_ERR_SWFW_SYNC.
OK.
On Wed, May 20, 2015 at 7:14 AM, Marcel Ziswiler marcel@ziswiler.com wrote:
On Wed, 2015-05-20 at 06:15 -0700, Tim Harvey wrote:
<snip> > > Tested on Apalis T30 1GB V1.1A with properly fused i211 > > Tested on Apalis T30 2GB V1.1A with iNVM fused i210 > > Tested on Apalis T30 1GB V1.0A with tools only aka non fused i211 > > Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > > --- > > BTW: Still fails on Apalis T30 2GB V1.0E with tools only aka non fused > > i210 as follows: > > e1000: e1000#0: ERROR: Hardware Initialization Failed > > In our downstream production U-Boot we temporarily hacked this as > > follows for now: > > http://git.toradex.com/cgit/u-boot-toradex.git/commit/?h=2015.04-toradex&id=2d8ea651b6da79047b6fa729863d25b5eb9e15d7 > > I don't understand your results above. What I'm most interested in is > if this patch series (adding the proper semaphore release and removing > your patch that uses the wrong register for i210) resolves the need > for you having added this particular patch for whatever board you > needed it for. Is the configuration that was failing for you requiring > 17da7120249bfdef877f46be5bbcb3cc01212eb9 resolved with this series > applied?
Yes, exactly.
ok - thats great news
When you say it 'still fails on Apalis T30 2GB V1.0E' does that mean you have that particular failure both before and after this patch series? That would indicate to me there is something more needed specifically for that configuration.
Yes, exactly. As once mentioned before Intel actually claims tools only mode anyway not being operational at all on the other hand the Linux driver worked just fine for us with each and every such combination. Unfortunately so far I did not get to tracking this any further.
It does make sense to me that an 'unprogrammed' device would work just fine as long as the programmed device-id's were supported by the driver (which they are) and the default mode matches your configuration. All 'programmed' means on an i210/i211 is that you've added some register writes to 'override' power-on defaults. As long as the power-on defaults work for your config then your ok. The default power-on config for i210/i211 is internal phy copper which is what you have.
Tim
On Wed, May 20, 2015 at 8:00 AM, Tim Harvey tharvey@gateworks.com wrote:
On Wed, May 20, 2015 at 7:14 AM, Marcel Ziswiler marcel@ziswiler.com wrote:
On Wed, 2015-05-20 at 06:15 -0700, Tim Harvey wrote:
<snip> > > Tested on Apalis T30 1GB V1.1A with properly fused i211 > > Tested on Apalis T30 2GB V1.1A with iNVM fused i210 > > Tested on Apalis T30 1GB V1.0A with tools only aka non fused i211 > > Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > > --- > > BTW: Still fails on Apalis T30 2GB V1.0E with tools only aka non fused > > i210 as follows: > > e1000: e1000#0: ERROR: Hardware Initialization Failed > > In our downstream production U-Boot we temporarily hacked this as > > follows for now: > > http://git.toradex.com/cgit/u-boot-toradex.git/commit/?h=2015.04-toradex&id=2d8ea651b6da79047b6fa729863d25b5eb9e15d7 > > I don't understand your results above. What I'm most interested in is > if this patch series (adding the proper semaphore release and removing > your patch that uses the wrong register for i210) resolves the need > for you having added this particular patch for whatever board you > needed it for. Is the configuration that was failing for you requiring > 17da7120249bfdef877f46be5bbcb3cc01212eb9 resolved with this series > applied?
Yes, exactly.
ok - thats great news
When you say it 'still fails on Apalis T30 2GB V1.0E' does that mean you have that particular failure both before and after this patch series? That would indicate to me there is something more needed specifically for that configuration.
Yes, exactly. As once mentioned before Intel actually claims tools only mode anyway not being operational at all on the other hand the Linux driver worked just fine for us with each and every such combination. Unfortunately so far I did not get to tracking this any further.
It does make sense to me that an 'unprogrammed' device would work just fine as long as the programmed device-id's were supported by the driver (which they are) and the default mode matches your configuration. All 'programmed' means on an i210/i211 is that you've added some register writes to 'override' power-on defaults. As long as the power-on defaults work for your config then your ok. The default power-on config for i210/i211 is internal phy copper which is what you have.
Tim
Marcel,
Could you give an 'acked-by' if you agree with this series? I would like to see it merged:
https://patchwork.ozlabs.org/patch/473997/ https://patchwork.ozlabs.org/patch/473998/ https://patchwork.ozlabs.org/patch/473996/
Regards,
Tim
On Fri, 2015-07-10 at 08:47 -0700, Tim Harvey wrote:
Marcel,
Could you give an 'acked-by' if you agree with this series? I would like to see it merged:
https://patchwork.ozlabs.org/patch/473997/ https://patchwork.ozlabs.org/patch/473998/ https://patchwork.ozlabs.org/patch/473996/
Regards,
Tim
Hi Tim
Sure. Sorry, I thought my previously given tested-by would be stronger than just an acked-by. Here you go for the whole series:
Acked-by Marcel Ziswiler marcel.ziswiler@toradex.com
Cheers
Marcel
On Sat, Jul 11, 2015 at 3:11 PM, Marcel Ziswiler marcel@ziswiler.com wrote:
On Fri, 2015-07-10 at 08:47 -0700, Tim Harvey wrote:
Marcel,
Could you give an 'acked-by' if you agree with this series? I would like to see it merged:
https://patchwork.ozlabs.org/patch/473997/ https://patchwork.ozlabs.org/patch/473998/ https://patchwork.ozlabs.org/patch/473996/
Regards,
Tim
Hi Tim
Sure. Sorry, I thought my previously given tested-by would be stronger than just an acked-by. Here you go for the whole series:
Acked-by Marcel Ziswiler marcel.ziswiler@toradex.com
Cheers
Marcel
Joe,
Can you take a look at this series? You are the current delegate in patchwork.
Thanks,
Tim
On Mon, Jul 13, 2015 at 8:45 AM, Tim Harvey tharvey@gateworks.com wrote:
On Sat, Jul 11, 2015 at 3:11 PM, Marcel Ziswiler marcel@ziswiler.com wrote:
On Fri, 2015-07-10 at 08:47 -0700, Tim Harvey wrote:
Marcel,
Could you give an 'acked-by' if you agree with this series? I would like to see it merged:
https://patchwork.ozlabs.org/patch/473997/ https://patchwork.ozlabs.org/patch/473998/ https://patchwork.ozlabs.org/patch/473996/
Regards,
Tim
Hi Tim
Sure. Sorry, I thought my previously given tested-by would be stronger than just an acked-by. Here you go for the whole series:
Acked-by Marcel Ziswiler marcel.ziswiler@toradex.com
Cheers
Marcel
Joe,
Can you take a look at this series? You are the current delegate in patchwork.
Thanks,
Tim
Tom,
Joe seems to be MIA - can you take a look at these please?
https://patchwork.ozlabs.org/patch/473997/ https://patchwork.ozlabs.org/patch/473998/ https://patchwork.ozlabs.org/patch/473996/
Thanks,
Tim
Hi Tim,
On Tue, May 19, 2015 at 12:01 PM, Tim Harvey tharvey@gateworks.com wrote:
This reverts commit 17da7120249bfdef877f46be5bbcb3cc01212eb9.
The i210/i211 do have the SW_FW_SYNC (0x5b5c) register and this is what should be used when acquiring the semaphore.
I believe the issue that this patch was trying to resolve is now resolved by properly releasing the semaphore once no longer needed.
Cc: Marcel Ziswiler marcel@ziswiler.com Cc: Marek Vasut marex@denx.de Cc: Aneesh Bansal aneesh.bansal@freescale.com Cc: Naveen Burmi NaveenBurmi@freescale.com Cc: Po Liu po.liu@freescale.com Cc: Bin Meng bmeng.cn@gmail.com Cc: Alison Wang alison.wang@freescale.com Cc: Reinhard Arlt reinhard.arlt@esd-electronics.com Cc: Shengzhou Liu Shengzhou.Liu@freescale.com Cc: York Sun yorksun@freescale.com Signed-off-by: Tim Harvey tharvey@gateworks.com
Applied to u-boot-net, thanks! -Joe
remove unnecessary clearing of SWSM.SWSM_SMBI when obtaining the SW semaphore. This was introduced in 951860634fdb557bbb58e0f99215391bc0c29779 while adding i210 support and should be now resolved by releasing the semaphore when no longer needed.
Cc: Marcel Ziswiler marcel@ziswiler.com Cc: Marek Vasut marex@denx.de Cc: Aneesh Bansal aneesh.bansal@freescale.com Cc: Naveen Burmi NaveenBurmi@freescale.com Cc: Po Liu po.liu@freescale.com Cc: Bin Meng bmeng.cn@gmail.com Cc: Alison Wang alison.wang@freescale.com Cc: Reinhard Arlt reinhard.arlt@esd-electronics.com Cc: Shengzhou Liu Shengzhou.Liu@freescale.com Cc: York Sun yorksun@freescale.com Signed-off-by: Tim Harvey tharvey@gateworks.com --- drivers/net/e1000.c | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/drivers/net/e1000.c b/drivers/net/e1000.c index f960024..a78ffc4 100644 --- a/drivers/net/e1000.c +++ b/drivers/net/e1000.c @@ -996,10 +996,6 @@ e1000_get_software_semaphore(struct e1000_hw *hw)
DEBUGFUNC();
- swsm = E1000_READ_REG(hw, SWSM); - swsm &= ~E1000_SWSM_SMBI; - E1000_WRITE_REG(hw, SWSM, swsm); - if (hw->mac_type != e1000_80003es2lan) return E1000_SUCCESS;
On Wed, May 20, 2015 at 1:01 AM, Tim Harvey tharvey@gateworks.com wrote:
remove unnecessary clearing of SWSM.SWSM_SMBI when obtaining the SW semaphore. This was introduced in 951860634fdb557bbb58e0f99215391bc0c29779 while adding i210 support and should be now resolved by releasing the semaphore when no longer needed.
Cc: Marcel Ziswiler marcel@ziswiler.com Cc: Marek Vasut marex@denx.de Cc: Aneesh Bansal aneesh.bansal@freescale.com Cc: Naveen Burmi NaveenBurmi@freescale.com Cc: Po Liu po.liu@freescale.com Cc: Bin Meng bmeng.cn@gmail.com Cc: Alison Wang alison.wang@freescale.com Cc: Reinhard Arlt reinhard.arlt@esd-electronics.com Cc: Shengzhou Liu Shengzhou.Liu@freescale.com Cc: York Sun yorksun@freescale.com Signed-off-by: Tim Harvey tharvey@gateworks.com
drivers/net/e1000.c | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/drivers/net/e1000.c b/drivers/net/e1000.c index f960024..a78ffc4 100644 --- a/drivers/net/e1000.c +++ b/drivers/net/e1000.c @@ -996,10 +996,6 @@ e1000_get_software_semaphore(struct e1000_hw *hw)
DEBUGFUNC();
swsm = E1000_READ_REG(hw, SWSM);swsm &= ~E1000_SWSM_SMBI;E1000_WRITE_REG(hw, SWSM, swsm);if (hw->mac_type != e1000_80003es2lan) return E1000_SUCCESS;--
Tested on QEMU v2.3.0
Tested-by: Bin Meng bmeng.cn@gmail.com
On Tue, 2015-05-19 at 10:01 -0700, Tim Harvey wrote:
remove unnecessary clearing of SWSM.SWSM_SMBI when obtaining the SW semaphore. This was introduced in 951860634fdb557bbb58e0f99215391bc0c29779 while adding i210 support and should be now resolved by releasing the semaphore when no longer needed.
Cc: Marcel Ziswiler marcel@ziswiler.com Cc: Marek Vasut marex@denx.de Cc: Aneesh Bansal aneesh.bansal@freescale.com Cc: Naveen Burmi NaveenBurmi@freescale.com Cc: Po Liu po.liu@freescale.com Cc: Bin Meng bmeng.cn@gmail.com Cc: Alison Wang alison.wang@freescale.com Cc: Reinhard Arlt reinhard.arlt@esd-electronics.com Cc: Shengzhou Liu Shengzhou.Liu@freescale.com Cc: York Sun yorksun@freescale.com Signed-off-by: Tim Harvey tharvey@gateworks.com
drivers/net/e1000.c | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/drivers/net/e1000.c b/drivers/net/e1000.c index f960024..a78ffc4 100644 --- a/drivers/net/e1000.c +++ b/drivers/net/e1000.c @@ -996,10 +996,6 @@ e1000_get_software_semaphore(struct e1000_hw *hw)
DEBUGFUNC();
swsm = E1000_READ_REG(hw, SWSM);swsm &= ~E1000_SWSM_SMBI;E1000_WRITE_REG(hw, SWSM, swsm);- if (hw->mac_type != e1000_80003es2lan) return E1000_SUCCESS;
Tested on Apalis T30 1GB V1.1A with properly fused i211 Tested on Apalis T30 2GB V1.1A with iNVM fused i210 Tested on Apalis T30 1GB V1.0A with tools only aka non fused i211 Tested-by: Marcel Ziswiler marcel.ziswiler@toradex.com --- BTW: Still fails on Apalis T30 2GB V1.0E with tools only aka non fused i210 as follows: e1000: e1000#0: ERROR: Hardware Initialization Failed In our downstream production U-Boot we temporarily hacked this as follows for now: http://git.toradex.com/cgit/u-boot-toradex.git/commit/?h=2015.04-toradex&...
Hi Tim,
On Tue, May 19, 2015 at 12:01 PM, Tim Harvey tharvey@gateworks.com wrote:
remove unnecessary clearing of SWSM.SWSM_SMBI when obtaining the SW semaphore. This was introduced in 951860634fdb557bbb58e0f99215391bc0c29779 while adding i210 support and should be now resolved by releasing the semaphore when no longer needed.
Cc: Marcel Ziswiler marcel@ziswiler.com Cc: Marek Vasut marex@denx.de Cc: Aneesh Bansal aneesh.bansal@freescale.com Cc: Naveen Burmi NaveenBurmi@freescale.com Cc: Po Liu po.liu@freescale.com Cc: Bin Meng bmeng.cn@gmail.com Cc: Alison Wang alison.wang@freescale.com Cc: Reinhard Arlt reinhard.arlt@esd-electronics.com Cc: Shengzhou Liu Shengzhou.Liu@freescale.com Cc: York Sun yorksun@freescale.com Signed-off-by: Tim Harvey tharvey@gateworks.com
Applied to u-boot-net, thanks! -Joe
participants (4)
-
Bin Meng -
Joe Hershberger -
Marcel Ziswiler -
Tim Harvey