netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 resubmit] fec: Restart PPS after link state change
@ 2022-08-22  8:10 Csókás Bence
  2022-08-24  9:00 ` patchwork-bot+netdevbpf
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Csókás Bence @ 2022-08-22  8:10 UTC (permalink / raw)
  To: netdev
  Cc: Richard Cochran, David S. Miller, Jakub Kicinski,
	qiangqing.zhang, Andrew Lunn, Csókás Bence

On link state change, the controller gets reset,
causing PPS to drop out and the PHC to lose its
time and calibration. So we restart it if needed,
restoring calibration and time registers.

Changes since v2:
* Add `fec_ptp_save_state()`/`fec_ptp_restore_state()`
* Use `ktime_get_real_ns()`
* Use `BIT()` macro
Changes since v1:
* More ECR #define's
* Stop PPS in `fec_ptp_stop()`

Signed-off-by: Csókás Bence <csokas.bence@prolan.hu>
---
 drivers/net/ethernet/freescale/fec.h      | 10 ++++++
 drivers/net/ethernet/freescale/fec_main.c | 42 ++++++++++++++++++++---
 drivers/net/ethernet/freescale/fec_ptp.c  | 29 ++++++++++++++++
 3 files changed, 77 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index ed7301b69169..0cebe4b63adb 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -634,6 +634,13 @@ struct fec_enet_private {
 	int pps_enable;
 	unsigned int next_counter;
 
+	struct {
+		struct timespec64 ts_phc;
+		u64 ns_sys;
+		u32 at_corr;
+		u8 at_inc_corr;
+	} ptp_saved_state;
+
 	u64 ethtool_stats[];
 };
 
@@ -644,5 +651,8 @@ void fec_ptp_disable_hwts(struct net_device *ndev);
 int fec_ptp_set(struct net_device *ndev, struct ifreq *ifr);
 int fec_ptp_get(struct net_device *ndev, struct ifreq *ifr);
 
+void fec_ptp_save_state(struct fec_enet_private *fep);
+int fec_ptp_restore_state(struct fec_enet_private *fep);
+
 /****************************************************************************/
 #endif /* FEC_H */
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index e8e2aa1e7f01..b0d60f898249 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -285,8 +285,11 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
 #define FEC_MMFR_TA		(2 << 16)
 #define FEC_MMFR_DATA(v)	(v & 0xffff)
 /* FEC ECR bits definition */
-#define FEC_ECR_MAGICEN		(1 << 2)
-#define FEC_ECR_SLEEP		(1 << 3)
+#define FEC_ECR_RESET   BIT(0)
+#define FEC_ECR_ETHEREN BIT(1)
+#define FEC_ECR_MAGICEN BIT(2)
+#define FEC_ECR_SLEEP   BIT(3)
+#define FEC_ECR_EN1588  BIT(4)
 
 #define FEC_MII_TIMEOUT		30000 /* us */
 
@@ -982,6 +985,9 @@ fec_restart(struct net_device *ndev)
 	u32 temp_mac[2];
 	u32 rcntl = OPT_FRAME_SIZE | 0x04;
 	u32 ecntl = 0x2; /* ETHEREN */
+	struct ptp_clock_request ptp_rq = { .type = PTP_CLK_REQ_PPS };
+
+	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
@@ -1135,7 +1141,7 @@ fec_restart(struct net_device *ndev)
 	}
 
 	if (fep->bufdesc_ex)
-		ecntl |= (1 << 4);
+		ecntl |= FEC_ECR_EN1588;
 
 	if (fep->quirks & FEC_QUIRK_DELAYED_CLKS_SUPPORT &&
 	    fep->rgmii_txc_dly)
@@ -1156,6 +1162,14 @@ fec_restart(struct net_device *ndev)
 	if (fep->bufdesc_ex)
 		fec_ptp_start_cyclecounter(ndev);
 
+	/* Restart PPS if needed */
+	if (fep->pps_enable) {
+		/* Clear flag so fec_ptp_enable_pps() doesn't return immediately */
+		fep->pps_enable = 0;
+		fec_ptp_restore_state(fep);
+		fep->ptp_caps.enable(&fep->ptp_caps, &ptp_rq, 1);
+	}
+
 	/* Enable interrupts we wish to service */
 	if (fep->link)
 		writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
@@ -1206,6 +1220,8 @@ fec_stop(struct net_device *ndev)
 	struct fec_enet_private *fep = netdev_priv(ndev);
 	u32 rmii_mode = readl(fep->hwp + FEC_R_CNTRL) & (1 << 8);
 	u32 val;
+	struct ptp_clock_request ptp_rq = { .type = PTP_CLK_REQ_PPS };
+	u32 ecntl = 0;
 
 	/* We cannot expect a graceful transmit stop without link !!! */
 	if (fep->link) {
@@ -1215,6 +1231,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.
@@ -1234,12 +1252,28 @@ fec_stop(struct net_device *ndev)
 	writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
 	writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
 
+	if (fep->bufdesc_ex)
+		ecntl |= FEC_ECR_EN1588;
+
 	/* We have to keep ENET enabled to have MII interrupt stay working */
 	if (fep->quirks & FEC_QUIRK_ENET_MAC &&
 		!(fep->wol_flag & FEC_WOL_FLAG_SLEEP_ON)) {
-		writel(2, fep->hwp + FEC_ECNTRL);
+		ecntl |= FEC_ECR_ETHEREN;
 		writel(rmii_mode, fep->hwp + FEC_R_CNTRL);
 	}
+
+	writel(ecntl, fep->hwp + FEC_ECNTRL);
+
+	if (fep->bufdesc_ex)
+		fec_ptp_start_cyclecounter(ndev);
+
+	/* Restart PPS if needed */
+	if (fep->pps_enable) {
+		/* Clear flag so fec_ptp_enable_pps() doesn't return immediately */
+		fep->pps_enable = 0;
+		fec_ptp_restore_state(fep);
+		fep->ptp_caps.enable(&fep->ptp_caps, &ptp_rq, 1);
+	}
 }
 
 
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index 3dc3c0b626c2..c74d04f4b2fd 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -633,7 +633,36 @@ void fec_ptp_stop(struct platform_device *pdev)
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct fec_enet_private *fep = netdev_priv(ndev);
 
+	if (fep->pps_enable)
+		fec_ptp_enable_pps(fep, 0);
+
 	cancel_delayed_work_sync(&fep->time_keep);
 	if (fep->ptp_clock)
 		ptp_clock_unregister(fep->ptp_clock);
 }
+
+void fec_ptp_save_state(struct fec_enet_private *fep)
+{
+	u32 atime_inc_corr;
+
+	fec_ptp_gettime(&fep->ptp_caps, &fep->ptp_saved_state.ts_phc);
+	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);
+}
+
+int fec_ptp_restore_state(struct fec_enet_private *fep)
+{
+	u32 atime_inc = readl(fep->hwp + FEC_ATIME_INC) & FEC_T_INC_MASK;
+	u64 ns_sys;
+
+	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_sys = ktime_get_ns() - fep->ptp_saved_state.ns_sys;
+	timespec64_add_ns(&fep->ptp_saved_state.ts_phc, ns_sys);
+	return fec_ptp_settime(&fep->ptp_caps, &fep->ptp_saved_state.ts_phc);
+}
-- 
2.25.1


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

* Re: [PATCH v3 resubmit] fec: Restart PPS after link state change
  2022-08-22  8:10 [PATCH v3 resubmit] fec: Restart PPS after link state change Csókás Bence
@ 2022-08-24  9:00 ` patchwork-bot+netdevbpf
  2022-08-27 16:09 ` BUG: " Marc Kleine-Budde
  2022-09-05 18:05 ` Guenter Roeck
  2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-08-24  9:00 UTC (permalink / raw)
  To: =?utf-8?b?Q3PDs2vDoXMgQmVuY2UgPGNzb2thcy5iZW5jZUBwcm9sYW4uaHU+?=
  Cc: netdev, richardcochran, davem, kuba, qiangqing.zhang, andrew

Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Mon, 22 Aug 2022 10:10:52 +0200 you wrote:
> On link state change, the controller gets reset,
> causing PPS to drop out and the PHC to lose its
> time and calibration. So we restart it if needed,
> restoring calibration and time registers.
> 
> Changes since v2:
> * Add `fec_ptp_save_state()`/`fec_ptp_restore_state()`
> * Use `ktime_get_real_ns()`
> * Use `BIT()` macro
> Changes since v1:
> * More ECR #define's
> * Stop PPS in `fec_ptp_stop()`
> 
> [...]

