netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: fec: ptp: fix convergence issue to support LinuxPTP stack
@ 2014-10-14  5:39 Fugang Duan
  2014-10-14 10:36 ` Richard Cochran
  0 siblings, 1 reply; 12+ messages in thread
From: Fugang Duan @ 2014-10-14  5:39 UTC (permalink / raw)
  To: davem; +Cc: netdev, bhutchings, b20596, b38611

IEEE 1588 module has one hw issue in capturing the ATVR register. According
to the user manual it is:
		ENET0->ATCR |= ENET_ATCR_CAPTURE_MASK;
		while(ENET0->ATCR & ENET_ATCR_CAPTURE_MASK);
		ts_counter_ns = ENET0->ATVR;

Incorrect behavior for ENET_ATCR[Capture and Restart Bits]. These bits will always
read a value zero. According to SPEC, when these bits are set to 1'b1, these should
hold value 1'b1 until the counter value is capture in the register clock domain.

Unfortunately there is a bug with the way the bit "ENET_ATCR_CAPTURE" clears.
So need something like:
		ENET0->ATCR |= ENET_ATCR_CAPTURE_MASK;
		wait();
		ts_counter_ns = ENET0->ATVR;

The wait-time to be at least 6 clock cycle of the slower clock between the register
clock and the 1588 clock. The 1588 ts_clk is 25Mhz, register clock is 66Mhz, so the
wait-time must be greater than 240ns (40ns * 6). The workaround is that adding 1us
delay before read ATVR.

Signed-off-by: Fugang Duan <B38611@freescale.com>
---
 drivers/net/ethernet/freescale/fec.h      |   47 +++++++++++++++++++++++++++++
 drivers/net/ethernet/freescale/fec_main.c |   43 +-------------------------
 drivers/net/ethernet/freescale/fec_ptp.c  |    5 +++
 3 files changed, 53 insertions(+), 42 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 1d5e182..b4dccec 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -367,6 +367,53 @@ struct bufdesc_ex {
 #define FEC_VLAN_TAG_LEN       0x04
 #define FEC_ETHTYPE_LEN                0x02
 
+/* Controller is ENET-MAC */
+#define FEC_QUIRK_ENET_MAC		(1 << 0)
+/* Controller needs driver to swap frame */
+#define FEC_QUIRK_SWAP_FRAME		(1 << 1)
+/* Controller uses gasket */
+#define FEC_QUIRK_USE_GASKET		(1 << 2)
+/* Controller has GBIT support */
+#define FEC_QUIRK_HAS_GBIT		(1 << 3)
+/* Controller has extend desc buffer */
+#define FEC_QUIRK_HAS_BUFDESC_EX	(1 << 4)
+/* Controller has hardware checksum support */
+#define FEC_QUIRK_HAS_CSUM		(1 << 5)
+/* Controller has hardware vlan support */
+#define FEC_QUIRK_HAS_VLAN		(1 << 6)
+/* ENET IP errata ERR006358
+ *
+ * If the ready bit in the transmit buffer descriptor (TxBD[R]) is previously
+ * detected as not set during a prior frame transmission, then the
+ * ENET_TDAR[TDAR] bit is cleared at a later time, even if additional TxBDs
+ * were added to the ring and the ENET_TDAR[TDAR] bit is set. This results in
+ * frames not being transmitted until there is a 0-to-1 transition on
+ * ENET_TDAR[TDAR].
+ */
+#define FEC_QUIRK_ERR006358            (1 << 7)
+/* ENET IP hw AVB
+ *
+ * i.MX6SX ENET IP add Audio Video Bridging (AVB) feature support.
+ * - Two class indicators on receive with configurable priority
+ * - Two class indicators and line speed timer on transmit allowing
+ *   implementation class credit based shapers externally
+ * - Additional DMA registers provisioned to allow managing up to 3
+ *   independent rings
+ */
+#define FEC_QUIRK_HAS_AVB		(1 << 8)
+/* There is a TDAR race condition for mutliQ when the software sets TDAR
+ * and the UDMA clears TDAR simultaneously or in a small window (2-4 cycles).
+ * This will cause the udma_tx and udma_tx_arbiter state machines to hang.
+ * The issue exist at i.MX6SX enet IP.
+ */
+#define FEC_QUIRK_ERR007885		(1 << 9)
+/* ENET Block Guide/ Chapter for the iMX6SLX (PELE) address one issue:
+ * Incorrect behavior for ENET_ATCR[Capture and Restart Bits]. These bits will
+ * always read a value zero. When these bits are set to 1'b1, these should hold
+ * value 1'b1 until the counter value is capture in the register clock domain.
+ */
+#define FEC_QUIRK_BUG_CAPTURE		(1 << 10)
+
 struct fec_enet_priv_tx_q {
 	int index;
 	unsigned char *tx_bounce[TX_RING_SIZE];
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 7a8209e..26d0525 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -78,47 +78,6 @@ static void fec_enet_itr_coal_init(struct net_device *ndev);
 #define FEC_ENET_RAFL_V	0x8
 #define FEC_ENET_OPD_V	0xFFF0
 
-/* Controller is ENET-MAC */
-#define FEC_QUIRK_ENET_MAC		(1 << 0)
-/* Controller needs driver to swap frame */
-#define FEC_QUIRK_SWAP_FRAME		(1 << 1)
-/* Controller uses gasket */
-#define FEC_QUIRK_USE_GASKET		(1 << 2)
-/* Controller has GBIT support */
-#define FEC_QUIRK_HAS_GBIT		(1 << 3)
-/* Controller has extend desc buffer */
-#define FEC_QUIRK_HAS_BUFDESC_EX	(1 << 4)
-/* Controller has hardware checksum support */
-#define FEC_QUIRK_HAS_CSUM		(1 << 5)
-/* Controller has hardware vlan support */
-#define FEC_QUIRK_HAS_VLAN		(1 << 6)
-/* ENET IP errata ERR006358
- *
- * If the ready bit in the transmit buffer descriptor (TxBD[R]) is previously
- * detected as not set during a prior frame transmission, then the
- * ENET_TDAR[TDAR] bit is cleared at a later time, even if additional TxBDs
- * were added to the ring and the ENET_TDAR[TDAR] bit is set. This results in
- * frames not being transmitted until there is a 0-to-1 transition on
- * ENET_TDAR[TDAR].
- */
-#define FEC_QUIRK_ERR006358            (1 << 7)
-/* ENET IP hw AVB
- *
- * i.MX6SX ENET IP add Audio Video Bridging (AVB) feature support.
- * - Two class indicators on receive with configurable priority
- * - Two class indicators and line speed timer on transmit allowing
- *   implementation class credit based shapers externally
- * - Additional DMA registers provisioned to allow managing up to 3
- *   independent rings
- */
-#define FEC_QUIRK_HAS_AVB		(1 << 8)
-/* There is a TDAR race condition for mutliQ when the software sets TDAR
- * and the UDMA clears TDAR simultaneously or in a small window (2-4 cycles).
- * This will cause the udma_tx and udma_tx_arbiter state machines to hang.
- * The issue exist at i.MX6SX enet IP.
- */
-#define FEC_QUIRK_ERR007885		(1 << 9)
-
 static struct platform_device_id fec_devtype[] = {
 	{
 		/* keep it for coldfire */
@@ -146,7 +105,7 @@ static struct platform_device_id fec_devtype[] = {
 		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT |
 				FEC_QUIRK_HAS_BUFDESC_EX | FEC_QUIRK_HAS_CSUM |
 				FEC_QUIRK_HAS_VLAN | FEC_QUIRK_HAS_AVB |
-				FEC_QUIRK_ERR007885,
+				FEC_QUIRK_ERR007885 | FEC_QUIRK_BUG_CAPTURE,
 	}, {
 		/* sentinel */
 	}
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index cca3617..380bb10 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -82,12 +82,17 @@ static cycle_t fec_ptp_read(const struct cyclecounter *cc)
 {
 	struct fec_enet_private *fep =
 		container_of(cc, struct fec_enet_private, cc);
+	const struct platform_device_id *id_entry =
+		platform_get_device_id(fep->pdev);
 	u32 tempval;
 
 	tempval = readl(fep->hwp + FEC_ATIME_CTRL);
 	tempval |= FEC_T_CTRL_CAPTURE;
 	writel(tempval, fep->hwp + FEC_ATIME_CTRL);
 
+	if (id_entry->driver_data & FEC_QUIRK_BUG_CAPTURE)
+		udelay(1);
+
 	return readl(fep->hwp + FEC_ATIME);
 }
 
-- 
1.7.8

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

* Re: [PATCH] net: fec: ptp: fix convergence issue to support LinuxPTP stack
  2014-10-14  5:39 [PATCH] net: fec: ptp: fix convergence issue to support LinuxPTP stack Fugang Duan
@ 2014-10-14 10:36 ` Richard Cochran
  2014-10-14 16:52   ` David Miller
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Cochran @ 2014-10-14 10:36 UTC (permalink / raw)
  To: Fugang Duan; +Cc: davem, netdev, bhutchings, b20596

On Tue, Oct 14, 2014 at 01:39:47PM +0800, Fugang Duan wrote:
> IEEE 1588 module has one hw issue in capturing the ATVR register. According
> to the user manual it is:
> 		ENET0->ATCR |= ENET_ATCR_CAPTURE_MASK;
> 		while(ENET0->ATCR & ENET_ATCR_CAPTURE_MASK);
> 		ts_counter_ns = ENET0->ATVR;

...

> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
> index cca3617..380bb10 100644
> --- a/drivers/net/ethernet/freescale/fec_ptp.c
> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
> @@ -82,12 +82,17 @@ static cycle_t fec_ptp_read(const struct cyclecounter *cc)
>  {
>  	struct fec_enet_private *fep =
>  		container_of(cc, struct fec_enet_private, cc);
> +	const struct platform_device_id *id_entry =
> +		platform_get_device_id(fep->pdev);
>  	u32 tempval;
>  
>  	tempval = readl(fep->hwp + FEC_ATIME_CTRL);
>  	tempval |= FEC_T_CTRL_CAPTURE;
>  	writel(tempval, fep->hwp + FEC_ATIME_CTRL);
>  
> +	if (id_entry->driver_data & FEC_QUIRK_BUG_CAPTURE)
> +		udelay(1);
> +

What? You never had

	while(ENET0->ATCR & ENET_ATCR_CAPTURE_MASK);

in the first place. Maybe you should try that.

Did this code ever work? I guess not.

>  	return readl(fep->hwp + FEC_ATIME);
>  }

Thanks,
Richard

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

* Re: [PATCH] net: fec: ptp: fix convergence issue to support LinuxPTP stack
  2014-10-14 10:36 ` Richard Cochran
