Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-10 Thread Segher Boessenkool
/scratch/tony/working/arch/powerpc/kernel/cacheinfo.c: In function  
'associativity_show':
/scratch/tony/working/arch/powerpc/kernel/cacheinfo.c:562:  
warning: 'associativity' may be used uninitialized in this function
/scratch/tony/working/arch/powerpc/kernel/cacheinfo.c: In function  
'size_show':
/scratch/tony/working/arch/powerpc/kernel/cacheinfo.c:513:  
warning: 'size_kb' may be used uninitialized in this function


Thanks.

So I think I've convinced myself that the warnings are incorrect and
that uninitialized use is not possible.


Strictly speaking the warnings aren't incorrect: the variables
may be used uninitialized, they just never are.  I agree this
isn't very helpful.

But I find it odd that gcc gives warnings for these sites but not  
others

in the file that use the same idiom (e.g. line_size_show,
nr_sets_show).


Yeah.


I'd guess that inlining is implicated somehow.  Would I
be justified in worrying that this version of gcc is generating
incorrect code?


Not really.  Sub-optimal code perhaps, but not incorrect.


If not, then I'm fine with the uninitialized_var() changes, but do
please include the warnings and the compiler version in the changelog.


If this happens with a non-ancient GCC version, can we have a bugreport
please?


Segher

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-10 Thread Scott Wood

Segher Boessenkool wrote:

Unfortunately -Wno-uninitialized also suppresses the warnings that point
to real bugs.


It's a double-edged sword, yes.  Warnings are always like that:
if the compiler could know that something _is_ wrong for certain,
it wouldn't need a warning (it would use an error, instead -- and
it does do this in certain cases); if it would know something is
not really wrong, it would just shut up.


The problem is that GCC does not give an error (only a warning) even for 
things like this where it should be trivial to detect that the usage 
*is* uninitialized, not just might be:


int foo(void)
{
   int a;

   return a;
}

And further, there is no separation of warning classes into 
might-be-uninitialized and is-uninitialized-compiler-can-tell-for-sure.


In other words, there should be a way to tell the compiler to err on the 
side of not complaining if it's unsure, but still report the obvious 
ones (or make the latter an error but the former a warning).  That's not 
ESP or DWIM.


-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-10 Thread Andreas Schwab
Scott Wood scottw...@freescale.com writes:

 The problem is that GCC does not give an error (only a warning) even for
 things like this where it should be trivial to detect that the usage *is*
 uninitialized, not just might be:

 int foo(void)
 {
int a;

return a;
 }

The compiler must not reject this code, because the undefined behavior
only occurs if executed.  There is no constraint violated.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-10 Thread Scott Wood

Andreas Schwab wrote:

Scott Wood scottw...@freescale.com writes:


The problem is that GCC does not give an error (only a warning) even for
things like this where it should be trivial to detect that the usage *is*
uninitialized, not just might be:

int foo(void)
{
   int a;

   return a;
}


The compiler must not reject this code, because the undefined behavior
only occurs if executed.  There is no constraint violated.


