[U-Boot] PATCH Nios2 kernel bootstrap error due to missing processor data cache flush: fix

From b75bd27f89ac6c105cebb6507cf082b6f5fffc7d Mon Sep 17 00:00:00 2001 From: Renato Andreola renato.andreola@imagos.it Date: Fri, 10 Apr 2009 12:32:29 +0200 Subject: Nios2: do_boom_linux(): kernel gunzip input data integrity problem due to mi ssing cache flush
Added instruction and data caches flush
Signed-off-by: Renato Andreola renato.andreola@imagos.it --- lib_nios2/bootm.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/lib_nios2/bootm.c b/lib_nios2/bootm.c index 53fd569..1e8034b 100644 --- a/lib_nios2/bootm.c +++ b/lib_nios2/bootm.c @@ -2,6 +2,9 @@ * (C) Copyright 2003, Psyent Corporation <www.psyent.com> * Scott McNutt smcnutt@psyent.com * + * (C) Copyright 2009, Imagos sas <www.imagos.it> + * Renato Andreola renato.andreola@imagos.it + * * See file CREDITS for list of people who contributed to this * project. * @@ -24,6 +27,7 @@ #include <common.h> #include <command.h> #include <asm/byteorder.h> +#include <asm/cache.h>
int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) { @@ -31,7 +35,9 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
if ((flag != 0) && (flag != BOOTM_STATE_OS_GO)) return 1; - + /* flushes data and instruction caches before calling the kernel */ + flush_dcache (0,CONFIG_SYS_DCACHE_SIZE ); + flush_icache (0,CONFIG_SYS_ICACHE_SIZE); /* For now we assume the Microtronix linux ... which only * needs to be called ;-) */

Dear Scott,
In message 49DF39CD.9010202@imagos.it you wrote:
From b75bd27f89ac6c105cebb6507cf082b6f5fffc7d Mon Sep 17 00:00:00 2001 From: Renato Andreola renato.andreola@imagos.it Date: Fri, 10 Apr 2009 12:32:29 +0200 Subject: Nios2: do_boom_linux(): kernel gunzip input data integrity problem due to mi ssing cache flush
Added instruction and data caches flush Signed-off-by: Renato Andreola <renato.andreola@imagos.it>
lib_nios2/bootm.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)
AFAICT this has neither been applied nor rejected yet. Please check.
Best regards,
Wolfgang Denk

See comments.
diff --git a/lib_nios2/bootm.c b/lib_nios2/bootm.c index 53fd569..1e8034b 100644 --- a/lib_nios2/bootm.c +++ b/lib_nios2/bootm.c @@ -2,6 +2,9 @@
- (C) Copyright 2003, Psyent Corporation <www.psyent.com>
- Scott McNutt smcnutt@psyent.com
- (C) Copyright 2009, Imagos sas <www.imagos.it>
- Renato Andreola renato.andreola@imagos.it
Claiming a copyright ...
- /* flushes data and instruction caches before calling the kernel */
- flush_dcache (0,CONFIG_SYS_DCACHE_SIZE );
- flush_icache (0,CONFIG_SYS_ICACHE_SIZE);
... for a two line bug fix?
This is hardly a valid reason to claim copyright on the module.
This practice will only discourage the contribution of original work to the project. Nobody wants to have their work hijacked in such a manner.
Please resubmit.
Regards, --Scott

Ok, please update the file with the correct bug fix with no more copyright. My intent is only to fix the bug that caused random boot failures and to keep free the good work on Nios2.
I was warried about that the official main UBoot distribution contained a bug that has a big impact on the ability of someone to test a linux distribution on Nios2 with UBoot.
We have spent two weeks with low level prints to find that the bug was related to the missing data cache flush and not to the kernel or the pre-kernel "trampoline" code.
Please note that the other bug related to incorrect flash timeout into the CFI flash code has not been applied into the last Uboot release yet. That bug was related to an integer division that leads to a zero timeout in case of a less that 1000Hz timer (e.g: if the calculated timer frequency is 999.99Hz, due to a truncation into the macro definition, the resulting time to sleep is Zero: this brings to non reliable clear/program cycles in Intel/Numonyx flash chips that works with the status flag polling algorithm).
Regards, Renato
Scott McNutt wrote:
See comments.
diff --git a/lib_nios2/bootm.c b/lib_nios2/bootm.c index 53fd569..1e8034b 100644 --- a/lib_nios2/bootm.c +++ b/lib_nios2/bootm.c @@ -2,6 +2,9 @@
- (C) Copyright 2003, Psyent Corporation <www.psyent.com>
- Scott McNutt smcnutt@psyent.com
- (C) Copyright 2009, Imagos sas <www.imagos.it>
- Renato Andreola renato.andreola@imagos.it
Claiming a copyright ...
- /* flushes data and instruction caches before calling the kernel */
- flush_dcache (0,CONFIG_SYS_DCACHE_SIZE );
- flush_icache (0,CONFIG_SYS_ICACHE_SIZE);
... for a two line bug fix?
This is hardly a valid reason to claim copyright on the module.
This practice will only discourage the contribution of original work to the project. Nobody wants to have their work hijacked in such a manner.
Please resubmit.
Regards, --Scott

Dear Renato Andreola,
In message 4A66C746.4050909@imagos.it you wrote:
Ok, please update the file with the correct bug fix with no more copyright. My intent is only to fix the bug that caused random boot failures and to keep free the good work on Nios2.
Please re-submit a cleaned up patch. Thanks in advance.
Best regards,
Wolfgang Denk

Dear Scott, Wolfgang,
On Tue, Jul 21, 2009 at 02:02:43PM -0400, Scott McNutt wrote:
... for a two line bug fix?
This is hardly a valid reason to claim copyright on the module.
This practice will only discourage the contribution of original work to the project. Nobody wants to have their work hijacked in such a manner.
So you really think your practice will encourage anybody to submit a clean patch for a bug he spent days (or weeks) to find?
I found it very difficult to supply a clean patch for some of the fixes or improvements I did for my private coldfire tree. For few of them, I sent bug reports with the part to fix - but I am not even sure if they finally got picked up by the maintainer.
On the one hand, you want patches to be tested (which is a good thing, of course), on the other hand I can not test a patch without other things changed for my board (which is not really useful for inclusion in the official tree, I think). Keeping several trees up-to-date to test the patches out of the vanilla tree and then rolling them back into the vanilly tree is at least beyond my time scale (and, to be honest, sometimes my knowledge of git, which is not self-explanatory at all).
I can understand the maintainers have not that much time, I can also understand that the original work has to get its due credit - but then those sending patches should get their credit, too, or else the maintainer has to do the work of integrating the fix himself.
Just my 2C, of course...
Regards, Wolfgang

Dear Wolfgang Wegner,
In message 20090722094127.GV20598@leila.ping.de you wrote:
So you really think your practice will encourage anybody to submit a clean patch for a bug he spent days (or weeks) to find?
That's how the public review process works: You submit a patch, and it either gets accepted or rejected. If it gets rejected, you gotta rework it again (and again, if necessary) until it gets accepted.
This is not different for you or for me or for anybody else.
If you submit a patch that gets accepted you at least 1) know that the problem is fixed in mainline, i. e. once and for ever, and 2) you receive the full credit for it because you show up as the author of the commit.
If you don't do that, then either somebody else will clean up your patch and commit it and earn the credits for your work, or nobody will care and the problem remains, which means your work was basicly wasted.
Your choice.
On the one hand, you want patches to be tested (which is a good thing, of course), on the other hand I can not test a patch without other things changed for my board (which is not really useful for inclusion in the official tree, I think). Keeping several trees up-to-date to test the patches out of the vanilla tree and then rolling them back into the vanilly tree is at least beyond my time scale (and, to be honest, sometimes my knowledge of git, which is not self-explanatory at all).
Well, we cannot save you the effort of learning git. We all had to go through this ourself. But I can promise that it is very well invested effort which will pay back very quickly.
Um... and instead of maintaining several private source trees you should consider poushing your code upstream, so that others do the maintenance for you.
I can understand the maintainers have not that much time, I can also understand that the original work has to get its due credit - but then those sending patches should get their credit, too, or else the maintainer has to do the work of integrating the fix himself.
You get the credit by being the author of the commit, and by your Signed-off-by: line in it. "git log" will show it, "git blame" will show it, and you will find your place in the U-Boot statistics page, too. You do not have any entitlement on a (C) Copyright entry in a bigger source file if you change just a few lines in it. That's not reasonable.
Best regards,
Wolfgang Denk

Dear Wolfgang,
On Wed, Jul 22, 2009 at 12:56:52PM +0200, Wolfgang Denk wrote: [...]
If you don't do that, then either somebody else will clean up your patch and commit it and earn the credits for your work, or nobody will care and the problem remains, which means your work was basicly wasted.
that was why I wrote now, because it seemed to me that the latter might happen in the current case. But maybe I misunderstood Renatos intention here.
[...]
Um... and instead of maintaining several private source trees you should consider poushing your code upstream, so that others do the maintenance for you.
The intention is to do so with my next project at work - but again it depends on how much time is left after sorting out all the low-level hard- and software bugs.
You get the credit by being the author of the commit, and by your Signed-off-by: line in it. "git log" will show it, "git blame" will show it, and you will find your place in the U-Boot statistics page, too. You do not have any entitlement on a (C) Copyright entry in a bigger source file if you change just a few lines in it. That's not reasonable.
Thanks for clearing this up.
Best regards, Wolfgang

