Patchwork Quieten arch/powerpc in a allmodconfig build.

login
register
mail settings
Submitter Tony Breeds
Date April 8, 2009, 4:36 a.m.
Message ID <2c4bcf8d1d7083ff53ce5b556765e96676a007fb.1239165378.git.tony@bakeyournoodle.com>
Download mbox | patch
Permalink /patch/25705/
State Changes Requested, archived
Headers show

Comments

Tony Breeds - April 8, 2009, 4:36 a.m.
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 <tony@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(-)
Michael Ellerman - April 8, 2009, 5:08 a.m.
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 <tony@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)
>  {
> -	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 =
> @@ -676,8 +676,8 @@ static u32 iseries_readl_be(const volatile void __iomem *addr)
>   */
>  static void iseries_writeb(u8 data, volatile void __iomem *addr)
>  {
> -	u64 bar_offset;
> -	u64 dsa;
> +	u64 uninitialized_var(bar_offset);
> +	u64 uninitialized_var(dsa);
>  	int retry = 0;
>  	u64 rc;
>  	struct device_node *dn =
> @@ -692,8 +692,8 @@ static void iseries_writeb(u8 data, volatile void __iomem *addr)
>  
>  static void iseries_writew_be(u16 data, volatile void __iomem *addr)
>  {
> -	u64 bar_offset;
> -	u64 dsa;
> +	u64 uninitialized_var(bar_offset);
> +	u64 uninitialized_var(dsa);
>  	int retry = 0;
>  	u64 rc;
>  	struct device_node *dn =
> @@ -708,8 +708,8 @@ static void iseries_writew_be(u16 data, volatile void __iomem *addr)
>  
>  static void iseries_writel_be(u32 data, volatile void __iomem *addr)
>  {
> -	u64 bar_offset;
> -	u64 dsa;
> +	u64 uninitialized_var(bar_offset);
> +	u64 uninitialized_var(dsa);
>  	int retry = 0;
>  	u64 rc;
>  	struct device_node *dn =
> diff --git a/arch/powerpc/platforms/powermac/low_i2c.c b/arch/powerpc/platforms/powermac/low_i2c.c
> index 21226b7..5989427 100644
> --- a/arch/powerpc/platforms/powermac/low_i2c.c
> +++ b/arch/powerpc/platforms/powermac/low_i2c.c
> @@ -1090,7 +1090,7 @@ EXPORT_SYMBOL_GPL(pmac_low_i2c_unlock);
>  
>  int pmac_i2c_open(struct pmac_i2c_bus *bus, int polled)
>  {
> -	int rc;
> +	int rc = 0;
>  
>  	mutex_lock(&bus->mutex);
>  	bus->polled = polled || pmac_i2c_force_poll;
> @@ -1099,9 +1099,8 @@ int pmac_i2c_open(struct pmac_i2c_bus *bus, int polled)
>  	if (bus->open && (rc = bus->open(bus)) != 0) {
>  		bus->opened = 0;
>  		mutex_unlock(&bus->mutex);
> -		return rc;
>  	}
> -	return 0;
> +	return rc;

This one is a bug surely?

1092 {
1093         int rc;
1094        
1095         mutex_lock(&bus->mutex);
1096         bus->polled = polled || pmac_i2c_force_poll;
1097         bus->opened = 1;
1098         bus->mode = pmac_i2c_mode_std;
1099         if (bus->open && (rc = bus->open(bus)) != 0) {
1100                 bus->opened = 0;
1101                 mutex_unlock(&bus->mutex);
1102                 return rc;
1103         }
1104         return 0;
1105 }


I can't reproduce it though with a simple test case :?

> diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
> index bf2e1ac..d92f593 100644
> --- a/arch/powerpc/platforms/pseries/msi.c
> +++ b/arch/powerpc/platforms/pseries/msi.c
> @@ -282,7 +282,7 @@ static int msi_quota_for_device(struct pci_dev *dev, int request)
>  {
>  	struct device_node *pe_dn;
>  	struct msi_counts counts;
> -	int total;
> +	int uninitialized_var(total);
>  
>  	pr_debug("rtas_msi: calc quota for %s, request %d\n", pci_name(dev),
>  		  request);

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

cheers
Tony Breeds - April 8, 2009, 5:51 a.m.
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
Tony Breeds - April 8, 2009, 6:13 a.m.
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
Michael Ellerman - April 8, 2009, 6:23 a.m.
On Wed, 2009-04-08 at 16:13 +1000, Tony Breeds wrote:
> 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).

CONFIG_MAKE_MY_KERNEL_SUCK I think it is.

In that case I don't blame gcc.

If people want to use that option I think they can just deal with the
warnings, we can make -Werror depend on !CONFIG_PROFILE_ALL_BRANCHES.

cheers
Tony Breeds - April 9, 2009, 10:45 p.m.
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
Segher Boessenkool - April 9, 2009, 10:46 p.m.
>> 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
Stephen Rothwell - April 9, 2009, 11:11 p.m.
On Fri, 10 Apr 2009 08:45:53 +1000 Tony Breeds <tony@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 :-))
Segher Boessenkool - April 9, 2009, 11:18 p.m.
>> -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
Segher Boessenkool - April 9, 2009, 11:23 p.m.
> 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
Scott Wood - April 10, 2009, 6:03 p.m.
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
Andreas Schwab - April 10, 2009, 6:35 p.m.
Scott Wood <scottwood@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.
Scott Wood - April 10, 2009, 6:43 p.m.
Andreas Schwab wrote:
> Scott Wood <scottwood@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
Segher Boessenkool - April 10, 2009, 8:28 p.m.
>>> 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
Segher Boessenkool - April 10, 2009, 8:45 p.m.
> 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
Scott Wood - April 10, 2009, 9:51 p.m.
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

Patch

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 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)
 {
-	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 =
@@ -676,8 +676,8 @@  static u32 iseries_readl_be(const volatile void __iomem *addr)
  */
 static void iseries_writeb(u8 data, volatile void __iomem *addr)
 {
-	u64 bar_offset;
-	u64 dsa;
+	u64 uninitialized_var(bar_offset);
+	u64 uninitialized_var(dsa);
 	int retry = 0;
 	u64 rc;
 	struct device_node *dn =
@@ -692,8 +692,8 @@  static void iseries_writeb(u8 data, volatile void __iomem *addr)
 
 static void iseries_writew_be(u16 data, volatile void __iomem *addr)
 {
-	u64 bar_offset;
-	u64 dsa;
+	u64 uninitialized_var(bar_offset);
+	u64 uninitialized_var(dsa);
 	int retry = 0;
 	u64 rc;
 	struct device_node *dn =
@@ -708,8 +708,8 @@  static void iseries_writew_be(u16 data, volatile void __iomem *addr)
 
 static void iseries_writel_be(u32 data, volatile void __iomem *addr)
 {
-	u64 bar_offset;
-	u64 dsa;
+	u64 uninitialized_var(bar_offset);
+	u64 uninitialized_var(dsa);
 	int retry = 0;
 	u64 rc;
 	struct device_node *dn =
diff --git a/arch/powerpc/platforms/powermac/low_i2c.c b/arch/powerpc/platforms/powermac/low_i2c.c
index 21226b7..5989427 100644
--- a/arch/powerpc/platforms/powermac/low_i2c.c
+++ b/arch/powerpc/platforms/powermac/low_i2c.c
@@ -1090,7 +1090,7 @@  EXPORT_SYMBOL_GPL(pmac_low_i2c_unlock);
 
 int pmac_i2c_open(struct pmac_i2c_bus *bus, int polled)
 {
-	int rc;
+	int rc = 0;
 
 	mutex_lock(&bus->mutex);
 	bus->polled = polled || pmac_i2c_force_poll;
@@ -1099,9 +1099,8 @@  int pmac_i2c_open(struct pmac_i2c_bus *bus, int polled)
 	if (bus->open && (rc = bus->open(bus)) != 0) {
 		bus->opened = 0;
 		mutex_unlock(&bus->mutex);
-		return rc;
 	}
-	return 0;
+	return rc;
 }
 EXPORT_SYMBOL_GPL(pmac_i2c_open);
 
diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
index bf2e1ac..d92f593 100644
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -282,7 +282,7 @@  static int msi_quota_for_device(struct pci_dev *dev, int request)
 {
 	struct device_node *pe_dn;
 	struct msi_counts counts;
-	int total;
+	int uninitialized_var(total);
 
 	pr_debug("rtas_msi: calc quota for %s, request %d\n", pci_name(dev),
 		  request);