Fine (though GCC could have something similar to -Werror but more 
limited in scope to the really serious stuff that *should* be illegal 
even if it isn't), but it should at least be a separate warning class.


My point was to counter Segher's assertion that the compiler currently 
gives an error on the obvious stuff.


-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-10 Thread Segher Boessenkool
The problem is that GCC does not give an error (only a warning)  
even for
things like this where it should be trivial to detect that the  
usage *is*

uninitialized, not just might be:

int foo(void)
{
   int a;

   return a;
}
The compiler must not reject this code, because the undefined  
behavior

only occurs if executed.  There is no constraint violated.


Fine (though GCC could have something similar to -Werror but more  
limited in scope to the really serious stuff that *should* be  
illegal even if it isn't), but it should at least be a separate  
warning class.


My point was to counter Segher's assertion that the compiler  
currently gives an error on the obvious stuff.


I never said that, or didn't intend to anyway; what I was trying to say
is that the compiler makes a difference between cases where it knows
something is uninitialized vs. cases where it cannot prove either way:

$ cat mm.c
int bork(void)
{
int a;

return a;
}

int main(void)
{
return bork();
}

$ powerpc-linux-gcc -Wall -W -Os -c mm.c
mm.c: In function 'bork':
mm.c:5: warning: 'a' is used uninitialized in this function

Note: _is_ used uninitialized, not may be like in cases where the  
compiler

isn't sure.

I don't know why this isn't an error; perhaps GCC does not assume main 
() to

always be executed.  I don't think it could prove much anything to be
executed in non-toy examples, anyway.


Segher

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-10 Thread Segher Boessenkool
And further, there is no separation of warning classes into might- 
be-uninitialized and is-uninitialized-compiler-can-tell-for-sure.


Indeed.  Please file a bug report.


Segher

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-10 Thread Scott Wood

Segher Boessenkool wrote:
And further, there is no separation of warning classes into 
might-be-uninitialized and is-uninitialized-compiler-can-tell-for-sure.


Indeed.  Please file a bug report.


Done: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39731

-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-09 Thread Segher Boessenkool
Gah, gcc sucks. It should just not warn in these cases where it  
doesn't

know wth is going on.


I don't think you'll get any arguments.  it only there was a - 
Wnowarnunused!


-Wno-unused or -Wno-unused-pparameter and/or -Wno-unused-variable.   
But I

thought this was about uninitialised var warnings?  -Wno-uninitialized
for that one.

If you are asking for a GCC option that will warn for all suspect cases
_except_ for the ones where it is obvious to you there is no problem:
you could try -Wesp.


Segher

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-09 Thread Tony Breeds
On Fri, Apr 10, 2009 at 12:46:06AM +0200, Segher Boessenkool wrote:

 -Wno-unused or -Wno-unused-pparameter and/or -Wno-unused-variable.  But I
 thought this was about uninitialised var warnings?  -Wno-uninitialized
 for that one.

 If you are asking for a GCC option that will warn for all suspect cases
 _except_ for the ones where it is obvious to you there is no problem:
 you could try -Wesp.

Ooops I was asking for -Wno-uninitialized.  I'll try a build with:
-Wno-uninitialized -Wesp
and see how that goes.

Thanks.

Yours Tony
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-09 Thread Stephen Rothwell
On Fri, 10 Apr 2009 08:45:53 +1000 Tony Breeds t...@bakeyournoodle.com wrote:

 On Fri, Apr 10, 2009 at 12:46:06AM +0200, Segher Boessenkool wrote:
 
  -Wno-unused or -Wno-unused-pparameter and/or -Wno-unused-variable.  But I
  thought this was about uninitialised var warnings?  -Wno-uninitialized
  for that one.
 
 Ooops I was asking for -Wno-uninitialized.  I'll try a build with:
   -Wno-uninitialized -Wesp
 and see how that goes.

Unfortunately -Wno-uninitialized also suppresses the warnings that point
to real bugs. (but, I guess, the -Wesp might help there :-))

-- 
Cheers,
Stephen Rothwells...@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/


pgprU1u2ye9Yc.pgp
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-09 Thread Segher Boessenkool
-Wno-unused or -Wno-unused-pparameter and/or -Wno-unused- 
variable.  But I
thought this was about uninitialised var warnings?  -Wno- 
uninitialized

for that one.


If you are asking for a GCC option that will warn for all suspect  
cases

_except_ for the ones where it is obvious to you there is no problem:
you could try -Wesp.


Ooops I was asking for -Wno-uninitialized.  I'll try a build with:
-Wno-uninitialized -Wesp
and see how that goes.


-Wesp was a joke: I was trying to say that the compiler cannot read  
minds.

It doesn't know what potentially unused variables are obviously (to you)
actually used.


Segher

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-09 Thread Segher Boessenkool
Unfortunately -Wno-uninitialized also suppresses the warnings that  
point

to real bugs.


It's a double-edged sword, yes.  Warnings are always like that:
if the compiler could know that something _is_ wrong for certain,
it wouldn't need a warning (it would use an error, instead -- and
it does do this in certain cases); if it would know something is
not really wrong, it would just shut up.


(but, I guess, the -Wesp might help there :-))


Yeah, it's the holy grail of compiler architecture.  Do what I want,
not what I say :-)


