Support for DaVinci EMAC on TI AM35xx

DaVinci EMAC is found not only on DaVinci SoCs but on some OMAP3 SoCs also. This patch moves common defines from arch-davinci/emac_defs.h to drivers/net/davinci_emac.h

Signed-off-by Ilya Yanok <[email protected]>

---
 arch/arm/include/asm/arch-davinci/emac_defs.h |  293 -----------------------
 drivers/net/davinci_emac.c                    |    1 +
 drivers/net/davinci_emac.h                    |  317 +++++++++++++++++++++++++
 3 files changed, 318 insertions(+), 293 deletions(-)
 create mode 100644 drivers/net/davinci_emac.h

Quoted 5 lines

 arch/arm/include/asm/arch-davinci/emac_defs.h |  293 -----------------------
 drivers/net/davinci_emac.c                    |    1 +
 drivers/net/davinci_emac.h                    |  317 +++++++++++++++++++++++++
 3 files changed, 318 insertions(+), 293 deletions(-)
 create mode 100644 drivers/net/davinci_emac.h

Checkpatch says:

total: 1 errors, 8 warnings, 623 lines checked

On AM35xx CPPI RAM had different addresses when accessed from the CPU and from the EMAC. We need to account this to deal with the buffer descriptors correctly.

Signed-off-by Ilya Yanok <[email protected]>

---
 drivers/net/davinci_emac.c |   28 +++++++++++++++++++---------
 1 files changed, 19 insertions(+), 9 deletions(-)

drivers/net/davinci_emac.c +6 -0

 
+#ifdef DAVINCI_EMAC_DCACHE
+static inline void davinci_flush(void *addr, int size)
+{
+	flush_dcache_range((unsigned long)addr,
+			(unsigned long)addr + size);
+}

Quoted 10 lines

drivers/net/davinci_emac.c +10 -0

+#ifdef DAVINCI_EMAC_DCACHE
+static inline void davinci_flush(void *addr, int size)
+{
+	flush_dcache_range((unsigned long)addr,
+			(unsigned long)addr + size);
+}
+
+static inline void davinci_invalidate(void *addr, int size)
+{
+	invalidate_dcache_range((unsigned long)addr,

drivers/net/davinci_emac.c static volatile u_int8_t active_phy_addr = 0xff; · line 114

Quoted 6 lines

drivers/net/davinci_emac.c +6 -0

+			(unsigned long)addr + size);
+}
+#else
+#define davinci_flush(addr, size)	do {} while (0)
+#define davinci_invalidate(addr, size)	do {} while (0)
+#endif

does it really make sense to have this be conditional ?

arm926ejs doesn't have {invalidate,flush}_dcache_range(), so we have to add this not to break the driver on DaVinci boards (maybe we need to add empty cache functions on arm926ejs instead?)

Quoted 10 lines · Mike Frysinger

Quoted 10 lines

drivers/net/davinci_emac.c +10 -0

+#ifdef DAVINCI_EMAC_DCACHE
+static inline void davinci_flush(void *addr, int size)
+{
+	flush_dcache_range((unsigned long)addr,
+			(unsigned long)addr + size);
+}
+
+static inline void davinci_invalidate(void *addr, int size)
+{
+	invalidate_dcache_range((unsigned long)addr,

Quoted 8 lines

drivers/net/davinci_emac.c static volatile u_int8_t active_phy_addr = 0xff; · line 114

Quoted 6 lines

drivers/net/davinci_emac.c +6 -0

+			(unsigned long)addr + size);
+}
+#else
+#define davinci_flush(addr, size)	do {} while (0)
+#define davinci_invalidate(addr, size)	do {} while (0)
+#endif

does it really make sense to have this be conditional ?

arm926ejs doesn't have {invalidate,flush}_dcache_range(), so we have to add this not to break the driver on DaVinci boards (maybe we need to add empty cache functions on arm926ejs instead?)

Quoted 3 lines

arm926ejs doesn't have {invalidate,flush}_dcache_range(), so we have to add this not to break the driver on DaVinci boards (maybe we need to add empty cache functions on arm926ejs instead?)