Here is the summary with links:
  - [v3,resubmit] fec: Restart PPS after link state change
    https://git.kernel.org/netdev/net/c/f79959220fa5

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

* BUG: Re: [PATCH v3 resubmit] fec: Restart PPS after link state change
  2022-08-22  8:10 [PATCH v3 resubmit] fec: Restart PPS after link state change Csókás Bence
  2022-08-24  9:00 ` patchwork-bot+netdevbpf
@ 2022-08-27 16:09 ` Marc Kleine-Budde
  2022-09-05 18:05 ` Guenter Roeck
  2 siblings, 0 replies; 4+ messages in thread
From: Marc Kleine-Budde @ 2022-08-27 16:09 UTC (permalink / raw)
  To: Csókás Bence
  Cc: netdev, Richard Cochran, David S. Miller, Jakub Kicinski,
	qiangqing.zhang, Andrew Lunn, kernel

[-- Attachment #1: Type: text/plain, Size: 4842 bytes --]

On 22.08.2022 10:10:52, Csókás Bence wrote:
> On link state change, the controller gets reset,
> causing PPS to drop out and the PHC to lose its
> time and calibration. So we restart it if needed,
> restoring calibration and time registers.
> 
> Changes since v2:
> * Add `fec_ptp_save_state()`/`fec_ptp_restore_state()`
> * Use `ktime_get_real_ns()`
> * Use `BIT()` macro
> Changes since v1:
> * More ECR #define's
> * Stop PPS in `fec_ptp_stop()`
> 
> Signed-off-by: Csókás Bence <csokas.bence@prolan.hu>

current net-next/main fails on my imx6 with:

| [   14.001542] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:283                                                                                                 
| [   14.010604] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 13, name: kworker/0:1                                                                                                     
| [   14.018737] preempt_count: 201, expected: 0                                                                                                                                                  
| [   14.022931] CPU: 0 PID: 13 Comm: kworker/0:1 Not tainted 6.0.0-rc2+ #225                                                                                                                     
| [   14.029643] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)                                                                                                                       
| [   14.036175] Workqueue: events_power_efficient phy_state_machine                                                                                                                              
| [   14.042121] [<c010ffe0>] (unwind_backtrace) from [<c010ac04>] (show_stack+0x10/0x14)                                                                                                         
| [   14.049889] [<c010ac04>] (show_stack) from [<c0ccab04>] (dump_stack_lvl+0x40/0x4c)                                                                                                           
| [   14.057479] [<c0ccab04>] (dump_stack_lvl) from [<c014ed7c>] (__might_resched+0x11c/0x154)                                                                                                    
| [   14.065678] [<c014ed7c>] (__might_resched) from [<c0cd9c20>] (mutex_lock+0x18/0x58)                                                                                                          
| [   14.073356] [<c0cd9c20>] (mutex_lock) from [<c082b59c>] (fec_ptp_gettime+0x2c/0xc4)                                                                                                          
| [   14.081035] [<c082b59c>] (fec_ptp_gettime) from [<c082bff4>] (fec_ptp_save_state+0x14/0x50)                                                                                                  
| [   14.089403] [<c082bff4>] (fec_ptp_save_state) from [<c0826ee0>] (fec_restart+0x40/0x6f4)                                                                                                     
| [   14.097510] [<c0826ee0>] (fec_restart) from [<c082b170>] (fec_enet_adjust_link+0xb0/0x21c)                                                                                                   
| [   14.105789] [<c082b170>] (fec_enet_adjust_link) from [<c0819bb4>] (phy_link_change+0x28/0x54)                                                                                                
| [   14.114333] [<c0819bb4>] (phy_link_change) from [<c0815688>] (phy_check_link_status+0x78/0xb4)
| [   14.122969] [<c0815688>] (phy_check_link_status) from [<c0816bec>] (phy_state_machine+0x68/0x29c)
| [   14.131857] [<c0816bec>] (phy_state_machine) from [<c0140604>] (process_one_work+0x1f8/0x410)
| [   14.140398] [<c0140604>] (process_one_work) from [<c01419c8>] (worker_thread+0x2c/0x544)
| [   14.148502] [<c01419c8>] (worker_thread) from [<c0148a4c>] (kthread+0xe4/0xf0)
| [   14.155739] [<c0148a4c>] (kthread) from [<c0100170>] (ret_from_fork+0x14/0x24)
| [   14.162973] Exception stack(0xc2097fb0 to 0xc2097ff8)
| [   14.168032] 7fa0:                                     00000000 00000000 00000000 00000000
| [   14.176217] 7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
| [   14.184402] 7fe0: 00000000 00000000 00000000 00000000 00000013 00000000
| [   14.191309] fec 2188000.ethernet lan0: Link is Up - 100Mbps/Full - flow control rx/tx

Reverting this patch "fixes" the problem.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v3 resubmit] fec: Restart PPS after link state change
  2022-08-22  8:10 [PATCH v3 resubmit] fec: Restart PPS after link state change Csókás Bence
  2022-08-24  9:00 ` patchwork-bot+netdevbpf
  2022-08-27 16:09 ` BUG: " Marc Kleine-Budde
@ 2022-09-05 18:05 ` Guenter Roeck
  2 siblings, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2022-09-05 18:05 UTC (permalink / raw)
  To: Csókás Bence
  Cc: netdev, Richard Cochran, David S. Miller, Jakub Kicinski,
	qiangqing.zhang, Andrew Lunn

