diff mbox series

net: smc91x: Fix m68k kernel compilation for ColdFire CPU

Message ID 20240509121713.190076-2-thorsten.blum@toblux.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: smc91x: Fix m68k kernel compilation for ColdFire CPU | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 927 this patch: 927
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 937 this patch: 937
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 938 this patch: 938
netdev/checkpatch warning WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-05-09--15-00 (tests: 1006)

Commit Message

Thorsten Blum May 9, 2024, 12:17 p.m. UTC
Compiling the m68k kernel with support for the ColdFire CPU family fails
with the following error:

In file included from drivers/net/ethernet/smsc/smc91x.c:80:
drivers/net/ethernet/smsc/smc91x.c: In function ‘smc_reset’:
drivers/net/ethernet/smsc/smc91x.h:160:40: error: implicit declaration of function ‘_swapw’; did you mean ‘swap’? [-Werror=implicit-function-declaration]
  160 | #define SMC_outw(lp, v, a, r)   writew(_swapw(v), (a) + (r))
      |                                        ^~~~~~
drivers/net/ethernet/smsc/smc91x.h:904:25: note: in expansion of macro ‘SMC_outw’
  904 |                         SMC_outw(lp, x, ioaddr, BANK_SELECT);           \
      |                         ^~~~~~~~
drivers/net/ethernet/smsc/smc91x.c:250:9: note: in expansion of macro ‘SMC_SELECT_BANK’
  250 |         SMC_SELECT_BANK(lp, 2);
      |         ^~~~~~~~~~~~~~~
cc1: some warnings being treated as errors

The function _swapw() was removed in commit d97cf70af097 ("m68k: use
asm-generic/io.h for non-MMU io access functions"), but is still used in
drivers/net/ethernet/smsc/smc91x.h.

Re-adding the previously deleted _swapw() function resolves the error.

Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com>
---
 drivers/net/ethernet/smsc/smc91x.h | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Andrew Lunn May 9, 2024, 8:36 p.m. UTC | #1
On Thu, May 09, 2024 at 02:17:14PM +0200, Thorsten Blum wrote:
> Compiling the m68k kernel with support for the ColdFire CPU family fails
> with the following error:
> 
> In file included from drivers/net/ethernet/smsc/smc91x.c:80:
> drivers/net/ethernet/smsc/smc91x.c: In function ‘smc_reset’:
> drivers/net/ethernet/smsc/smc91x.h:160:40: error: implicit declaration of function ‘_swapw’; did you mean ‘swap’? [-Werror=implicit-function-declaration]
>   160 | #define SMC_outw(lp, v, a, r)   writew(_swapw(v), (a) + (r))
>       |                                        ^~~~~~
> drivers/net/ethernet/smsc/smc91x.h:904:25: note: in expansion of macro ‘SMC_outw’
>   904 |                         SMC_outw(lp, x, ioaddr, BANK_SELECT);           \
>       |                         ^~~~~~~~
> drivers/net/ethernet/smsc/smc91x.c:250:9: note: in expansion of macro ‘SMC_SELECT_BANK’
>   250 |         SMC_SELECT_BANK(lp, 2);
>       |         ^~~~~~~~~~~~~~~
> cc1: some warnings being treated as errors
> 
> The function _swapw() was removed in commit d97cf70af097 ("m68k: use
> asm-generic/io.h for non-MMU io access functions"), but is still used in
> drivers/net/ethernet/smsc/smc91x.h.
> 
> Re-adding the previously deleted _swapw() function resolves the error.

This seems like the wrong fix.

commit d97cf70af09721ef416c61faa44543e3b84c9a55
Author: Greg Ungerer <gerg@linux-m68k.org>
Date:   Fri Mar 23 23:39:10 2018 +1000

    m68k: use asm-generic/io.h for non-MMU io access functions
    
    There is nothing really special about the non-MMU m68k IO access functions.
    So we can easily switch to using the asm-generic/io.h functions.

So it rather than put something back which there is an aim to remove,
please find the generic replacement. This _swapw() swaps a 16 bit
word. The generic for that is swab16().

I'm also surprised it took 6 years to find this. Has something else
changed recently?

    Andrew

---
pw-bot: cr
Arnd Bergmann May 9, 2024, 8:47 p.m. UTC | #2
On Thu, May 9, 2024, at 14:17, Thorsten Blum wrote:
> 
> +static inline unsigned short _swapw(volatile unsigned short v)
> +{
> +	return ((v << 8) | (v >> 8));
> +}
> +
>  #define SMC_inw(a, r)		_swapw(readw((a) + (r)))
>  #define SMC_outw(lp, v, a, r)	writew(_swapw(v), (a) + (r))
>  #define SMC_insw(a, r, p, l)	mcf_insw(a + r, p, l)

I think you can just use iowrite16_be() and ioread16_be()
here in place of the little-endian access plus swap.

Also, it looks like it's been broken for six years without
anyone noticing the problem, so I wonder if there are even
still any machines that use this driver and get kernel
updates.

     Arnd
Thorsten Blum May 9, 2024, 9:26 p.m. UTC | #3
On 9. May 2024, at 22:36, Andrew Lunn <andrew@lunn.ch> wrote:
> This seems like the wrong fix.
> 
> commit d97cf70af09721ef416c61faa44543e3b84c9a55
> Author: Greg Ungerer <gerg@linux-m68k.org>
> Date:   Fri Mar 23 23:39:10 2018 +1000
> 
>    m68k: use asm-generic/io.h for non-MMU io access functions
> 
>    There is nothing really special about the non-MMU m68k IO access functions.
>    So we can easily switch to using the asm-generic/io.h functions.
> 
> So it rather than put something back which there is an aim to remove,
> please find the generic replacement. This _swapw() swaps a 16 bit
> word. The generic for that is swab16().

Thanks. I will use ioread16be() and iowrite16be() as suggested by Arnd
instead and submit a v2 shortly.

Thorsten
diff mbox series

Patch

diff --git a/drivers/net/ethernet/smsc/smc91x.h b/drivers/net/ethernet/smsc/smc91x.h
index 46eee747c699..e5d7f49915c6 100644
--- a/drivers/net/ethernet/smsc/smc91x.h
+++ b/drivers/net/ethernet/smsc/smc91x.h
@@ -156,6 +156,11 @@  static inline void mcf_outsw(void *a, unsigned char *p, int l)
 		writew(*wp++, a);
 }
 
+static inline unsigned short _swapw(volatile unsigned short v)
+{
+	return ((v << 8) | (v >> 8));
+}
+
 #define SMC_inw(a, r)		_swapw(readw((a) + (r)))
 #define SMC_outw(lp, v, a, r)	writew(_swapw(v), (a) + (r))
 #define SMC_insw(a, r, p, l)	mcf_insw(a + r, p, l)