linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dsa: b53: avoid 'maybe-uninitialized' warning
@ 2016-06-27  9:19 Arnd Bergmann
  2016-06-29  9:07 ` David Miller
  0 siblings, 1 reply; 2+ messages in thread
From: Arnd Bergmann @ 2016-06-27  9:19 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: Arnd Bergmann, David S. Miller, netdev, linux-kernel

In some configurations, gcc produces a warning for correct code
in this driver:

drivers/net/dsa/b53/b53_mmap.c: In function 'b53_mmap_read64':
drivers/net/dsa/b53/b53_mmap.c:107:10: error: 'hi' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  *val = ((u64)hi << 32) | lo;
          ^~~~~~~
drivers/net/dsa/b53/b53_mmap.c: In function 'b53_mmap_read48':
drivers/net/dsa/b53/b53_mmap.c:91:11: error: 'hi' may be used uninitialized in this function [-Werror=maybe-uninitialized]
   *val = ((u64)hi << 32) | lo;
           ^~~~~~~
drivers/net/dsa/b53/b53_mmap.c:83:11: error: 'hi' may be used uninitialized in this function [-Werror=maybe-uninitialized]
   *val = ((u64)hi << 16) | lo;

I have seen the warning before and at the time thought I had fixed
it with 55e7f6abe131 ("dsa: b53: fix big-endian register access"),
however it now came back in a different randconfig build that happens
to have different inlining decisions in the compiler.

The mistake that gcc makes here is that it thinks the second call to
readl() might fail because the address 'reg + 4' is not a multiple
of four despite having knowing that 'reg' itself is a multiple of four.

By open-coding the two reads without the redundant alignment check,
we can avoid the warning and produce slightly better object code, but
get slightly longer source code instead.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/dsa/b53/b53_mmap.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_mmap.c b/drivers/net/dsa/b53/b53_mmap.c
index b70daf9174d4..21f1068b0804 100644
--- a/drivers/net/dsa/b53/b53_mmap.c
+++ b/drivers/net/dsa/b53/b53_mmap.c
@@ -70,6 +70,8 @@ static int b53_mmap_read32(struct b53_device *dev, u8 page, u8 reg, u32 *val)
 
 static int b53_mmap_read48(struct b53_device *dev, u8 page, u8 reg, u64 *val)
 {
+	u8 __iomem *regs = dev->priv;
+
 	if (WARN_ON(reg % 2))
 		return -EINVAL;
 
@@ -77,16 +79,26 @@ static int b53_mmap_read48(struct b53_device *dev, u8 page, u8 reg, u64 *val)
 		u16 lo;
 		u32 hi;
 
-		b53_mmap_read16(dev, page, reg, &lo);
-		b53_mmap_read32(dev, page, reg + 2, &hi);
+		if (dev->pdata && dev->pdata->big_endian) {
+			lo = ioread16be(regs + (page << 8) + reg);
+			hi = ioread32be(regs + (page << 8) + reg + 2);
+		} else {
+			lo = readw(regs + (page << 8) + reg);
+			hi = readl(regs + (page << 8) + reg + 2);
+		}
 
 		*val = ((u64)hi << 16) | lo;
 	} else {
 		u32 lo;
 		u16 hi;
 
-		b53_mmap_read32(dev, page, reg, &lo);
-		b53_mmap_read16(dev, page, reg + 4, &hi);
+		if (dev->pdata && dev->pdata->big_endian) {
+			lo = ioread32be(regs + (page << 8) + reg);
+			hi = ioread16be(regs + (page << 8) + reg + 4);
+		} else {
+			lo = readl(regs + (page << 8) + reg);
+			hi = readw(regs + (page << 8) + reg + 4);
+		}
 
 		*val = ((u64)hi << 32) | lo;
 	}
@@ -96,13 +108,19 @@ static int b53_mmap_read48(struct b53_device *dev, u8 page, u8 reg, u64 *val)
 
 static int b53_mmap_read64(struct b53_device *dev, u8 page, u8 reg, u64 *val)
 {
+	u8 __iomem *regs = dev->priv;
 	u32 hi, lo;
 
 	if (WARN_ON(reg % 4))
 		return -EINVAL;
 
-	b53_mmap_read32(dev, page, reg, &lo);
-	b53_mmap_read32(dev, page, reg + 4, &hi);
+	if (dev->pdata && dev->pdata->big_endian) {
+		lo = ioread32be(regs + (page << 8) + reg);
+		hi = ioread32be(regs + (page << 8) + reg + 4);
+	} else {
+		lo = readl(regs + (page << 8) + reg);
+		hi = readl(regs + (page << 8) + reg + 4);
+	}
 
 	*val = ((u64)hi << 32) | lo;
 
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] dsa: b53: avoid 'maybe-uninitialized' warning
  2016-06-27  9:19 [PATCH] dsa: b53: avoid 'maybe-uninitialized' warning Arnd Bergmann
@ 2016-06-29  9:07 ` David Miller
  0 siblings, 0 replies; 2+ messages in thread
From: David Miller @ 2016-06-29  9:07 UTC (permalink / raw)
  To: arnd; +Cc: f.fainelli, netdev, linux-kernel

From: Arnd Bergmann <arnd@arndb.de>
Date: Mon, 27 Jun 2016 11:19:13 +0200

> In some configurations, gcc produces a warning for correct code
> in this driver:
 ...
> I have seen the warning before and at the time thought I had fixed
> it with 55e7f6abe131 ("dsa: b53: fix big-endian register access"),
> however it now came back in a different randconfig build that happens
> to have different inlining decisions in the compiler.
> 
> The mistake that gcc makes here is that it thinks the second call to
> readl() might fail because the address 'reg + 4' is not a multiple
> of four despite having knowing that 'reg' itself is a multiple of four.
> 
> By open-coding the two reads without the redundant alignment check,
> we can avoid the warning and produce slightly better object code, but
> get slightly longer source code instead.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied, thanks Arnd.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2016-06-29  9:08 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-27  9:19 [PATCH] dsa: b53: avoid 'maybe-uninitialized' warning Arnd Bergmann
2016-06-29  9:07 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).