linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: mdio-octeon: Fix pointer/integer casts
@ 2019-11-11  0:42 Olof Johansson
  2019-11-11  2:32 ` Andrew Lunn
  2019-11-12  5:46 ` David Miller
  0 siblings, 2 replies; 6+ messages in thread
From: Olof Johansson @ 2019-11-11  0:42 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Heiner Kallweit
  Cc: netdev, linux-kernel, David S . Miller, Olof Johansson

Fixes a bunch of these warnings on arm allmodconfig:

In file included from /build/drivers/net/phy/mdio-cavium.c:11:
/build/drivers/net/phy/mdio-cavium.c: In function 'cavium_mdiobus_set_mode':
/build/drivers/net/phy/mdio-cavium.h:114:37: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
  114 | #define oct_mdio_readq(addr)  readq((void *)addr)
      |                                     ^
/build/drivers/net/phy/mdio-cavium.c:21:16: note: in expansion of macro 'oct_mdio_readq'
   21 |  smi_clk.u64 = oct_mdio_readq(p->register_base + SMI_CLK);
      |                ^~~~~~~~~~~~~~

Fixes: 171a9bae68c7 ("staging/octeon: Allow test build on !MIPS")
Signed-off-by: Olof Johansson <olof@lixom.net>
---
 drivers/net/phy/mdio-cavium.h  | 14 +++++++-------
 drivers/net/phy/mdio-octeon.c  |  5 ++---
 drivers/net/phy/mdio-thunder.c |  2 +-
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/net/phy/mdio-cavium.h b/drivers/net/phy/mdio-cavium.h
index b7f89ad27465f..1cf81f0bc585f 100644
--- a/drivers/net/phy/mdio-cavium.h
+++ b/drivers/net/phy/mdio-cavium.h
@@ -90,7 +90,7 @@ union cvmx_smix_wr_dat {
 
 struct cavium_mdiobus {
 	struct mii_bus *mii_bus;
-	u64 register_base;
+	void __iomem *register_base;
 	enum cavium_mdiobus_mode mode;
 };
 
@@ -98,20 +98,20 @@ struct cavium_mdiobus {
 
 #include <asm/octeon/octeon.h>
 
-static inline void oct_mdio_writeq(u64 val, u64 addr)
+static inline void oct_mdio_writeq(u64 val, void __iomem *addr)
 {
-	cvmx_write_csr(addr, val);
+	cvmx_write_csr((u64)addr, val);
 }
 
-static inline u64 oct_mdio_readq(u64 addr)
+static inline u64 oct_mdio_readq(void __iomem *addr)
 {
-	return cvmx_read_csr(addr);
+	return cvmx_read_csr((u64)addr);
 }
 #else
 #include <linux/io-64-nonatomic-lo-hi.h>
 
-#define oct_mdio_writeq(val, addr)	writeq(val, (void *)addr)
-#define oct_mdio_readq(addr)		readq((void *)addr)
+#define oct_mdio_writeq(val, addr)	writeq(val, addr)
+#define oct_mdio_readq(addr)		readq(addr)
 #endif
 
 int cavium_mdiobus_read(struct mii_bus *bus, int phy_id, int regnum);
diff --git a/drivers/net/phy/mdio-octeon.c b/drivers/net/phy/mdio-octeon.c
index 8327382aa5689..c58ab8acd485a 100644
--- a/drivers/net/phy/mdio-octeon.c
+++ b/drivers/net/phy/mdio-octeon.c
@@ -44,8 +44,7 @@ static int octeon_mdiobus_probe(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
-	bus->register_base =
-		(u64)devm_ioremap(&pdev->dev, mdio_phys, regsize);
+	bus->register_base = devm_ioremap(&pdev->dev, mdio_phys, regsize);
 	if (!bus->register_base) {
 		dev_err(&pdev->dev, "dev_ioremap failed\n");
 		return -ENOMEM;
@@ -56,7 +55,7 @@ static int octeon_mdiobus_probe(struct platform_device *pdev)
 	oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN);
 
 	bus->mii_bus->name = KBUILD_MODNAME;
-	snprintf(bus->mii_bus->id, MII_BUS_ID_SIZE, "%llx", bus->register_base);
+	snprintf(bus->mii_bus->id, MII_BUS_ID_SIZE, "%p", bus->register_base);
 	bus->mii_bus->parent = &pdev->dev;
 
 	bus->mii_bus->read = cavium_mdiobus_read;
diff --git a/drivers/net/phy/mdio-thunder.c b/drivers/net/phy/mdio-thunder.c
index b6128ae7f14f3..280cf84d4116e 100644
--- a/drivers/net/phy/mdio-thunder.c
+++ b/drivers/net/phy/mdio-thunder.c
@@ -84,7 +84,7 @@ static int thunder_mdiobus_pci_probe(struct pci_dev *pdev,
 		nexus->buses[i] = bus;
 		i++;
 
-		bus->register_base = (u64)nexus->bar0 +
+		bus->register_base = nexus->bar0 +
 			r.start - pci_resource_start(pdev, 0);
 
 		smi_en.u64 = 0;
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] net: mdio-octeon: Fix pointer/integer casts
  2019-11-11  0:42 [PATCH] net: mdio-octeon: Fix pointer/integer casts Olof Johansson
@ 2019-11-11  2:32 ` Andrew Lunn
  2019-11-11  2:36   ` Olof Johansson
  2019-11-12  5:46 ` David Miller
  1 sibling, 1 reply; 6+ messages in thread
From: Andrew Lunn @ 2019-11-11  2:32 UTC (permalink / raw)
  To: Olof Johansson
  Cc: Florian Fainelli, Heiner Kallweit, netdev, linux-kernel,
	David S . Miller

On Sun, Nov 10, 2019 at 04:42:11PM -0800, Olof Johansson wrote:
> Fixes a bunch of these warnings on arm allmodconfig:
> 
> In file included from /build/drivers/net/phy/mdio-cavium.c:11:
> /build/drivers/net/phy/mdio-cavium.c: In function 'cavium_mdiobus_set_mode':
> /build/drivers/net/phy/mdio-cavium.h:114:37: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
>   114 | #define oct_mdio_readq(addr)  readq((void *)addr)
>       |                                     ^
> /build/drivers/net/phy/mdio-cavium.c:21:16: note: in expansion of macro 'oct_mdio_readq'
>    21 |  smi_clk.u64 = oct_mdio_readq(p->register_base + SMI_CLK);
>       |                ^~~~~~~~~~~~~~
> 
> Fixes: 171a9bae68c7 ("staging/octeon: Allow test build on !MIPS")
> Signed-off-by: Olof Johansson <olof@lixom.net>
> ---
>  drivers/net/phy/mdio-cavium.h  | 14 +++++++-------
>  drivers/net/phy/mdio-octeon.c  |  5 ++---
>  drivers/net/phy/mdio-thunder.c |  2 +-
>  3 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/phy/mdio-cavium.h b/drivers/net/phy/mdio-cavium.h
> index b7f89ad27465f..1cf81f0bc585f 100644
> --- a/drivers/net/phy/mdio-cavium.h
> +++ b/drivers/net/phy/mdio-cavium.h
> @@ -90,7 +90,7 @@ union cvmx_smix_wr_dat {
>  
>  struct cavium_mdiobus {
>  	struct mii_bus *mii_bus;
> -	u64 register_base;
> +	void __iomem *register_base;
>  	enum cavium_mdiobus_mode mode;
>  };
>  
> @@ -98,20 +98,20 @@ struct cavium_mdiobus {
>  
>  #include <asm/octeon/octeon.h>
>  
> -static inline void oct_mdio_writeq(u64 val, u64 addr)
> +static inline void oct_mdio_writeq(u64 val, void __iomem *addr)
>  {
> -	cvmx_write_csr(addr, val);
> +	cvmx_write_csr((u64)addr, val);
>  }

Hi Olof

Humm. The warning goes away, but is it really any better?

Did you try also changing the stub function in
drivers/staging/octeon/octeon-stubs.h so it takes void __iomem?  Or
did that cause a lot more warnings from other places?

Thanks
     Andrew

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] net: mdio-octeon: Fix pointer/integer casts
  2019-11-11  2:32 ` Andrew Lunn
@ 2019-11-11  2:36   ` Olof Johansson
  0 siblings, 0 replies; 6+ messages in thread
From: Olof Johansson @ 2019-11-11  2:36 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Florian Fainelli, Heiner Kallweit, Network Development,
	Linux Kernel Mailing List, David S . Miller

On Sun, Nov 10, 2019 at 6:32 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Sun, Nov 10, 2019 at 04:42:11PM -0800, Olof Johansson wrote:
> > Fixes a bunch of these warnings on arm allmodconfig:
> >
> > In file included from /build/drivers/net/phy/mdio-cavium.c:11:
> > /build/drivers/net/phy/mdio-cavium.c: In function 'cavium_mdiobus_set_mode':
> > /build/drivers/net/phy/mdio-cavium.h:114:37: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
> >   114 | #define oct_mdio_readq(addr)  readq((void *)addr)
> >       |                                     ^
> > /build/drivers/net/phy/mdio-cavium.c:21:16: note: in expansion of macro 'oct_mdio_readq'
> >    21 |  smi_clk.u64 = oct_mdio_readq(p->register_base + SMI_CLK);
> >       |                ^~~~~~~~~~~~~~
> >
> > Fixes: 171a9bae68c7 ("staging/octeon: Allow test build on !MIPS")
> > Signed-off-by: Olof Johansson <olof@lixom.net>
> > ---
> >  drivers/net/phy/mdio-cavium.h  | 14 +++++++-------
> >  drivers/net/phy/mdio-octeon.c  |  5 ++---
> >  drivers/net/phy/mdio-thunder.c |  2 +-
> >  3 files changed, 10 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/phy/mdio-cavium.h b/drivers/net/phy/mdio-cavium.h
> > index b7f89ad27465f..1cf81f0bc585f 100644
> > --- a/drivers/net/phy/mdio-cavium.h
> > +++ b/drivers/net/phy/mdio-cavium.h
> > @@ -90,7 +90,7 @@ union cvmx_smix_wr_dat {
> >
> >  struct cavium_mdiobus {
> >       struct mii_bus *mii_bus;
> > -     u64 register_base;
> > +     void __iomem *register_base;
> >       enum cavium_mdiobus_mode mode;
> >  };
> >
> > @@ -98,20 +98,20 @@ struct cavium_mdiobus {
> >
> >  #include <asm/octeon/octeon.h>
> >
> > -static inline void oct_mdio_writeq(u64 val, u64 addr)
> > +static inline void oct_mdio_writeq(u64 val, void __iomem *addr)
> >  {
> > -     cvmx_write_csr(addr, val);
> > +     cvmx_write_csr((u64)addr, val);
> >  }
>
> Hi Olof
>
> Humm. The warning goes away, but is it really any better?
>
> Did you try also changing the stub function in
> drivers/staging/octeon/octeon-stubs.h so it takes void __iomem?  Or
> did that cause a lot more warnings from other places?

That percolates through a bunch of MIPS code that I didn't feel like
getting into. So indeed, I stopped at that point.


-Olof

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] net: mdio-octeon: Fix pointer/integer casts
  2019-11-11  0:42 [PATCH] net: mdio-octeon: Fix pointer/integer casts Olof Johansson
  2019-11-11  2:32 ` Andrew Lunn
@ 2019-11-12  5:46 ` David Miller
  2019-11-12 13:23   ` Andrew Lunn
  1 sibling, 1 reply; 6+ messages in thread
From: David Miller @ 2019-11-12  5:46 UTC (permalink / raw)
  To: olof; +Cc: andrew, f.fainelli, hkallweit1, netdev, linux-kernel

From: Olof Johansson <olof@lixom.net>
Date: Sun, 10 Nov 2019 16:42:11 -0800

> -static inline void oct_mdio_writeq(u64 val, u64 addr)
> +static inline void oct_mdio_writeq(u64 val, void __iomem *addr)
>  {
> -	cvmx_write_csr(addr, val);
> +	cvmx_write_csr((u64)addr, val);
>  }

I hate stuff like this, I think you really need to fix this from the bottom
up or similar.  MMIO and such addresses are __iomem pointers, period.


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] net: mdio-octeon: Fix pointer/integer casts
  2019-11-12  5:46 ` David Miller
@ 2019-11-12 13:23   ` Andrew Lunn
  2019-11-12 19:51     ` Olof Johansson
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Lunn @ 2019-11-12 13:23 UTC (permalink / raw)
  To: David Miller; +Cc: olof, f.fainelli, hkallweit1, netdev, linux-kernel

On Mon, Nov 11, 2019 at 09:46:58PM -0800, David Miller wrote:
> From: Olof Johansson <olof@lixom.net>
> Date: Sun, 10 Nov 2019 16:42:11 -0800
> 
> > -static inline void oct_mdio_writeq(u64 val, u64 addr)
> > +static inline void oct_mdio_writeq(u64 val, void __iomem *addr)
> >  {
> > -	cvmx_write_csr(addr, val);
> > +	cvmx_write_csr((u64)addr, val);
> >  }
> 
> I hate stuff like this, I think you really need to fix this from the bottom
> up or similar.  MMIO and such addresses are __iomem pointers, period.

Yes, i agree, but did not want to push the work to Olof. The point of
COMPILE_TEST is to find issues like this, code which should be
architecture independent, but is not. The cast just papers over the
cracks.

At a minimum, could we fix the stub cvmx_write_csr() used for
everything !MIPS. That should hopefully fix everything !MIPS, but
cause MIPS to start issuing warning. The MIPS folks can then cleanup
their code, which is really what is broken here.

      Andrew

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] net: mdio-octeon: Fix pointer/integer casts
  2019-11-12 13:23   ` Andrew Lunn
@ 2019-11-12 19:51     ` Olof Johansson
  0 siblings, 0 replies; 6+ messages in thread
From: Olof Johansson @ 2019-11-12 19:51 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David Miller, Florian Fainelli, Heiner Kallweit,
	Network Development, Linux Kernel Mailing List

On Tue, Nov 12, 2019 at 5:23 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Mon, Nov 11, 2019 at 09:46:58PM -0800, David Miller wrote:
> > From: Olof Johansson <olof@lixom.net>
> > Date: Sun, 10 Nov 2019 16:42:11 -0800
> >
> > > -static inline void oct_mdio_writeq(u64 val, u64 addr)
> > > +static inline void oct_mdio_writeq(u64 val, void __iomem *addr)
> > >  {
> > > -   cvmx_write_csr(addr, val);
> > > +   cvmx_write_csr((u64)addr, val);
> > >  }
> >
> > I hate stuff like this, I think you really need to fix this from the bottom
> > up or similar.  MMIO and such addresses are __iomem pointers, period.
>
> Yes, i agree, but did not want to push the work to Olof. The point of
> COMPILE_TEST is to find issues like this, code which should be
> architecture independent, but is not. The cast just papers over the
> cracks.
>
> At a minimum, could we fix the stub cvmx_write_csr() used for
> everything !MIPS. That should hopefully fix everything !MIPS, but
> cause MIPS to start issuing warning. The MIPS folks can then cleanup
> their code, which is really what is broken here.

I'm not disagreeing with Dave, going all the way down the rabbit hole
is preferred. In this case I mostly pushed the lack of __iomem usage
down one layer but not the whole way.

I'm unlikely to find time to do it in the near future myself (this was
a bit of a weekend drive-by from my side), but I don't mind doing it.
If someone else beats me to it, feel free to take it over.


-Olof

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-12 19:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-11  0:42 [PATCH] net: mdio-octeon: Fix pointer/integer casts Olof Johansson
2019-11-11  2:32 ` Andrew Lunn
2019-11-11  2:36   ` Olof Johansson
2019-11-12  5:46 ` David Miller
2019-11-12 13:23   ` Andrew Lunn
2019-11-12 19:51     ` Olof Johansson

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).