| Message ID | 20231208153327.3306798-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 68cbdb150d55834ed0a52352684ba2cc554c8a08 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v1,1/1] net: dl2k: Use proper conversion of dev_addr before IO to device | expand |
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!
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>
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) {
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(-)