Re: [U-Boot] [PATCH] sc520: fix build warning about unused temp var
Hi Mike, On 03/06/2012 08:55 AM, Mike Frysinger wrote: > Building the eNET_SRAM board fails for me: > sc520_timer.c: In function 'sc520_udelay': > sc520_timer.c:81:7: error: variable 'temp' set but not used > [-Werror=unused-but-set-variable] > cc1: all warnings being treated as errors > make[1]: *** [sc520_timer.o] Error 1 > > Signed-off-by: Mike Frysinger Applied Regards, Graeme ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] sc520: fix build warning about unused temp var
Hi Mike, On Tue, Mar 6, 2012 at 10:11 AM, Mike Frysinger wrote: > On Monday 05 March 2012 17:31:43 Graeme Russ wrote: >> So do I apply this to fix the build warning knowing there is another more >> serious bug and knowing this arch is getting scrapped soon, or do I just >> leave it as is? > > considering this warning breaks `MAKEALL` testing, it's adding noise to people > trying to verify their own unrelated changes. so yes, i think this fix should > be merged, or the code dropped. leaving it in between is negatively affecting > others. OK, I will apply a send a pull request hopefully tonight Regards, Graeme ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] sc520: fix build warning about unused temp var
On Monday 05 March 2012 17:31:43 Graeme Russ wrote:
> On Tue, Mar 6, 2012 at 8:55 AM, Mike Frysinger wrote:
> > Building the eNET_SRAM board fails for me:
> >sc520_timer.c: In function 'sc520_udelay':
> >sc520_timer.c:81:7: error: variable 'temp' set but not used
> >[-Werror=unused-but-set-variable]
> >cc1: all warnings being treated as errors
> >make[1]: *** [sc520_timer.o] Error 1
> >
> > Signed-off-by: Mike Frysinger
> > ---
> > arch/x86/cpu/sc520/sc520_timer.c |5 ++---
> > 1 files changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/cpu/sc520/sc520_timer.c
> > b/arch/x86/cpu/sc520/sc520_timer.c index 495a694..41f121f 100644
> > --- a/arch/x86/cpu/sc520/sc520_timer.c
> > +++ b/arch/x86/cpu/sc520/sc520_timer.c
> > @@ -78,10 +78,9 @@ void sc520_udelay(unsigned long usec)
> > {
> >int m = 0;
> >long u;
> > - long temp;
> >
> > - temp = readw(&sc520_mmcr->swtmrmilli);
> > - temp = readw(&sc520_mmcr->swtmrmicro);
> > + readw(&sc520_mmcr->swtmrmilli);
> > + readw(&sc520_mmcr->swtmrmicro);
> >
> >do {
> >m += readw(&sc520_mmcr->swtmrmilli);
>
> I really appreciate the effort you (and others) have put into fixing this
> and I really should have made a more formal response earlier (sorry), but
> here goes...
>
> Although this will fix the build warning, the actual implementation of
> sc520_udelay is wrong and can produce a timeout which is up to 1000us
> short (which I think we can all agree is a bad thing). On top of this,
> there is an sc520 silicon bug which means even a technically correct
> software implementation may produce erroneous results (although from
> memory that produces a timeout which could be 1000us long which is
> better than a short timeout)
>
> And then there is the fact that this is a depricated platform - There is
> only one physical sc520 board which U-Boot has ever been loaded onto and
> that is buried on a shelf in my study somewhere - I'm working on another
> x86 board at the moment, but I'm not at liberty to release the code yet.
> As soon as that is released, I'm going to remove the sc520 anyway
>
> So do I apply this to fix the build warning knowing there is another more
> serious bug and knowing this arch is getting scrapped soon, or do I just
> leave it as is?
considering this warning breaks `MAKEALL` testing, it's adding noise to people
trying to verify their own unrelated changes. so yes, i think this fix should
be merged, or the code dropped. leaving it in between is negatively affecting
others.
-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] sc520: fix build warning about unused temp var
Hi Mike,
On Tue, Mar 6, 2012 at 8:55 AM, Mike Frysinger wrote:
> Building the eNET_SRAM board fails for me:
> sc520_timer.c: In function 'sc520_udelay':
> sc520_timer.c:81:7: error: variable 'temp' set but not used
> [-Werror=unused-but-set-variable]
> cc1: all warnings being treated as errors
> make[1]: *** [sc520_timer.o] Error 1
>
> Signed-off-by: Mike Frysinger
> ---
> arch/x86/cpu/sc520/sc520_timer.c | 5 ++---
> 1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/cpu/sc520/sc520_timer.c
> b/arch/x86/cpu/sc520/sc520_timer.c
> index 495a694..41f121f 100644
> --- a/arch/x86/cpu/sc520/sc520_timer.c
> +++ b/arch/x86/cpu/sc520/sc520_timer.c
> @@ -78,10 +78,9 @@ void sc520_udelay(unsigned long usec)
> {
> int m = 0;
> long u;
> - long temp;
>
> - temp = readw(&sc520_mmcr->swtmrmilli);
> - temp = readw(&sc520_mmcr->swtmrmicro);
> + readw(&sc520_mmcr->swtmrmilli);
> + readw(&sc520_mmcr->swtmrmicro);
>
> do {
> m += readw(&sc520_mmcr->swtmrmilli);
I really appreciate the effort you (and others) have put into fixing this
and I really should have made a more formal response earlier (sorry), but
here goes...
Although this will fix the build warning, the actual implementation of
sc520_udelay is wrong and can produce a timeout which is up to 1000us
short (which I think we can all agree is a bad thing). On top of this,
there is an sc520 silicon bug which means even a technically correct
software implementation may produce erroneous results (although from
memory that produces a timeout which could be 1000us long which is
better than a short timeout)
And then there is the fact that this is a depricated platform - There is
only one physical sc520 board which U-Boot has ever been loaded onto and
that is buried on a shelf in my study somewhere - I'm working on another
x86 board at the moment, but I'm not at liberty to release the code yet.
As soon as that is released, I'm going to remove the sc520 anyway
So do I apply this to fix the build warning knowing there is another more
serious bug and knowing this arch is getting scrapped soon, or do I just
leave it as is?
Regards,
Graeme
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] sc520: fix build warning about unused temp var
Building the eNET_SRAM board fails for me:
sc520_timer.c: In function 'sc520_udelay':
sc520_timer.c:81:7: error: variable 'temp' set but not used
[-Werror=unused-but-set-variable]
cc1: all warnings being treated as errors
make[1]: *** [sc520_timer.o] Error 1
Signed-off-by: Mike Frysinger
---
arch/x86/cpu/sc520/sc520_timer.c |5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/x86/cpu/sc520/sc520_timer.c b/arch/x86/cpu/sc520/sc520_timer.c
index 495a694..41f121f 100644
--- a/arch/x86/cpu/sc520/sc520_timer.c
+++ b/arch/x86/cpu/sc520/sc520_timer.c
@@ -78,10 +78,9 @@ void sc520_udelay(unsigned long usec)
{
int m = 0;
long u;
- long temp;
- temp = readw(&sc520_mmcr->swtmrmilli);
- temp = readw(&sc520_mmcr->swtmrmicro);
+ readw(&sc520_mmcr->swtmrmilli);
+ readw(&sc520_mmcr->swtmrmicro);
do {
m += readw(&sc520_mmcr->swtmrmilli);
--
1.7.8.4
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
