* [PATCH net] bnxt_en: reset PHC frequency in free-running mode
@ 2023-03-06 16:53 Vadim Fedorenko
2023-03-06 17:11 ` Pavan Chebbi
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Vadim Fedorenko @ 2023-03-06 16:53 UTC (permalink / raw)
To: Jakub Kicinski, Pavan Chebbi, Andy Gospodarek, Michael Chan
Cc: Vadim Fedorenko, Richard Cochran, Vadim Fedorenko, netdev
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 | 57 +++++++++++--------
3 files changed, 36 insertions(+), 29 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 5d4b1f2ebeac..8472ff79adf3 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -6989,11 +6989,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..41e4bb7b8acb 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_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..99c1a53231aa 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_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_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_RTC(ptp->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_RTC(bp))
return -ENODEV;
if (!phc_cfg) {
@@ -932,13 +937,15 @@ 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_PTP_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);
+ if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC)
+ bnxt_ptp_adjfreq_rtc(bp, 0);
}
ptp->ptp_info = bnxt_ptp_caps;
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH net] bnxt_en: reset PHC frequency in free-running mode 2023-03-06 16:53 [PATCH net] bnxt_en: reset PHC frequency in free-running mode Vadim Fedorenko @ 2023-03-06 17:11 ` Pavan Chebbi 2023-03-06 19:12 ` Vadim Fedorenko 2023-03-06 18:06 ` Jakub Kicinski 2023-03-07 2:40 ` kernel test robot 2 siblings, 1 reply; 8+ messages in thread From: Pavan Chebbi @ 2023-03-06 17:11 UTC (permalink / raw) To: Vadim Fedorenko Cc: Jakub Kicinski, Andy Gospodarek, Michael Chan, Richard Cochran, Vadim Fedorenko, netdev [-- Attachment #1: Type: text/plain, Size: 7275 bytes --] On Mon, Mar 6, 2023 at 10:23 PM 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. > Thanks for the patch. I see what you are trying to do. But I think we have some build issues with this. Haven't looked at the whole patch, but one error I can spot is down at the bottom. > 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 | 57 +++++++++++-------- > 3 files changed, 36 insertions(+), 29 deletions(-) > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c > index 5d4b1f2ebeac..8472ff79adf3 100644 > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c > @@ -6989,11 +6989,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..41e4bb7b8acb 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_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..99c1a53231aa 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_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_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_RTC(ptp->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_RTC(bp)) > return -ENODEV; > > if (!phc_cfg) { > @@ -932,13 +937,15 @@ 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_PTP_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); > + if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC) > + bnxt_ptp_adjfreq_rtc(bp, 0); You meant bnxt_ptp_adjfine_rtc(), right. Anyway, let me go through the patch in detail, while you may submit corrections for the build. > } > > ptp->ptp_info = bnxt_ptp_caps; > -- > 2.30.2 > [-- Attachment #2: S/MIME Cryptographic Signature --] [-- Type: application/pkcs7-signature, Size: 4209 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net] bnxt_en: reset PHC frequency in free-running mode 2023-03-06 17:11 ` Pavan Chebbi @ 2023-03-06 19:12 ` Vadim Fedorenko 2023-03-07 5:07 ` Pavan Chebbi 0 siblings, 1 reply; 8+ messages in thread From: Vadim Fedorenko @ 2023-03-06 19:12 UTC (permalink / raw) To: Pavan Chebbi, Vadim Fedorenko Cc: Jakub Kicinski, Andy Gospodarek, Michael Chan, Richard Cochran, netdev On 06/03/2023 17:11, Pavan Chebbi wrote: > On Mon, Mar 6, 2023 at 10:23 PM 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. >> > Thanks for the patch. > I see what you are trying to do. But I think we have some build issues > with this. > Haven't looked at the whole patch, but one error I can spot is down at > the bottom. > >> 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 | 57 +++++++++++-------- >> 3 files changed, 36 insertions(+), 29 deletions(-) >> >> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c >> index 5d4b1f2ebeac..8472ff79adf3 100644 >> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c >> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c >> @@ -6989,11 +6989,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..41e4bb7b8acb 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_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..99c1a53231aa 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_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_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_RTC(ptp->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_RTC(bp)) >> return -ENODEV; >> >> if (!phc_cfg) { >> @@ -932,13 +937,15 @@ 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_PTP_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); >> + if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC) >> + bnxt_ptp_adjfreq_rtc(bp, 0); > > You meant bnxt_ptp_adjfine_rtc(), right. > Anyway, let me go through the patch in detail, while you may submit > corrections for the build. > Oh, yeah, right, artefact of rebasing. Will fix it in v2, thanks. >> } >> >> ptp->ptp_info = bnxt_ptp_caps; >> -- >> 2.30.2 >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net] bnxt_en: reset PHC frequency in free-running mode 2023-03-06 19:12 ` Vadim Fedorenko @ 2023-03-07 5:07 ` Pavan Chebbi 2023-03-07 10:25 ` Vadim Fedorenko 0 siblings, 1 reply; 8+ messages in thread From: Pavan Chebbi @ 2023-03-07 5:07 UTC (permalink / raw) To: Vadim Fedorenko Cc: Vadim Fedorenko, Jakub Kicinski, Andy Gospodarek, Michael Chan, Richard Cochran, netdev [-- Attachment #1: Type: text/plain, Size: 1758 bytes --] On Tue, Mar 7, 2023 at 12:43 AM Vadim Fedorenko <vadim.fedorenko@linux.dev> wrote: > > On 06/03/2023 17:11, Pavan Chebbi wrote: > > On Mon, Mar 6, 2023 at 10:23 PM Vadim Fedorenko <vadfed@meta.com> wrote: > >> > >> @@ -932,13 +937,15 @@ 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_PTP_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); > >> + if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC) > >> + bnxt_ptp_adjfreq_rtc(bp, 0); I am not sure if the intended objective of resetting the PHC is going to be achieved with this. The FW will always apply the new ppb on the base PHC frequency. I am not sure what you mean by "reset the hardware frequency of PHC to zero" in the commit message. If you want PHC to start counting from 0 on init, you may use bnxt_ptp_cfg_settime() with 0. Also, the RTC flag may not be set on newer firmwares running on MH systems. I see no harm in resetting the PHC unconditionally if we are not in RTC mode. > > > > You meant bnxt_ptp_adjfine_rtc(), right. > > Anyway, let me go through the patch in detail, while you may submit > > corrections for the build. > > > Oh, yeah, right, artefact of rebasing. Will fix it in v2, thanks. > > > >> } > >> > >> ptp->ptp_info = bnxt_ptp_caps; > >> -- > >> 2.30.2 > >> > [-- Attachment #2: S/MIME Cryptographic Signature --] [-- Type: application/pkcs7-signature, Size: 4209 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net] bnxt_en: reset PHC frequency in free-running mode 2023-03-07 5:07 ` Pavan Chebbi @ 2023-03-07 10:25 ` Vadim Fedorenko 0 siblings, 0 replies; 8+ messages in thread From: Vadim Fedorenko @ 2023-03-07 10:25 UTC (permalink / raw) To: Pavan Chebbi Cc: Vadim Fedorenko, Jakub Kicinski, Andy Gospodarek, Michael Chan, Richard Cochran, netdev On 07/03/2023 05:07, Pavan Chebbi wrote: > On Tue, Mar 7, 2023 at 12:43 AM Vadim Fedorenko > <vadim.fedorenko@linux.dev> wrote: >> >> On 06/03/2023 17:11, Pavan Chebbi wrote: >>> On Mon, Mar 6, 2023 at 10:23 PM Vadim Fedorenko <vadfed@meta.com> wrote: >>>> >>>> @@ -932,13 +937,15 @@ 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_PTP_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); >>>> + if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC) >>>> + bnxt_ptp_adjfreq_rtc(bp, 0); > > I am not sure if the intended objective of resetting the PHC is going > to be achieved with this. The FW will always apply the new ppb on the > base PHC frequency. I am not sure what you mean by "reset the hardware > frequency of PHC to zero" in the commit message. Well, I meant reset it to base frequency and remove any adjustments applied before. I'll re-phrase it in the next version. > If you want PHC to > start counting from 0 on init, you may use bnxt_ptp_cfg_settime() with > 0. That's not what we want, the current counter behavior is ok. > Also, the RTC flag may not be set on newer firmwares running on MH > systems. I see no harm in resetting the PHC unconditionally if we are > not in RTC mode. And that's perfectly fine! Each firmware upgrade requires full NIC reset and it will end up with reset of PHC to base frequency. But in the situation when we have several version of kernel running on the SLED, on kernel upgrade we end up with adjustment stored in the PHC using old kernel which was tuning RTC, but then booted into new kernel which stops tuning RTC. If the last stored adjustment was close to boundaries for some reasons, timecounter will never compensate the difference and all hosts in the sled will be drifting hard. The only way to bring it back to working state is to power reset the sled, which is disruptive action. The fix was proved to work actually in our setup. >>> >>> You meant bnxt_ptp_adjfine_rtc(), right. >>> Anyway, let me go through the patch in detail, while you may submit >>> corrections for the build. >>> >> Oh, yeah, right, artefact of rebasing. Will fix it in v2, thanks. >> >> >>>> } >>>> >>>> ptp->ptp_info = bnxt_ptp_caps; >>>> -- >>>> 2.30.2 >>>> >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net] bnxt_en: reset PHC frequency in free-running mode 2023-03-06 16:53 [PATCH net] bnxt_en: reset PHC frequency in free-running mode Vadim Fedorenko 2023-03-06 17:11 ` Pavan Chebbi @ 2023-03-06 18:06 ` Jakub Kicinski 2023-03-06 19:13 ` Vadim Fedorenko 2023-03-07 2:40 ` kernel test robot 2 siblings, 1 reply; 8+ messages in thread From: Jakub Kicinski @ 2023-03-06 18:06 UTC (permalink / raw) To: Vadim Fedorenko Cc: Pavan Chebbi, Andy Gospodarek, Michael Chan, Richard Cochran, Vadim Fedorenko, netdev On Mon, 6 Mar 2023 08:53:44 -0800 Vadim Fedorenko wrote: > +#define BNXT_PTP_RTC(bp) (!BNXT_MH(bp) && \ > + ((bp)->fw_cap & BNXT_FW_CAP_PTP_RTC)) nit: maybe BNXT_PTP_USE_RTC() ? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net] bnxt_en: reset PHC frequency in free-running mode 2023-03-06 18:06 ` Jakub Kicinski @ 2023-03-06 19:13 ` Vadim Fedorenko 0 siblings, 0 replies; 8+ messages in thread From: Vadim Fedorenko @ 2023-03-06 19:13 UTC (permalink / raw) To: Jakub Kicinski, Vadim Fedorenko Cc: Pavan Chebbi, Andy Gospodarek, Michael Chan, Richard Cochran, netdev On 06/03/2023 18:06, Jakub Kicinski wrote: > On Mon, 6 Mar 2023 08:53:44 -0800 Vadim Fedorenko wrote: >> +#define BNXT_PTP_RTC(bp) (!BNXT_MH(bp) && \ >> + ((bp)->fw_cap & BNXT_FW_CAP_PTP_RTC)) > > nit: maybe BNXT_PTP_USE_RTC() ? That's fair point. I'll change it in v2. Thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net] bnxt_en: reset PHC frequency in free-running mode 2023-03-06 16:53 [PATCH net] bnxt_en: reset PHC frequency in free-running mode Vadim Fedorenko 2023-03-06 17:11 ` Pavan Chebbi 2023-03-06 18:06 ` Jakub Kicinski @ 2023-03-07 2:40 ` kernel test robot 2 siblings, 0 replies; 8+ messages in thread From: kernel test robot @ 2023-03-07 2:40 UTC (permalink / raw) To: Vadim Fedorenko, Jakub Kicinski, Pavan Chebbi, Andy Gospodarek, Michael Chan Cc: llvm, oe-kbuild-all, Vadim Fedorenko, Richard Cochran, netdev Hi Vadim, I love your patch! Yet something to improve: [auto build test ERROR on net/master] url: https://github.com/intel-lab-lkp/linux/commits/Vadim-Fedorenko/bnxt_en-reset-PHC-frequency-in-free-running-mode/20230307-005700 patch link: https://lore.kernel.org/r/20230306165344.350387-1-vadfed%40meta.com patch subject: [PATCH net] bnxt_en: reset PHC frequency in free-running mode config: riscv-randconfig-r042-20230306 (https://download.01.org/0day-ci/archive/20230307/202303071016.BkAkCGA9-lkp@intel.com/config) compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install riscv cross compiling tool for clang build # apt-get install binutils-riscv64-linux-gnu # https://github.com/intel-lab-lkp/linux/commit/26d7b40eb9dd69f66a477fab7cf51d98c3fe63de git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Vadim-Fedorenko/bnxt_en-reset-PHC-frequency-in-free-running-mode/20230307-005700 git checkout 26d7b40eb9dd69f66a477fab7cf51d98c3fe63de # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/net/ethernet/broadcom/bnxt/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> | Link: https://lore.kernel.org/oe-kbuild-all/202303071016.BkAkCGA9-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c:948:4: error: call to undeclared function 'bnxt_ptp_adjfreq_rtc'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] bnxt_ptp_adjfreq_rtc(bp, 0); ^ drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c:948:4: note: did you mean 'bnxt_ptp_adjfine_rtc'? drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c:208:12: note: 'bnxt_ptp_adjfine_rtc' declared here static int bnxt_ptp_adjfine_rtc(struct bnxt *bp, long scaled_ppm) ^ 1 error generated. vim +/bnxt_ptp_adjfreq_rtc +948 drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c 919 920 int bnxt_ptp_init(struct bnxt *bp, bool phc_cfg) 921 { 922 struct bnxt_ptp_cfg *ptp = bp->ptp_cfg; 923 int rc; 924 925 if (!ptp) 926 return 0; 927 928 rc = bnxt_map_ptp_regs(bp); 929 if (rc) 930 return rc; 931 932 if (ptp->ptp_clock && bnxt_pps_config_ok(bp)) 933 return 0; 934 935 bnxt_ptp_free(bp); 936 937 atomic_set(&ptp->tx_avail, BNXT_MAX_TX_TS); 938 spin_lock_init(&ptp->ptp_lock); 939 940 if (BNXT_PTP_RTC(ptp->bp)) { 941 bnxt_ptp_timecounter_init(bp, false); 942 rc = bnxt_ptp_init_rtc(bp, phc_cfg); 943 if (rc) 944 goto out; 945 } else { 946 bnxt_ptp_timecounter_init(bp, true); 947 if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC) > 948 bnxt_ptp_adjfreq_rtc(bp, 0); 949 } 950 951 ptp->ptp_info = bnxt_ptp_caps; 952 if ((bp->fw_cap & BNXT_FW_CAP_PTP_PPS)) { 953 if (bnxt_ptp_pps_init(bp)) 954 netdev_err(bp->dev, "1pps not initialized, continuing without 1pps support\n"); 955 } 956 ptp->ptp_clock = ptp_clock_register(&ptp->ptp_info, &bp->pdev->dev); 957 if (IS_ERR(ptp->ptp_clock)) { 958 int err = PTR_ERR(ptp->ptp_clock); 959 960 ptp->ptp_clock = NULL; 961 rc = err; 962 goto out; 963 } 964 if (bp->flags & BNXT_FLAG_CHIP_P5) { 965 spin_lock_bh(&ptp->ptp_lock); 966 bnxt_refclk_read(bp, NULL, &ptp->current_time); 967 WRITE_ONCE(ptp->old_time, ptp->current_time); 968 spin_unlock_bh(&ptp->ptp_lock); 969 ptp_schedule_worker(ptp->ptp_clock, 0); 970 } 971 return 0; 972 973 out: 974 bnxt_ptp_free(bp); 975 bnxt_unmap_ptp_regs(bp); 976 return rc; 977 } 978 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-03-07 10:25 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-03-06 16:53 [PATCH net] bnxt_en: reset PHC frequency in free-running mode Vadim Fedorenko 2023-03-06 17:11 ` Pavan Chebbi 2023-03-06 19:12 ` Vadim Fedorenko 2023-03-07 5:07 ` Pavan Chebbi 2023-03-07 10:25 ` Vadim Fedorenko 2023-03-06 18:06 ` Jakub Kicinski 2023-03-06 19:13 ` Vadim Fedorenko 2023-03-07 2:40 ` kernel test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).