diff mbox series

[net,v3] bnxt_en: reset PHC frequency in free-running mode

Message ID 20230309215347.2764771-1-vadfed@meta.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net,v3] bnxt_en: reset PHC frequency in free-running mode | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 25 this patch: 25
netdev/cc_maintainers fail 2 blamed authors not CCed: leon@kernel.org pabeni@redhat.com; 4 maintainers not CCed: leon@kernel.org pabeni@redhat.com edumazet@google.com davem@davemloft.net
netdev/build_clang success Errors and warnings before: 20 this patch: 20
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 24 this patch: 24
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 122 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Vadim Fedorenko March 9, 2023, 9:53 p.m. UTC 
When using a PHC in shared between multiple hosts, the previous
frequency value may not be reset and could lead to host being unable to
compensate the offset with timecounter adjustments. To avoid such state
reset the hardware frequency of PHC to zero on init. Some refactoring is
needed to make code readable.

Fixes: 85036aee1938 ("bnxt_en: Add a non-real time mode to access NIC clock")
Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  6 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  2 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c | 60 ++++++++++---------
 3 files changed, 37 insertions(+), 31 deletions(-)

Comments

Pavan Chebbi March 10, 2023, 5:04 a.m. UTC | #1 
On Fri, Mar 10, 2023 at 3:24 AM Vadim Fedorenko <vadfed@meta.com> wrote:
>
> When using a PHC in shared between multiple hosts, the previous
> frequency value may not be reset and could lead to host being unable to
> compensate the offset with timecounter adjustments. To avoid such state
> reset the hardware frequency of PHC to zero on init. Some refactoring is
> needed to make code readable.
>
> Fixes: 85036aee1938 ("bnxt_en: Add a non-real time mode to access NIC clock")
> Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  6 +-
>  drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  2 +
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c | 60 ++++++++++---------
>  3 files changed, 37 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 808236dc898b..e2e2c986c82b 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -6990,11 +6990,9 @@ static int bnxt_hwrm_func_qcfg(struct bnxt *bp)
>                 if (flags & FUNC_QCFG_RESP_FLAGS_FW_DCBX_AGENT_ENABLED)
>                         bp->fw_cap |= BNXT_FW_CAP_DCBX_AGENT;
>         }
> -       if (BNXT_PF(bp) && (flags & FUNC_QCFG_RESP_FLAGS_MULTI_HOST)) {
> +       if (BNXT_PF(bp) && (flags & FUNC_QCFG_RESP_FLAGS_MULTI_HOST))
>                 bp->flags |= BNXT_FLAG_MULTI_HOST;
> -               if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC)
> -                       bp->fw_cap &= ~BNXT_FW_CAP_PTP_RTC;
> -       }
> +
>         if (flags & FUNC_QCFG_RESP_FLAGS_RING_MONITOR_ENABLED)
>                 bp->fw_cap |= BNXT_FW_CAP_RING_MONITOR;
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> index dcb09fbe4007..c0628ac1b798 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> @@ -2000,6 +2000,8 @@ struct bnxt {
>         u32                     fw_dbg_cap;
>
>  #define BNXT_NEW_RM(bp)                ((bp)->fw_cap & BNXT_FW_CAP_NEW_RM)
> +#define BNXT_PTP_USE_RTC(bp)   (!BNXT_MH(bp) && \
> +                                ((bp)->fw_cap & BNXT_FW_CAP_PTP_RTC))
>         u32                     hwrm_spec_code;
>         u16                     hwrm_cmd_seq;
>         u16                     hwrm_cmd_kong_seq;
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> index 4ec8bba18cdd..a96adde97e60 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> @@ -63,7 +63,7 @@ static int bnxt_ptp_settime(struct ptp_clock_info *ptp_info,
>                                                 ptp_info);
>         u64 ns = timespec64_to_ns(ts);
>
> -       if (ptp->bp->fw_cap & BNXT_FW_CAP_PTP_RTC)
> +       if (BNXT_PTP_USE_RTC(ptp->bp))
>                 return bnxt_ptp_cfg_settime(ptp->bp, ns);
>
>         spin_lock_bh(&ptp->ptp_lock);
> @@ -196,7 +196,7 @@ static int bnxt_ptp_adjtime(struct ptp_clock_info *ptp_info, s64 delta)
>         struct bnxt_ptp_cfg *ptp = container_of(ptp_info, struct bnxt_ptp_cfg,
>                                                 ptp_info);
>
> -       if (ptp->bp->fw_cap & BNXT_FW_CAP_PTP_RTC)
> +       if (BNXT_PTP_USE_RTC(ptp->bp))
>                 return bnxt_ptp_adjphc(ptp, delta);
>
>         spin_lock_bh(&ptp->ptp_lock);
> @@ -205,34 +205,39 @@ static int bnxt_ptp_adjtime(struct ptp_clock_info *ptp_info, s64 delta)
>         return 0;
>  }
>
> +static int bnxt_ptp_adjfine_rtc(struct bnxt *bp, long scaled_ppm)
> +{
> +       s32 ppb = scaled_ppm_to_ppb(scaled_ppm);
> +       struct hwrm_port_mac_cfg_input *req;
> +       int rc;
> +
> +       rc = hwrm_req_init(bp, req, HWRM_PORT_MAC_CFG);
> +       if (rc)
> +               return rc;
> +
> +       req->ptp_freq_adj_ppb = cpu_to_le32(ppb);
> +       req->enables = cpu_to_le32(PORT_MAC_CFG_REQ_ENABLES_PTP_FREQ_ADJ_PPB);
> +       rc = hwrm_req_send(bp, req);
> +       if (rc)
> +               netdev_err(bp->dev,
> +                          "ptp adjfine failed. rc = %d\n", rc);
> +       return rc;
> +}
> +
>  static int bnxt_ptp_adjfine(struct ptp_clock_info *ptp_info, long scaled_ppm)
>  {
>         struct bnxt_ptp_cfg *ptp = container_of(ptp_info, struct bnxt_ptp_cfg,
>                                                 ptp_info);
> -       struct hwrm_port_mac_cfg_input *req;
>         struct bnxt *bp = ptp->bp;
> -       int rc = 0;
>
> -       if (!(ptp->bp->fw_cap & BNXT_FW_CAP_PTP_RTC)) {
> -               spin_lock_bh(&ptp->ptp_lock);
> -               timecounter_read(&ptp->tc);
> -               ptp->cc.mult = adjust_by_scaled_ppm(ptp->cmult, scaled_ppm);
> -               spin_unlock_bh(&ptp->ptp_lock);
> -       } else {
> -               s32 ppb = scaled_ppm_to_ppb(scaled_ppm);
> -
> -               rc = hwrm_req_init(bp, req, HWRM_PORT_MAC_CFG);
> -               if (rc)
> -                       return rc;
> +       if (BNXT_PTP_USE_RTC(bp))
> +               return bnxt_ptp_adjfine_rtc(bp, scaled_ppm);
>
> -               req->ptp_freq_adj_ppb = cpu_to_le32(ppb);
> -               req->enables = cpu_to_le32(PORT_MAC_CFG_REQ_ENABLES_PTP_FREQ_ADJ_PPB);
> -               rc = hwrm_req_send(ptp->bp, req);
> -               if (rc)
> -                       netdev_err(ptp->bp->dev,
> -                                  "ptp adjfine failed. rc = %d\n", rc);
> -       }
> -       return rc;
> +       spin_lock_bh(&ptp->ptp_lock);
> +       timecounter_read(&ptp->tc);
> +       ptp->cc.mult = adjust_by_scaled_ppm(ptp->cmult, scaled_ppm);
> +       spin_unlock_bh(&ptp->ptp_lock);
> +       return 0;
>  }
>
>  void bnxt_ptp_pps_event(struct bnxt *bp, u32 data1, u32 data2)
> @@ -879,7 +884,7 @@ int bnxt_ptp_init_rtc(struct bnxt *bp, bool phc_cfg)
>         u64 ns;
>         int rc;
>
> -       if (!bp->ptp_cfg || !(bp->fw_cap & BNXT_FW_CAP_PTP_RTC))
> +       if (!bp->ptp_cfg || !BNXT_PTP_USE_RTC(bp))
>                 return -ENODEV;
>
>         if (!phc_cfg) {
> @@ -932,13 +937,14 @@ int bnxt_ptp_init(struct bnxt *bp, bool phc_cfg)
>         atomic_set(&ptp->tx_avail, BNXT_MAX_TX_TS);
>         spin_lock_init(&ptp->ptp_lock);
>
> -       if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC) {
> +       if (BNXT_MH(bp)) {

Using BNXT_MH(bp) inside bnxt_ptp_init() will not work because
bnxt_ptp_init() is being called much before we determine it is an MH
system.
I may have alluded to this change in v2 comments but I realize that we
don't have the MH determined yet. I am sure your test would not pass
with this patch.
We plan to make that change in the upstream driver to move the
bnxt_ptp_init() to a later point, after we know the type of host.
For now, I think you can simply call bnxt_ptp_adjfine_rtc(bp, 0); when
you see it is a non-RTC host. Much like in v2, except the condition.

if (BNXT_PTP_USE_RTC(ptp->bp)) {
        bnxt_ptp_timecounter_init(bp, false);
        rc = bnxt_ptp_init_rtc(bp, phc_cfg);
        if (rc)
           goto out;
} else {
        bnxt_ptp_timecounter_init(bp, true);
        bnxt_ptp_adjfine_rtc(bp, 0);
}

Rest everything looks OK.

> +               bnxt_ptp_timecounter_init(bp, true);
> +               bnxt_ptp_adjfine_rtc(bp, 0);
> +       } else {
>                 bnxt_ptp_timecounter_init(bp, false);
>                 rc = bnxt_ptp_init_rtc(bp, phc_cfg);
>                 if (rc)
>                         goto out;
> -       } else {
> -               bnxt_ptp_timecounter_init(bp, true);
>         }
>
>         ptp->ptp_info = bnxt_ptp_caps;
> --
> 2.34.1
>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 808236dc898b..e2e2c986c82b 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -6990,11 +6990,9 @@  static int bnxt_hwrm_func_qcfg(struct bnxt *bp)
 		if (flags & FUNC_QCFG_RESP_FLAGS_FW_DCBX_AGENT_ENABLED)
 			bp->fw_cap |= BNXT_FW_CAP_DCBX_AGENT;
 	}
