All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] tg3: APE heartbeat changes
@ 2018-02-16  4:31 Satish Baddipadige
  2018-02-16 21:26 ` David Miller
  0 siblings, 1 reply; 8+ messages in thread
From: Satish Baddipadige @ 2018-02-16  4:31 UTC (permalink / raw)
  To: davem
  Cc: netdev, michael.chan, prashant, siva.kallam, Prashant Sreedharan,
	Satish Baddipadige

From: Prashant Sreedharan <prashant.sreedharan@broadcom.com>

In ungraceful host shutdown or driver crash case BMC connectivity is
lost. APE firmware is missing the driver state in this
case to keep the BMC connectivity alive.
This patch has below change to address this issue.

Heartbeat mechanism with APE firmware. This heartbeat mechanism
is needed to notify the APE firmware about driver state.

This patch also has the change in wait time for APE event from
1ms to 20ms as there can be some delay in getting response.

Signed-off-by: Prashant Sreedharan <prashant.sreedharan@broadcom.com>
Signed-off-by: Satish Baddipadige <satish.baddipadige@broadcom.com>
Signed-off-by: Siva Reddy Kallam <siva.kallam@broadcom.com>
Acked-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 35 ++++++++++++++++++++++++-----------
 drivers/net/ethernet/broadcom/tg3.h |  5 +++++
 2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index a77ee2f..36d8d1e 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -820,7 +820,7 @@ static int tg3_ape_event_lock(struct tg3 *tp, u32 timeout_us)
 
 		tg3_ape_unlock(tp, TG3_APE_LOCK_MEM);
 
-		udelay(10);
+		usleep_range(10, 20);
 		timeout_us -= (timeout_us > 10) ? 10 : timeout_us;
 	}
 
@@ -922,8 +922,8 @@ static int tg3_ape_send_event(struct tg3 *tp, u32 event)
 	if (!(apedata & APE_FW_STATUS_READY))
 		return -EAGAIN;
 
-	/* Wait for up to 1 millisecond for APE to service previous event. */
-	err = tg3_ape_event_lock(tp, 1000);
+	/* Wait for up to 20 millisecond for APE to service previous event. */
+	err = tg3_ape_event_lock(tp, 20000);
 	if (err)
 		return err;
 
@@ -946,6 +946,7 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
 
 	switch (kind) {
 	case RESET_KIND_INIT:
+		tg3_ape_write32(tp, TG3_APE_HOST_HEARTBEAT_COUNT, tp->ape_hb++);
 		tg3_ape_write32(tp, TG3_APE_HOST_SEG_SIG,
 				APE_HOST_SEG_SIG_MAGIC);
 		tg3_ape_write32(tp, TG3_APE_HOST_SEG_LEN,
@@ -962,13 +963,6 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
 		event = APE_EVENT_STATUS_STATE_START;
 		break;
 	case RESET_KIND_SHUTDOWN:
-		/* With the interface we are currently using,
-		 * APE does not track driver state.  Wiping
-		 * out the HOST SEGMENT SIGNATURE forces
-		 * the APE to assume OS absent status.
-		 */
-		tg3_ape_write32(tp, TG3_APE_HOST_SEG_SIG, 0x0);
-
 		if (device_may_wakeup(&tp->pdev->dev) &&
 		    tg3_flag(tp, WOL_ENABLE)) {
 			tg3_ape_write32(tp, TG3_APE_HOST_WOL_SPEED,
@@ -990,6 +984,18 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
 	tg3_ape_send_event(tp, event);
 }
 
+static inline void tg3_send_ape_heartbeat(struct tg3 *tp,
+					  unsigned long interval)
+{
+	/* Check if hb interval has exceeded */
+	if (!tg3_flag(tp, ENABLE_APE) ||
+	    time_before(jiffies, tp->ape_hb_jiffies + interval))
+		return;
+
+	tg3_ape_write32(tp, TG3_APE_HOST_HEARTBEAT_COUNT, tp->ape_hb++);
+	tp->ape_hb_jiffies = jiffies;
+}
+
 static void tg3_disable_ints(struct tg3 *tp)
 {
 	int i;
@@ -7262,6 +7268,7 @@ static int tg3_poll_msix(struct napi_struct *napi, int budget)
 		}
 	}
 
+	tg3_send_ape_heartbeat(tp, TG3_APE_HB_INTERVAL << 1);
 	return work_done;
 
 tx_recovery:
@@ -7344,6 +7351,7 @@ static int tg3_poll(struct napi_struct *napi, int budget)
 		}
 	}
 
+	tg3_send_ape_heartbeat(tp, TG3_APE_HB_INTERVAL << 1);
 	return work_done;
 
 tx_recovery:
@@ -10732,7 +10740,7 @@ static int tg3_reset_hw(struct tg3 *tp, bool reset_phy)
 	if (tg3_flag(tp, ENABLE_APE))
 		/* Write our heartbeat update interval to APE. */
 		tg3_ape_write32(tp, TG3_APE_HOST_HEARTBEAT_INT_MS,
-				APE_HOST_HEARTBEAT_INT_DISABLE);
+				APE_HOST_HEARTBEAT_INT_5SEC);
 
 	tg3_write_sig_post_reset(tp, RESET_KIND_INIT);
 
@@ -11077,6 +11085,9 @@ static void tg3_timer(struct timer_list *t)
 		tp->asf_counter = tp->asf_multiplier;
 	}
 
+	/* Update the APE heartbeat every 5 seconds.*/
+	tg3_send_ape_heartbeat(tp, TG3_APE_HB_INTERVAL);
+
 	spin_unlock(&tp->lock);
 
 restart_timer:
@@ -16653,6 +16664,8 @@ static int tg3_get_invariants(struct tg3 *tp, const struct pci_device_id *ent)
 				       pci_state_reg);
 
 		tg3_ape_lock_init(tp);
+		tp->ape_hb_interval =
+			msecs_to_jiffies(APE_HOST_HEARTBEAT_INT_5SEC);
 	}
 
 	/* Set up tp->grc_local_ctrl before calling
diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
index 47f51cc..1d61aa3 100644
--- a/drivers/net/ethernet/broadcom/tg3.h
+++ b/drivers/net/ethernet/broadcom/tg3.h
@@ -2508,6 +2508,7 @@
 #define TG3_APE_LOCK_PHY3		5
 #define TG3_APE_LOCK_GPIO		7
 
+#define TG3_APE_HB_INTERVAL             (tp->ape_hb_interval)
 #define TG3_EEPROM_SB_F1R2_MBA_OFF	0x10
 
 
@@ -3423,6 +3424,10 @@ struct tg3 {
 	struct device			*hwmon_dev;
 	bool				link_up;
 	bool				pcierr_recovery;
+
+	u32                             ape_hb;
+	unsigned long                   ape_hb_interval;
+	unsigned long                   ape_hb_jiffies;
 };
 
 /* Accessor macros for chip and asic attributes
-- 
2.1.0

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

* Re: [PATCH net] tg3: APE heartbeat changes
  2018-02-16  4:31 [PATCH net] tg3: APE heartbeat changes Satish Baddipadige
@ 2018-02-16 21:26 ` David Miller
  0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2018-02-16 21:26 UTC (permalink / raw)
  To: satish.baddipadige
  Cc: netdev, michael.chan, prashant, siva.kallam, prashant.sreedharan