Dear Wolfgang Wegner,
In message 20090722103358.GW20598@leila.ping.de you wrote:
Um... and instead of maintaining several private source trees you should consider poushing your code upstream, so that others do the maintenance for you.
The intention is to do so with my next project at work - but again it depends on how much time is left after sorting out all the low-level hard- and software bugs.
I strongly recommend to include the time for the upstream-pushing into your next project schedule, right from the beginning. Consider it an important quality-assurance measure - where else do you get a large number of highly qualified experts reviewing your code _for_ _free_ ?
Otherwise you might find yourself in a situation (again) where you will have to realize that we never get assigned enough time to do things right - but always to do them twice.
Best regards,
Wolfgang Denk

Wolfgang Wegner wrote:
Dear Scott, Wolfgang,
On Tue, Jul 21, 2009 at 02:02:43PM -0400, Scott McNutt wrote:
... for a two line bug fix?
This is hardly a valid reason to claim copyright on the module.
This practice will only discourage the contribution of original work to the project. Nobody wants to have their work hijacked in such a manner.
So you really think your practice will encourage anybody to submit a clean patch for a bug he spent days (or weeks) to find?
Many contributors have done so, many times, for many years, across many different architectures, myself included.
The amount of time I have spent doing so is insignificant when compared to the benefits I have enjoyed from the generous contributions of _others_. It's one of the small ways I can express my gratitude for the privilege of using such contributions.
But this is all irrelevant and off point, no? I am very happy to hear that someone can benefit from the code I spent many, many weeks to develop in my spare time ... code that I have offered freely. So don't enjoy the benefit, then claim co-ownership after adding two lines. At the very best, that's presumptuous.
And yes, if that was considered accepted practice, I doubt I'd make any significant original contributions again.
Best Regards, --Scott

Dear Renato Andreola,
In message 49DF39CD.9010202@imagos.it you wrote:
From b75bd27f89ac6c105cebb6507cf082b6f5fffc7d Mon Sep 17 00:00:00 2001 From: Renato Andreola renato.andreola@imagos.it Date: Fri, 10 Apr 2009 12:32:29 +0200 Subject: Nios2: do_boom_linux(): kernel gunzip input data integrity problem due to mi ssing cache flush
Added instruction and data caches flush Signed-off-by: Renato Andreola <renato.andreola@imagos.it>
lib_nios2/bootm.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/lib_nios2/bootm.c b/lib_nios2/bootm.c index 53fd569..1e8034b 100644 --- a/lib_nios2/bootm.c +++ b/lib_nios2/bootm.c @@ -2,6 +2,9 @@
- (C) Copyright 2003, Psyent Corporation <www.psyent.com>
- Scott McNutt smcnutt@psyent.com
- (C) Copyright 2009, Imagos sas <www.imagos.it>
- Renato Andreola renato.andreola@imagos.it
We are still waiting for you to resubmit this patch without adding a Copyright note for just two lines of code.
Thanks in advance.
Best regards,
Wolfgang Denk

Wolfgang,
I checked on this again ... and missed it as well. This was resubmitted on Thu, 6 Aug 2009. I'll add the resubmitted patch.
My apologies for any inconvenience.
Regards, --Scott
Wolfgang Denk wrote:
Dear Renato Andreola,
In message 49DF39CD.9010202@imagos.it you wrote:
From b75bd27f89ac6c105cebb6507cf082b6f5fffc7d Mon Sep 17 00:00:00 2001 From: Renato Andreola renato.andreola@imagos.it Date: Fri, 10 Apr 2009 12:32:29 +0200 Subject: Nios2: do_boom_linux(): kernel gunzip input data integrity problem due to mi ssing cache flush
Added instruction and data caches flush Signed-off-by: Renato Andreola <renato.andreola@imagos.it>
lib_nios2/bootm.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/lib_nios2/bootm.c b/lib_nios2/bootm.c index 53fd569..1e8034b 100644 --- a/lib_nios2/bootm.c +++ b/lib_nios2/bootm.c @@ -2,6 +2,9 @@
- (C) Copyright 2003, Psyent Corporation <www.psyent.com>
- Scott McNutt smcnutt@psyent.com
- (C) Copyright 2009, Imagos sas <www.imagos.it>
- Renato Andreola renato.andreola@imagos.it
We are still waiting for you to resubmit this patch without adding a Copyright note for just two lines of code.
Thanks in advance.
Best regards,
Wolfgang Denk
participants (4)
-
Renato Andreola
-
Scott McNutt
-
Wolfgang Denk
-
wolfgang@leila.ping.de