All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot]  [PATCH 0/3] e1000: fix semaphore sync issues
@ 2015-05-19 17:01 Tim Harvey
  2015-05-19 17:01 ` [U-Boot] [Patch 1/3] e1000: releasing semaphore once no longer needed Tim Harvey
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Tim Harvey @ 2015-05-19 17:01 UTC (permalink / raw)
  To: u-boot

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(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [Patch 1/3] e1000: releasing semaphore once no longer needed
  2015-05-19 17:01 [U-Boot] [PATCH 0/3] e1000: fix semaphore sync issues Tim Harvey
@ 2015-05-19 17:01 ` Tim Harvey
  2015-05-20 10:06   ` Bin Meng
                     ` (2 more replies)
  2015-05-19 17:01 ` [U-Boot] [Patch 2/3] Revert "e1000: fix sw fw sync on igb i210/i211" Tim Harvey
  2015-05-19 17:01 ` [U-Boot] [Patch 3/3] e1000: remove unnecessary clearing of SWSM.SWSM_SMBI Tim Harvey
  2 siblings, 3 replies; 20+ messages in thread
From: Tim Harvey @ 2015-05-19 17:01 UTC (permalink / raw)
  To: u-boot

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)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [Patch 2/3] Revert "e1000: fix sw fw sync on igb i210/i211"
  2015-05-19 17:01 [U-Boot] [PATCH 0/3] e1000: fix semaphore sync issues Tim Harvey
  2015-05-19 17:01 ` [U-Boot] [Patch 1/3] e1000: releasing semaphore once no longer needed Tim Harvey
@ 2015-05-19 17:01 ` Tim Harvey
  2015-05-20 10:06   ` Bin Meng
                     ` (2 more replies)
  2015-05-19 17:01 ` [U-Boot] [Patch 3/3] e1000: remove unnecessary clearing of SWSM.SWSM_SMBI Tim Harvey
  2 siblings, 3 replies; 20+ messages in thread
From: Tim Harvey @ 2015-05-19 17:01 UTC (permalink / raw)
  To: u-boot

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 */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [Patch 3/3] e1000: remove unnecessary clearing of SWSM.SWSM_SMBI
  2015-05-19 17:01 [U-Boot] [PATCH 0/3] e1000: fix semaphore sync issues Tim Harvey
  2015-05-19 17:01 ` [U-Boot] [Patch 1/3] e1000: releasing semaphore once no longer needed Tim Harvey
  2015-05-19 17:01 ` [U-Boot] [Patch 2/3] Revert "e1000: fix sw fw sync on igb i210/i211" Tim Harvey
@ 2015-05-19 17:01 ` Tim Harvey
  2015-05-20 10:07   ` Bin Meng
                     ` (2 more replies)
  2 siblings, 3 replies; 20+ messages in thread
From: Tim Harvey @ 2015-05-19 17:01 UTC (permalink / raw)
  To: u-boot

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;
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [Patch 1/3] e1000: releasing semaphore once no longer needed
  2015-05-19 17:01 ` [U-Boot] [Patch 1/3] e1000: releasing semaphore once no longer needed Tim Harvey
@ 2015-05-20 10:06   ` Bin Meng
  2015-05-20 11:26   ` Marcel Ziswiler
  2015-08-12 19:31   ` Joe Hershberger
  2 siblings, 0 replies; 20+ messages in thread
From: Bin Meng @ 2015-05-20 10:06 UTC (permalink / raw)
  To: u-boot

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>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [Patch 2/3] Revert "e1000: fix sw fw sync on igb i210/i211"
  2015-05-19 17:01 ` [U-Boot] [Patch 2/3] Revert "e1000: fix sw fw sync on igb i210/i211" Tim Harvey
@ 2015-05-20 10:06   ` Bin Meng
  2015-05-20 11:27   ` Marcel Ziswiler
  2015-08-12 19:31   ` Joe Hershberger
  2 siblings, 0 replies; 20+ messages in thread
From: Bin Meng @ 2015-05-20 10:06 UTC (permalink / raw)
  To: u-boot

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);
> -               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 */
> --

Tested on QEMU v2.3.0