From: Satish Baddipadige <satish.baddipadige@broadcom.com>
Date: Fri, 16 Feb 2018 10:01:29 +0530

> @@ -990,6 +984,18 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
>  	tg3_ape_send_event(tp, event);
>  }
>  
> +static inline void tg3_send_ape_heartbeat(struct tg3 *tp,

Inline functions are not appropriate in foo.c files, please drop the
inline keyword and let the compiler device.

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

* Re: [PATCH net] tg3: APE heartbeat changes
  2018-02-19  6:57 Satish Baddipadige
@ 2018-02-19 19:17 ` David Miller
  0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2018-02-19 19:17 UTC (permalink / raw)
  To: satish.baddipadige
  Cc: netdev, michael.chan, prashant, siva.kallam, prashant.sreedharan

From: Satish Baddipadige <satish.baddipadige@broadcom.com>
Date: Mon, 19 Feb 2018 12:27:04 +0530

> From: Prashant Sreedharan <prashant.sreedharan@broadcom.com>
> 
> In ungraceful host shutdown or driver crash case BMC connectivity is
> lost. APE firmware is missing the driver state in this
> case to keep the BMC connectivity alive.
> This patch has below change to address this issue.
> 
> Heartbeat mechanism with APE firmware. This heartbeat mechanism
> is needed to notify the APE firmware about driver state.
> 
> This patch also has the change in wait time for APE event from
> 1ms to 20ms as there can be some delay in getting response.
> 
> v2: Drop inline keyword as per David suggestion.
> 
> Signed-off-by: Prashant Sreedharan <prashant.sreedharan@broadcom.com>
> Signed-off-by: Satish Baddipadige <satish.baddipadige@broadcom.com>
> Signed-off-by: Siva Reddy Kallam <siva.kallam@broadcom.com>
> Acked-by: Michael Chan <michael.chan@broadcom.com>

Looks good, applied, thank you.

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

* [PATCH net] tg3: APE heartbeat changes
@ 2018-02-19  6:57 Satish Baddipadige
  2018-02-19 19:17 ` David Miller
  0 siblings, 1 reply; 8+ messages in thread
From: Satish Baddipadige @ 2018-02-19  6:57 UTC (permalink / raw)
  To: davem
  Cc: netdev, michael.chan, prashant, siva.kallam, Prashant Sreedharan,
	Satish Baddipadige

From: Prashant Sreedharan <prashant.sreedharan@broadcom.com>

In ungraceful host shutdown or driver crash case BMC connectivity is
lost. APE firmware is missing the driver state in this
case to keep the BMC connectivity alive.
This patch has below change to address this issue.

Heartbeat mechanism with APE firmware. This heartbeat mechanism
is needed to notify the APE firmware about driver state.

This patch also has the change in wait time for APE event from
1ms to 20ms as there can be some delay in getting response.

v2: Drop inline keyword as per David suggestion.

Signed-off-by: Prashant Sreedharan <prashant.sreedharan@broadcom.com>
Signed-off-by: Satish Baddipadige <satish.baddipadige@broadcom.com>
Signed-off-by: Siva Reddy Kallam <siva.kallam@broadcom.com>
Acked-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 35 ++++++++++++++++++++++++-----------
 drivers/net/ethernet/broadcom/tg3.h |  5 +++++
 2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index a77ee2f..c1841db 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -820,7 +820,7 @@ static int tg3_ape_event_lock(struct tg3 *tp, u32 timeout_us)
 
 		tg3_ape_unlock(tp, TG3_APE_LOCK_MEM);
 
-		udelay(10);
+		usleep_range(10, 20);
 		timeout_us -= (timeout_us > 10) ? 10 : timeout_us;
 	}
 
