[U-Boot-Users] [PATCH] ColdFire: Fix FEC transmit issue for MCF5275

[U-Boot-Users] [PATCH] ColdFire: Fix FEC transmit issue for MCF5275

[Tsi-Chung Liew]

Signed-off-by: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
---
 drivers/net/mcffec.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/net/mcffec.c b/drivers/net/mcffec.c
index 3b81258..71d1960 100644
--- a/drivers/net/mcffec.c
+++ b/drivers/net/mcffec.c
@@ -166,6 +166,13 @@ int fec_send(struct eth_device *dev, volatile void *packet, int length)
 	/* Activate transmit Buffer Descriptor polling */
 	fecp->tdar = 0x01000000;	/* Descriptor polling active    */
 
+	/* FEC fix for MCF5275, FEC unable to initial transmit data packet.
+	 * A nop will ensure the descriptor polling active completed.
+	 */
+#ifdef CONFIG_M5275
+	__asm__ ("nop");
+#endif
+
 #ifdef CFG_UNIFY_CACHE
 	icache_invalid();
 #endif
-- 
1.5.4.1

[U-Boot-Users] [PATCH] ColdFire: Fix FEC transmit issue for MCF5275

[Wolfgang Denk]

In message <1205791696-17263-1-git-send-email-Tsi-Chung.Liew@freescale.com> you wrote:
> Signed-off-by: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
> ---
>  drivers/net/mcffec.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/mcffec.c b/drivers/net/mcffec.c
> index 3b81258..71d1960 100644
> --- a/drivers/net/mcffec.c
> +++ b/drivers/net/mcffec.c
> @@ -166,6 +166,13 @@ int fec_send(struct eth_device *dev, volatile void *packet, int length)
>  	/* Activate transmit Buffer Descriptor polling */
>  	fecp->tdar = 0x01000000;	/* Descriptor polling active    */
>  
> +	/* FEC fix for MCF5275, FEC unable to initial transmit data packet.
> +	 * A nop will ensure the descriptor polling active completed.
> +	 */

Please note:

The preferred style for long (multi-line) comments is: 

        /*
         * This is the preferred style for multi-line
         * comments in the Linux kernel source code.
         * Please use it consistently.
         *
         * Description:  A column of asterisks on the left side,
         * with beginning and ending almost-blank lines.
         */


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A meeting is an event at which the minutes are kept and the hours are
lost.

[U-Boot-Users] [PATCH] ColdFire: Fix FEC transmit issue for MCF5275

[Tsi-Chung Liew]

Signed-off-by: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
---
 drivers/net/mcffec.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/net/mcffec.c b/drivers/net/mcffec.c
index 3b81258..40d7bf2 100644
--- a/drivers/net/mcffec.c
+++ b/drivers/net/mcffec.c
@@ -166,6 +166,14 @@ int fec_send(struct eth_device *dev, volatile void *packet, int length)
 	/* Activate transmit Buffer Descriptor polling */
 	fecp->tdar = 0x01000000;	/* Descriptor polling active    */
 
+	/*
+	 * FEC fix for MCF5275, FEC unable to initial transmit data packet.
+	 * A nop will ensure the descriptor polling active completed.
+	 */
+#ifdef CONFIG_M5275
+	__asm__ ("nop");
+#endif
+
 #ifdef CFG_UNIFY_CACHE
 	icache_invalid();
 #endif
-- 
1.5.4.1

[U-Boot-Users] [PATCH] ColdFire: Fix FEC transmit issue for MCF5275

[Jon Loeliger]

Tsi-Chung Liew wrote:

> +	/*
> +	 * FEC fix for MCF5275, FEC unable to initial transmit data packet.
> +	 * A nop will ensure the descriptor polling active completed.
> +	 */
> +#ifdef CONFIG_M5275
> +	__asm__ ("nop");
> +#endif
> +
>  #ifdef CFG_UNIFY_CACHE
>  	icache_invalid();
>  #endif

At the risk of being very picky, I think this should be
written to have the comment inside the #ifdef so that
the code and comment are not accidentally separated.

#if defined(CONFIG_M5275)
	/*
	 * FEC ...
	 */
	__asm__("nop");
#endif

But, that may be just me.  Take it or leave it. :-)

jdl

[U-Boot-Users] [PATCH] ColdFire: Fix FEC transmit issue for MCF5275

[Scott Wood]

On Wed, Mar 19, 2008 at 09:05:27AM -0500, Jon Loeliger wrote:
> Tsi-Chung Liew wrote:
> 
> > +	/*
> > +	 * FEC fix for MCF5275, FEC unable to initial transmit data packet.
> > +	 * A nop will ensure the descriptor polling active completed.
> > +	 */
> > +#ifdef CONFIG_M5275
> > +	__asm__ ("nop");
> > +#endif
> > +
> >  #ifdef CFG_UNIFY_CACHE
> >  	icache_invalid();
> >  #endif
> 
> At the risk of being very picky, I think this should be
> written to have the comment inside the #ifdef so that
> the code and comment are not accidentally separated.

At the risk of being somewhat less picky, I think there should be more
explanation of why a nop solves anything in a robust manner.

-Scott

[U-Boot-Users] [PATCH] ColdFire: Fix FEC transmit issue for MCF5275

[Tsi-Chung Liew]

Signed-off-by: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
---
 drivers/net/mcffec.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/net/mcffec.c b/drivers/net/mcffec.c
index 3b81258..10afa93 100644
--- a/drivers/net/mcffec.c
+++ b/drivers/net/mcffec.c
@@ -166,6 +166,18 @@ int fec_send(struct eth_device *dev, volatile void *packet, int length)
 	/* Activate transmit Buffer Descriptor polling */
 	fecp->tdar = 0x01000000;	/* Descriptor polling active    */
 
+#ifdef CONFIG_M5275
+	/*
+	 * FEC fix for MCF5275. This issue is only happened in cpu polling
+	 * but does not show up in interrupt. It is believe that there is
+	 * a sync issue when accessing from DRAM. This causes FEC unable
+	 * to initial transmit data packet. A NOP instruction will
+	 * ensure the transmit descriptor polling active completed before
+	 * the next instruction begin.
+	 */
+	__asm__ ("nop");
+#endif
+
 #ifdef CFG_UNIFY_CACHE
 	icache_invalid();
 #endif
-- 
1.5.4.1

[U-Boot-Users] [PATCH] ColdFire: Fix FEC transmit issue for MCF5275

[Ben Warren]

Tsi-Chung Liew wrote:
> Signed-off-by: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
> ---
>  drivers/net/mcffec.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/mcffec.c b/drivers/net/mcffec.c
> index 3b81258..71d1960 100644
> --- a/drivers/net/mcffec.c
> +++ b/drivers/net/mcffec.c
> @@ -166,6 +166,13 @@ int fec_send(struct eth_device *dev, volatile void *packet, int length)
>  	/* Activate transmit Buffer Descriptor polling */
>  	fecp->tdar = 0x01000000;	/* Descriptor polling active    */
>  
> +	/* FEC fix for MCF5275, FEC unable to initial transmit data packet.
> +	 * A nop will ensure the descriptor polling active completed.
> +	 */
> +#ifdef CONFIG_M5275
> +	__asm__ ("nop");
> +#endif
> +
>  #ifdef CFG_UNIFY_CACHE
>  	icache_invalid();
>  #endif
>   
Applied to net tree. As others have noted, adding a NOP here seems like 
a band-aid, but I can't really dispute it.

thanks,
Ben