Skip to contentSkip to conversation

sc520: fix build warning about unused temp var

PATCH New Mike Frysinger 2 participants 5 messages Mar 5, 2012 – Mar 6, 2012 x86

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 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
---
 arch/x86/cpu/sc520/sc520_timer.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)
arch/x86/cpu/sc520/sc520_timer.c +2 -3
index 495a694..41f121f 100644
@@ -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);
Signature · 1 line
1.7.8.4
#1 lore raw

PATCH 1/1 sc520: fix build warning about unused temp var

Hi Mike,

Quoted 19 lines · Mike Frysinger

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

---
?arch/x86/cpu/sc520/sc520_timer.c | ? ?5 ++---
?1 files changed, 2 insertions(+), 3 deletions(-)

arch/x86/cpu/sc520/sc520_timer.c +0 -0
index 495a694..41f121f 100644
@@ -78,10 +78,9 @@ void sc520_udelay(unsigned long usec)
 ?{
 ? ? ? ?int m = 0;

arch/x86/cpu/sc520/sc520_timer.c void sc520_udelay(unsigned long usec) · line 86

Quoted 10 lines
arch/x86/cpu/sc520/sc520_timer.c +2 -3
 ? ? ? ?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

#2 lore raw

PATCH 1/1 sc520: fix build warning about unused temp var

Quoted 49 lines · On Monday 05 March 2012 17:31:43 Graeme Russ wrote:
Quoted 19 lines · Mike Frysinger

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

---
 arch/x86/cpu/sc520/sc520_timer.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)
(quoted diff) +0 -0

b/arch/x86/cpu/sc520/sc520_timer.c index 495a694..41f121f 100644

arch/x86/cpu/sc520/sc520_timer.c +0 -0
@@ -78,10 +78,9 @@ void sc520_udelay(unsigned long usec)
 {
       int m = 0;

arch/x86/cpu/sc520/sc520_timer.c void sc520_udelay(unsigned long usec) · line 86

Quoted 10 lines
arch/x86/cpu/sc520/sc520_timer.c +2 -3
       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

Quoted 3 lines

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120305/76fd1fa4/attachment.pgp>

#3 lore raw

PATCH 1/1 sc520: fix build warning about unused temp var

Hi Mike,

Quoted 10 lines · Mike Frysinger
Quoted 3 lines · 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

Applied
#4 lore raw

PATCH 1/1 sc520: fix build warning about unused temp var

In reply to Mike Frysinger

Hi Mike,

Quoted 8 lines · Mike Frysinger

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

Applied

Regards,

Graeme