@@ -922,8 +922,8 @@ static int tg3_ape_send_event(struct tg3 *tp, u32 event)
 	if (!(apedata & APE_FW_STATUS_READY))
 		return -EAGAIN;
 
-	/* Wait for up to 1 millisecond for APE to service previous event. */
-	err = tg3_ape_event_lock(tp, 1000);
+	/* Wait for up to 20 millisecond for APE to service previous event. */
+	err = tg3_ape_event_lock(tp, 20000);
 	if (err)
 		return err;
 
@@ -946,6 +946,7 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
 
 	switch (kind) {
 	case RESET_KIND_INIT:
+		tg3_ape_write32(tp, TG3_APE_HOST_HEARTBEAT_COUNT, tp->ape_hb++);
 		tg3_ape_write32(tp, TG3_APE_HOST_SEG_SIG,
 				APE_HOST_SEG_SIG_MAGIC);
 		tg3_ape_write32(tp, TG3_APE_HOST_SEG_LEN,
@@ -962,13 +963,6 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
 		event = APE_EVENT_STATUS_STATE_START;
 		break;
 	case RESET_KIND_SHUTDOWN:
-		/* With the interface we are currently using,
-		 * APE does not track driver state.  Wiping
-		 * out the HOST SEGMENT SIGNATURE forces
-		 * the APE to assume OS absent status.
-		 */
-		tg3_ape_write32(tp, TG3_APE_HOST_SEG_SIG, 0x0);
-
 		if (device_may_wakeup(&tp->pdev->dev) &&
 		    tg3_flag(tp, WOL_ENABLE)) {
 			tg3_ape_write32(tp, TG3_APE_HOST_WOL_SPEED,
@@ -990,6 +984,18 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
 	tg3_ape_send_event(tp, event);
 }
 
+static void tg3_send_ape_heartbeat(struct tg3 *tp,
+				   unsigned long interval)
+{
+	/* Check if hb interval has exceeded */
+	if (!tg3_flag(tp, ENABLE_APE) ||
+	    time_before(jiffies, tp->ape_hb_jiffies + interval))
+		return;
+
+	tg3_ape_write32(tp, TG3_APE_HOST_HEARTBEAT_COUNT, tp->ape_hb++);
+	tp->ape_hb_jiffies = jiffies;
+}
+
 static void tg3_disable_ints(struct tg3 *tp)
 {
 	int i;
@@ -7262,6 +7268,7 @@ static int tg3_poll_msix(struct napi_struct *napi, int budget)
 		}
 	}
 
+	tg3_send_ape_heartbeat(tp, TG3_APE_HB_INTERVAL << 1);
 	return work_done;
 
 tx_recovery:
@@ -7344,6 +7351,7 @@ static int tg3_poll(struct napi_struct *napi, int budget)
 		}
 	}
 
+	tg3_send_ape_heartbeat(tp, TG3_APE_HB_INTERVAL << 1);
 	return work_done;
 
 tx_recovery:
@@ -10732,7 +10740,7 @@ static int tg3_reset_hw(struct tg3 *tp, bool reset_phy)
 	if (tg3_flag(tp, ENABLE_APE))
 		/* Write our heartbeat update interval to APE. */
 		tg3_ape_write32(tp, TG3_APE_HOST_HEARTBEAT_INT_MS,
-				APE_HOST_HEARTBEAT_INT_DISABLE);
+				APE_HOST_HEARTBEAT_INT_5SEC);
 
 	tg3_write_sig_post_reset(tp, RESET_KIND_INIT);
 
@@ -11077,6 +11085,9 @@ static void tg3_timer(struct timer_list *t)
 		tp->asf_counter = tp->asf_multiplier;
 	}
 