@ 2014-10-14 16:52   ` David Miller
  2014-10-14 18:27     ` Frank.Li
  0 siblings, 1 reply; 12+ messages in thread
From: David Miller @ 2014-10-14 16:52 UTC (permalink / raw)
  To: richardcochran; +Cc: b38611, netdev, bhutchings, b20596

From: Richard Cochran <richardcochran@gmail.com>
Date: Tue, 14 Oct 2014 12:36:10 +0200

> On Tue, Oct 14, 2014 at 01:39:47PM +0800, Fugang Duan wrote:
>> IEEE 1588 module has one hw issue in capturing the ATVR register. According
>> to the user manual it is:
>> 		ENET0->ATCR |= ENET_ATCR_CAPTURE_MASK;
>> 		while(ENET0->ATCR & ENET_ATCR_CAPTURE_MASK);
>> 		ts_counter_ns = ENET0->ATVR;
> 
> ...
> 
>> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
>> index cca3617..380bb10 100644
>> --- a/drivers/net/ethernet/freescale/fec_ptp.c
>> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
>> @@ -82,12 +82,17 @@ static cycle_t fec_ptp_read(const struct cyclecounter *cc)
>>  {
>>  	struct fec_enet_private *fep =
>>  		container_of(cc, struct fec_enet_private, cc);
>> +	const struct platform_device_id *id_entry =
>> +		platform_get_device_id(fep->pdev);
>>  	u32 tempval;
>>  
>>  	tempval = readl(fep->hwp + FEC_ATIME_CTRL);
>>  	tempval |= FEC_T_CTRL_CAPTURE;
>>  	writel(tempval, fep->hwp + FEC_ATIME_CTRL);
>>  
>> +	if (id_entry->driver_data & FEC_QUIRK_BUG_CAPTURE)
>> +		udelay(1);
>> +
> 
> What? You never had
> 
> 	while(ENET0->ATCR & ENET_ATCR_CAPTURE_MASK);
> 
> in the first place. Maybe you should try that.
> 
> Did this code ever work? I guess not.

Also see Luwei Zhou's series:

http://patchwork.ozlabs.org/patch/398467/
http://patchwork.ozlabs.org/patch/398465/
http://patchwork.ozlabs.org/patch/398466/

Let's get down to the bottom of all of this before I apply anything.  Does this
fec PTP code work at all?

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

* RE: [PATCH] net: fec: ptp: fix convergence issue to support LinuxPTP stack
  2014-10-14 16:52   ` David Miller
@ 2014-10-14 18:27     ` Frank.Li
  2014-10-14 18:39       ` Richard Cochran
  0 siblings, 1 reply; 12+ messages in thread
From: Frank.Li @ 2014-10-14 18:27 UTC (permalink / raw)
  To: David Miller, richardcochran@gmail.com
  Cc: fugang.duan@freescale.com, netdev@vger.kernel.org,
	bhutchings@solarflare.com



> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Tuesday, October 14, 2014 11:53 AM
> To: richardcochran@gmail.com
> Cc: Duan Fugang-B38611; netdev@vger.kernel.org; bhutchings@solarflare.com; Li
> Frank-B20596
> Subject: Re: [PATCH] net: fec: ptp: fix convergence issue to support LinuxPTP
> stack
> 
> From: Richard Cochran <richardcochran@gmail.com>
> Date: Tue, 14 Oct 2014 12:36:10 +0200
> 
> > On Tue, Oct 14, 2014 at 01:39:47PM +0800, Fugang Duan wrote:
> >> IEEE 1588 module has one hw issue in capturing the ATVR register.
> >> According to the user manual it is:
> >> 		ENET0->ATCR |= ENET_ATCR_CAPTURE_MASK;
> >> 		while(ENET0->ATCR & ENET_ATCR_CAPTURE_MASK);
> >> 		ts_counter_ns = ENET0->ATVR;
> >
> > ...
> >
> >> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c
> >> b/drivers/net/ethernet/freescale/fec_ptp.c
> >> index cca3617..380bb10 100644
> >> --- a/drivers/net/ethernet/freescale/fec_ptp.c
> >> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
> >> @@ -82,12 +82,17 @@ static cycle_t fec_ptp_read(const struct
> >> cyclecounter *cc)  {
> >>  	struct fec_enet_private *fep =
> >>  		container_of(cc, struct fec_enet_private, cc);
> >> +	const struct platform_device_id *id_entry =
> >> +		platform_get_device_id(fep->pdev);
> >>  	u32 tempval;
> >>
> >>  	tempval = readl(fep->hwp + FEC_ATIME_CTRL);
> >>  	tempval |= FEC_T_CTRL_CAPTURE;
> >>  	writel(tempval, fep->hwp + FEC_ATIME_CTRL);
> >>
> >> +	if (id_entry->driver_data & FEC_QUIRK_BUG_CAPTURE)
> >> +		udelay(1);
> >> +
> >
> > What? You never had
> >
> > 	while(ENET0->ATCR & ENET_ATCR_CAPTURE_MASK);
> >
> > in the first place. Maybe you should try that.
> >
> > Did this code ever work? I guess not.
> 
> Also see Luwei Zhou's series:
> 
> http://patchwork.ozlabs.org/patch/398467/
> http://patchwork.ozlabs.org/patch/398465/
> http://patchwork.ozlabs.org/patch/398466/
> 
> Let's get down to the bottom of all of this before I apply anything.  Does this
> fec PTP code work at all?

Yes, Luwei's 3 patch tested at MX6Q, MX6DL platform.

Only i.MX6SX has the problem, which fixed by fugang's patch.

i.MX6SX's ptp have the same issue even though without luwei's patch.

So luwei's patch is independent with fugang's patch.

Luwei's patch use hardware ptp adjustment method.
Fugang's patch fix the problem existed in MX6SX platform.

So I suggest apply luwei's 3 patches firstly.

Best regards
Frank Li

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

* Re: [PATCH] net: fec: ptp: fix convergence issue to support LinuxPTP stack
  2014-10-14 18:27     ` Frank.Li
@ 2014-10-14 18:39       ` Richard Cochran
  2014-10-14 18:43         ` Frank.Li
  2014-10-15  1:30         ` fugang.duan
  0 siblings, 2 replies; 12+ messages in thread
From: Richard Cochran @ 2014-10-14 18:39 UTC (permalink / raw)
  To: Frank.Li@freescale.com
  Cc: David Miller, fugang.duan@freescale.com, netdev@vger.kernel.org,
	bhutchings@solarflare.com

Frank,

On Tue, Oct 14, 2014 at 06:27:28PM +0000, Frank.Li@freescale.com wrote:
> Fugang's patch fix the problem existed in MX6SX platform.

You say that Fugang's patch fixes a quirk in the SX device.

But he said that the user's manual tells us to wait for some status
bits to clear. (See the third line, with the 'while' key word).

> > > On Tue, Oct 14, 2014 at 01:39:47PM +0800, Fugang Duan wrote:
> > >> IEEE 1588 module has one hw issue in capturing the ATVR register.
> > >> According to the user manual it is:
> > >> 		ENET0->ATCR |= ENET_ATCR_CAPTURE_MASK;
> > >> 		while(ENET0->ATCR & ENET_ATCR_CAPTURE_MASK);
> > >> 		ts_counter_ns = ENET0->ATVR;

That applies to all IMX devices, doesn't it?
In any case, Fugang's change log is not clear.

Confused,
Richard

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

* RE: [PATCH] net: fec: ptp: fix convergence issue to support LinuxPTP stack
  2014-10-14 18:39       ` Richard Cochran
@ 2014-10-14 18:43         ` Frank.Li
  2014-10-14 18:49           ` Richard Cochran
  2014-10-15  1:30         ` fugang.duan
  1 sibling, 1 reply; 12+ messages in thread
From: Frank.Li @ 2014-10-14 18:43 UTC (permalink / raw)
  To: Richard Cochran
  Cc: David Miller, fugang.duan@freescale.com, netdev@vger.kernel.org,
	bhutchings@solarflare.com



> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@gmail.com]
> Sent: Tuesday, October 14, 2014 1:40 PM
> To: Li Frank-B20596
> Cc: David Miller; Duan Fugang-B38611; netdev@vger.kernel.org;
> bhutchings@solarflare.com
> Subject: Re: [PATCH] net: fec: ptp: fix convergence issue to support LinuxPTP
> stack
> 
> Frank,
> 
> On Tue, Oct 14, 2014 at 06:27:28PM +0000, Frank.Li@freescale.com wrote:
> > Fugang's patch fix the problem existed in MX6SX platform.
> 
> You say that Fugang's patch fixes a quirk in the SX device.
> 
> But he said that the user's manual tells us to wait for some status bits to
> clear. (See the third line, with the 'while' key word).
> 
> > > > On Tue, Oct 14, 2014 at 01:39:47PM +0800, Fugang Duan wrote:
> > > >> IEEE 1588 module has one hw issue in capturing the ATVR register.
> > > >> According to the user manual it is:
> > > >> 		ENET0->ATCR |= ENET_ATCR_CAPTURE_MASK;
> > > >> 		while(ENET0->ATCR & ENET_ATCR_CAPTURE_MASK);
> > > >> 		ts_counter_ns = ENET0->ATVR;
> 
> That applies to all IMX devices, doesn't it?
> In any case, Fugang's change log is not clear.

Only MX6 SX. Only MX6 SX added FEC_QUIRK_BUG_CAPTURE.

.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT |
 				FEC_QUIRK_HAS_BUFDESC_EX | FEC_QUIRK_HAS_CSUM |
 				FEC_QUIRK_HAS_VLAN | FEC_QUIRK_HAS_AVB |
-				FEC_QUIRK_ERR007885,
+				FEC_QUIRK_ERR007885 | FEC_QUIRK_BUG_CAPTURE,

> 
> Confused,
> Richard

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

* Re: [PATCH] net: fec: ptp: fix convergence issue to support LinuxPTP stack
  2014-10-14 18:43         ` Frank.Li
@ 2014-10-14 18:49           ` Richard Cochran
  2014-10-15  1:28             ` fugang.duan
  2014-10-15  1:37             ` fugang.duan
  0 siblings, 2 replies; 12+ messages in thread
From: Richard Cochran @ 2014-10-14 18:49 UTC (permalink / raw)
  To: Frank.Li@freescale.com
  Cc: David Miller, fugang.duan@freescale.com, netdev@vger.kernel.org,
	bhutchings@solarflare.com

On Tue, Oct 14, 2014 at 06:43:51PM +0000, Frank.Li@freescale.com wrote:
> Only MX6 SX. Only MX6 SX added FEC_QUIRK_BUG_CAPTURE.

But what about this comment:

+/* ENET Block Guide/ Chapter for the iMX6SLX (PELE) address one issue:
+ * Incorrect behavior for ENET_ATCR[Capture and Restart Bits]. These bits will
+ * always read a value zero. When these bits are set to 1'b1, these should hold
+ * value 1'b1 until the counter value is capture in the register clock domain.
+ */

It sounds like the bits are "sticky" until the counter value has been
latched. Therefore you need to check the bits before reading the
counter, but the code does not do this for the case of !SX.

Thanks,
Richard

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

* RE: [PATCH] net: fec: ptp: fix convergence issue to support LinuxPTP stack
  2014-10-14 18:49           ` Richard Cochran
@ 2014-10-15  1:28             ` fugang.duan
  2014-10-15  1:37             ` fugang.duan
  1 sibling, 0 replies; 12+ messages in thread
From: fugang.duan @ 2014-10-15  1:28 UTC (permalink / raw)
  To: Richard Cochran, Frank.Li@freescale.com
  Cc: David Miller, netdev@vger.kernel.org, bhutchings@solarflare.com

From: Richard Cochran <richardcochran@gmail.com> Sent: Wednesday, October 15, 2014 2:50 AM
>To: Li Frank-B20596
>Cc: David Miller; Duan Fugang-B38611; netdev@vger.kernel.org;
>bhutchings@solarflare.com
>Subject: Re: [PATCH] net: fec: ptp: fix convergence issue to support
>LinuxPTP stack
>
>On Tue, Oct 14, 2014 at 06:43:51PM +0000, Frank.Li@freescale.com wrote:
>> Only MX6 SX. Only MX6 SX added FEC_QUIRK_BUG_CAPTURE.
>
>But what about this comment:
>
>+/* ENET Block Guide/ Chapter for the iMX6SLX (PELE) address one issue:
>+ * Incorrect behavior for ENET_ATCR[Capture and Restart Bits]. These
>+bits will
>+ * always read a value zero. When these bits are set to 1'b1, these
>+should hold
>+ * value 1'b1 until the counter value is capture in the register clock
>domain.
>+ */
>
>It sounds like the bits are "sticky" until the counter value has been
>latched. Therefore you need to check the bits before reading the counter,
>but the code does not do this for the case of !SX.
>
>Thanks,
>Richard


Hi, Richard,

Fristly, the patch just fix ptp issue for imx6sx.
Secondly, pls see the patch commit log:

    IEEE 1588 module has one hw issue in capturing the ATVR register. According
    to the user manual it is:
               ENET0->ATCR |= ENET_ATCR_CAPTURE_MASK;
               while(ENET0->ATCR & ENET_ATCR_CAPTURE_MASK);
               ts_counter_ns = ENET0->ATVR;
    Incorrect behavior for ENET_ATCR[Capture and Restart Bits]. These bits will always
    read a value zero. According to SPEC, when these bits are set to 1'b1, these should
    hold value 1'b1 until the counter value is capture in the register clock domain.
    
    Unfortunately there is a bug with the way the bit "ENET_ATCR_CAPTURE" clears.
    So need something like:
               ENET0->ATCR |= ENET_ATCR_CAPTURE_MASK;
               wait();
               ts_counter_ns = ENET0->ATVR;
    
    The wait-time to be at least 6 clock cycle of the slower clock between the register
    clock and the 1588 clock. The 1588 ts_clk is 25Mhz, register clock is 66Mhz, so the
    wait-time must be greater than 240ns (40ns * 6). The workaround is that adding 1us
    delay before read ATVR.



There need add delay instead of while().

Thanks,
Andy

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

* RE: [PATCH] net: fec: ptp: fix convergence issue to support LinuxPTP stack
  2014-10-14 18:39       ` Richard Cochran
  2014-10-14 18:43         ` Frank.Li
@ 2014-10-15  1:30         ` fugang.duan
  2014-10-15  7:25           ` Richard Cochran
  1 sibling, 1 reply; 12+ messages in thread
From: fugang.duan @ 2014-10-15  1:30 UTC (permalink / raw)
  To: Richard Cochran, Frank.Li@freescale.com
  Cc: David Miller, netdev@vger.kernel.org, bhutchings@solarflare.com

From: Richard Cochran <richardcochran@gmail.com> Sent: Wednesday, October 15, 2014 2:40 AM
>To: Li Frank-B20596
>Cc: David Miller; Duan Fugang-B38611; netdev@vger.kernel.org;
>bhutchings@solarflare.com
>Subject: Re: [PATCH] net: fec: ptp: fix convergence issue to support
>LinuxPTP stack
>
>Frank,
>
>On Tue, Oct 14, 2014 at 06:27:28PM +0000, Frank.Li@freescale.com wrote:
>> Fugang's patch fix the problem existed in MX6SX platform.
>
>You say that Fugang's patch fixes a quirk in the SX device.
>
>But he said that the user's manual tells us to wait for some status bits
>to clear. (See the third line, with the 'while' key word).
>
>> > > On Tue, Oct 14, 2014 at 01:39:47PM +0800, Fugang Duan wrote:
>> > >> IEEE 1588 module has one hw issue in capturing the ATVR register.
>> > >> According to the user manual it is:
>> > >> 		ENET0->ATCR |= ENET_ATCR_CAPTURE_MASK;
>> > >> 		while(ENET0->ATCR & ENET_ATCR_CAPTURE_MASK);
>> > >> 		ts_counter_ns = ENET0->ATVR;
>
>That applies to all IMX devices, doesn't it?
>In any case, Fugang's change log is not clear.
>
>Confused,
>Richard

Pls read the commit log in entire ?

Regrads,
Andy

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

* RE: [PATCH] net: fec: ptp: fix convergence issue to support LinuxPTP stack
  2014-10-14 18:49           ` Richard Cochran
  2014-10-15  1:28             ` fugang.duan
@ 2014-10-15  1:37             ` fugang.duan
  1 sibling, 0 replies; 12+ messages in thread
From: fugang.duan @ 2014-10-15  1:37 UTC (permalink / raw)
  To: Richard Cochran, Frank.Li@freescale.com
  Cc: David Miller, netdev@vger.kernel.org, bhutchings@solarflare.com

From: Richard Cochran <richardcochran@gmail.com> Sent: Wednesday, October 15, 2014 2:50 AM
>To: Li Frank-B20596
>Cc: David Miller; Duan Fugang-B38611; netdev@vger.kernel.org;
>bhutchings@solarflare.com
>Subject: Re: [PATCH] net: fec: ptp: fix convergence issue to support
>LinuxPTP stack
>
>On Tue, Oct 14, 2014 at 06:43:51PM +0000, Frank.Li@freescale.com wrote:
>> Only MX6 SX. Only MX6 SX added FEC_QUIRK_BUG_CAPTURE.
>
>But what about this comment:
>
>+/* ENET Block Guide/ Chapter for the iMX6SLX (PELE) address one issue:
>+ * Incorrect behavior for ENET_ATCR[Capture and Restart Bits]. These
>+bits will
>+ * always read a value zero. When these bits are set to 1'b1, these
>+should hold
>+ * value 1'b1 until the counter value is capture in the register clock
>domain.
>+ */
>
>It sounds like the bits are "sticky" until the counter value has been
>latched. Therefore you need to check the bits before reading the counter,
>but the code does not do this for the case of !SX.
>
>Thanks,
>Richard

Hi, Richard,

These "__should__" hold 1'b1 until the counter value is capture in the register clock domain.
But "Incorrect behavior for ENET_ATCR[Capture and Restart Bits]. These bits will always read a value zero"


Thanks,
Andy

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

* Re: [PATCH] net: fec: ptp: fix convergence issue to support LinuxPTP stack
  2014-10-15  1:30         ` fugang.duan
@ 2014-10-15  7:25           ` Richard Cochran
  2014-10-15  7:42             ` fugang.duan
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Cochran @ 2014-10-15  7:25 UTC (permalink / raw)
  To: fugang.duan@freescale.com
  Cc: Frank.Li@freescale.com, David Miller, netdev@vger.kernel.org,
	bhutchings@solarflare.com

On Wed, Oct 15, 2014 at 01:30:41AM +0000, fugang.duan@freescale.com wrote:
> 
> Pls read the commit log in entire ?

I did read it. 

It is incomprehensible.

Can you please fix it?

Thanks,
Richard

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

* RE: [PATCH] net: fec: ptp: fix convergence issue to support LinuxPTP stack
  2014-10-15  7:25           ` Richard Cochran
@ 2014-10-15  7:42             ` fugang.duan
  0 siblings, 0 replies; 12+ messages in thread
From: fugang.duan @ 2014-10-15  7:42 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Frank.Li@freescale.com, David Miller, netdev@vger.kernel.org,
	bhutchings@solarflare.com

From: Richard Cochran <richardcochran@gmail.com> Sent: Wednesday, October 15, 2014 3:26 PM
>To: Duan Fugang-B38611
>Cc: Li Frank-B20596; David Miller; netdev@vger.kernel.org;
>bhutchings@solarflare.com
>Subject: Re: [PATCH] net: fec: ptp: fix convergence issue to support
>LinuxPTP stack
>
>On Wed, Oct 15, 2014 at 01:30:41AM +0000, fugang.duan@freescale.com wrote:
>>
>> Pls read the commit log in entire ?
>
>I did read it.
>
>It is incomprehensible.
>
>Can you please fix it?
>
>Thanks,
>Richard

Ok, I will enhance the commit/comment log and then send out V2.

Thanks,
Andy

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

end of thread, other threads:[~2014-10-15  7:42 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-14  5:39 [PATCH] net: fec: ptp: fix convergence issue to support LinuxPTP stack Fugang Duan
2014-10-14 10:36 ` Richard Cochran
2014-10-14 16:52   ` David Miller
2014-10-14 18:27     ` Frank.Li
2014-10-14 18:39       ` Richard Cochran
2014-10-14 18:43         ` Frank.Li
2014-10-14 18:49           ` Richard Cochran
2014-10-15  1:28             ` fugang.duan
2014-10-15  1:37             ` fugang.duan
2014-10-15  1:30         ` fugang.duan
2014-10-15  7:25           ` Richard Cochran
2014-10-15  7:42             ` fugang.duan

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