On Mon, Aug 22, 2022 at 10:10:52AM +0200, Csókás Bence wrote:
> On link state change, the controller gets reset,
> causing PPS to drop out and the PHC to lose its
> time and calibration. So we restart it if needed,
> restoring calibration and time registers.
> 
> Changes since v2:
> * Add `fec_ptp_save_state()`/`fec_ptp_restore_state()`
> * Use `ktime_get_real_ns()`
> * Use `BIT()` macro
> Changes since v1:
> * More ECR #define's
> * Stop PPS in `fec_ptp_stop()`
> 
> Signed-off-by: Csókás Bence <csokas.bence@prolan.hu>

Besides the problem already reported (widespread BUG: sleeping function
called from invalid context at kernel/locking/mutex.c:580, called from
fec_ptp_gettime), this patch results in a crash when trying to boot the
m68k:mcf5208evb emulation in qemu. Reverting this patch fixes the problem.

Guenter

---
*** ILLEGAL INSTRUCTION ***   FORMAT=4
Current process id is 1
BAD KERNEL TRAP: 00000000
PC: [<00000000>] 0x0
SR: 2714  SP: (ptrval)  a2: 40829634
d0: 00002710    d1: 00002010    d2: 40829442    d3: 00002010
d4: 00000000    d5: 402e818a    a0: 00000000    a1: 40824000
Process swapper (pid: 1, task=(ptrval))
Frame format=4 eff addr=400681c2 pc=00000000
Stack from 40831cec:
        40829442 00002010 40831e0c 402e818a 40ba2000 00000008 408295a4 40829000
        401b0c42 40829634 40829420 00000000 40829420 40829000 00000000 00000001
        00000000 401b130e 408295a4 40829702 40347ee0 401ad026 40829420 40347eea
        00000000 40831e0c 402e818a 40ba2000 00000008 40347ee0 40829000 fffffff8
        4082945a 4082945a 40831e14 00000002 00000000 00000000 00000000 00000000
        00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