+	/* Update the APE heartbeat every 5 seconds.*/
+	tg3_send_ape_heartbeat(tp, TG3_APE_HB_INTERVAL);
+
 	spin_unlock(&tp->lock);
 
 restart_timer:
@@ -16653,6 +16664,8 @@ static int tg3_get_invariants(struct tg3 *tp, const struct pci_device_id *ent)
 				       pci_state_reg);
 
 		tg3_ape_lock_init(tp);
+		tp->ape_hb_interval =
+			msecs_to_jiffies(APE_HOST_HEARTBEAT_INT_5SEC);
 	}
 
 	/* Set up tp->grc_local_ctrl before calling
diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
index 47f51cc..1d61aa3 100644
--- a/drivers/net/ethernet/broadcom/tg3.h
+++ b/drivers/net/ethernet/broadcom/tg3.h
@@ -2508,6 +2508,7 @@
 #define TG3_APE_LOCK_PHY3		5
 #define TG3_APE_LOCK_GPIO		7
 
+#define TG3_APE_HB_INTERVAL             (tp->ape_hb_interval)
 #define TG3_EEPROM_SB_F1R2_MBA_OFF	0x10
 
 
@@ -3423,6 +3424,10 @@ struct tg3 {
 	struct device			*hwmon_dev;
 	bool				link_up;
 	bool				pcierr_recovery;
+
+	u32                             ape_hb;
+	unsigned long                   ape_hb_interval;
+	unsigned long                   ape_hb_jiffies;
 };
 
 /* Accessor macros for chip and asic attributes
-- 
2.1.0

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

* Re: [PATCH net] tg3: APE heartbeat changes
@ 2018-02-19  6:42 Satish Baddipadige
  0 siblings, 0 replies; 8+ messages in thread
From: Satish Baddipadige @ 2018-02-19  6:42 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, Michael Chan, Prashant Sreedharan,
	Siva Reddy (Siva) Kallam, Prashant Sreedharan

On Sat, Feb 17, 2018 at 2:56 AM, David Miller <davem@davemloft.net> wrote:
>
> From: Satish Baddipadige <satish.baddipadige@broadcom.com>
> Date: Fri, 16 Feb 2018 10:01:29 +0530
>
> > @@ -990,6 +984,18 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
> >       tg3_ape_send_event(tp, event);
> >  }
> >
> > +static inline void tg3_send_ape_heartbeat(struct tg3 *tp,
>
> Inline functions are not appropriate in foo.c files, please drop the
> inline keyword and let the compiler device.

Thanks David. Will send a v2 patch with inline dropped.

Thanks,
Satish

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

* Re: [PATCH net] tg3: APE heartbeat changes
  2018-02-16  5:52 Satish Baddipadige
@ 2018-02-16 13:40 ` Mauro Rodrigues
  0 siblings, 0 replies; 8+ messages in thread
From: Mauro Rodrigues @ 2018-02-16 13:40 UTC (permalink / raw)
  To: Satish Baddipadige
  Cc: davem, netdev, michael.chan, prashant, siva.kallam, Prashant Sreedharan

On Fri, Feb 16, 2018 at 11:22:43AM +0530, Satish Baddipadige wrote:
> From: Prashant Sreedharan <prashant.sreedharan@broadcom.com>
> 
> In ungraceful host shutdown or driver crash case BMC connectivity is
> lost. APE firmware is missing the driver state in this
> case to keep the BMC connectivity alive.
> This patch has below change to address this issue.
> 
> Heartbeat mechanism with APE firmware. This heartbeat mechanism
> is needed to notify the APE firmware about driver state.
> 
> This patch also has the change in wait time for APE event from
> 1ms to 20ms as there can be some delay in getting response.
> 
> Signed-off-by: Prashant Sreedharan <prashant.sreedharan@broadcom.com>
> Signed-off-by: Satish Baddipadige <satish.baddipadige@broadcom.com>
> Signed-off-by: Siva Reddy Kallam <siva.kallam@broadcom.com>
> Acked-by: Michael Chan <michael.chan@broadcom.com>
Tested-by: Mauro S. M. Rodrigues <maurosr@linux.vnet.ibm.com>

Thank you very much for the patch Satish.
> ---
>  drivers/net/ethernet/broadcom/tg3.c | 35 ++++++++++++++++++++++++-----------
>  drivers/net/ethernet/broadcom/tg3.h |  5 +++++
>  2 files changed, 29 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index a77ee2f..36d8d1e 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -820,7 +820,7 @@ static int tg3_ape_event_lock(struct tg3 *tp, u32 timeout_us)
> 
>  		tg3_ape_unlock(tp, TG3_APE_LOCK_MEM);
> 
> -		udelay(10);
> +		usleep_range(10, 20);
>  		timeout_us -= (timeout_us > 10) ? 10 : timeout_us;
>  	}
> 
> @@ -922,8 +922,8 @@ static int tg3_ape_send_event(struct tg3 *tp, u32 event)
>  	if (!(apedata & APE_FW_STATUS_READY))
>  		return -EAGAIN;
> 
> -	/* Wait for up to 1 millisecond for APE to service previous event. */
> -	err = tg3_ape_event_lock(tp, 1000);
> +	/* Wait for up to 20 millisecond for APE to service previous event. */
> +	err = tg3_ape_event_lock(tp, 20000);
>  	if (err)
>  		return err;
> 
> @@ -946,6 +946,7 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
> 
>  	switch (kind) {
>  	case RESET_KIND_INIT:
> +		tg3_ape_write32(tp, TG3_APE_HOST_HEARTBEAT_COUNT, tp->ape_hb++);
>  		tg3_ape_write32(tp, TG3_APE_HOST_SEG_SIG,
>  				APE_HOST_SEG_SIG_MAGIC);
>  		tg3_ape_write32(tp, TG3_APE_HOST_SEG_LEN,
> @@ -962,13 +963,6 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
>  		event = APE_EVENT_STATUS_STATE_START;
>  		break;
>  	case RESET_KIND_SHUTDOWN:
> -		/* With the interface we are currently using,
> -		 * APE does not track driver state.  Wiping
> -		 * out the HOST SEGMENT SIGNATURE forces
> -		 * the APE to assume OS absent status.
> -		 */
> -		tg3_ape_write32(tp, TG3_APE_HOST_SEG_SIG, 0x0);
> -
>  		if (device_may_wakeup(&tp->pdev->dev) &&
>  		    tg3_flag(tp, WOL_ENABLE)) {
>  			tg3_ape_write32(tp, TG3_APE_HOST_WOL_SPEED,
> @@ -990,6 +984,18 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
>  	tg3_ape_send_event(tp, event);
>  }
> 
> +static inline void tg3_send_ape_heartbeat(struct tg3 *tp,
> +					  unsigned long interval)
> +{
> +	/* Check if hb interval has exceeded */
> +	if (!tg3_flag(tp, ENABLE_APE) ||
> +	    time_before(jiffies, tp->ape_hb_jiffies + interval))
> +		return;
> +
> +	tg3_ape_write32(tp, TG3_APE_HOST_HEARTBEAT_COUNT, tp->ape_hb++);
> +	tp->ape_hb_jiffies = jiffies;
> +}
> +
>  static void tg3_disable_ints(struct tg3 *tp)
>  {
>  	int i;
> @@ -7262,6 +7268,7 @@ static int tg3_poll_msix(struct napi_struct *napi, int budget)
>  		}
>  	}
> 
> +	tg3_send_ape_heartbeat(tp, TG3_APE_HB_INTERVAL << 1);
>  	return work_done;
> 
>  tx_recovery:
> @@ -7344,6 +7351,7 @@ static int tg3_poll(struct napi_struct *napi, int budget)
>  		}
>  	}
> 
> +	tg3_send_ape_heartbeat(tp, TG3_APE_HB_INTERVAL << 1);
>  	return work_done;
> 
>  tx_recovery:
> @@ -10732,7 +10740,7 @@ static int tg3_reset_hw(struct tg3 *tp, bool reset_phy)
>  	if (tg3_flag(tp, ENABLE_APE))
>  		/* Write our heartbeat update interval to APE. */
>  		tg3_ape_write32(tp, TG3_APE_HOST_HEARTBEAT_INT_MS,
> -				APE_HOST_HEARTBEAT_INT_DISABLE);
> +				APE_HOST_HEARTBEAT_INT_5SEC);
> 
>  	tg3_write_sig_post_reset(tp, RESET_KIND_INIT);
> 
> @@ -11077,6 +11085,9 @@ static void tg3_timer(struct timer_list *t)
>  		tp->asf_counter = tp->asf_multiplier;
>  	}
> 
> +	/* Update the APE heartbeat every 5 seconds.*/
> +	tg3_send_ape_heartbeat(tp, TG3_APE_HB_INTERVAL);
> +
>  	spin_unlock(&tp->lock);
> 
>  restart_timer:
> @@ -16653,6 +16664,8 @@ static int tg3_get_invariants(struct tg3 *tp, const struct pci_device_id *ent)
>  				       pci_state_reg);
> 
>  		tg3_ape_lock_init(tp);
> +		tp->ape_hb_interval =
> +			msecs_to_jiffies(APE_HOST_HEARTBEAT_INT_5SEC);
>  	}
> 
>  	/* Set up tp->grc_local_ctrl before calling
> diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
> index 47f51cc..1d61aa3 100644
> --- a/drivers/net/ethernet/broadcom/tg3.h
> +++ b/drivers/net/ethernet/broadcom/tg3.h
> @@ -2508,6 +2508,7 @@
>  #define TG3_APE_LOCK_PHY3		5
>  #define TG3_APE_LOCK_GPIO		7
> 
> +#define TG3_APE_HB_INTERVAL             (tp->ape_hb_interval)
>  #define TG3_EEPROM_SB_F1R2_MBA_OFF	0x10
> 
> 
> @@ -3423,6 +3424,10 @@ struct tg3 {
>  	struct device			*hwmon_dev;
>  	bool				link_up;
>  	bool				pcierr_recovery;
> +
> +	u32                             ape_hb;
> +	unsigned long                   ape_hb_interval;
> +	unsigned long                   ape_hb_jiffies;
>  };
> 
>  /* Accessor macros for chip and asic attributes
> -- 
> 2.1.0
> 

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

* [PATCH net] tg3: APE heartbeat changes
@ 2018-02-16  5:52 Satish Baddipadige
  2018-02-16 13:40 ` Mauro Rodrigues
  0 siblings, 1 reply; 8+ messages in thread
From: Satish Baddipadige @ 2018-02-16  5:52 UTC (permalink / raw)
  To: davem
  Cc: netdev, michael.chan, prashant, siva.kallam, Prashant Sreedharan,
	Satish Baddipadige

From: Prashant Sreedharan <prashant.sreedharan@broadcom.com>

In ungraceful host shutdown or driver crash case BMC connectivity is
lost. APE firmware is missing the driver state in this
case to keep the BMC connectivity alive.
This patch has below change to address this issue.

Heartbeat mechanism with APE firmware. This heartbeat mechanism
is needed to notify the APE firmware about driver state.

This patch also has the change in wait time for APE event from
1ms to 20ms as there can be some delay in getting response.

Signed-off-by: Prashant Sreedharan <prashant.sreedharan@broadcom.com>
Signed-off-by: Satish Baddipadige <satish.baddipadige@broadcom.com>
Signed-off-by: Siva Reddy Kallam <siva.kallam@broadcom.com>
Acked-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 35 ++++++++++++++++++++++++-----------
 drivers/net/ethernet/broadcom/tg3.h |  5 +++++
 2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index a77ee2f..36d8d1e 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -820,7 +820,7 @@ static int tg3_ape_event_lock(struct tg3 *tp, u32 timeout_us)
 
 		tg3_ape_unlock(tp, TG3_APE_LOCK_MEM);
 
-		udelay(10);
+		usleep_range(10, 20);
 		timeout_us -= (timeout_us > 10) ? 10 : timeout_us;
 	}
 
@@ -922,8 +922,8 @@ static int tg3_ape_send_event(struct tg3 *tp, u32 event)
 	if (!(apedata & APE_FW_STATUS_READY))
 		return -EAGAIN;
 
-	/* Wait for up to 1 millisecond for APE to service previous event. */
-	err = tg3_ape_event_lock(tp, 1000);
+	/* Wait for up to 20 millisecond for APE to service previous event. */
+	err = tg3_ape_event_lock(tp, 20000);
 	if (err)
 		return err;
 