Tested-by: Bin Meng <bmeng.cn@gmail.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [Patch 3/3] e1000: remove unnecessary clearing of SWSM.SWSM_SMBI
  2015-05-19 17:01 ` [U-Boot] [Patch 3/3] e1000: remove unnecessary clearing of SWSM.SWSM_SMBI Tim Harvey
@ 2015-05-20 10:07   ` Bin Meng
  2015-05-20 11:27   ` Marcel Ziswiler
  2015-08-12 19:32   ` Joe Hershberger
  2 siblings, 0 replies; 20+ messages in thread
From: Bin Meng @ 2015-05-20 10:07 UTC (permalink / raw)
  To: u-boot

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>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [Patch 1/3] e1000: releasing semaphore once no longer needed
  2015-05-19 17:01 ` [U-Boot] [Patch 1/3] e1000: releasing semaphore once no longer needed Tim Harvey
  2015-05-20 10:06   ` Bin Meng
@ 2015-05-20 11:26   ` Marcel Ziswiler
  2015-08-12 19:31   ` Joe Hershberger
  2 siblings, 0 replies; 20+ messages in thread
From: Marcel Ziswiler @ 2015-05-20 11:26 UTC (permalink / raw)
  To: u-boot

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&id=2d8ea651b6da79047b6fa729863d25b5eb9e15d7

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [Patch 2/3] Revert "e1000: fix sw fw sync on igb i210/i211"
  2015-05-19 17:01 ` [U-Boot] [Patch 2/3] Revert "e1000: fix sw fw sync on igb i210/i211" Tim Harvey
  2015-05-20 10:06   ` Bin Meng
@ 2015-05-20 11:27   ` Marcel Ziswiler
  2015-05-20 13:15     ` Tim Harvey
  2015-08-12 19:31   ` Joe Hershberger
  2 siblings, 1 reply; 20+ messages in thread
From: Marcel Ziswiler @ 2015-05-20 11:27 UTC (permalink / raw)
  To: u-boot

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);
> -		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 */

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [Patch 3/3] e1000: remove unnecessary clearing of SWSM.SWSM_SMBI
  2015-05-19 17:01 ` [U-Boot] [Patch 3/3] e1000: remove unnecessary clearing of SWSM.SWSM_SMBI Tim Harvey
  2015-05-20 10:07   ` Bin Meng
@ 2015-05-20 11:27   ` Marcel Ziswiler
  2015-08-12 19:32   ` Joe Hershberger
  2 siblings, 0 replies; 20+ messages in thread
From: Marcel Ziswiler @ 2015-05-20 11:27 UTC (permalink / raw)
  To: u-boot

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&id=2d8ea651b6da79047b6fa729863d25b5eb9e15d7

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [Patch 2/3] Revert "e1000: fix sw fw sync on igb i210/i211"
  2015-05-20 11:27   ` Marcel Ziswiler
@ 2015-05-20 13:15     ` Tim Harvey
  2015-05-20 14:14       ` Marcel Ziswiler
  0 siblings, 1 reply; 20+ messages in thread
From: Tim Harvey @ 2015-05-20 13:15 UTC (permalink / raw)
  To: u-boot

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&id=2d8ea651b6da79047b6fa729863d25b5eb9e15d7
>

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [Patch 2/3] Revert "e1000: fix sw fw sync on igb i210/i211"
  2015-05-20 13:15     ` Tim Harvey
@ 2015-05-20 14:14       ` Marcel Ziswiler
  2015-05-20 15:00         ` Tim Harvey
  0 siblings, 1 reply; 20+ messages in thread
From: Marcel Ziswiler @ 2015-05-20 14:14 UTC (permalink / raw)
  To: u-boot

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.

> 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=c4c3c7449bdb15c53bfebb0a29c73b24ea810d23

Plus the tools only PCI IDs are anyway missing in Linux:
http://git.toradex.com/cgit/linux-toradex.git/commit/?h=tegra&id=2c7123458270c9b3ec9b5ed668f9d55a7f8dbad9

> 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.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [Patch 2/3] Revert "e1000: fix sw fw sync on igb i210/i211"
  2015-05-20 14:14       ` Marcel Ziswiler
@ 2015-05-20 15:00         ` Tim Harvey
  2015-07-10 15:47           ` Tim Harvey
  0 siblings, 1 reply; 20+ messages in thread
