Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
Dear Anton Staaf, In message caf6fiowhpea8_npa85e3spc17zdckzqkv2ros_eaab_9wv9...@mail.gmail.com you wrote: 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. ... 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: w...@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_ ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
On Thu, Oct 13, 2011 at 1:03 PM, Wolfgang Denk w...@denx.de wrote: Dear Anton Staaf, In message caf6fiowhpea8_npa85e3spc17zdckzqkv2ros_eaab_9wv9...@mail.gmail.com you wrote: 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. 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. :) Thanks, Anton ... 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: w...@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_ ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
Dear Anton Staaf, In message CAF6FioWnYJNBz0+4Af3-0vLCoGrgGgcN10z=k0df8yv87gu...@mail.gmail.com you wrote: 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: w...@denx.de Hokey religions and ancient weapons are no substitute for a good blaster at your side. - Han Solo ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
On Thursday 13 October 2011 16:03:37 Wolfgang Denk wrote: Anton Staaf wrote: ... 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 . 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 signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
On Thu, Oct 13, 2011 at 1:31 PM, Wolfgang Denk w...@denx.de wrote: Dear Anton Staaf, In message CAF6FioWnYJNBz0+4Af3-0vLCoGrgGgcN10z=k0df8yv87gu...@mail.gmail.com you wrote: 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? 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. Thanks, Anton 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: w...@denx.de Hokey religions and ancient weapons are no substitute for a good blaster at your side. - Han Solo ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
On Thu, Oct 13, 2011 at 1:36 PM, Mike Frysinger vap...@gentoo.org wrote: On Thursday 13 October 2011 16:03:37 Wolfgang Denk wrote: Anton Staaf wrote: ... 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 . 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. This makes sense to me, as it is pretty much what I did initially. I ended up not sending that patch up because I was able to define ARCH_DMA_MINALIGN for all architectures we care about. But in the future it might be a good pattern for static checking configs. Thanks, Anton -mike ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
Dear Anton Staaf, In message CAF6FioXcAAP=QOF3a5s_L6zUB5ArEUAQYutruAx=bpmt7rg...@mail.gmail.com you wrote: 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. OK. so I'll wait for your next version patch set. Thanks!! 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: w...@denx.de The more data I punch in this card, the lighter it becomes, and the lower the mailing cost. - Stan Kelly-Bootle, The Devil's DP Dictionary ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
Hi Mike, On 09.10.2011 19:56, Mike Frysinger wrote: 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. Ok, I've dropped DAVINCI_EMAC_DCACHE (please see my v2 patches) but now all DaVinci boards using this driver are broken... Regards, Ilya. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
Hi Stefano, On 10.10.2011 17:17, Stefano Babic wrote: 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 Yes, that's what I need. But it looks like it's defined mostly for the PPC boards.. can help in our case ( cache: add ALLOC_CACHE_ALIGN_BUFFER macro): http://patchwork.ozlabs.org/patch/117698/ Hm.. Well, actually I need to align the static buffer, so I don't need this, __aligned(CONFIG_SYS_CACHELINE_SIZE) does the trick. Regards, Ilya. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
On 10/06/2011 01:36 AM, Ilya Yanok wrote: 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 ya...@emcraft.com Hi Ilya, --- drivers/net/davinci_emac.c | 47 1 files changed, 47 insertions(+), 0 deletions(-) diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c index 222a0d0..b787213 100644 --- a/drivers/net/davinci_emac.c +++ b/drivers/net/davinci_emac.c @@ -96,6 +96,40 @@ static volatile u_int8_t active_phy_addr = 0xff; phy_tphy; +#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 ? Best regards, Stefano Babic -- = DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
Hi Stefano, On 10.10.2011 16:35, Stefano Babic wrote: +#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 ? 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? For now I've just hardcoded the 64 byte alignment but this seems to be not perfect solution... Regards, Ilya. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
On 10/10/2011 03:03 PM, Ilya Yanok wrote: Hi Stefano, Hi Ilya, On 10.10.2011 16:35, Stefano Babic wrote: +#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 ? 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. Regards, Stefano -- = DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
Dear Stefano Babic, In message 4e92f05f.4030...@denx.de you wrote: 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. 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: w...@denx.de A modem is a baudy house. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
On Monday 10 October 2011 09:39:16 Wolfgang Denk wrote: Stefano Babic wrote: 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. 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 ... -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
Dear Mike Frysinger, In message 201110101124.22548.vap...@gentoo.org you wrote: 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: w...@denx.de Maintain an awareness for contribution -- to your schedule, your project, our company. - A Group of Employees ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
On Monday 10 October 2011 13:44:21 Wolfgang Denk wrote: Mike Frysinger wrote: 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 blah, this is the one e-mail in that thread that went into my trash somehow. i'll follow up in the original posting; thanks. -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
On Mon, Oct 10, 2011 at 10:44 AM, Wolfgang Denk w...@denx.de wrote: Dear Mike Frysinger, In message 201110101124.22548.vap...@gentoo.org you wrote: 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 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... -Anton 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: w...@denx.de Maintain an awareness for contribution -- to your schedule, your project, our company. - A Group of Employees ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
Dear Anton Staaf, In message CAF6FioWbLz9JKBnj5sJGALtj=bjylnbfvunna6fe62y42t6...@mail.gmail.com you wrote: 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... 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. 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 loong 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: w...@denx.de We don't have to protect the environment -- the Second Coming is at hand. - James Watt ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
On Mon, Oct 10, 2011 at 11:31 AM, Wolfgang Denk w...@denx.de wrote: Dear Anton Staaf, In message CAF6FioWbLz9JKBnj5sJGALtj=bjylnbfvunna6fe62y42t6...@mail.gmail.com you wrote: 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... Yes, sorry, I should have said almost no architectures define it, PPC being the exception. 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. Yes, that's my feeling as well. 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 loong 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 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: w...@denx.de We don't have to protect the environment -- the Second Coming is at hand. - James Watt ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
On Mon, Oct 10, 2011 at 11:45 AM, Anton Staaf robot...@chromium.org wrote: On Mon, Oct 10, 2011 at 11:31 AM, Wolfgang Denk w...@denx.de wrote: Dear Anton Staaf, In message CAF6FioWbLz9JKBnj5sJGALtj=bjylnbfvunna6fe62y42t6...@mail.gmail.com you wrote: 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... Yes, sorry, I should have said almost no architectures define it, PPC being the exception. 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. Yes, that's my feeling as well. 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 loong 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. So I broke down and am reading all the specs I can find to determine the L1 data cache line sizes for all of the supported architectures in U-Boot. But this means that my patch when it comes will need a good bit of review from people with more experience from each of these architectures. In most cases I am opting to have cache.h define the CONFIG_SYS_CACHELINE_SIZE macro only if it is not already defined by the board config. And then only if I can determine a reasonable upper bound for it's value for an architecture. This also means that my patch will take a little longer to show up, probably tomorrow or Wednesday would be my guess. I plan to submit this as a separate patch set from the one that uses the value of CONFIG_SYS_CACHELINE_SIZE to define the stack allocation macro and it's use. Thanks, Anton Thanks, Anton 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: w...@denx.de We don't have to protect the environment -- the Second Coming is at hand. - James Watt ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
Hi Mike, On 07.10.2011 21:34, Mike Frysinger wrote: +#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, +(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?) Regards, Ilya. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
On Sun, Oct 09, 2011 at 02:41:47PM +0400, Ilya Yanok wrote: 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?) Even better would be working cache functions, so that DA8xx boards (and presumably other DaVinci processors) can run with Ethernet and cache enabled. There have been a few proposals floating around to do this, but I'm not sure whether they've stalled or are continuing quietly along. Bye for now, -- Laurence Withers, lwith...@guralp.comhttp://www.guralp.com/ Direct tel:+447753988197 or tel:+44408643 Software Engineer General support queries: supp...@guralp.com CMG-DCM CMG-EAM CMG-NAM ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
On Sunday 09 October 2011 06:41:47 Ilya Yanok wrote: On 07.10.2011 21:34, Mike Frysinger wrote: +#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, + (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?) 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. -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
On Wednesday 05 October 2011 19:36:44 Ilya Yanok wrote: --- a/drivers/net/davinci_emac.c +++ b/drivers/net/davinci_emac.c +#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, + (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 ? -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 4/6] davinci_emac: fix for running with dcache enabled
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 ya...@emcraft.com --- drivers/net/davinci_emac.c | 47 1 files changed, 47 insertions(+), 0 deletions(-) diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c index 222a0d0..b787213 100644 --- a/drivers/net/davinci_emac.c +++ b/drivers/net/davinci_emac.c @@ -96,6 +96,40 @@ static volatile u_int8_t active_phy_addr = 0xff; phy_t phy; +#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, + (unsigned long)addr + size); +} +#else +#define davinci_flush(addr, size) do {} while (0) +#define davinci_invalidate(addr, size) do {} while (0) +#endif + +static inline void davinci_flush_rx_descs(void) +{ + davinci_flush((void *)emac_rx_desc, EMAC_MAX_RX_BUFFERS * + sizeof(emac_desc)); +} + +static inline void davinci_invalidate_rx_descs(void) +{ + davinci_invalidate((void *)emac_rx_desc, EMAC_MAX_RX_BUFFERS * + sizeof(emac_desc)); +} + +static inline void davinci_flush_desc(emac_desc *desc) +{ + davinci_flush((void *)desc, sizeof(*desc)); +} + static int davinci_eth_set_mac_addr(struct eth_device *dev) { unsigned long mac_hi; @@ -412,6 +446,8 @@ static int davinci_eth_open(struct eth_device *dev, bd_t *bis) emac_rx_active_tail = rx_desc; emac_rx_queue_active = 1; + davinci_flush_rx_descs(); + /* Enable TX/RX */ writel(EMAC_MAX_ETHERNET_PKT_SIZE, adap_emac-RXMAXLEN); writel(0, adap_emac-RXBUFFEROFFSET); @@ -568,6 +604,10 @@ static int davinci_eth_send_packet (struct eth_device *dev, EMAC_CPPI_SOP_BIT | EMAC_CPPI_OWNERSHIP_BIT | EMAC_CPPI_EOP_BIT); + + davinci_flush((void *)packet, length); + davinci_flush_desc(emac_tx_desc); + /* Send the packet */ writel(BD_TO_HW((unsigned long)emac_tx_desc), adap_emac-TX0HDP); @@ -600,6 +640,8 @@ static int davinci_eth_rcv_packet (struct eth_device *dev) volatile emac_desc *tail_desc; int status, ret = -1; + davinci_invalidate_rx_descs(); + rx_curr_desc = emac_rx_active_head; status = rx_curr_desc-pkt_flag_len; if ((rx_curr_desc) ((status EMAC_CPPI_OWNERSHIP_BIT) == 0)) { @@ -607,6 +649,8 @@ static int davinci_eth_rcv_packet (struct eth_device *dev) /* Error in packet - discard it and requeue desc */ printf (WARN: emac_rcv_pkt: Error in packet\n); } else { + davinci_invalidate(rx_curr_desc-buffer, + rx_curr_desc-buff_off_len 0x); NetReceive (rx_curr_desc-buffer, (rx_curr_desc-buff_off_len 0x)); ret = rx_curr_desc-buff_off_len 0x; @@ -632,6 +676,7 @@ static int davinci_eth_rcv_packet (struct eth_device *dev) rx_curr_desc-buff_off_len = EMAC_MAX_ETHERNET_PKT_SIZE; rx_curr_desc-pkt_flag_len = EMAC_CPPI_OWNERSHIP_BIT; rx_curr_desc-next = 0; + davinci_flush_desc(rx_curr_desc); if (emac_rx_active_head == 0) { printf (INFO: emac_rcv_pkt: active queue head = 0\n); @@ -649,11 +694,13 @@ static int davinci_eth_rcv_packet (struct eth_device *dev) tail_desc-next = BD_TO_HW((unsigned int) curr_desc); status = tail_desc-pkt_flag_len; if (status EMAC_CPPI_EOQ_BIT) { + davinci_flush_desc(tail_desc); writel(BD_TO_HW((unsigned long)curr_desc), adap_emac-RX0HDP); status = ~EMAC_CPPI_EOQ_BIT; tail_desc-pkt_flag_len = status; } + davinci_flush_desc(tail_desc); } return (ret); } -- 1.7.6.2 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot