* [PATCH 1/2] net: fec: Restart PPS after link state change @ 2024-09-16 14:19 Csókás, Bence 2024-09-16 14:19 ` [PATCH 2/2] net: fec: Reload PTP registers after link-state change Csókás, Bence 2024-09-19 8:18 ` [PATCH 1/2] net: fec: Restart PPS after link state change Paolo Abeni 0 siblings, 2 replies; 6+ messages in thread From: Csókás, Bence @ 2024-09-16 14:19 UTC (permalink / raw) To: imx, netdev, linux-kernel Cc: Csókás, Bence, Wei Fang, Shenwei Wang, Clark Wang, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Richard Cochran On link state change, the controller gets reset, causing PPS to drop out. Re-enable PPS if it was enabled before the controller reset. Signed-off-by: Csókás, Bence <csokas.bence@prolan.hu> --- drivers/net/ethernet/freescale/fec.h | 1 + drivers/net/ethernet/freescale/fec_main.c | 7 ++++++- drivers/net/ethernet/freescale/fec_ptp.c | 12 ++++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index a19cb2a786fd..afa0bfb974e6 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -695,6 +695,7 @@ struct fec_enet_private { }; void fec_ptp_init(struct platform_device *pdev, int irq_idx); +void fec_ptp_restore_state(struct fec_enet_private *fep); void fec_ptp_stop(struct platform_device *pdev); void fec_ptp_start_cyclecounter(struct net_device *ndev); int fec_ptp_set(struct net_device *ndev, struct kernel_hwtstamp_config *config, diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index a923cb95cdc6..531b51091e7d 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1244,8 +1244,10 @@ fec_restart(struct net_device *ndev) writel(ecntl, fep->hwp + FEC_ECNTRL); fec_enet_active_rxring(ndev); - if (fep->bufdesc_ex) + if (fep->bufdesc_ex) { fec_ptp_start_cyclecounter(ndev); + fec_ptp_restore_state(fep); + } /* Enable interrupts we wish to service */ if (fep->link) @@ -1366,6 +1368,9 @@ fec_stop(struct net_device *ndev) val = readl(fep->hwp + FEC_ECNTRL); val |= FEC_ECR_EN1588; writel(val, fep->hwp + FEC_ECNTRL); + + fec_ptp_start_cyclecounter(ndev); + fec_ptp_restore_state(fep); } } diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c index e32f6724f568..c5b89352373a 100644 --- a/drivers/net/ethernet/freescale/fec_ptp.c +++ b/drivers/net/ethernet/freescale/fec_ptp.c @@ -770,6 +770,18 @@ void fec_ptp_init(struct platform_device *pdev, int irq_idx) schedule_delayed_work(&fep->time_keep, HZ); } +/* Restore PTP functionality after a reset */ +void fec_ptp_restore_state(struct fec_enet_private *fep) +{ + /* Restart PPS if needed */ + if (fep->pps_enable) { + /* Reset turned it off, so adjust our status flag */ + fep->pps_enable = 0; + /* Re-enable PPS */ + fec_ptp_enable_pps(fep, 1); + } +} + void fec_ptp_stop(struct platform_device *pdev) { struct net_device *ndev = platform_get_drvdata(pdev); -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] net: fec: Reload PTP registers after link-state change 2024-09-16 14:19 [PATCH 1/2] net: fec: Restart PPS after link state change Csókás, Bence @ 2024-09-16 14:19 ` Csókás, Bence 2024-09-16 15:05 ` Frank Li 2024-09-19 8:18 ` [PATCH 1/2] net: fec: Restart PPS after link state change Paolo Abeni 1 sibling, 1 reply; 6+ messages in thread From: Csókás, Bence @ 2024-09-16 14:19 UTC (permalink / raw) To: imx, netdev, linux-kernel Cc: Csókás, Bence, Wei Fang, Shenwei Wang, Clark Wang, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Richard Cochran On link-state change, the controller gets reset, which clears all PTP registers, including PHC time, calibrated clock correction values etc. For correct IEEE 1588 operation we need to restore these after the reset. Signed-off-by: Csókás, Bence <csokas.bence@prolan.hu> --- drivers/net/ethernet/freescale/fec.h | 7 +++++ drivers/net/ethernet/freescale/fec_main.c | 4 +++ drivers/net/ethernet/freescale/fec_ptp.c | 35 +++++++++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index afa0bfb974e6..efe770fe337d 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -691,11 +691,18 @@ struct fec_enet_private { /* XDP BPF Program */ struct bpf_prog *xdp_prog; + struct { + u64 ns_sys, ns_phc; + u32 at_corr; + u8 at_inc_corr; + } ptp_saved_state; + u64 ethtool_stats[]; }; void fec_ptp_init(struct platform_device *pdev, int irq_idx); void fec_ptp_restore_state(struct fec_enet_private *fep); +void fec_ptp_save_state(struct fec_enet_private *fep); void fec_ptp_stop(struct platform_device *pdev); void fec_ptp_start_cyclecounter(struct net_device *ndev); int fec_ptp_set(struct net_device *ndev, struct kernel_hwtstamp_config *config, diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 531b51091e7d..570f8a14d975 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1077,6 +1077,8 @@ fec_restart(struct net_device *ndev) u32 rcntl = OPT_FRAME_SIZE | 0x04; u32 ecntl = FEC_ECR_ETHEREN; + fec_ptp_save_state(fep); + /* Whack a reset. We should wait for this. * For i.MX6SX SOC, enet use AXI bus, we use disable MAC * instead of reset MAC itself. @@ -1338,6 +1340,8 @@ fec_stop(struct net_device *ndev) netdev_err(ndev, "Graceful transmit stop did not complete!\n"); } + fec_ptp_save_state(fep); + /* Whack a reset. We should wait for this. * For i.MX6SX SOC, enet use AXI bus, we use disable MAC * instead of reset MAC itself. diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c index c5b89352373a..8011a6f3c4be 100644 --- a/drivers/net/ethernet/freescale/fec_ptp.c +++ b/drivers/net/ethernet/freescale/fec_ptp.c @@ -770,9 +770,44 @@ void fec_ptp_init(struct platform_device *pdev, int irq_idx) schedule_delayed_work(&fep->time_keep, HZ); } +void fec_ptp_save_state(struct fec_enet_private *fep) +{ + unsigned long flags; + u32 atime_inc_corr; + + spin_lock_irqsave(&fep->tmreg_lock, flags); + + fep->ptp_saved_state.ns_phc = timecounter_read(&fep->tc); + fep->ptp_saved_state.ns_sys = ktime_get_ns(); + + fep->ptp_saved_state.at_corr = readl(fep->hwp + FEC_ATIME_CORR); + atime_inc_corr = readl(fep->hwp + FEC_ATIME_INC) & FEC_T_INC_CORR_MASK; + fep->ptp_saved_state.at_inc_corr = (u8)(atime_inc_corr >> FEC_T_INC_CORR_OFFSET); + + spin_unlock_irqrestore(&fep->tmreg_lock, flags); +} + /* Restore PTP functionality after a reset */ void fec_ptp_restore_state(struct fec_enet_private *fep) { + u32 atime_inc = readl(fep->hwp + FEC_ATIME_INC) & FEC_T_INC_MASK; + unsigned long flags; + u32 counter; + u64 ns; + + spin_lock_irqsave(&fep->tmreg_lock, flags); + + writel(fep->ptp_saved_state.at_corr, fep->hwp + FEC_ATIME_CORR); + atime_inc |= ((u32)fep->ptp_saved_state.at_inc_corr) << FEC_T_INC_CORR_OFFSET; + writel(atime_inc, fep->hwp + FEC_ATIME_INC); + + ns = ktime_get_ns() - fep->ptp_saved_state.ns_sys + fep->ptp_saved_state.ns_phc; + counter = ns & fep->cc.mask; + writel(counter, fep->hwp + FEC_ATIME); + timecounter_init(&fep->tc, &fep->cc, ns); + + spin_unlock_irqrestore(&fep->tmreg_lock, flags); + /* Restart PPS if needed */ if (fep->pps_enable) { /* Reset turned it off, so adjust our status flag */ -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] net: fec: Reload PTP registers after link-state change 2024-09-16 14:19 ` [PATCH 2/2] net: fec: Reload PTP registers after link-state change Csókás, Bence @ 2024-09-16 15:05 ` Frank Li 2024-09-17 7:53 ` Csókás Bence 0 siblings, 1 reply; 6+ messages in thread From: Frank Li @ 2024-09-16 15:05 UTC (permalink / raw) To: Csókás, Bence Cc: imx, netdev, linux-kernel, Wei Fang, Shenwei Wang, Clark Wang, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Richard Cochran On Mon, Sep 16, 2024 at 04:19:31PM +0200, Csókás, Bence wrote: > On link-state change, the controller gets reset, > which clears all PTP registers, including PHC time, > calibrated clock correction values etc. For correct > IEEE 1588 operation we need to restore these after > the reset. I am not sure if it necessary. timer will be big offset after reset. ptpd should set_time then do clock frequency adjust, supposed just few ms, ptp time will get resync. of course, restore these value may reduce the resync time. Frank > > Signed-off-by: Csókás, Bence <csokas.bence@prolan.hu> > --- > drivers/net/ethernet/freescale/fec.h | 7 +++++ > drivers/net/ethernet/freescale/fec_main.c | 4 +++ > drivers/net/ethernet/freescale/fec_ptp.c | 35 +++++++++++++++++++++++ > 3 files changed, 46 insertions(+) > > diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h > index afa0bfb974e6..efe770fe337d 100644 > --- a/drivers/net/ethernet/freescale/fec.h > +++ b/drivers/net/ethernet/freescale/fec.h > @@ -691,11 +691,18 @@ struct fec_enet_private { > /* XDP BPF Program */ > struct bpf_prog *xdp_prog; > > + struct { > + u64 ns_sys, ns_phc; > + u32 at_corr; > + u8 at_inc_corr; > + } ptp_saved_state; > + > u64 ethtool_stats[]; > }; > > void fec_ptp_init(struct platform_device *pdev, int irq_idx); > void fec_ptp_restore_state(struct fec_enet_private *fep); > +void fec_ptp_save_state(struct fec_enet_private *fep); > void fec_ptp_stop(struct platform_device *pdev); > void fec_ptp_start_cyclecounter(struct net_device *ndev); > int fec_ptp_set(struct net_device *ndev, struct kernel_hwtstamp_config *config, > diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c > index 531b51091e7d..570f8a14d975 100644 > --- a/drivers/net/ethernet/freescale/fec_main.c > +++ b/drivers/net/ethernet/freescale/fec_main.c > @@ -1077,6 +1077,8 @@ fec_restart(struct net_device *ndev) > u32 rcntl = OPT_FRAME_SIZE | 0x04; > u32 ecntl = FEC_ECR_ETHEREN; > > + fec_ptp_save_state(fep); > + > /* Whack a reset. We should wait for this. > * For i.MX6SX SOC, enet use AXI bus, we use disable MAC > * instead of reset MAC itself. > @@ -1338,6 +1340,8 @@ fec_stop(struct net_device *ndev) > netdev_err(ndev, "Graceful transmit stop did not complete!\n"); > } > > + fec_ptp_save_state(fep); > + > /* Whack a reset. We should wait for this. > * For i.MX6SX SOC, enet use AXI bus, we use disable MAC > * instead of reset MAC itself. > diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c > index c5b89352373a..8011a6f3c4be 100644 > --- a/drivers/net/ethernet/freescale/fec_ptp.c > +++ b/drivers/net/ethernet/freescale/fec_ptp.c > @@ -770,9 +770,44 @@ void fec_ptp_init(struct platform_device *pdev, int irq_idx) > schedule_delayed_work(&fep->time_keep, HZ); > } > > +void fec_ptp_save_state(struct fec_enet_private *fep) > +{ > + unsigned long flags; > + u32 atime_inc_corr; > + > + spin_lock_irqsave(&fep->tmreg_lock, flags); > + > + fep->ptp_saved_state.ns_phc = timecounter_read(&fep->tc); > + fep->ptp_saved_state.ns_sys = ktime_get_ns(); > + > + fep->ptp_saved_state.at_corr = readl(fep->hwp + FEC_ATIME_CORR); > + atime_inc_corr = readl(fep->hwp + FEC_ATIME_INC) & FEC_T_INC_CORR_MASK; > + fep->ptp_saved_state.at_inc_corr = (u8)(atime_inc_corr >> FEC_T_INC_CORR_OFFSET); > + > + spin_unlock_irqrestore(&fep->tmreg_lock, flags); > +} > + > /* Restore PTP functionality after a reset */ > void fec_ptp_restore_state(struct fec_enet_private *fep) > { > + u32 atime_inc = readl(fep->hwp + FEC_ATIME_INC) & FEC_T_INC_MASK; > + unsigned long flags; > + u32 counter; > + u64 ns; > + > + spin_lock_irqsave(&fep->tmreg_lock, flags); > + > + writel(fep->ptp_saved_state.at_corr, fep->hwp + FEC_ATIME_CORR); > + atime_inc |= ((u32)fep->ptp_saved_state.at_inc_corr) << FEC_T_INC_CORR_OFFSET; > + writel(atime_inc, fep->hwp + FEC_ATIME_INC); > + > + ns = ktime_get_ns() - fep->ptp_saved_state.ns_sys + fep->ptp_saved_state.ns_phc; > + counter = ns & fep->cc.mask; > + writel(counter, fep->hwp + FEC_ATIME); > + timecounter_init(&fep->tc, &fep->cc, ns); > + > + spin_unlock_irqrestore(&fep->tmreg_lock, flags); > + > /* Restart PPS if needed */ > if (fep->pps_enable) { > /* Reset turned it off, so adjust our status flag */ > -- > 2.34.1 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] net: fec: Reload PTP registers after link-state change 2024-09-16 15:05 ` Frank Li @ 2024-09-17 7:53 ` Csókás Bence 2024-09-17 15:23 ` Frank Li 0 siblings, 1 reply; 6+ messages in thread From: Csókás Bence @ 2024-09-17 7:53 UTC (permalink / raw) To: Frank Li Cc: imx, netdev, linux-kernel, Wei Fang, Shenwei Wang, Clark Wang, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Richard Cochran Hi! On 9/16/24 17:05, Frank Li wrote: > On Mon, Sep 16, 2024 at 04:19:31PM +0200, Csókás, Bence wrote: >> On link-state change, the controller gets reset, >> which clears all PTP registers, including PHC time, >> calibrated clock correction values etc. For correct >> IEEE 1588 operation we need to restore these after >> the reset. > > I am not sure if it necessary. timer will be big offset after reset. ptpd > should set_time then do clock frequency adjust, supposed just few ms, ptp > time will get resync. > > of course, restore these value may reduce the resync time. > > Frank There's 3 problems with that: 1. ATCORR, ATINC and ATPER will not be restored, therefore precision will be immediately lost. 2. ptpd does NOT set the time, only once, on startup. Currently, on link-down, ptpd tries to correct for the missing 54 years by making the PHC tick 3% faster (therefore the PPS signal will have a frequency error as well), which will never get it there. One work-around is to periodically re-start ptpd, but this is obviously sub-optimal. 3. If the PTP server goes away, there's no way to restore the time. Whereas if you save and reload it, you can continue, although with degraded precision. Bence ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] net: fec: Reload PTP registers after link-state change 2024-09-17 7:53 ` Csókás Bence @ 2024-09-17 15:23 ` Frank Li 0 siblings, 0 replies; 6+ messages in thread From: Frank Li @ 2024-09-17 15:23 UTC (permalink / raw) To: Csókás Bence Cc: imx, netdev, linux-kernel, Wei Fang, Shenwei Wang, Clark Wang, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Richard Cochran On Tue, Sep 17, 2024 at 09:53:07AM +0200, Csókás Bence wrote: > Hi! > > On 9/16/24 17:05, Frank Li wrote: > > On Mon, Sep 16, 2024 at 04:19:31PM +0200, Csókás, Bence wrote: > > > On link-state change, the controller gets reset, > > > which clears all PTP registers, including PHC time, > > > calibrated clock correction values etc. For correct > > > IEEE 1588 operation we need to restore these after > > > the reset. > > > > I am not sure if it necessary. timer will be big offset after reset. ptpd > > should set_time then do clock frequency adjust, supposed just few ms, ptp > > time will get resync. > > > > of course, restore these value may reduce the resync time. > > > > Frank > > There's 3 problems with that: > 1. ATCORR, ATINC and ATPER will not be restored, therefore precision will be > immediately lost. Yes, It will be good if you have compare time back to sync with/without save and restore. I think ptpd always to make it sync again, just take little bit longer. > 2. ptpd does NOT set the time, only once, on startup. Currently, on > link-down, ptpd tries to correct for the missing 54 years by making the PHC > tick 3% faster (therefore the PPS signal will have a frequency error as > well), which will never get it there. One work-around is to periodically > re-start ptpd, but this is obviously sub-optimal. Supposed it should be ptpd bug. I remember time draft is bigger enough, it should be set the time again. Maybe ptp change the policy. For example, if system suspend 1min and resume back, absolute offset will be 1min offset. ptpd should set the time to catch up offset firstly, then resync frequency. > 3. If the PTP server goes away, there's no way to restore the time. Whereas > if you save and reload it, you can continue, although with degraded > precision. This one make sense, you can add to commit message. > > Bence > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] net: fec: Restart PPS after link state change 2024-09-16 14:19 [PATCH 1/2] net: fec: Restart PPS after link state change Csókás, Bence 2024-09-16 14:19 ` [PATCH 2/2] net: fec: Reload PTP registers after link-state change Csókás, Bence @ 2024-09-19 8:18 ` Paolo Abeni 1 sibling, 0 replies; 6+ messages in thread From: Paolo Abeni @ 2024-09-19 8:18 UTC (permalink / raw) To: Csókás, Bence, imx, netdev, linux-kernel Cc: Wei Fang, Shenwei Wang, Clark Wang, David S. Miller, Eric Dumazet, Jakub Kicinski, Richard Cochran On 9/16/24 16:19, Csókás, Bence wrote: > On link state change, the controller gets reset, > causing PPS to drop out. Re-enable PPS if it was > enabled before the controller reset. > > Signed-off-by: Csókás, Bence <csokas.bence@prolan.hu> This and the next patch look like a bugfix, as such they should target the 'net' tree: https://elixir.bootlin.com/linux/v6.11/source/Documentation/process/maintainer-netdev.rst#L64 and you should include suitable fixes tag in each of them. Thanks, Paolo ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-19 8:18 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-09-16 14:19 [PATCH 1/2] net: fec: Restart PPS after link state change Csókás, Bence 2024-09-16 14:19 ` [PATCH 2/2] net: fec: Reload PTP registers after link-state change Csókás, Bence 2024-09-16 15:05 ` Frank Li 2024-09-17 7:53 ` Csókás Bence 2024-09-17 15:23 ` Frank Li 2024-09-19 8:18 ` [PATCH 1/2] net: fec: Restart PPS after link state change Paolo Abeni
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox