| Message ID | 20190731185023.20954-1-natechancellor@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | net: mdio-octeon: Fix build error and Kconfig warning | expand |
On 7/31/19 2:55 PM, Randy Dunlap wrote: > On 7/31/19 11:50 AM, Nathan Chancellor wrote: >> arm allyesconfig warns: >> >> WARNING: unmet direct dependencies detected for MDIO_OCTEON >> Depends on [n]: NETDEVICES [=y] && MDIO_DEVICE [=y] && MDIO_BUS [=y] >> && 64BIT && HAS_IOMEM [=y] && OF_MDIO [=y] >> Selected by [y]: >> - OCTEON_ETHERNET [=y] && STAGING [=y] && (CAVIUM_OCTEON_SOC && >> NETDEVICES [=y] || COMPILE_TEST [=y]) >> >> and errors: >> >> In file included from ../drivers/net/phy/mdio-octeon.c:14: >> ../drivers/net/phy/mdio-octeon.c: In function 'octeon_mdiobus_probe': >> ../drivers/net/phy/mdio-cavium.h:111:36: error: implicit declaration of >> function 'writeq'; did you mean 'writeb'? >> [-Werror=implicit-function-declaration] >> 111 | #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr) >> | ^~~~~~ >> ../drivers/net/phy/mdio-octeon.c:56:2: note: in expansion of macro >> 'oct_mdio_writeq' >> 56 | oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN); >> | ^~~~~~~~~~~~~~~ >> cc1: some warnings being treated as errors >> >> This allows MDIO_OCTEON to be built with COMPILE_TEST as well and >> includes the proper header for readq/writeq. This does not address >> the several -Wint-to-pointer-cast and -Wpointer-to-int-cast warnings >> that appeared as a result of commit 171a9bae68c7 ("staging/octeon: >> Allow test build on !MIPS") in these files. >> >> Fixes: 171a9bae68c7 ("staging/octeon: Allow test build on !MIPS") >> Reported-by: kbuild test robot <lkp@intel.com> >> Reported-by: Mark Brown <broonie@kernel.org> >> Reported-by: Randy Dunlap <rdunlap@infradead.org> >> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > > > With today's linux-next (20190731), I am still seeing a Kconfig warning and > build errors (building for i386): > > and applying Greg's "depends on NETDEVICES" patch and this patch: > > WARNING: unmet direct dependencies detected for MDIO_OCTEON > Depends on [n]: NETDEVICES [=y] && MDIO_DEVICE [=m] && MDIO_BUS [=m] && (64BIT [=n] || COMPILE_TEST [=y]) && HAS_IOMEM [=y] && OF_MDIO [=n] > Selected by [m]: > - OCTEON_ETHERNET [=m] && STAGING [=y] && (CAVIUM_OCTEON_SOC || COMPILE_TEST [=y]) && NETDEVICES [=y] > > ERROR: "cavium_mdiobus_write" [drivers/net/phy/mdio-octeon.ko] undefined! > ERROR: "cavium_mdiobus_read" [drivers/net/phy/mdio-octeon.ko] undefined! > > > kernel .config file is attached. > > Am I missing another patch? > > thanks. If I add this to drivers/staging/octeon/Kconfig: select MDIO_OCTEON + select MDIO_CAVIUM help then the build succeeds. This isn't being done by make *config because MDIO_OCTEON depends on OF_MDIO, which is not set in my .config file, so the "select MDIO_CAVIUM" in MDIO_OCTEON is not done. However, there are lots of type/cast warnings in both mdio-octeon and mdio-cavium: ../drivers/net/phy/mdio-octeon.c: In function ‘octeon_mdiobus_probe’: ../drivers/net/phy/mdio-octeon.c:48:3: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] (u64)devm_ioremap(&pdev->dev, mdio_phys, regsize); ^ In file included from ../drivers/net/phy/mdio-octeon.c:14:0: ../drivers/net/phy/mdio-cavium.h:113:48: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr) ^ ../drivers/net/phy/mdio-octeon.c:56:2: note: in expansion of macro ‘oct_mdio_writeq’ oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN); ^~~~~~~~~~~~~~~ ../drivers/net/phy/mdio-cavium.h:113:48: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr) ^ ../drivers/net/phy/mdio-octeon.c:77:2: note: in expansion of macro ‘oct_mdio_writeq’ oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN); ^~~~~~~~~~~~~~~ ../drivers/net/phy/mdio-octeon.c: In function ‘octeon_mdiobus_remove’: ../drivers/net/phy/mdio-cavium.h:113:48: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr) ^ ../drivers/net/phy/mdio-octeon.c:91:2: note: in expansion of macro ‘oct_mdio_writeq’ oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN); ^~~~~~~~~~~~~~~ and CC [M] drivers/net/phy/mdio-cavium.o In file included from ../drivers/net/phy/mdio-cavium.c:11:0: ../drivers/net/phy/mdio-cavium.c: In function ‘cavium_mdiobus_set_mode’: ../drivers/net/phy/mdio-cavium.h:114:37: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] #define oct_mdio_readq(addr) readq((void *)addr) ^ ../drivers/net/phy/mdio-cavium.c:21:16: note: in expansion of macro ‘oct_mdio_readq’ smi_clk.u64 = oct_mdio_readq(p->register_base + SMI_CLK); ^~~~~~~~~~~~~~ ../drivers/net/phy/mdio-cavium.h:113:48: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr) ^ ../drivers/net/phy/mdio-cavium.c:24:2: note: in expansion of macro ‘oct_mdio_writeq’ oct_mdio_writeq(smi_clk.u64, p->register_base + SMI_CLK); ^~~~~~~~~~~~~~~ ../drivers/net/phy/mdio-cavium.c: In function ‘cavium_mdiobus_c45_addr’: ../drivers/net/phy/mdio-cavium.h:113:48: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr) ^ ../drivers/net/phy/mdio-cavium.c:39:2: note: in expansion of macro ‘oct_mdio_writeq’ oct_mdio_writeq(smi_wr.u64, p->register_base + SMI_WR_DAT); ^~~~~~~~~~~~~~~ ../drivers/net/phy/mdio-cavium.h:113:48: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr) ^ ../drivers/net/phy/mdio-cavium.c:47:2: note: in expansion of macro ‘oct_mdio_writeq’ oct_mdio_writeq(smi_cmd.u64, p->register_base + SMI_CMD); ^~~~~~~~~~~~~~~ ../drivers/net/phy/mdio-cavium.h:114:37: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] #define oct_mdio_readq(addr) readq((void *)addr) ^ ../drivers/net/phy/mdio-cavium.c:54:16: note: in expansion of macro ‘oct_mdio_readq’ smi_wr.u64 = oct_mdio_readq(p->register_base + SMI_WR_DAT); ^~~~~~~~~~~~~~ ../drivers/net/phy/mdio-cavium.c: In function ‘cavium_mdiobus_read’: ../drivers/net/phy/mdio-cavium.h:113:48: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr) ^ ../drivers/net/phy/mdio-cavium.c:86:2: note: in expansion of macro ‘oct_mdio_writeq’ oct_mdio_writeq(smi_cmd.u64, p->register_base + SMI_CMD); ^~~~~~~~~~~~~~~ ../drivers/net/phy/mdio-cavium.h:114:37: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] #define oct_mdio_readq(addr) readq((void *)addr) ^ ../drivers/net/phy/mdio-cavium.c:93:16: note: in expansion of macro ‘oct_mdio_readq’ smi_rd.u64 = oct_mdio_readq(p->register_base + SMI_RD_DAT); ^~~~~~~~~~~~~~ ../drivers/net/phy/mdio-cavium.c: In function ‘cavium_mdiobus_write’: ../drivers/net/phy/mdio-cavium.h:113:48: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr) ^ ../drivers/net/phy/mdio-cavium.c:125:2: note: in expansion of macro ‘oct_mdio_writeq’ oct_mdio_writeq(smi_wr.u64, p->register_base + SMI_WR_DAT); ^~~~~~~~~~~~~~~ ../drivers/net/phy/mdio-cavium.h:113:48: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr) ^ ../drivers/net/phy/mdio-cavium.c:131:2: note: in expansion of macro ‘oct_mdio_writeq’ oct_mdio_writeq(smi_cmd.u64, p->register_base + SMI_CMD); ^~~~~~~~~~~~~~~ ../drivers/net/phy/mdio-cavium.h:114:37: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] #define oct_mdio_readq(addr) readq((void *)addr) ^ ../drivers/net/phy/mdio-cavium.c:138:16: note: in expansion of macro ‘oct_mdio_readq’ smi_wr.u64 = oct_mdio_readq(p->register_base + SMI_WR_DAT); ^~~~~~~~~~~~~~ >> --- >> drivers/net/phy/Kconfig | 2 +- >> drivers/net/phy/mdio-cavium.h | 2 ++ >> 2 files changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig >> index 20f14c5fbb7e..ed2edf4b5b0e 100644 >> --- a/drivers/net/phy/Kconfig >> +++ b/drivers/net/phy/Kconfig >> @@ -159,7 +159,7 @@ config MDIO_MSCC_MIIM >> >> config MDIO_OCTEON >> tristate "Octeon and some ThunderX SOCs MDIO buses" >> - depends on 64BIT >> + depends on 64BIT || COMPILE_TEST >> depends on HAS_IOMEM && OF_MDIO >> select MDIO_CAVIUM >> help >> diff --git a/drivers/net/phy/mdio-cavium.h b/drivers/net/phy/mdio-cavium.h >> index ed5f9bb5448d..b7f89ad27465 100644 >> --- a/drivers/net/phy/mdio-cavium.h >> +++ b/drivers/net/phy/mdio-cavium.h >> @@ -108,6 +108,8 @@ static inline u64 oct_mdio_readq(u64 addr) >> return cvmx_read_csr(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) >> #endif >> > >
From: Nathan Chancellor <natechancellor@gmail.com> Date: Wed, 31 Jul 2019 11:50:24 -0700 > arm allyesconfig warns: > > WARNING: unmet direct dependencies detected for MDIO_OCTEON > Depends on [n]: NETDEVICES [=y] && MDIO_DEVICE [=y] && MDIO_BUS [=y] > && 64BIT && HAS_IOMEM [=y] && OF_MDIO [=y] > Selected by [y]: > - OCTEON_ETHERNET [=y] && STAGING [=y] && (CAVIUM_OCTEON_SOC && > NETDEVICES [=y] || COMPILE_TEST [=y]) > > and errors: > > In file included from ../drivers/net/phy/mdio-octeon.c:14: > ../drivers/net/phy/mdio-octeon.c: In function 'octeon_mdiobus_probe': > ../drivers/net/phy/mdio-cavium.h:111:36: error: implicit declaration of > function 'writeq'; did you mean 'writeb'? The proper way to fix this is to include either linux/io-64-nonatomic-hi-lo.h or linux/io-64-nonatomic-lo-hi.h whichever is appropriate.
On Fri, Aug 02, 2019 at 06:11:32PM -0700, David Miller wrote: > From: Nathan Chancellor <natechancellor@gmail.com> > Date: Wed, 31 Jul 2019 11:50:24 -0700 > > > arm allyesconfig warns: > > > > WARNING: unmet direct dependencies detected for MDIO_OCTEON > > Depends on [n]: NETDEVICES [=y] && MDIO_DEVICE [=y] && MDIO_BUS [=y] > > && 64BIT && HAS_IOMEM [=y] && OF_MDIO [=y] > > Selected by [y]: > > - OCTEON_ETHERNET [=y] && STAGING [=y] && (CAVIUM_OCTEON_SOC && > > NETDEVICES [=y] || COMPILE_TEST [=y]) > > > > and errors: > > > > In file included from ../drivers/net/phy/mdio-octeon.c:14: > > ../drivers/net/phy/mdio-octeon.c: In function 'octeon_mdiobus_probe': > > ../drivers/net/phy/mdio-cavium.h:111:36: error: implicit declaration of > > function 'writeq'; did you mean 'writeb'? > > The proper way to fix this is to include either > > linux/io-64-nonatomic-hi-lo.h > > or > > linux/io-64-nonatomic-lo-hi.h > > whichever is appropriate. Hmmmm, is that not what I did? Although I did not know about io-64-nonatomic-hi-lo.h. What is the difference and which one is needed here? There is apparently another failure when OF_MDIO is not set, I guess I can try to look into that as well and respin into a series if necessary. Cheers, Nathan
On Fri, Aug 02, 2019 at 06:30:31PM -0700, Nathan Chancellor wrote: > On Fri, Aug 02, 2019 at 06:11:32PM -0700, David Miller wrote: > > The proper way to fix this is to include either > > > > linux/io-64-nonatomic-hi-lo.h > > > > or > > > > linux/io-64-nonatomic-lo-hi.h > > > > whichever is appropriate. > > Hmmmm, is that not what I did? > > Although I did not know about io-64-nonatomic-hi-lo.h. What is the > difference and which one is needed here? Whether you write the high or low 32 bits first. For this, it doesn't matter, since the compiled driver will never be run on real hardware. > There is apparently another failure when OF_MDIO is not set, I guess I > can try to look into that as well and respin into a series if > necessary. Thanks for taking care of that!
On Fri, Aug 02, 2019 at 06:39:52PM -0700, Matthew Wilcox wrote: > On Fri, Aug 02, 2019 at 06:30:31PM -0700, Nathan Chancellor wrote: > > On Fri, Aug 02, 2019 at 06:11:32PM -0700, David Miller wrote: > > > The proper way to fix this is to include either > > > > > > linux/io-64-nonatomic-hi-lo.h > > > > > > or > > > > > > linux/io-64-nonatomic-lo-hi.h > > > > > > whichever is appropriate. > > > > Hmmmm, is that not what I did? > > > > Although I did not know about io-64-nonatomic-hi-lo.h. What is the > > difference and which one is needed here? > > Whether you write the high or low 32 bits first. For this, it doesn't > matter, since the compiled driver will never be run on real hardware. That's what I figured. I have only seen lo-hi used personally, which is what I went with here. Thanks for the confirmation! > > > There is apparently another failure when OF_MDIO is not set, I guess I > > can try to look into that as well and respin into a series if > > necessary. > > Thanks for taking care of that!
On Thu, Aug 1, 2019 at 1:52 AM Randy Dunlap <rdunlap@infradead.org> wrote: > However, there are lots of type/cast warnings in both mdio-octeon and mdio-cavium: > > ../drivers/net/phy/mdio-octeon.c: In function ‘octeon_mdiobus_probe’: > ../drivers/net/phy/mdio-octeon.c:48:3: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] > (u64)devm_ioremap(&pdev->dev, mdio_phys, regsize); > ^ cavium_mdiobus.register_base should be "void __iomem *" instead of "u64", and the cast should be dropped. > In file included from ../drivers/net/phy/mdio-octeon.c:14:0: > ../drivers/net/phy/mdio-cavium.h:113:48: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] > #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr) > ^ ... which allows to drop this cast as well. Casts are evil, and usually a sign that you're doing something wrong. Gr{oetje,eeting}s, Geert
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 20f14c5fbb7e..ed2edf4b5b0e 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -159,7 +159,7 @@ config MDIO_MSCC_MIIM config MDIO_OCTEON tristate "Octeon and some ThunderX SOCs MDIO buses" - depends on 64BIT + depends on 64BIT || COMPILE_TEST depends on HAS_IOMEM && OF_MDIO select MDIO_CAVIUM help diff --git a/drivers/net/phy/mdio-cavium.h b/drivers/net/phy/mdio-cavium.h index ed5f9bb5448d..b7f89ad27465 100644 --- a/drivers/net/phy/mdio-cavium.h +++ b/drivers/net/phy/mdio-cavium.h @@ -108,6 +108,8 @@ static inline u64 oct_mdio_readq(u64 addr) return cvmx_read_csr(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) #endif
arm allyesconfig warns: WARNING: unmet direct dependencies detected for MDIO_OCTEON Depends on [n]: NETDEVICES [=y] && MDIO_DEVICE [=y] && MDIO_BUS [=y] && 64BIT && HAS_IOMEM [=y] && OF_MDIO [=y] Selected by [y]: - OCTEON_ETHERNET [=y] && STAGING [=y] && (CAVIUM_OCTEON_SOC && NETDEVICES [=y] || COMPILE_TEST [=y]) and errors: In file included from ../drivers/net/phy/mdio-octeon.c:14: ../drivers/net/phy/mdio-octeon.c: In function 'octeon_mdiobus_probe': ../drivers/net/phy/mdio-cavium.h:111:36: error: implicit declaration of function 'writeq'; did you mean 'writeb'? [-Werror=implicit-function-declaration] 111 | #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr) | ^~~~~~ ../drivers/net/phy/mdio-octeon.c:56:2: note: in expansion of macro 'oct_mdio_writeq' 56 | oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN); | ^~~~~~~~~~~~~~~ cc1: some warnings being treated as errors This allows MDIO_OCTEON to be built with COMPILE_TEST as well and includes the proper header for readq/writeq. This does not address the several -Wint-to-pointer-cast and -Wpointer-to-int-cast warnings that appeared as a result of commit 171a9bae68c7 ("staging/octeon: Allow test build on !MIPS") in these files. Fixes: 171a9bae68c7 ("staging/octeon: Allow test build on !MIPS") Reported-by: kbuild test robot <lkp@intel.com> Reported-by: Mark Brown <broonie@kernel.org> Reported-by: Randy Dunlap <rdunlap@infradead.org> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> --- drivers/net/phy/Kconfig | 2 +- drivers/net/phy/mdio-cavium.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)