@@ -946,6 +946,7 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
 
 	switch (kind) {
 	case RESET_KIND_INIT:
+		tg3_ape_write32(tp, TG3_APE_HOST_HEARTBEAT_COUNT, tp->ape_hb++);
 		tg3_ape_write32(tp, TG3_APE_HOST_SEG_SIG,
 				APE_HOST_SEG_SIG_MAGIC);
 		tg3_ape_write32(tp, TG3_APE_HOST_SEG_LEN,
@@ -962,13 +963,6 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
 		event = APE_EVENT_STATUS_STATE_START;
 		break;
 	case RESET_KIND_SHUTDOWN:
-		/* With the interface we are currently using,
-		 * APE does not track driver state.  Wiping
-		 * out the HOST SEGMENT SIGNATURE forces
-		 * the APE to assume OS absent status.
-		 */
-		tg3_ape_write32(tp, TG3_APE_HOST_SEG_SIG, 0x0);
-
 		if (device_may_wakeup(&tp->pdev->dev) &&
 		    tg3_flag(tp, WOL_ENABLE)) {
 			tg3_ape_write32(tp, TG3_APE_HOST_WOL_SPEED,
@@ -990,6 +984,18 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
 	tg3_ape_send_event(tp, event);
 }
 
+static inline void tg3_send_ape_heartbeat(struct tg3 *tp,
+					  unsigned long interval)
+{
+	/* Check if hb interval has exceeded */
+	if (!tg3_flag(tp, ENABLE_APE) ||
+	    time_before(jiffies, tp->ape_hb_jiffies + interval))
+		return;
+
+	tg3_ape_write32(tp, TG3_APE_HOST_HEARTBEAT_COUNT, tp->ape_hb++);
+	tp->ape_hb_jiffies = jiffies;
+}
+
 static void tg3_disable_ints(struct tg3 *tp)
 {
 	int i;
@@ -7262,6 +7268,7 @@ static int tg3_poll_msix(struct napi_struct *napi, int budget)
 		}
 	}
 
+	tg3_send_ape_heartbeat(tp, TG3_APE_HB_INTERVAL << 1);
 	return work_done;
 
 tx_recovery:
@@ -7344,6 +7351,7 @@ static int tg3_poll(struct napi_struct *napi, int budget)
 		}
 	}
 