Segher

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-09 Thread Nathan Lynch
Tony Breeds t...@bakeyournoodle.com wrote:

 On Wed, Apr 08, 2009 at 01:47:36PM -0500, Nathan Lynch wrote:
 
  I think I had some reason for doing it this way, but I'm drawing a
  blank right now.
  
  In the meantime, can someone post the warnings that gcc emits for
  cacheinfo.c, as well as the gcc version?  I can't reproduce them with
  4.3.2 on Fedora.
 
 ---
 [t...@thor ~]$ egrep cacheinfo tmp/build.log 
 /scratch/tony/working/arch/powerpc/kernel/cacheinfo.c: In function 
 'associativity_show':
 /scratch/tony/working/arch/powerpc/kernel/cacheinfo.c:562: warning: 
 'associativity' may be used uninitialized in this function
 /scratch/tony/working/arch/powerpc/kernel/cacheinfo.c: In function 
 'size_show':
 /scratch/tony/working/arch/powerpc/kernel/cacheinfo.c:513: warning: 'size_kb' 
 may be used uninitialized in this function

Thanks.

So I think I've convinced myself that the warnings are incorrect and
that uninitialized use is not possible.

But I find it odd that gcc gives warnings for these sites but not others
in the file that use the same idiom (e.g. line_size_show,
nr_sets_show).  I'd guess that inlining is implicated somehow.  Would I
be justified in worrying that this version of gcc is generating
incorrect code?

If not, then I'm fine with the uninitialized_var() changes, but do
please include the warnings and the compiler version in the changelog.

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-08 Thread Tony Breeds
On Wed, Apr 08, 2009 at 03:51:26PM +1000, Tony Breeds wrote:
 
 Hmm actually I think you're right.  I dont want to push my luck with the gcc
 hackers though

Replying to myself.

Yes this is a gcc bug but one introduced by CONFIG_TRACE_ALL_BRANCHES (or
whatever that's called).

Yours Tony
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-08 Thread Michael Ellerman
On Wed, 2009-04-08 at 15:51 +1000, Tony Breeds wrote:
 On Wed, Apr 08, 2009 at 03:08:55PM +1000, Michael Ellerman wrote:
 
  The getter routines in here could really multiplex their return values
  with a negative error code, which I generally prefer, but this works I
  guess.
 
 I was hoping someone would notice and suggest it.  tag you're it!

I meant we /could/ change them, but we could also leave them, it's a bit
of a coin-flip which is better. Nathan might have an opinion?

Something like this:

diff --git a/arch/powerpc/kernel/cacheinfo.c b/arch/powerpc/kernel/cacheinfo.c
index bb37b1d..9f3a155 100644
--- a/arch/powerpc/kernel/cacheinfo.c
+++ b/arch/powerpc/kernel/cacheinfo.c
@@ -191,7 +191,7 @@ static void cache_cpu_set(struct cache *cache, int cpu)
}
 }
 