if the prototype is in include/common.h, then code may assume it exists. if it doesn't exist for a particular cpu, then that cpu is broken and common code (which is what drivers/ is) should not be adding hacks to workaround broken cpus. please add stubs to the cpu you're referring to and drop the DAVINCI_EMAC_DCACHE define. or let whoever cares about that cpu add the defines, but still drop DAVINCI_EMAC_DCACHE.

DaVinci EMAC is present on TI AM35xx SoCs (ARMv7) which run with D-Cache enabled by default. So we have to take care and flush/invalidate the cache before/after the DMA operations.

Signed-off-by Ilya Yanok <[email protected]>

---
 drivers/net/davinci_emac.c |   47 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 47 insertions(+), 0 deletions(-)

drivers/net/davinci_emac.c +0 -0

index 222a0d0..b787213 100644

drivers/net/davinci_emac.c static volatile u_int8_t active_phy_addr = 0xff; · line 104

Quoted 6 lines

drivers/net/davinci_emac.c +6 -0

+#ifdef DAVINCI_EMAC_DCACHE
+static inline void davinci_flush(void *addr, int size)
+{
+	flush_dcache_range((unsigned long)addr,
+			(unsigned long)addr + size);
+}

There is no check with the cache linesize. I get this error:

ERROR: v7_dcache_inval_range - stop address is not aligned - 0x5c0200a0

Should we not be sure that size is rounded up to align with the cache line size ?

Hi Stefano,

Quoted 9 lines

drivers/net/davinci_emac.c static volatile u_int8_t active_phy_addr = 0xff; · line 104

Quoted 6 lines

drivers/net/davinci_emac.c +6 -0

+#ifdef DAVINCI_EMAC_DCACHE
+static inline void davinci_flush(void *addr, int size)
+{
+	flush_dcache_range((unsigned long)addr,
+			(unsigned long)addr + size);
+}

There is no check with the cache linesize. I get this error:

Quoted 4 lines

ERROR: v7_dcache_inval_range - stop address is not aligned - 0x5c0200a0

Should we not be sure that size is rounded up to align with the cache line size ?

Surely we should. Actually it's not the size that has to be aligned but the buffer itself. Is there any generic API to get the cache line size?

There is a CONFIG_SYS_CACHELINE_SIZE. However, I see recent patches that can help in our case ( cache: add ALLOC_CACHE_ALIGN_BUFFER macro):

http://patchwork.ozlabs.org/patch/117698/

Wolfgang replied he has already applied, but I have not yet seen on u-boot TOT.

Quoted 7 lines · Stefano Babic

There is a CONFIG_SYS_CACHELINE_SIZE. However, I see recent patches that can help in our case ( cache: add ALLOC_CACHE_ALIGN_BUFFER macro):

http://patchwork.ozlabs.org/patch/117698/

Wolfgang replied he has already applied, but I have not yet seen on u-boot TOT.

See the rest of the thread. I had applied this patch set to a loal tree, but it was breaking hundreds of systems, so had to back out the patches again.

I'm eager to get this code in myself, but it needs to be compile-clean at least and harmless to all boards that don't actually reference that code.

Quoted 8 lines

See the rest of the thread. I had applied this patch set to a loal tree, but it was breaking hundreds of systems, so had to back out the patches again.

I'm eager to get this code in myself, but it needs to be compile-clean at least and harmless to all boards that don't actually reference that code.

sorry, but the rest of what thread ? i missed that there were issues and was wondering why they weren't in the published master branch yet ...

Quoted 10 lines · Mike Frysinger

Quoted 7 lines

See the rest of the thread. I had applied this patch set to a loal tree, but it was breaking hundreds of systems, so had to back out the patches again.

I'm eager to get this code in myself, but it needs to be compile-clean at least and harmless to all boards that don't actually reference that code.

sorry, but the rest of what thread ? i missed that there were issues and was wondering why they weren't in the published master branch yet ...

There are actually three parts to this storey:

Thisi s the original patch series, which I applied to a local test branch with the intention to pull into mainline:

10/04 Anton Staaf [U-Boot] [PATCH v2 0/7] Add cache line alignment
support http://article.gmane.org/gmane.comp.boot-loaders.u-boot/111026