+	tg3_send_ape_heartbeat(tp, TG3_APE_HB_INTERVAL << 1);
 	return work_done;
 
 tx_recovery:
@@ -10732,7 +10740,7 @@ static int tg3_reset_hw(struct tg3 *tp, bool reset_phy)
 	if (tg3_flag(tp, ENABLE_APE))
 		/* Write our heartbeat update interval to APE. */
 		tg3_ape_write32(tp, TG3_APE_HOST_HEARTBEAT_INT_MS,
-				APE_HOST_HEARTBEAT_INT_DISABLE);
+				APE_HOST_HEARTBEAT_INT_5SEC);
 
 	tg3_write_sig_post_reset(tp, RESET_KIND_INIT);
 
@@ -11077,6 +11085,9 @@ static void tg3_timer(struct timer_list *t)
 		tp->asf_counter = tp->asf_multiplier;
 	}
 
+	/* Update the APE heartbeat every 5 seconds.*/
+	tg3_send_ape_heartbeat(tp, TG3_APE_HB_INTERVAL);
+
 	spin_unlock(&tp->lock);
 
 restart_timer:
@@ -16653,6 +16664,8 @@ static int tg3_get_invariants(struct tg3 *tp, const struct pci_device_id *ent)
 				       pci_state_reg);
 
 		tg3_ape_lock_init(tp);