-	if (BNXT_PF(bp) && (flags & FUNC_QCFG_RESP_FLAGS_MULTI_HOST)) {
+	if (BNXT_PF(bp) && (flags & FUNC_QCFG_RESP_FLAGS_MULTI_HOST))
 		bp->flags |= BNXT_FLAG_MULTI_HOST;
-		if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC)
-			bp->fw_cap &= ~BNXT_FW_CAP_PTP_RTC;
-	}
+
 	if (flags & FUNC_QCFG_RESP_FLAGS_RING_MONITOR_ENABLED)
 		bp->fw_cap |= BNXT_FW_CAP_RING_MONITOR;
 
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index dcb09fbe4007..c0628ac1b798 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2000,6 +2000,8 @@  struct bnxt {
 	u32			fw_dbg_cap;
 
 #define BNXT_NEW_RM(bp)		((bp)->fw_cap & BNXT_FW_CAP_NEW_RM)
+#define BNXT_PTP_USE_RTC(bp)	(!BNXT_MH(bp) && \
+				 ((bp)->fw_cap & BNXT_FW_CAP_PTP_RTC))
 	u32			hwrm_spec_code;
 	u16			hwrm_cmd_seq;
 	u16                     hwrm_cmd_kong_seq;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
index 4ec8bba18cdd..a96adde97e60 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
@@ -63,7 +63,7 @@  static int bnxt_ptp_settime(struct ptp_clock_info *ptp_info,
 						ptp_info);
 	u64 ns = timespec64_to_ns(ts);
 
