2024-05-09 14:45:20

by Thorsten Blum

[permalink] [raw]
Subject: [PATCH] net: smc91x: Fix m68k kernel compilation for ColdFire CPU

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 <[email protected]>
---
drivers/net/ethernet/smsc/smc91x.h | 5 +++++
1 file changed, 5 insertions(+)

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



2024-05-09 20:38:55

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH] net: smc91x: Fix m68k kernel compilation for ColdFire CPU

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 <[email protected]>
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

2024-05-09 20:47:40

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] net: smc91x: Fix m68k kernel compilation for ColdFire CPU

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

2024-05-09 21:27:15

by Thorsten Blum

[permalink] [raw]
Subject: Re: [PATCH] net: smc91x: Fix m68k kernel compilation for ColdFire CPU

On 9. May 2024, at 22:36, Andrew Lunn <[email protected]> wrote:
> This seems like the wrong fix.
>
> commit d97cf70af09721ef416c61faa44543e3b84c9a55
> Author: Greg Ungerer <[email protected]>
> 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

2024-05-09 21:42:23

by Thorsten Blum

[permalink] [raw]
Subject: [PATCH v2] net: smc91x: Fix m68k kernel compilation for ColdFire CPU

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.

Use ioread16be() and iowrite16be() to resolve the error.

Signed-off-by: Thorsten Blum <[email protected]>
---
Changes in v2:
- Use ioread16be() and iowrite16be() directly instead of re-adding
_swapw() as suggested by Arnd Bergmann and Andrew Lunn
---
drivers/net/ethernet/smsc/smc91x.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/smsc/smc91x.h b/drivers/net/ethernet/smsc/smc91x.h
index 46eee747c699..45ef5ac0788a 100644
--- a/drivers/net/ethernet/smsc/smc91x.h
+++ b/drivers/net/ethernet/smsc/smc91x.h
@@ -156,8 +156,8 @@ static inline void mcf_outsw(void *a, unsigned char *p, int l)
writew(*wp++, a);
}

-#define SMC_inw(a, r) _swapw(readw((a) + (r)))
-#define SMC_outw(lp, v, a, r) writew(_swapw(v), (a) + (r))
+#define SMC_inw(a, r) ioread16be((a) + (r))
+#define SMC_outw(lp, v, a, r) iowrite16be(v, (a) + (r))
#define SMC_insw(a, r, p, l) mcf_insw(a + r, p, l)
#define SMC_outsw(a, r, p, l) mcf_outsw(a + r, p, l)

--
2.45.0


2024-05-10 15:19:58

by Thorsten Blum

[permalink] [raw]
Subject: [PATCH v3] net: smc91x: Fix m68k kernel compilation for ColdFire CPU

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.

Use ioread16be() and iowrite16be() to resolve the error.

Fixes: d97cf70af097 ("m68k: use asm-generic/io.h for non-MMU io access functions")
Signed-off-by: Thorsten Blum <[email protected]>
---
Changes in v2:
- Use ioread16be() and iowrite16be() directly instead of re-adding
_swapw() as suggested by Arnd Bergmann and Andrew Lunn

Changes in v3:
- Add Fixes: tag
---
drivers/net/ethernet/smsc/smc91x.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/smsc/smc91x.h b/drivers/net/ethernet/smsc/smc91x.h
index 46eee747c699..45ef5ac0788a 100644
--- a/drivers/net/ethernet/smsc/smc91x.h
+++ b/drivers/net/ethernet/smsc/smc91x.h
@@ -156,8 +156,8 @@ static inline void mcf_outsw(void *a, unsigned char *p, int l)
writew(*wp++, a);
}

-#define SMC_inw(a, r) _swapw(readw((a) + (r)))
-#define SMC_outw(lp, v, a, r) writew(_swapw(v), (a) + (r))
+#define SMC_inw(a, r) ioread16be((a) + (r))
+#define SMC_outw(lp, v, a, r) iowrite16be(v, (a) + (r))
#define SMC_insw(a, r, p, l) mcf_insw(a + r, p, l)
#define SMC_outsw(a, r, p, l) mcf_outsw(a + r, p, l)

--
2.45.0


2024-05-10 16:23:32

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v3] net: smc91x: Fix m68k kernel compilation for ColdFire CPU

On Fri, May 10, 2024 at 01:30:55PM +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.
>
> Use ioread16be() and iowrite16be() to resolve the error.
>
> Fixes: d97cf70af097 ("m68k: use asm-generic/io.h for non-MMU io access functions")
> Signed-off-by: Thorsten Blum <[email protected]>
> ---
> Changes in v2:
> - Use ioread16be() and iowrite16be() directly instead of re-adding
> _swapw() as suggested by Arnd Bergmann and Andrew Lunn
>
> Changes in v3:
> - Add Fixes: tag

This is for net.

Cc: <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>

Andrew

2024-05-13 22:50:39

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH v3] net: smc91x: Fix m68k kernel compilation for ColdFire CPU

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:

On Fri, 10 May 2024 13:30:55 +0200 you 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
>
> [...]

Here is the summary with links:
- [v3] net: smc91x: Fix m68k kernel compilation for ColdFire CPU
https://git.kernel.org/netdev/net/c/5eefb477d21a

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html