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

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

[Renato Andreola]

 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 ;-)
      */
-- 
1.5.5

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

[Wolfgang Denk]

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

-- 
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
There's an old proverb that says just about whatever you want it to.

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

[Scott McNutt]

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

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

[Renato Andreola]

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

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

[Wolfgang Denk]

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

-- 
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
Democracy is mob rule, but with income taxes.

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

[Wolfgang Wegner]

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

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

[Wolfgang Denk]

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

-- 
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
"There are some good people in it, but the orchestra as  a  whole  is
equivalent to a gang bent on destruction."      - John Cage, composer

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

[Wolfgang Wegner]

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

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

[Wolfgang Denk]

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

-- 
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
Use the Force, Luke.

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

[Scott McNutt]

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

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

[Wolfgang Denk]

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

-- 
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
"In matters of principle, stand like a rock;  in  matters  of  taste,
swim with the current."                            - Thomas Jefferson

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

[Scott McNutt]

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
> 

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

[Renato Andreola]

 From caddbcecc99d7f96fdf7ca9c3d89ffac9cb999e1 Mon Sep 17 00:00:00 2001
From: Renato Andreola <renato.andreola@imagos.it>
Date: Thu, 6 Aug 2009 11:03:19 +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 |    5 +++++
  1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/lib_nios2/bootm.c b/lib_nios2/bootm.c
index 53fd569..65bb4cb 100644
--- a/lib_nios2/bootm.c
+++ b/lib_nios2/bootm.c
@@ -24,6 +24,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)
  {
@@ -32,6 +33,10 @@ 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 ;-)
  	 */
-- 
1.5.5