From: Tim Harvey @ 2015-05-20 15:00 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [Patch 2/3] Revert "e1000: fix sw fw sync on igb i210/i211"
  2015-05-20 15:00         ` Tim Harvey
@ 2015-07-10 15:47           ` Tim Harvey
  2015-07-11 22:11             ` Marcel Ziswiler
  0 siblings, 1 reply; 20+ messages in thread
From: Tim Harvey @ 2015-07-10 15:47 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [Patch 2/3] Revert "e1000: fix sw fw sync on igb i210/i211"
  2015-07-10 15:47           ` Tim Harvey
@ 2015-07-11 22:11             ` Marcel Ziswiler
  2015-07-13 15:45               ` Tim Harvey
  0 siblings, 1 reply; 20+ messages in thread
From: Marcel Ziswiler @ 2015-07-11 22:11 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [Patch 2/3] Revert "e1000: fix sw fw sync on igb i210/i211"
  2015-07-11 22:11             ` Marcel Ziswiler
@ 2015-07-13 15:45               ` Tim Harvey
  2015-07-28 14:45                 ` Tim Harvey
  0 siblings, 1 reply; 20+ messages in thread
From: Tim Harvey @ 2015-07-13 15:45 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [Patch 2/3] Revert "e1000: fix sw fw sync on igb i210/i211"
  2015-07-13 15:45               ` Tim Harvey
@ 2015-07-28 14:45                 ` Tim Harvey
  0 siblings, 0 replies; 20+ messages in thread
From: Tim Harvey @ 2015-07-28 14:45 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [Patch 1/3] e1000: releasing semaphore once no longer needed
  2015-05-19 17:01 ` [U-Boot] [Patch 1/3] e1000: releasing semaphore once no longer needed Tim Harvey
  2015-05-20 10:06   ` Bin Meng
  2015-05-20 11:26   ` Marcel Ziswiler
@ 2015-08-12 19:31   ` Joe Hershberger
  2 siblings, 0 replies; 20+ messages in thread
From: Joe Hershberger @ 2015-08-12 19:31 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [Patch 2/3] Revert "e1000: fix sw fw sync on igb i210/i211"
  2015-05-19 17:01 ` [U-Boot] [Patch 2/3] Revert "e1000: fix sw fw sync on igb i210/i211" Tim Harvey
  2015-05-20 10:06   ` Bin Meng
  2015-05-20 11:27   ` Marcel Ziswiler
@ 2015-08-12 19:31   ` Joe Hershberger
  2 siblings, 0 replies; 20+ messages in thread
From: Joe Hershberger @ 2015-08-12 19:31 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [Patch 3/3] e1000: remove unnecessary clearing of SWSM.SWSM_SMBI
  2015-05-19 17:01 ` [U-Boot] [Patch 3/3] e1000: remove unnecessary clearing of SWSM.SWSM_SMBI Tim Harvey
  2015-05-20 10:07   ` Bin Meng
  2015-05-20 11:27   ` Marcel Ziswiler
@ 2015-08-12 19:32   ` Joe Hershberger
  2 siblings, 0 replies; 20+ messages in thread
From: Joe Hershberger @ 2015-08-12 19:32 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2015-08-12 19:32 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-19 17:01 [U-Boot] [PATCH 0/3] e1000: fix semaphore sync issues Tim Harvey
2015-05-19 17:01 ` [U-Boot] [Patch 1/3] e1000: releasing semaphore once no longer needed Tim Harvey
2015-05-20 10:06   ` Bin Meng
2015-05-20 11:26   ` Marcel Ziswiler
2015-08-12 19:31   ` Joe Hershberger
2015-05-19 17:01 ` [U-Boot] [Patch 2/3] Revert "e1000: fix sw fw sync on igb i210/i211" Tim Harvey
2015-05-20 10:06   ` Bin Meng
2015-05-20 11:27   ` Marcel Ziswiler
2015-05-20 13:15     ` Tim Harvey
2015-05-20 14:14       ` Marcel Ziswiler
2015-05-20 15:00         ` Tim Harvey
2015-07-10 15:47           ` Tim Harvey
2015-07-11 22:11             ` Marcel Ziswiler
2015-07-13 15:45               ` Tim Harvey
2015-07-28 14:45                 ` Tim Harvey
2015-08-12 19:31   ` Joe Hershberger
2015-05-19 17:01 ` [U-Boot] [Patch 3/3] e1000: remove unnecessary clearing of SWSM.SWSM_SMBI Tim Harvey
2015-05-20 10:07   ` Bin Meng
2015-05-20 11:27   ` Marcel Ziswiler
2015-08-12 19:32   ` Joe Hershberger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.