Call Trace: [<401b0c42>] fec_ptp_gettime+0x3a/0x8c
 [<401b130e>] fec_ptp_save_state+0x12/0x3e
 [<401ad026>] fec_restart+0x5a/0x770
 [<401ae256>] fec_probe+0x74a/0xd06
 [<402c10a0>] strcpy+0x0/0x18
 [<402d3824>] mutex_unlock+0x0/0x40
 [<402d37e4>] mutex_lock+0x0/0x40
 [<401840ba>] uart_get_icount+0x9c/0x198
 [<40193876>] platform_probe+0x22/0x60
 [<40191c52>] really_probe+0xb0/0x2e4
 [<40191f40>] driver_probe_device+0x24/0x112
 [<40192290>] __driver_attach+0x7a/0x200
 [<402b6458>] klist_next+0x0/0x154
 [<40192216>] __driver_attach+0x0/0x200
 [<4036cac2>] do_one_initcall+0x0/0x22c
 [<4019025c>] bus_for_each_dev+0x6a/0xae
 [<40192866>] driver_attach+0x16/0x1c
 [<40192216>] __driver_attach+0x0/0x200
 [<40190b5a>] bus_add_driver+0x154/0x222
 [<40192f9a>] driver_register+0x6c/0xf0
 [<40377d4a>] fec_driver_init+0x0/0x12
 [<40377d58>] fec_driver_init+0xe/0x12
 [<4036cb1e>] do_one_initcall+0x5c/0x22c
 [<402c10a0>] strcpy+0x0/0x18
 [<4036cac2>] do_one_initcall+0x0/0x22c
 [<402c10a0>] strcpy+0x0/0x18
 [<4036cac2>] do_one_initcall+0x0/0x22c
 [<4003d686>] parse_args+0x0/0x390
 [<4036ce8a>] kernel_init_freeable+0x144/0x1a4
 [<4003d686>] parse_args+0x0/0x390
 [<4036ce98>] kernel_init_freeable+0x152/0x1a4
 [<40377d4a>] fec_driver_init+0x0/0x12
 [<400977d6>] kfree+0x0/0x206
 [<402d2288>] schedule+0x0/0x120
 [<4003d686>] parse_args+0x0/0x390
 [<402cb5d0>] _printk+0x0/0x18
 [<402d0bb0>] kernel_init+0x0/0xf0
 [<402d0bca>] kernel_init+0x1a/0xf0
 [<400208d4>] ret_from_kernel_thread+0xc/0x14
Code: 0000 0000 0000 0000 0000 0000 0000 0000 <0000> 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000
Disabling lock debugging due to kernel taint
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

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

end of thread, other threads:[~2022-09-05 18:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-22  8:10 [PATCH v3 resubmit] fec: Restart PPS after link state change Csókás Bence
2022-08-24  9:00 ` patchwork-bot+netdevbpf
2022-08-27 16:09 ` BUG: " Marc Kleine-Budde
2022-09-05 18:05 ` Guenter Roeck

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).