+		tp->ape_hb_interval =
+			msecs_to_jiffies(APE_HOST_HEARTBEAT_INT_5SEC);
 	}
 
 	/* Set up tp->grc_local_ctrl before calling
diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
index 47f51cc..1d61aa3 100644
--- a/drivers/net/ethernet/broadcom/tg3.h
+++ b/drivers/net/ethernet/broadcom/tg3.h
@@ -2508,6 +2508,7 @@
 #define TG3_APE_LOCK_PHY3		5
 #define TG3_APE_LOCK_GPIO		7
 
+#define TG3_APE_HB_INTERVAL             (tp->ape_hb_interval)
 #define TG3_EEPROM_SB_F1R2_MBA_OFF	0x10
 
 
@@ -3423,6 +3424,10 @@ struct tg3 {
 	struct device			*hwmon_dev;
 	bool				link_up;
 	bool				pcierr_recovery;
+
+	u32                             ape_hb;
+	unsigned long                   ape_hb_interval;
+	unsigned long                   ape_hb_jiffies;
 };
 
 /* Accessor macros for chip and asic attributes
-- 
2.1.0

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

* [PATCH net] tg3: APE heartbeat changes
@ 2018-02-16  5:16 Satish Baddipadige
  0 siblings, 0 replies; 8+ messages in thread
From: Satish Baddipadige @ 2018-02-16  5:16 UTC (permalink / raw)
  To: davem
  Cc: netdev, michael.chan, prashant, siva.kallam, Prashant Sreedharan,
	Satish Baddipadige

From: Prashant Sreedharan <prashant.sreedharan@broadcom.com>

In ungraceful host shutdown or driver crash case BMC connectivity is
lost. APE firmware is missing the driver state in this
case to keep the BMC connectivity alive.
This patch has below change to address this issue.

Heartbeat mechanism with APE firmware. This heartbeat mechanism
is needed to notify the APE firmware about driver state.

This patch also has the change in wait time for APE event from
1ms to 20ms as there can be some delay in getting response.

Signed-off-by: Prashant Sreedharan <prashant.sreedharan@broadcom.com>
Signed-off-by: Satish Baddipadige <satish.baddipadige@broadcom.com>
Signed-off-by: Siva Reddy Kallam <siva.kallam@broadcom.com>
Acked-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 35 ++++++++++++++++++++++++-----------
 drivers/net/ethernet/broadcom/tg3.h |  5 +++++
 2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index a77ee2f..36d8d1e 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -820,7 +820,7 @@ static int tg3_ape_event_lock(struct tg3 *tp, u32 timeout_us)
 
 		tg3_ape_unlock(tp, TG3_APE_LOCK_MEM);
 
-		udelay(10);
+		usleep_range(10, 20);
 		timeout_us -= (timeout_us > 10) ? 10 : timeout_us;
 	}
 
@@ -922,8 +922,8 @@ static int tg3_ape_send_event(struct tg3 *tp, u32 event)
 	if (!(apedata & APE_FW_STATUS_READY))
 		return -EAGAIN;
 
-	/* Wait for up to 1 millisecond for APE to service previous event. */
-	err = tg3_ape_event_lock(tp, 1000);
+	/* Wait for up to 20 millisecond for APE to service previous event. */
+	err = tg3_ape_event_lock(tp, 20000);
 	if (err)
 		return err;
 
