The driver is using iowriteXX()/ioreadXX() APIs which are LE IO
accessors simplified as
1. Convert given value _from_ CPU _to_ LE
2. Write it to the device as is
The dev_addr is a byte stream, but because the driver uses 16-bit
IO accessors, it wants to perform double conversion on BE CPUs,
but it took it wrong, as it effectivelly does two times _from_ CPU
_to_ LE. What it has to do is to consider dev_addr as an array of
LE16 and hence do _from_ LE _to_ CPU conversion, followed by implied
_from_ CPU _to_ LE in the iowrite16().
To achieve that, use get_unaligned_le16(). This will make it correct
and allows to avoid sparse warning as reported by LKP.
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/net/ethernet/dlink/dl2k.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/dlink/dl2k.c b/drivers/net/ethernet/dlink/dl2k.c
index db6615aa921b..7bfeae04b52b 100644
--- a/drivers/net/ethernet/dlink/dl2k.c
+++ b/drivers/net/ethernet/dlink/dl2k.c
@@ -565,8 +565,7 @@ static void rio_hw_init(struct net_device *dev)
* too. However, it doesn't work on IP1000A so we use 16-bit access.
*/
for (i = 0; i < 3; i++)
- dw16(StationAddr0 + 2 * i,
- cpu_to_le16(((const u16 *)dev->dev_addr)[i]));
+ dw16(StationAddr0 + 2 * i, get_unaligned_le16(&dev->dev_addr[2 * i]));
set_multicast (dev);
if (np->coalesce) {
--
2.43.0.rc1.1.gbec44491f096
Hello:
This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <[email protected]>:
On Fri, 8 Dec 2023 17:33:27 +0200 you wrote:
> The driver is using iowriteXX()/ioreadXX() APIs which are LE IO
> accessors simplified as
>
> 1. Convert given value _from_ CPU _to_ LE
> 2. Write it to the device as is
>
> The dev_addr is a byte stream, but because the driver uses 16-bit
> IO accessors, it wants to perform double conversion on BE CPUs,
> but it took it wrong, as it effectivelly does two times _from_ CPU
> _to_ LE. What it has to do is to consider dev_addr as an array of
> LE16 and hence do _from_ LE _to_ CPU conversion, followed by implied
> _from_ CPU _to_ LE in the iowrite16().
>
> [...]
Here is the summary with links:
- [net-next,v1,1/1] net: dl2k: Use proper conversion of dev_addr before IO to device
https://git.kernel.org/netdev/net-next/c/68cbdb150d55
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
On Fri, Dec 08, 2023 at 05:33:27PM +0200, Andy Shevchenko wrote:
> The driver is using iowriteXX()/ioreadXX() APIs which are LE IO
> accessors simplified as
>
> 1. Convert given value _from_ CPU _to_ LE
> 2. Write it to the device as is
>
> The dev_addr is a byte stream, but because the driver uses 16-bit
> IO accessors, it wants to perform double conversion on BE CPUs,
> but it took it wrong, as it effectivelly does two times _from_ CPU
> _to_ LE. What it has to do is to consider dev_addr as an array of
> LE16 and hence do _from_ LE _to_ CPU conversion, followed by implied
> _from_ CPU _to_ LE in the iowrite16().
>
> To achieve that, use get_unaligned_le16(). This will make it correct
> and allows to avoid sparse warning as reported by LKP.
>
> Reported-by: kernel test robot <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> Signed-off-by: Andy Shevchenko <[email protected]>
Thanks Andy,
I agree with your reasoning that the explicit conversion is reversed.
Reviewed-by: Simon Horman <[email protected]>