[PATCH net-next v1 1/1] net: dl2k: Use proper conversion of dev_addr before IO to device

[PATCH net-next v1 1/1] net: dl2k: Use proper conversion of dev_addr before IO to device

[Andy Shevchenko]

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 <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202312030058.hfZPTXd7-lkp@intel.com/
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 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

Re: [PATCH net-next v1 1/1] net: dl2k: Use proper conversion of dev_addr before IO to device

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

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

Re: [PATCH net-next v1 1/1] net: dl2k: Use proper conversion of dev_addr before IO to device

[Simon Horman]

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 <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202312030058.hfZPTXd7-lkp@intel.com/
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thanks Andy,

I agree with your reasoning that the explicit conversion is reversed.

Reviewed-by: Simon Horman <horms@kernel.org>