@@ -946,6 +946,7 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
 
 	switch (kind) {
 	case RESET_KIND_INIT:
+		tg3_ape_write32(tp, TG3_APE_HOST_HEARTBEAT_COUNT, tp->ape_hb++);
 		tg3_ape_write32(tp, TG3_APE_HOST_SEG_SIG,
 				APE_HOST_SEG_SIG_MAGIC);
 		tg3_ape_write32(tp, TG3_APE_HOST_SEG_LEN,
@@ -962,13 +963,6 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
 		event = APE_EVENT_STATUS_STATE_START;
 		break;
 	case RESET_KIND_SHUTDOWN:
-		/* With the interface we are currently using,
-		 * APE does not track driver state.  Wiping
-		 * out the HOST SEGMENT SIGNATURE forces
-		 * the APE to assume OS absent status.
-		 */
-		tg3_ape_write32(tp, TG3_APE_HOST_SEG_SIG, 0x0);
-
 		if (device_may_wakeup(&tp->pdev->dev) &&
 		    tg3_flag(tp, WOL_ENABLE)) {
 			tg3_ape_write32(tp, TG3_APE_HOST_WOL_SPEED,
@@ -990,6 +984,18 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
 	tg3_ape_send_event(tp, event);
 }
 
+static inline void tg3_send_ape_heartbeat(struct tg3 *tp,
+					  unsigned long interval)
+{
+	/* Check if hb interval has exceeded */
+	if (!tg3_flag(tp, ENABLE_APE) ||
+	    time_before(jiffies, tp->ape_hb_jiffies + interval))
+		return;
+
+	tg3_ape_write32(tp, TG3_APE_HOST_HEARTBEAT_COUNT, tp->ape_hb++);
+	tp->ape_hb_jiffies = jiffies;
+}
+
 static void tg3_disable_ints(struct tg3 *tp)
 {
 	int i;
@@ -7262,6 +7268,7 @@ static int tg3_poll_msix(struct napi_struct *napi, int budget)
 		}
 	}
 
+	tg3_send_ape_heartbeat(tp, TG3_APE_HB_INTERVAL << 1);
 	return work_done;
 
 tx_recovery:
@@ -7344,6 +7351,7 @@ static int tg3_poll(struct napi_struct *napi, int budget)
 		}
 	}
 
+	tg3_send_ape_heartbeat(tp, TG3_APE_HB_INTERVAL << 1);
 	return work_done;
 
 tx_recovery:
@@ -10732,7 +10740,7 @@ static int tg3_reset_hw(struct tg3 *tp, bool reset_phy)
 	if (tg3_flag(tp, ENABLE_APE))
 		/* Write our heartbeat update interval to APE. */
 		tg3_ape_write32(tp, TG3_APE_HOST_HEARTBEAT_INT_MS,
-				APE_HOST_HEARTBEAT_INT_DISABLE);
+				APE_HOST_HEARTBEAT_INT_5SEC);
 
 	tg3_write_sig_post_reset(tp, RESET_KIND_INIT);
 
@@ -11077,6 +11085,9 @@ static void tg3_timer(struct timer_list *t)
 		tp->asf_counter = tp->asf_multiplier;
 	}
 
+	/* Update the APE heartbeat every 5 seconds.*/
+	tg3_send_ape_heartbeat(tp, TG3_APE_HB_INTERVAL);
+
 	spin_unlock(&tp->lock);
 
 restart_timer:
@@ -16653,6 +16664,8 @@ static int tg3_get_invariants(struct tg3 *tp, const struct pci_device_id *ent)
 				       pci_state_reg);
 
 		tg3_ape_lock_init(tp);
+		tp->ape_hb_interval =
+			msecs_to_jiffies(APE_HOST_HEARTBEAT_INT_5SEC);
 	}
 
 	/* Set up tp->grc_local_ctrl before calling
diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
index 47f51cc..1d61aa3 100644
--- a/drivers/net/ethernet/broadcom/tg3.h
+++ b/drivers/net/ethernet/broadcom/tg3.h
@@ -2508,6 +2508,7 @@
 #define TG3_APE_LOCK_PHY3		5
 #define TG3_APE_LOCK_GPIO		7
 
+#define TG3_APE_HB_INTERVAL             (tp->ape_hb_interval)
 #define TG3_EEPROM_SB_F1R2_MBA_OFF	0x10
 
 
@@ -3423,6 +3424,10 @@ struct tg3 {
 	struct device			*hwmon_dev;
 	bool				link_up;
 	bool				pcierr_recovery;
+
+	u32                             ape_hb;
+	unsigned long                   ape_hb_interval;
+	unsigned long                   ape_hb_jiffies;
 };
 
 /* Accessor macros for chip and asic attributes
-- 
2.1.0

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

end of thread, other threads:[~2018-02-19 19:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-16  4:31 [PATCH net] tg3: APE heartbeat changes Satish Baddipadige
2018-02-16 21:26 ` David Miller
2018-02-16  5:16 Satish Baddipadige
2018-02-16  5:52 Satish Baddipadige
2018-02-16 13:40 ` Mauro Rodrigues
2018-02-19  6:42 Satish Baddipadige
2018-02-19  6:57 Satish Baddipadige
2018-02-19 19:17 ` David Miller

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.