Then I noticed that this broke all PPC4xx boards, and asked Stefan to fix this problem. Stefan did:

10/07 Stefan Roese [PATCH 1/2] ppc: Include <asm/cache.h> in common.h
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/111482

When PPC was building again, I tested it on ARM (which I assumed was OK, given that this was Anton's primary architecture). That was when I finally gave up, see

10/09 To:Anton Staaf Re: [U-Boot] [PATCH v2 0/7] Add cache line
alignment support
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/111713

Dear Mike Frysinger,

Quoted 11 lines · In message <[email protected]> you wrote:

Quoted 8 lines

See the rest of the thread. ?I had applied this patch set to a loal tree, but it was breaking hundreds of systems, so had to back out the patches again.

I'm eager to get this code in myself, but it needs to be compile-clean at least and harmless to all boards that don't actually reference that code.

sorry, but the rest of what thread ? ?i missed that there were issues and was wondering why they weren't in the published master branch yet ...

There are actually three parts to this storey:

Thisi s the original patch series, which I applied to a local test branch with the intention to pull into mainline:

10/04 Anton Staaf ? ? ? ?[U-Boot] [PATCH v2 0/7] Add cache line alignment support
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/111026

Then I noticed that this broke all PPC4xx boards, and asked Stefan to fix this problem. ?Stefan did:

10/07 Stefan Roese ? ? ? [PATCH 1/2] ppc: Include <asm/cache.h> in common.h
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/111482

When PPC was building again, I tested it on ARM (which I assumed was OK, given that this was Anton's primary architecture). ?That was when I finally gave up, see

10/09 To:Anton Staaf ? ? Re: [U-Boot] [PATCH v2 0/7] Add cache line alignment support
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/111713

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 "Maintain an awareness for contribution -- ?to ?your ?schedule, ?your project, our company." ? ? ? ? ? ? ? ? ? ? ? ? - A Group of Employees
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Quoted 4 lines

When PPC was building again, I tested it on ARM (which I assumed was OK, given that this was Anton's primary architecture). =A0That was when I finally gave up, see

Yes, the patches expose the fact that almost no boards define CONFIG_SYS_CACHELINE_SIZE.

I am working on a "solution" for this. My first thought is to add defines for
CONFIG_SYS_CACHELINE_SIZE in all of the arch cache.h files that currently do not have them.
This would be all cache.h files other than the PPC one. But this could be a huge amount of work
to look up all of the arch cacheline sizes. So I am thinking of putting in a wrong, but large power
of two so that boards will build and probably work. But will certainly need to be fixed up...

Another solution would be to do the above and define CONFIG_SYS_CACHELINE_SIZE as a
large (128?) value and then indicate that that config is to deprecated in favor of Mikes suggestion
of using the Linux CACHE BITS defines. Then we can move boards over to that mechanism
over time, and in the mean time all boards will compile, and architectures/boards that correctly
define their cacheline size will function correctly, and architectures/boards that use the large default
will most likely function correctly...

Dear Anton Staaf,

Quoted 7 lines · In message <CAF6FioWbLz9JKBnj5sJGALtj=bJyLNbFvunNa6FE62y42t6-hw@mail.gmail.com>…

Quoted 4 lines

When PPC was building again, I tested it on ARM (which I assumed was OK, given that this was Anton's primary architecture). =A0That was when I finally gave up, see

Yes, the patches expose the fact that almost no boards define CONFIG_SYS_CACHELINE_SIZE.

Um...

-> ls include/configs/* | wc -l
577
-> grep CONFIG_SYS_CACHELINE_SIZE include/configs/* | wc -l 199

So that's more than one third of all boards - most of them PPC...

Quoted 10 lines

I am working on a "solution" for this. ?My first thought is to add defines for
CONFIG_SYS_CACHELINE_SIZE in all of the arch cache.h files that currently do not have them.
This would be all cache.h files other than the PPC one. ?But this could be a huge amount of work
to look up all of the arch cacheline sizes. ?So I am thinking of putting in a wrong, but large power
of two so that boards will build and probably work. ?But will certainly need to be fixed up...

Please don't - bogus stuff that appears to be working is known to never get fixed. ?Let's rather break the boards - this enforces the needed clean-up.

Quoted 11 lines

Another solution would be to do the above and define CONFIG_SYS_CACHELINE_SIZE as a
large (128?) value and then indicate that that config is to deprecated in favor of Mikes suggestion
of using the Linux CACHE BITS defines. ?Then we can move boards over to that mechanism
over time, and in the mean time all boards will compile, and architectures/boards that correctly
define their cacheline size will function correctly, and architectures/boards that use the large default
will most likely function correctly...

At least a big, dfat build warning must be issued, then (but only one, not a loooooong list).

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 "We don't have to protect the environment -- the Second Coming is ?at hand." ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? - James Watt

Quoted 11 lines · Wolfgang Denk

Dear Anton Staaf,

Quoted 7 lines · In message <CAF6FioWbLz9JKBnj5sJGALtj=bJyLNbFvunNa6FE62y42t6-hw@mail.gmail.com>…

Quoted 4 lines

When PPC was building again, I tested it on ARM (which I assumed was OK, given that this was Anton's primary architecture). =A0That was when I finally gave up, see

Yes, the patches expose the fact that almost no boards define CONFIG_SYS_CACHELINE_SIZE.

Quoted 8 lines

Um...

-> ls include/configs/* | wc -l
577
-> grep CONFIG_SYS_CACHELINE_SIZE include/configs/* | wc -l 199

So that's more than one third of all boards - most of them PPC...

Yes, sorry, I should have said almost no architectures define it, PPC being the exception.

Quoted 11 lines

Quoted 10 lines

I am working on a "solution" for this. ?My first thought is to add defines for
CONFIG_SYS_CACHELINE_SIZE in all of the arch cache.h files that currently do not have them.
This would be all cache.h files other than the PPC one. ?But this could be a huge amount of work
to look up all of the arch cacheline sizes. ?So I am thinking of putting in a wrong, but large power
of two so that boards will build and probably work. ?But will certainly need to be fixed up...

Quoted 3 lines

Please don't - bogus stuff that appears to be working is known to never get fixed. ?Let's rather break the boards - this enforces the needed clean-up.

Yes, that's my feeling as well.

Quoted 12 lines

Quoted 11 lines

Another solution would be to do the above and define CONFIG_SYS_CACHELINE_SIZE as a
large (128?) value and then indicate that that config is to deprecated in favor of Mikes suggestion
of using the Linux CACHE BITS defines. ?Then we can move boards over to that mechanism
over time, and in the mean time all boards will compile, and architectures/boards that correctly
define their cacheline size will function correctly, and architectures/boards that use the large default
will most likely function correctly...

Quoted 2 lines

At least a big, dfat build warning must be issued, then (but only one, not a loooooong list).

Would you be OK with a build warning for the lack of definition of CONFIG_SYS_CACHELINE_SIZE like I have now if it only happened
once per board? ?I could move it from common.h to a c file that is always built. ?Perhaps I could add a checks.c file to libgeneric? ?I'm not really sure if that's the right place for it. ?Do you have a suggestion?

This new file would include asm/cache.h and then common.h. ?It would then have a #ifdef to check for CONFIG_SYS_CACHELINE_SIZE. ?So any arch that defined it in it's asm/cache.h or any board that defined it in it's config file
would be OK. ?All other boards would generate a single compiler warning when building that file.

What do you think?

I would have to add a couple of empty asm/cache.h files for the architectures that don't currently have them.

Thanks,
? ? Anton

Quoted 11 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 "We don't have to protect the environment -- the Second Coming is ?at hand." ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? - James Watt

Would you be OK with a build warning for the lack of definition of CONFIG_SYS_CACHELINE_SIZE like I have now if it only happened once per board? ...

... I could move it from common.h to a c file that is always
built. Perhaps I could add a checks.c file to libgeneric? I'm not really sure if that's the right place for it. Do you have a suggestion?

Dear Anton Staaf,

Quoted 4 lines · In message <CAF6FioWHpeA8_nPA85e3SPc17ZDCkzqkV2ROs_EAaB_9wV9vEA@mail.gmail.com>…

Would you be OK with a build warning for the lack of definition of CONFIG_SYS_CACHELINE_SIZE like I have now if it only happened once per board? ...

Yes, that would be great.

Quoted 3 lines

? ? ? ? ? ? ?... I could move it from common.h to a c file that is always built. ?Perhaps I could add a checks.c file to libgeneric? ?I'm not really sure if that's the right place for it. ?Do you have a suggestion?

I'd like to avoid a new file. ?But I don't have a good idea either. Well, heck, go and add the test to the top of common/main.c .

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 What can it profit a man to gain the whole world and to come ?to ?his property with a gastric ulcer, a blown prostate, and bifocals? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? -- John Steinbeck, _Cannery Row_

Turns out I was able to define ARCH_DMA_MINALIGN for all U-Boot architectures and use that
instead of the actual cache line size to generate correctly aligned buffers. My latest
patch set implements this. So we shouldn't have any warnings being generated and we should
have no need for the extra file (or addition to main.c) to generate said warnings. :)

Dear Anton Staaf,

Quoted 9 lines · In message <CAF6FioWnYJNBz0+4Af3-0vLCoGrgGgcN10z=K0df8Yv87GUVjw@mail.gmail.com>…

Turns out I was able to define ARCH_DMA_MINALIGN for all U-Boot architectures and use that
instead of the actual cache line size to generate correctly aligned buffers. ?My latest
patch set implements this. ?So we shouldn't have any warnings being generated and we should
have no need for the extra file (or addition to main.c) to generate said warnings. ?:)

Great. ?So this latest patch set is the final one?

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 Hokey religions and ancient weapons are ?no ?substitute ?for ?a ?good blaster at your side. ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?- Han Solo

I believe it is correct yes. But it would be good to have each architecture custodian
weigh in on the architectures I couldn't test directly. Also, Mike has submitted a
patch that adds the asm/cache.h file for blackfin from the Linux kernel. This patch
obsoletes one of the patches in my series. My thinking is that we should wait a bit to
hear from other custodians, and then I'll send a second version of the patch set that
addresses any comments. But I am also pretty confident that we could submit the existing patch set (minus the blackfin asm/cache.h patch) and we would be safe.

Quoted 4 lines · Anton Staaf

... I could move it from common.h to a c file that is always

built. Perhaps I could add a checks.c file to libgeneric? I'm not really sure if that's the right place for it. Do you have a suggestion?

I'd like to avoid a new file. But I don't have a good idea either. Well, heck, go and add the test to the top of common/main.c .

Quoted 6 lines · On Thursday 13 October 2011 16:03:37 Wolfgang Denk wrote:

Quoted 4 lines · Anton Staaf

? ? ? ? ? ? ?... I could move it from common.h to a c file that is always

built. ?Perhaps I could add a checks.c file to libgeneric? ?I'm not really sure if that's the right place for it. ?Do you have a suggestion?

Quoted 2 lines

I'd like to avoid a new file. ?But I don't have a good idea either. Well, heck, go and add the test to the top of common/main.c .

fwiw, we added a dummy file in the Blackfin linux port called "arch_checks.c" where we kept all of our #ifdef sanity checks. ?that way it got checked once (rather than in every header include), and we didn't have to clutter up semi- related files with it.

-mike

Quoted 5 lines

Quoted 2 lines

Should we not be sure that size is rounded up to align with the cache line size ?

Surely we should. Actually it's not the size that has to be aligned but the buffer itself. Is there any generic API to get the cache line size?

There is a CONFIG_SYS_CACHELINE_SIZE. However, I see recent patches that

can help in our case ( cache: add ALLOC_CACHE_ALIGN_BUFFER macro):

http://patchwork.ozlabs.org/patch/117698/

AM35xx has DaVinci-compatible EMAC.

Signed-off-by Ilya Yanok <[email protected]>

---
 arch/arm/include/asm/arch-omap3/emac_defs.h |   57 +++++++++++++++++++++++++++
 1 files changed, 57 insertions(+), 0 deletions(-)
 create mode 100755 arch/arm/include/asm/arch-omap3/emac_defs.h