-	if (ptp->bp->fw_cap & BNXT_FW_CAP_PTP_RTC)
+	if (BNXT_PTP_USE_RTC(ptp->bp))
 		return bnxt_ptp_cfg_settime(ptp->bp, ns);
 
 	spin_lock_bh(&ptp->ptp_lock);
@@ -196,7 +196,7 @@  static int bnxt_ptp_adjtime(struct ptp_clock_info *ptp_info, s64 delta)
 	struct bnxt_ptp_cfg *ptp = container_of(ptp_info, struct bnxt_ptp_cfg,
 						ptp_info);
 
-	if (ptp->bp->fw_cap & BNXT_FW_CAP_PTP_RTC)
+	if (BNXT_PTP_USE_RTC(ptp->bp))
 		return bnxt_ptp_adjphc(ptp, delta);
 
 	spin_lock_bh(&ptp->ptp_lock);
@@ -205,34 +205,39 @@  static int bnxt_ptp_adjtime(struct ptp_clock_info *ptp_info, s64 delta)
 	return 0;
 }
 
+static int bnxt_ptp_adjfine_rtc(struct bnxt *bp, long scaled_ppm)
+{
+	s32 ppb = scaled_ppm_to_ppb(scaled_ppm);
+	struct hwrm_port_mac_cfg_input *req;
+	int rc;
+
+	rc = hwrm_req_init(bp, req, HWRM_PORT_MAC_CFG);
+	if (rc)
+		return rc;
+
+	req->ptp_freq_adj_ppb = cpu_to_le32(ppb);
+	req->enables = cpu_to_le32(PORT_MAC_CFG_REQ_ENABLES_PTP_FREQ_ADJ_PPB);
+	rc = hwrm_req_send(bp, req);
+	if (rc)
+		netdev_err(bp->dev,
+			   "ptp adjfine failed. rc = %d\n", rc);
+	return rc;
+}
+
 static int bnxt_ptp_adjfine(struct ptp_clock_info *ptp_info, long scaled_ppm)
 {
 	struct bnxt_ptp_cfg *ptp = container_of(ptp_info, struct bnxt_ptp_cfg,
 						ptp_info);
-	struct hwrm_port_mac_cfg_input *req;
 	struct bnxt *bp = ptp->bp;
-	int rc = 0;
 
-	if (!(ptp->bp->fw_cap & BNXT_FW_CAP_PTP_RTC)) {
-		spin_lock_bh(&ptp->ptp_lock);
-		timecounter_read(&ptp->tc);
-		ptp->cc.mult = adjust_by_scaled_ppm(ptp->cmult, scaled_ppm);
-		spin_unlock_bh(&ptp->ptp_lock);
-	} else {
-		s32 ppb = scaled_ppm_to_ppb(scaled_ppm);
-
-		rc = hwrm_req_init(bp, req, HWRM_PORT_MAC_CFG);
-		if (rc)
-			return rc;
+	if (BNXT_PTP_USE_RTC(bp))
+		return bnxt_ptp_adjfine_rtc(bp, scaled_ppm);
 
-		req->ptp_freq_adj_ppb = cpu_to_le32(ppb);
-		req->enables = cpu_to_le32(PORT_MAC_CFG_REQ_ENABLES_PTP_FREQ_ADJ_PPB);
-		rc = hwrm_req_send(ptp->bp, req);
-		if (rc)
-			netdev_err(ptp->bp->dev,
-				   "ptp adjfine failed. rc = %d\n", rc);
-	}
-	return rc;
+	spin_lock_bh(&ptp->ptp_lock);
+	timecounter_read(&ptp->tc);
+	ptp->cc.mult = adjust_by_scaled_ppm(ptp->cmult, scaled_ppm);
+	spin_unlock_bh(&ptp->ptp_lock);
+	return 0;
 }
 
 void bnxt_ptp_pps_event(struct bnxt *bp, u32 data1, u32 data2)
@@ -879,7 +884,7 @@  int bnxt_ptp_init_rtc(struct bnxt *bp, bool phc_cfg)
 	u64 ns;
 	int rc;
 
-	if (!bp->ptp_cfg || !(bp->fw_cap & BNXT_FW_CAP_PTP_RTC))
+	if (!bp->ptp_cfg || !BNXT_PTP_USE_RTC(bp))
 		return -ENODEV;
 
 	if (!phc_cfg) {
@@ -932,13 +937,14 @@  int bnxt_ptp_init(struct bnxt *bp, bool phc_cfg)
 	atomic_set(&ptp->tx_avail, BNXT_MAX_TX_TS);
 	spin_lock_init(&ptp->ptp_lock);
 
-	if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC) {
+	if (BNXT_MH(bp)) {
+		bnxt_ptp_timecounter_init(bp, true);
+		bnxt_ptp_adjfine_rtc(bp, 0);
+	} else {
 		bnxt_ptp_timecounter_init(bp, false);
 		rc = bnxt_ptp_init_rtc(bp, phc_cfg);
 		if (rc)
 			goto out;
-	} else {
-		bnxt_ptp_timecounter_init(bp, true);
 	}
 
 	ptp->ptp_info = bnxt_ptp_caps;