| Message ID | 20230308144209.150456-1-vadfed@meta.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] bnxt_en: reset PHC frequency in free-running mode | expand |
On Wed, Mar 8, 2023 at 8:12 PM Vadim Fedorenko <vadfed@meta.com> wrote: > > +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); nit: can be a single line. > + return rc; > +} > + > @@ -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_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); > + if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC) I understand from your response on v1 as to why it will not affect you if a new firmware does not report RTC on MH. However, once you update the fw, any subsequent kernels upgrades will prevent resetting the freq stored in the PHC. Would changing the check to if (BNXT_MH(bp)) instead be a better option? > + bnxt_ptp_adjfine_rtc(bp, 0); > } > > ptp->ptp_info = bnxt_ptp_caps; > -- > 2.30.2 >
On 09.03.2023 04:40, Pavan Chebbi wrote: > On Wed, Mar 8, 2023 at 8:12 PM Vadim Fedorenko <vadfed@meta.com> wrote: >> >> +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); > > nit: can be a single line. > >> + return rc; >> +} >> + > >> @@ -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_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); >> + if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC) > > I understand from your response on v1 as to why it will not affect you > if a new firmware does not report RTC on MH. > However, once you update the fw, any subsequent kernels upgrades will > prevent resetting the freq stored in the PHC. > Would changing the check to if (BNXT_MH(bp)) instead be a better option? How will it affect hardware without RTC support? The one which doesn't have BNXT_FW_CAP_PTP_RTC in a single-host configuration. Asking because if FW will not expose BNXT_FW_CAP_PTP_RTC, the check BNXT_PTP_USE_RTC() will be equal to !BNXT_MH() and there will be no need for additional check in this else clause. >> + bnxt_ptp_adjfine_rtc(bp, 0); >> } >> >> ptp->ptp_info = bnxt_ptp_caps; >> -- >> 2.30.2 >>
On Thu, Mar 9, 2023 at 3:02 PM Vadim Fedorenko <vadim.fedorenko@linux.dev> wrote: > > On 09.03.2023 04:40, Pavan Chebbi wrote: > > On Wed, Mar 8, 2023 at 8:12 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_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); > >> + if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC) > > > > I understand from your response on v1 as to why it will not affect you > > if a new firmware does not report RTC on MH. > > However, once you update the fw, any subsequent kernels upgrades will > > prevent resetting the freq stored in the PHC. > > Would changing the check to if (BNXT_MH(bp)) instead be a better option? > > How will it affect hardware without RTC support? The one which doesn't have > BNXT_FW_CAP_PTP_RTC in a single-host configuration. Asking because if FW will For single hosts, it should not matter if we reset the PHC frequency. bnxt_ptp_init() is [re]initializing the host-timecounter, and this function being called on a single host means everything is going to [re]start from scratch. > not expose BNXT_FW_CAP_PTP_RTC, the check BNXT_PTP_USE_RTC() will be equal to > !BNXT_MH() and there will be no need for additional check in this else clause. You are right, hence my original suggestion of resetting the PHC freq unconditionally is better. One more thing, the function bnxt_ptp_adjfine() should select non-realtime adjustments only for MH systems. You may need to flip the check, something like if (!BNXT_MH(ptp->bp)) return bnxt_ptp_adjfine_rtc(bp, scaled_ppm); This is because there can be a very old firmware which does not have RTC on single hosts but we still want to make HW adjustments.
On 09/03/2023 10:11, Pavan Chebbi wrote: > On Thu, Mar 9, 2023 at 3:02 PM Vadim Fedorenko > <vadim.fedorenko@linux.dev> wrote: >> >> On 09.03.2023 04:40, Pavan Chebbi wrote: >>> On Wed, Mar 8, 2023 at 8:12 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_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); >>>> + if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC) >>> >>> I understand from your response on v1 as to why it will not affect you >>> if a new firmware does not report RTC on MH. >>> However, once you update the fw, any subsequent kernels upgrades will >>> prevent resetting the freq stored in the PHC. >>> Would changing the check to if (BNXT_MH(bp)) instead be a better option? >> >> How will it affect hardware without RTC support? The one which doesn't have >> BNXT_FW_CAP_PTP_RTC in a single-host configuration. Asking because if FW will > > For single hosts, it should not matter if we reset the PHC frequency. > bnxt_ptp_init() is [re]initializing the host-timecounter, and this > function being called on a single host means everything is going to > [re]start from scratch. > >> not expose BNXT_FW_CAP_PTP_RTC, the check BNXT_PTP_USE_RTC() will be equal to >> !BNXT_MH() and there will be no need for additional check in this else clause. > > You are right, hence my original suggestion of resetting the PHC freq > unconditionally is better. > One more thing, the function bnxt_ptp_adjfine() should select > non-realtime adjustments only for MH systems. > You may need to flip the check, something like > > if (!BNXT_MH(ptp->bp)) > return bnxt_ptp_adjfine_rtc(bp, scaled_ppm); > > This is because there can be a very old firmware which does not have > RTC on single hosts but we still want to make HW adjustments. Well, I just want to be sure that we will support all possible combinations of FW in the driver. AFAIU, there 3 different options: 1. Very old firmware. Doesn't have RTC support and doesn't expose BNXT_FW_CAP_PTP_RTC. Call to bnxt_ptp_adjfine_rtc on this variant may make HW unusable. We MUST not call it in this case. The timecounter is also not supported in this configuration, right? 2. Firmware which supports RTC and exposes BNXT_FW_CAP_PTP_RTC. Timecounter should be used only in MH case then. 3. Firmware which supports RTC, but doesn't expose BNXT_FW_CAP_PTP_RTC for MH configuration. How can we understand that it's not variant 1 in MH configuration? Or are we sure FUNC_QCAPS_RESP_FLAGS_PTP_SUPPORTED is not set on old firmware?
On Thu, Mar 9, 2023 at 4:20 PM Vadim Fedorenko <vadim.fedorenko@linux.dev> wrote: > > On 09/03/2023 10:11, Pavan Chebbi wrote: > > On Thu, Mar 9, 2023 at 3:02 PM Vadim Fedorenko > > <vadim.fedorenko@linux.dev> wrote: > >> > >> On 09.03.2023 04:40, Pavan Chebbi wrote: > >>> On Wed, Mar 8, 2023 at 8:12 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_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); > >>>> + if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC) > >>> > >>> I understand from your response on v1 as to why it will not affect you > >>> if a new firmware does not report RTC on MH. > >>> However, once you update the fw, any subsequent kernels upgrades will > >>> prevent resetting the freq stored in the PHC. > >>> Would changing the check to if (BNXT_MH(bp)) instead be a better option? > >> > >> How will it affect hardware without RTC support? The one which doesn't have > >> BNXT_FW_CAP_PTP_RTC in a single-host configuration. Asking because if FW will > > > > For single hosts, it should not matter if we reset the PHC frequency. > > bnxt_ptp_init() is [re]initializing the host-timecounter, and this > > function being called on a single host means everything is going to > > [re]start from scratch. > > > >> not expose BNXT_FW_CAP_PTP_RTC, the check BNXT_PTP_USE_RTC() will be equal to > >> !BNXT_MH() and there will be no need for additional check in this else clause. > > > > You are right, hence my original suggestion of resetting the PHC freq > > unconditionally is better. > > One more thing, the function bnxt_ptp_adjfine() should select > > non-realtime adjustments only for MH systems. > > You may need to flip the check, something like > > > > if (!BNXT_MH(ptp->bp)) > > return bnxt_ptp_adjfine_rtc(bp, scaled_ppm); > > > > This is because there can be a very old firmware which does not have > > RTC on single hosts but we still want to make HW adjustments. > > Well, I just want to be sure that we will support all possible > combinations of FW in the driver. AFAIU, there 3 different options: > > 1. Very old firmware. Doesn't have RTC support and doesn't expose > BNXT_FW_CAP_PTP_RTC. Call to bnxt_ptp_adjfine_rtc on this variant may > make HW unusable. We MUST not call it in this case. The timecounter is > also not supported in this configuration, right? No, the first ever version of bnxt PTP solution has the FW support for HW adjustment but does not expose BNXT_FW_CAP_PTP_RTC. So we need to be mindful of this just to keep backward compatibility intact. > 2. Firmware which supports RTC and exposes BNXT_FW_CAP_PTP_RTC. > Timecounter should be used only in MH case then. Timecounter is used in all variations. Single host (SH) and Multi host (MH), with/without RTC. The only difference is how timecounter is used. With RTC, we wanted to provide an MH solution to achieve synchronization across all the hosts using a common timebase. But we had to implement the recently added non-realtime design to achieve that goal. But we still need RTC on single hosts when we want to adjust phase. > 3. Firmware which supports RTC, but doesn't expose BNXT_FW_CAP_PTP_RTC > for MH configuration. How can we understand that it's not variant 1 in > MH configuration? Or are we sure FUNC_QCAPS_RESP_FLAGS_PTP_SUPPORTED is > not set on old firmware? As such we don't use RTC (even if exposed) at all in MH case. So simply ignore the RTC if you determine a MH system. Let me just summarize: we can have three variants of Firmware. 1. That does not have RTC exposed on both SH and MH (very old) 2. That has RTC exposed both on SH and MH (current production) 3. That has RTC exposed on SH and not on MH (future) In all the above cases, we make use of the timecounter for timestamps, but we must make HW freq adjustments in SH, and timecounter freq adjustments in MH.
On 09/03/2023 12:23, Pavan Chebbi wrote: > On Thu, Mar 9, 2023 at 4:20 PM Vadim Fedorenko > <vadim.fedorenko@linux.dev> wrote: >> >> On 09/03/2023 10:11, Pavan Chebbi wrote: >>> On Thu, Mar 9, 2023 at 3:02 PM Vadim Fedorenko >>> <vadim.fedorenko@linux.dev> wrote: >>>> >>>> On 09.03.2023 04:40, Pavan Chebbi wrote: >>>>> On Wed, Mar 8, 2023 at 8:12 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_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); >>>>>> + if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC) >>>>> >>>>> I understand from your response on v1 as to why it will not affect you >>>>> if a new firmware does not report RTC on MH. >>>>> However, once you update the fw, any subsequent kernels upgrades will >>>>> prevent resetting the freq stored in the PHC. >>>>> Would changing the check to if (BNXT_MH(bp)) instead be a better option? >>>> >>>> How will it affect hardware without RTC support? The one which doesn't have >>>> BNXT_FW_CAP_PTP_RTC in a single-host configuration. Asking because if FW will >>> >>> For single hosts, it should not matter if we reset the PHC frequency. >>> bnxt_ptp_init() is [re]initializing the host-timecounter, and this >>> function being called on a single host means everything is going to >>> [re]start from scratch. >>> >>>> not expose BNXT_FW_CAP_PTP_RTC, the check BNXT_PTP_USE_RTC() will be equal to >>>> !BNXT_MH() and there will be no need for additional check in this else clause. >>> >>> You are right, hence my original suggestion of resetting the PHC freq >>> unconditionally is better. >>> One more thing, the function bnxt_ptp_adjfine() should select >>> non-realtime adjustments only for MH systems. >>> You may need to flip the check, something like >>> >>> if (!BNXT_MH(ptp->bp)) >>> return bnxt_ptp_adjfine_rtc(bp, scaled_ppm); >>> >>> This is because there can be a very old firmware which does not have >>> RTC on single hosts but we still want to make HW adjustments. >> >> Well, I just want to be sure that we will support all possible >> combinations of FW in the driver. AFAIU, there 3 different options: >> >> 1. Very old firmware. Doesn't have RTC support and doesn't expose >> BNXT_FW_CAP_PTP_RTC. Call to bnxt_ptp_adjfine_rtc on this variant may >> make HW unusable. We MUST not call it in this case. The timecounter is >> also not supported in this configuration, right? > > No, the first ever version of bnxt PTP solution has the FW support for > HW adjustment but does not expose BNXT_FW_CAP_PTP_RTC. > So we need to be mindful of this just to keep backward compatibility intact. > >> 2. Firmware which supports RTC and exposes BNXT_FW_CAP_PTP_RTC. >> Timecounter should be used only in MH case then. > > Timecounter is used in all variations. Single host (SH) and Multi host > (MH), with/without RTC. The only difference is how timecounter is > used. > With RTC, we wanted to provide an MH solution to achieve > synchronization across all the hosts using a common timebase. > But we had to implement the recently added non-realtime design to > achieve that goal. But we still need RTC on single hosts when we want > to adjust phase. > >> 3. Firmware which supports RTC, but doesn't expose BNXT_FW_CAP_PTP_RTC >> for MH configuration. How can we understand that it's not variant 1 in >> MH configuration? Or are we sure FUNC_QCAPS_RESP_FLAGS_PTP_SUPPORTED is >> not set on old firmware? > > As such we don't use RTC (even if exposed) at all in MH case. So > simply ignore the RTC if you determine a MH system. > > Let me just summarize: > we can have three variants of Firmware. > 1. That does not have RTC exposed on both SH and MH (very old) > 2. That has RTC exposed both on SH and MH (current production) > 3. That has RTC exposed on SH and not on MH (future) > > In all the above cases, we make use of the timecounter for timestamps, > but we must make HW freq adjustments in SH, and timecounter freq > adjustments in MH. OK, got it. I'll send a v3 soon with the comments addressed. Thanks for feedback and explanations.
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..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..4ee8581b5fea 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(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_USE_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_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); + if (bp->fw_cap & BNXT_FW_CAP_PTP_RTC) + bnxt_ptp_adjfine_rtc(bp, 0); } ptp->ptp_info = bnxt_ptp_caps;
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 base frequency 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(-)