-static int cache_size(const struct cache *cache, unsigned int *ret)
+static int cache_size(const struct cache *cache)
 {
const char *propname;
const u32 *cache_size;
@@ -202,19 +202,18 @@ static int cache_size(const struct cache *cache, unsigned 
if (!cache_size)
return -ENODEV;
 
-   *ret = *cache_size;
-   return 0;
+   return cache_size;
 }
 
-static int cache_size_kb(const struct cache *cache, unsigned int *ret)
+static int cache_size_kb(const struct cache *cache)
 {
unsigned int size;
 
-   if (cache_size(cache, size))
-   return -ENODEV;
+   size = cache_size(cache);
+   if (size  0)
+   return size;
 
-   *ret = size / 1024;
-   return 0;
+   return size / 1024;
 }
 
 /* not cache_line_size() because that's a macro in include/linux/cache.h */
@@ -515,8 +514,9 @@ static ssize_t size_show(struct kobject *k, struct kobj_attr
 
cache = index_kobj_to_cache(k);
 
-   if (cache_size_kb(cache, size_kb))
-   return -ENODEV;
+   size_kb = cache_size_kb(cache);
+   if (size_kb  0)
+   return size_kb;
 
return sprintf(buf, %uK\n, size_kb);
 }


cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person


signature.asc
Description: This is a digitally signed message part
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-08 Thread Geert Uytterhoeven
On Wed, 8 Apr 2009, Michael Ellerman wrote:
 On Wed, 2009-04-08 at 15:51 +1000, Tony Breeds wrote:
  On Wed, Apr 08, 2009 at 03:08:55PM +1000, Michael Ellerman wrote:
  
   The getter routines in here could really multiplex their return values
   with a negative error code, which I generally prefer, but this works I
   guess.
  
  I was hoping someone would notice and suggest it.  tag you're it!
 
 I meant we /could/ change them, but we could also leave them, it's a bit
 of a coin-flip which is better. Nathan might have an opinion?
 
 Something like this:
 
 diff --git a/arch/powerpc/kernel/cacheinfo.c b/arch/powerpc/kernel/cacheinfo.c
 index bb37b1d..9f3a155 100644
 --- a/arch/powerpc/kernel/cacheinfo.c
 +++ b/arch/powerpc/kernel/cacheinfo.c

 -static int cache_size_kb(const struct cache *cache, unsigned int *ret)
 +static int cache_size_kb(const struct cache *cache)
  {
 unsigned int size;
  
`int size', or Roel will come to get you ;-)

  
 -   if (cache_size(cache, size))
 -   return -ENODEV;
 +   size = cache_size(cache);
 +   if (size  0)
  
 +   return size;

I can't check the others, due to lacking context...

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:+32 (0)2 700 8453
Fax:  +32 (0)2 700 8622
E-mail:   geert.uytterhoe...@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-08 Thread Nathan Lynch
Michael Ellerman mich...@ellerman.id.au wrote:

 On Wed, 2009-04-08 at 15:51 +1000, Tony Breeds wrote:
  On Wed, Apr 08, 2009 at 03:08:55PM +1000, Michael Ellerman wrote:
  
   The getter routines in here could really multiplex their return values
   with a negative error code, which I generally prefer, but this works I
   guess.
  
  I was hoping someone would notice and suggest it.  tag you're it!
 
 I meant we /could/ change them, but we could also leave them, it's a bit
 of a coin-flip which is better. Nathan might have an opinion?

I think I had some reason for doing it this way, but I'm drawing a
blank right now.

In the meantime, can someone post the warnings that gcc emits for
cacheinfo.c, as well as the gcc version?  I can't reproduce them with
4.3.2 on Fedora.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-08 Thread Tony Breeds
On Wed, Apr 08, 2009 at 01:47:36PM -0500, Nathan Lynch wrote:

 I think I had some reason for doing it this way, but I'm drawing a
 blank right now.
 
 In the meantime, can someone post the warnings that gcc emits for
 cacheinfo.c, as well as the gcc version?  I can't reproduce them with
 4.3.2 on Fedora.

---
[t...@thor ~]$ egrep cacheinfo tmp/build.log 
/scratch/tony/working/arch/powerpc/kernel/cacheinfo.c: In function 
'associativity_show':
/scratch/tony/working/arch/powerpc/kernel/cacheinfo.c:562: warning: 
'associativity' may be used uninitialized in this function
/scratch/tony/working/arch/powerpc/kernel/cacheinfo.c: In function 'size_show':
/scratch/tony/working/arch/powerpc/kernel/cacheinfo.c:513: warning: 'size_kb' 
may be used uninitialized in this function
t...@sprygo:~/scratch$ 
/opt/crosstool/gcc-4.4.0-20090216-nolibc/powerpc-linux/bin/powerpc-linux-gcc -v
Using built-in specs.
Target: powerpc-linux
Configured with: /home/tony/buildall/src/gcc/configure --target=powerpc-linux 
--enable-targets=all 
--prefix=/opt/crosstool/gcc-4.4.0-20090216-nolibc/powerpc-linux 
--program-prefix=powerpc-linux- --disable-bootstrap --with-mpfr=/usr 
--enable-languages=c --without-headers --enable-sjlj-exceptions 
--with-system-libunwind --disable-nls --disable-threads --disable-shared 
--disable-libmudflap --disable-libssp --disable-libgomp --disable-decimal-float 
--enable-checking=release
Thread model: single
gcc version 4.4.0 20090216 (experimental) (GCC) 
t...@sprygo:~/scratch$ 
---

That's the compiler in rawhide

Yours Tony
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


[PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-07 Thread Tony Breeds
This patch silences all the warnings generated in arch/powerpc for
allmodconfig build.

It does:
 * Where appropriate use the uninitialized_var() macro to help GCC
   understand we know what's going on.
 * Explicitly casts PHYSICAL_START in one printk()
 * Initialise a few variables, as it's neater than using uninitialized_var()

Signed-off-by: Tony Breeds t...@bakeyournoodle.com
---
Only compile tested.

 arch/powerpc/kernel/cacheinfo.c   |4 ++--
 arch/powerpc/kernel/pci_dn.c  |2 +-
 arch/powerpc/kernel/setup_64.c|4 ++--
 arch/powerpc/platforms/cell/axon_msi.c|2 +-
 arch/powerpc/platforms/cell/beat_iommu.c  |2 +-
 arch/powerpc/platforms/iseries/pci.c  |   24 
 arch/powerpc/platforms/powermac/low_i2c.c |5 ++---
 arch/powerpc/platforms/pseries/msi.c  |2 +-
 8 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/arch/powerpc/kernel/cacheinfo.c b/arch/powerpc/kernel/cacheinfo.c
index bb37b1d..fd6aef9 100644
--- a/arch/powerpc/kernel/cacheinfo.c
+++ b/arch/powerpc/kernel/cacheinfo.c
@@ -510,7 +510,7 @@ static struct cache *index_kobj_to_cache(struct kobject *k)
 
 static ssize_t size_show(struct kobject *k, struct kobj_attribute *attr, char 
*buf)
 {
-   unsigned int size_kb;
+   unsigned int uninitialized_var(size_kb);
struct cache *cache;
 
cache = index_kobj_to_cache(k);
@@ -559,7 +559,7 @@ static struct kobj_attribute cache_nr_sets_attr =
 
 static ssize_t associativity_show(struct kobject *k, struct kobj_attribute 
*attr, char *buf)
 {
-   unsigned int associativity;
+   unsigned int uninitialized_var(associativity);
struct cache *cache;
 
cache = index_kobj_to_cache(k);
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index 1c67de5..b9d66ed 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -83,7 +83,7 @@ void *traverse_pci_devices(struct device_node *start, 
traverse_func pre,
void *data)
 {
struct device_node *dn, *nextdn;
-   void *ret;
+   void *uninitialized_var(ret);
 
/* We started with a phb, iterate all childs */
for (dn = start-child; dn; dn = nextdn) {
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index c410c60..38968f1 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -421,8 +421,8 @@ void __init setup_system(void)
printk(htab_address  = 0x%p\n, htab_address);
printk(htab_hash_mask= 0x%lx\n, htab_hash_mask);
if (PHYSICAL_START  0)
-   printk(physical_start= 0x%lx\n,
-  PHYSICAL_START);
+   printk(physical_start= 0x%llx\n,
+  (unsigned long long)PHYSICAL_START);
printk(-\n);
 
DBG( - setup_system()\n);
diff --git a/arch/powerpc/platforms/cell/axon_msi.c 
b/arch/powerpc/platforms/cell/axon_msi.c
index 0ce45c2..dae4c7c 100644
--- a/arch/powerpc/platforms/cell/axon_msi.c
+++ b/arch/powerpc/platforms/cell/axon_msi.c
@@ -151,7 +151,7 @@ static struct axon_msic *find_msi_translator(struct pci_dev 
*dev)
 {
struct irq_host *irq_host;
struct device_node *dn, *tmp;
-   const phandle *ph;
+   const phandle *uninitialized_var(ph);
struct axon_msic *msic = NULL;
 
dn = of_node_get(pci_device_to_OF_node(dev));
diff --git a/arch/powerpc/platforms/cell/beat_iommu.c 
b/arch/powerpc/platforms/cell/beat_iommu.c
index 93b0efd..8230cd8 100644
--- a/arch/powerpc/platforms/cell/beat_iommu.c
+++ b/arch/powerpc/platforms/cell/beat_iommu.c
@@ -57,7 +57,7 @@ static unsigned long celleb_dma_direct_offset;
 static void __init celleb_init_direct_mapping(void)
 {
u64 lpar_addr, io_addr;
-   u64 io_space_id, ioid, dma_base, dma_size, io_page_size;
+   u64 io_space_id=0, ioid=0, dma_base=0, dma_size=0, io_page_size=0;
 
if (!find_dma_window(io_space_id, ioid, dma_base, dma_size,
 io_page_size)) {
diff --git a/arch/powerpc/platforms/iseries/pci.c 
b/arch/powerpc/platforms/iseries/pci.c
index 02a634f..05f047d 100644
--- a/arch/powerpc/platforms/iseries/pci.c
+++ b/arch/powerpc/platforms/iseries/pci.c
@@ -616,8 +616,8 @@ static inline struct device_node *xlate_iomm_address(
  */
 static u8 iseries_readb(const volatile void __iomem *addr)
 {
-   u64 bar_offset;
-   u64 dsa;
+   u64 uninitialized_var(bar_offset);
+   u64 uninitialized_var(dsa);
int retry = 0;
struct HvCallPci_LoadReturn ret;
struct device_node *dn =
@@ -634,8 +634,8 @@ static u8 iseries_readb(const volatile void __iomem *addr)
 
 static u16 iseries_readw_be(const volatile void __iomem *addr)
 {
-   u64 bar_offset;
-   u64 dsa;
+   u64 uninitialized_var(bar_offset);
+   u64 

Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-07 Thread Michael Ellerman
On Wed, 2009-04-08 at 14:36 +1000, Tony Breeds wrote:
 This patch silences all the warnings generated in arch/powerpc for
 allmodconfig build.
 
 It does:
  * Where appropriate use the uninitialized_var() macro to help GCC
understand we know what's going on.
  * Explicitly casts PHYSICAL_START in one printk()
  * Initialise a few variables, as it's neater than using uninitialized_var()
 
 Signed-off-by: Tony Breeds t...@bakeyournoodle.com
 ---
 Only compile tested.
 
  arch/powerpc/kernel/cacheinfo.c   |4 ++--
  arch/powerpc/kernel/pci_dn.c  |2 +-
  arch/powerpc/kernel/setup_64.c|4 ++--
  arch/powerpc/platforms/cell/axon_msi.c|2 +-
  arch/powerpc/platforms/cell/beat_iommu.c  |2 +-
  arch/powerpc/platforms/iseries/pci.c  |   24 
  arch/powerpc/platforms/powermac/low_i2c.c |5 ++---
  arch/powerpc/platforms/pseries/msi.c  |2 +-
  8 files changed, 22 insertions(+), 23 deletions(-)
 
 diff --git a/arch/powerpc/kernel/cacheinfo.c b/arch/powerpc/kernel/cacheinfo.c
 index bb37b1d..fd6aef9 100644
 --- a/arch/powerpc/kernel/cacheinfo.c
 +++ b/arch/powerpc/kernel/cacheinfo.c
 @@ -510,7 +510,7 @@ static struct cache *index_kobj_to_cache(struct kobject 
 *k)
  
  static ssize_t size_show(struct kobject *k, struct kobj_attribute *attr, 
 char *buf)
  {
 - unsigned int size_kb;
 + unsigned int uninitialized_var(size_kb);
   struct cache *cache;
  
   cache = index_kobj_to_cache(k);
 @@ -559,7 +559,7 @@ static struct kobj_attribute cache_nr_sets_attr =
  
  static ssize_t associativity_show(struct kobject *k, struct kobj_attribute 
 *attr, char *buf)
  {
 - unsigned int associativity;
 + unsigned int uninitialized_var(associativity);
   struct cache *cache;

The getter routines in here could really multiplex their return values
with a negative error code, which I generally prefer, but this works I
guess.


 diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
 index 1c67de5..b9d66ed 100644
 --- a/arch/powerpc/kernel/pci_dn.c
 +++ b/arch/powerpc/kernel/pci_dn.c
 @@ -83,7 +83,7 @@ void *traverse_pci_devices(struct device_node *start, 
 traverse_func pre,
   void *data)
  {
   struct device_node *dn, *nextdn;
 - void *ret;
 + void *uninitialized_var(ret);

The code causing this one is a little ugly anyway, I think we should
change it to be:

-   if (pre  ((ret = pre(dn, data)) != NULL))
-   return ret;
+   if (pre) {
+   ret = pre(dn, data);
+   if (ret != NULL)
+   return ret;
+   }


 diff --git a/arch/powerpc/platforms/cell/axon_msi.c 
 b/arch/powerpc/platforms/cell/axon_msi.c
 index 0ce45c2..dae4c7c 100644
 --- a/arch/powerpc/platforms/cell/axon_msi.c
 +++ b/arch/powerpc/platforms/cell/axon_msi.c
 @@ -151,7 +151,7 @@ static struct axon_msic *find_msi_translator(struct 
 pci_dev *dev)
  {
   struct irq_host *irq_host;
   struct device_node *dn, *tmp;
 - const phandle *ph;
 + const phandle *uninitialized_var(ph);
   struct axon_msic *msic = NULL;

Freakin gcc. This is just:

if (!dn)
return;

for (; dn; ..)
ph = ..

if (!ph)

And it can't work out that it's never used uninitialised?

 diff --git a/arch/powerpc/platforms/cell/beat_iommu.c 
 b/arch/powerpc/platforms/cell/beat_iommu.c
 index 93b0efd..8230cd8 100644
 --- a/arch/powerpc/platforms/cell/beat_iommu.c
 +++ b/arch/powerpc/platforms/cell/beat_iommu.c
 @@ -57,7 +57,7 @@ static unsigned long celleb_dma_direct_offset;
  static void __init celleb_init_direct_mapping(void)
  {
   u64 lpar_addr, io_addr;
 - u64 io_space_id, ioid, dma_base, dma_size, io_page_size;
 + u64 io_space_id=0, ioid=0, dma_base=0, dma_size=0, io_page_size=0;

Do you need a new spacebar? :D

 diff --git a/arch/powerpc/platforms/iseries/pci.c 
 b/arch/powerpc/platforms/iseries/pci.c
 index 02a634f..05f047d 100644
 --- a/arch/powerpc/platforms/iseries/pci.c
 +++ b/arch/powerpc/platforms/iseries/pci.c
 @@ -616,8 +616,8 @@ static inline struct device_node *xlate_iomm_address(
   */
  static u8 iseries_readb(const volatile void __iomem *addr)
  {
 - u64 bar_offset;
 - u64 dsa;
 + u64 uninitialized_var(bar_offset);
 + u64 uninitialized_var(dsa);
   int retry = 0;
   struct HvCallPci_LoadReturn ret;
   struct device_node *dn =
 @@ -634,8 +634,8 @@ static u8 iseries_readb(const volatile void __iomem *addr)
  
  static u16 iseries_readw_be(const volatile void __iomem *addr)
  {
 - u64 bar_offset;
 - u64 dsa;
 + u64 uninitialized_var(bar_offset);
 + u64 uninitialized_var(dsa);
   int retry = 0;
   struct HvCallPci_LoadReturn ret;
   struct device_node *dn =
 @@ -653,8 +653,8 @@ static u16 iseries_readw_be(const volatile void __iomem 
 *addr)
  
  static u32 iseries_readl_be(const volatile void __iomem *addr)
  {
 -   

Re: [PATCH] Quieten arch/powerpc in a allmodconfig build.

2009-04-07 Thread Tony Breeds
On Wed, Apr 08, 2009 at 03:08:55PM +1000, Michael Ellerman wrote:

 The getter routines in here could really multiplex their return values
 with a negative error code, which I generally prefer, but this works I
 guess.

I was hoping someone would notice and suggest it.  tag you're it!

 Do you need a new spacebar? :D

nowhydoyouask?
 
 This one is a bug surely?

Hmm actually I think you're right.  I dont want to push my luck with the gcc
hackers though

 Gah, gcc sucks. It should just not warn in these cases where it doesn't
 know wth is going on.

I don't think you'll get any arguments.  it only there was a -Wnowarnunused!

Yours Tony
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev