Linux-mediatek Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian
@ 2025-10-27 17:17 Caleb James DeLisle
  2025-10-28 20:19 ` Jonas Gorski
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Caleb James DeLisle @ 2025-10-27 17:17 UTC (permalink / raw)
  To: nbd, lorenzo
  Cc: ryder.lee, shayne.chen, sean.wang, matthias.bgg,
	angelogioacchino.delregno, linux-wireless, linux-kernel,
	linux-mediatek, Caleb James DeLisle

When on a Big Endian machine, PCI swaps words to/from LE when
reading/writing them. This presents a problem when we're trying
to copy an opaque byte array such as firmware or encryption key.

Byte-swapping during copy results in two swaps, but solves the
problem.

Fixes:
mt76x2e 0000:02:00.0: ROM patch build: 20141115060606a
mt76x2e 0000:02:00.0: Firmware Version: 0.0.00
mt76x2e 0000:02:00.0: Build: 1
mt76x2e 0000:02:00.0: Build Time: 201607111443____
mt76x2e 0000:02:00.0: Firmware failed to start
mt76x2e 0000:02:00.0: probe with driver mt76x2e failed with error -145

Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
---
 drivers/net/wireless/mediatek/mt76/mmio.c | 34 +++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/net/wireless/mediatek/mt76/mmio.c b/drivers/net/wireless/mediatek/mt76/mmio.c
index cd2e9737c3bf..776dbaacc8a3 100644
--- a/drivers/net/wireless/mediatek/mt76/mmio.c
+++ b/drivers/net/wireless/mediatek/mt76/mmio.c
@@ -30,15 +30,49 @@ static u32 mt76_mmio_rmw(struct mt76_dev *dev, u32 offset, u32 mask, u32 val)
 	return val;
 }
 
+static void mt76_mmio_write_copy_portable(void __iomem *dst,
+					  const u8 *src, int len)
+{
+	__le32 val;
+	int i = 0;
+
+	for (i = 0; i < ALIGN(len, 4); i += 4) {
+		memcpy(&val, src + i, sizeof(val));
+		writel(cpu_to_le32(val), dst + i);
+	}
+}
+
 static void mt76_mmio_write_copy(struct mt76_dev *dev, u32 offset,
 				 const void *data, int len)
 {
+	if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) {
+		mt76_mmio_write_copy_portable(dev->mmio.regs + offset, data,
+					      len);
+		return;
+	}
 	__iowrite32_copy(dev->mmio.regs + offset, data, DIV_ROUND_UP(len, 4));
 }
 
+static void mt76_mmio_read_copy_portable(u8 *dst,
+					 const void __iomem *src, int len)
+{
+	u32 val;
+	int i;
+
+	for (i = 0; i < ALIGN(len, 4); i += 4) {
+		val = le32_to_cpu(readl(src + i));
+		memcpy(dst + i, &val, sizeof(val));
+	}
+}
+
 static void mt76_mmio_read_copy(struct mt76_dev *dev, u32 offset,
 				void *data, int len)
 {
+	if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) {
+		mt76_mmio_read_copy_portable(data, dev->mmio.regs + offset,
+					     len);
+		return;
+	}
 	__ioread32_copy(data, dev->mmio.regs + offset, DIV_ROUND_UP(len, 4));
 }
 
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian
  2025-10-27 17:17 [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian Caleb James DeLisle
@ 2025-10-28 20:19 ` Jonas Gorski
  2025-10-28 21:41   ` Caleb James DeLisle
  2025-10-31  1:18 ` kernel test robot
  2025-11-01 16:25 ` kernel test robot
  2 siblings, 1 reply; 12+ messages in thread
From: Jonas Gorski @ 2025-10-28 20:19 UTC (permalink / raw)
  To: Caleb James DeLisle
  Cc: nbd, lorenzo, ryder.lee, shayne.chen, sean.wang, matthias.bgg,
	angelogioacchino.delregno, linux-wireless, linux-kernel,
	linux-mediatek

Hi,

On Mon, Oct 27, 2025 at 6:19 PM Caleb James DeLisle <cjd@cjdns.fr> wrote:
>
> When on a Big Endian machine, PCI swaps words to/from LE when
> reading/writing them. This presents a problem when we're trying
> to copy an opaque byte array such as firmware or encryption key.
>
> Byte-swapping during copy results in two swaps, but solves the
> problem.
>
> Fixes:
> mt76x2e 0000:02:00.0: ROM patch build: 20141115060606a
> mt76x2e 0000:02:00.0: Firmware Version: 0.0.00
> mt76x2e 0000:02:00.0: Build: 1
> mt76x2e 0000:02:00.0: Build Time: 201607111443____
> mt76x2e 0000:02:00.0: Firmware failed to start
> mt76x2e 0000:02:00.0: probe with driver mt76x2e failed with error -145
>
> Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
> ---
>  drivers/net/wireless/mediatek/mt76/mmio.c | 34 +++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mmio.c b/drivers/net/wireless/mediatek/mt76/mmio.c
> index cd2e9737c3bf..776dbaacc8a3 100644
> --- a/drivers/net/wireless/mediatek/mt76/mmio.c
> +++ b/drivers/net/wireless/mediatek/mt76/mmio.c
> @@ -30,15 +30,49 @@ static u32 mt76_mmio_rmw(struct mt76_dev *dev, u32 offset, u32 mask, u32 val)
>         return val;
>  }
>
> +static void mt76_mmio_write_copy_portable(void __iomem *dst,
> +                                         const u8 *src, int len)
> +{
> +       __le32 val;
> +       int i = 0;
> +
> +       for (i = 0; i < ALIGN(len, 4); i += 4) {
> +               memcpy(&val, src + i, sizeof(val));
> +               writel(cpu_to_le32(val), dst + i);
> +       }
> +}
> +
>  static void mt76_mmio_write_copy(struct mt76_dev *dev, u32 offset,
>                                  const void *data, int len)
>  {
> +       if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) {
> +               mt76_mmio_write_copy_portable(dev->mmio.regs + offset, data,
> +                                             len);
> +               return;
> +       }
>         __iowrite32_copy(dev->mmio.regs + offset, data, DIV_ROUND_UP(len, 4));

Maybe just replace this with memcpy_toio() which does no swapping at
all instead of double swapping on BE?

>  }
>
> +static void mt76_mmio_read_copy_portable(u8 *dst,
> +                                        const void __iomem *src, int len)
> +{
> +       u32 val;
> +       int i;
> +
> +       for (i = 0; i < ALIGN(len, 4); i += 4) {
> +               val = le32_to_cpu(readl(src + i));
> +               memcpy(dst + i, &val, sizeof(val));
> +       }
> +}
> +
>  static void mt76_mmio_read_copy(struct mt76_dev *dev, u32 offset,
>                                 void *data, int len)
>  {
> +       if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) {
> +               mt76_mmio_read_copy_portable(data, dev->mmio.regs + offset,
> +                                            len);
> +               return;
> +       }
>         __ioread32_copy(data, dev->mmio.regs + offset, DIV_ROUND_UP(len, 4));

And memcpy_fromio() here.

Best regards,
Jonas


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian
  2025-10-28 20:19 ` Jonas Gorski
@ 2025-10-28 21:41   ` Caleb James DeLisle
  2025-10-29  9:15     ` Jonas Gorski
  0 siblings, 1 reply; 12+ messages in thread
From: Caleb James DeLisle @ 2025-10-28 21:41 UTC (permalink / raw)
  To: Jonas Gorski
  Cc: nbd, lorenzo, ryder.lee, shayne.chen, sean.wang, matthias.bgg,
	angelogioacchino.delregno, linux-wireless, linux-kernel,
	linux-mediatek


On 28/10/2025 21:19, Jonas Gorski wrote:
> Hi,
>
> On Mon, Oct 27, 2025 at 6:19 PM Caleb James DeLisle <cjd@cjdns.fr> wrote:
>> When on a Big Endian machine, PCI swaps words to/from LE when
>> reading/writing them. This presents a problem when we're trying
>> to copy an opaque byte array such as firmware or encryption key.
>>
>> Byte-swapping during copy results in two swaps, but solves the
>> problem.
>>
>> Fixes:
>> mt76x2e 0000:02:00.0: ROM patch build: 20141115060606a
>> mt76x2e 0000:02:00.0: Firmware Version: 0.0.00
>> mt76x2e 0000:02:00.0: Build: 1
>> mt76x2e 0000:02:00.0: Build Time: 201607111443____
>> mt76x2e 0000:02:00.0: Firmware failed to start
>> mt76x2e 0000:02:00.0: probe with driver mt76x2e failed with error -145
>>
>> Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
>> ---
>>   drivers/net/wireless/mediatek/mt76/mmio.c | 34 +++++++++++++++++++++++
>>   1 file changed, 34 insertions(+)
>>
>> diff --git a/drivers/net/wireless/mediatek/mt76/mmio.c b/drivers/net/wireless/mediatek/mt76/mmio.c
>> index cd2e9737c3bf..776dbaacc8a3 100644
>> --- a/drivers/net/wireless/mediatek/mt76/mmio.c
>> +++ b/drivers/net/wireless/mediatek/mt76/mmio.c
>> @@ -30,15 +30,49 @@ static u32 mt76_mmio_rmw(struct mt76_dev *dev, u32 offset, u32 mask, u32 val)
>>          return val;
>>   }
>>
>> +static void mt76_mmio_write_copy_portable(void __iomem *dst,
>> +                                         const u8 *src, int len)
>> +{
>> +       __le32 val;
>> +       int i = 0;
>> +
>> +       for (i = 0; i < ALIGN(len, 4); i += 4) {
>> +               memcpy(&val, src + i, sizeof(val));
>> +               writel(cpu_to_le32(val), dst + i);
>> +       }
>> +}
>> +
>>   static void mt76_mmio_write_copy(struct mt76_dev *dev, u32 offset,
>>                                   const void *data, int len)
>>   {
>> +       if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) {
>> +               mt76_mmio_write_copy_portable(dev->mmio.regs + offset, data,
>> +                                             len);
>> +               return;
>> +       }
>>          __iowrite32_copy(dev->mmio.regs + offset, data, DIV_ROUND_UP(len, 4));
> Maybe just replace this with memcpy_toio() which does no swapping at
> all instead of double swapping on BE?


I'm not that informed about how PCI works so I had to test to confirm
my understanding, but I can confirm that memcpy_toio() does not solve
the problem.

The issue as I understand it is that rather than making every driver
carefully call cpu_to_le*() every MMIO write, someone decided to make
the PCI host bridge itself transparently byte-swap all MMIO on the
wire. Since most MMIO is hitting registers and most buffers are
transferred by DMA, for the most part everything works and nobody
notices.

But in the rare case that we need to write a blob to MMIO, it gets
transparently swapped in hardware so you need to use cpu_to_le in that
case. Doing a search of ./drivers for write.*cpu_to_le I can see this
issue comes up a bit.


Thanks,

Caleb


>
>>   }
>>
>> +static void mt76_mmio_read_copy_portable(u8 *dst,
>> +                                        const void __iomem *src, int len)
>> +{
>> +       u32 val;
>> +       int i;
>> +
>> +       for (i = 0; i < ALIGN(len, 4); i += 4) {
>> +               val = le32_to_cpu(readl(src + i));
>> +               memcpy(dst + i, &val, sizeof(val));
>> +       }
>> +}
>> +
>>   static void mt76_mmio_read_copy(struct mt76_dev *dev, u32 offset,
>>                                  void *data, int len)
>>   {
>> +       if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) {
>> +               mt76_mmio_read_copy_portable(data, dev->mmio.regs + offset,
>> +                                            len);
>> +               return;
>> +       }
>>          __ioread32_copy(data, dev->mmio.regs + offset, DIV_ROUND_UP(len, 4));
> And memcpy_fromio() here.
>
> Best regards,
> Jonas


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian
  2025-10-28 21:41   ` Caleb James DeLisle
@ 2025-10-29  9:15     ` Jonas Gorski
  2025-10-29 15:24       ` Caleb James DeLisle
  0 siblings, 1 reply; 12+ messages in thread
From: Jonas Gorski @ 2025-10-29  9:15 UTC (permalink / raw)
  To: Caleb James DeLisle
  Cc: nbd, lorenzo, ryder.lee, shayne.chen, sean.wang, matthias.bgg,
	angelogioacchino.delregno, linux-wireless, linux-kernel,
	linux-mediatek

On Tue, Oct 28, 2025 at 10:42 PM Caleb James DeLisle <cjd@cjdns.fr> wrote:
>
>
> On 28/10/2025 21:19, Jonas Gorski wrote:
> > Hi,
> >
> > On Mon, Oct 27, 2025 at 6:19 PM Caleb James DeLisle <cjd@cjdns.fr> wrote:
> >> When on a Big Endian machine, PCI swaps words to/from LE when
> >> reading/writing them. This presents a problem when we're trying
> >> to copy an opaque byte array such as firmware or encryption key.
> >>
> >> Byte-swapping during copy results in two swaps, but solves the
> >> problem.
> >>
> >> Fixes:
> >> mt76x2e 0000:02:00.0: ROM patch build: 20141115060606a
> >> mt76x2e 0000:02:00.0: Firmware Version: 0.0.00
> >> mt76x2e 0000:02:00.0: Build: 1
> >> mt76x2e 0000:02:00.0: Build Time: 201607111443____
> >> mt76x2e 0000:02:00.0: Firmware failed to start
> >> mt76x2e 0000:02:00.0: probe with driver mt76x2e failed with error -145
> >>
> >> Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
> >> ---
> >>   drivers/net/wireless/mediatek/mt76/mmio.c | 34 +++++++++++++++++++++++
> >>   1 file changed, 34 insertions(+)
> >>
> >> diff --git a/drivers/net/wireless/mediatek/mt76/mmio.c b/drivers/net/wireless/mediatek/mt76/mmio.c
> >> index cd2e9737c3bf..776dbaacc8a3 100644
> >> --- a/drivers/net/wireless/mediatek/mt76/mmio.c
> >> +++ b/drivers/net/wireless/mediatek/mt76/mmio.c
> >> @@ -30,15 +30,49 @@ static u32 mt76_mmio_rmw(struct mt76_dev *dev, u32 offset, u32 mask, u32 val)
> >>          return val;
> >>   }
> >>
> >> +static void mt76_mmio_write_copy_portable(void __iomem *dst,
> >> +                                         const u8 *src, int len)
> >> +{
> >> +       __le32 val;
> >> +       int i = 0;
> >> +
> >> +       for (i = 0; i < ALIGN(len, 4); i += 4) {
> >> +               memcpy(&val, src + i, sizeof(val));
> >> +               writel(cpu_to_le32(val), dst + i);
> >> +       }
> >> +}
> >> +
> >>   static void mt76_mmio_write_copy(struct mt76_dev *dev, u32 offset,
> >>                                   const void *data, int len)
> >>   {
> >> +       if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) {
> >> +               mt76_mmio_write_copy_portable(dev->mmio.regs + offset, data,
> >> +                                             len);
> >> +               return;
> >> +       }
> >>          __iowrite32_copy(dev->mmio.regs + offset, data, DIV_ROUND_UP(len, 4));
> > Maybe just replace this with memcpy_toio() which does no swapping at
> > all instead of double swapping on BE?
>
>
> I'm not that informed about how PCI works so I had to test to confirm
> my understanding, but I can confirm that memcpy_toio() does not solve
> the problem.

Ah, right, I misread _iowrite32_copy() to do conversion to LE, but it doesn't.

What architecture is this you have? PowerPC? ARM? MIPS? 32 bit? 64 bit?

So the differences I see are:

1. __iowrite32_copy() uses __raw_writel(), which has different memory
semantics than writel()
2. __iowrite32_copy() assumed src is aligned to 32 bit, while you
explicitly align it
3. memcpy_toio() will handle unaligned src properly, but does 64 bit
accesses on 64 bit systems (and uses __raw* again).

Is src aligned? If not, then the issue might be 2. And if your system
is 64 bit, it would explain why 3 didn't help.

As a first step you could try to replace the writel(cpu_to_le32(val)
with a iowrite32be(val, ...) which should do the same except avoiding
the doubled byte swapping. If that works, you can try to replace it
with __raw_writel(), which then would make this the same as
__iowrite32_copy, except making sure that src is aligned.

Also you could replace your memcpy() with get_unaligned((u32 *)(src +
i)); Should do the same but inline.

>
> The issue as I understand it is that rather than making every driver
> carefully call cpu_to_le*() every MMIO write, someone decided to make
> the PCI host bridge itself transparently byte-swap all MMIO on the
> wire. Since most MMIO is hitting registers and most buffers are
> transferred by DMA, for the most part everything works and nobody
> notices.
>
> But in the rare case that we need to write a blob to MMIO, it gets
> transparently swapped in hardware so you need to use cpu_to_le in that
> case. Doing a search of ./drivers for write.*cpu_to_le I can see this
> issue comes up a bit.

Every (PCI) driver does conversion to LE implicitly by using
writel/readl (or iowrite32/ioread32) etc do the conversion to/from LE.
So writel(foo, dst )is a __raw_writel(cpu_to_le32(foo), dst) etc. PCI
memory is assumed to be in LE. If you are on a little endian system,
then no byte swapping happens, and on BE it will do byte swapping
before writing the value.

Best regards,
Jonas


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian
  2025-10-29  9:15     ` Jonas Gorski
@ 2025-10-29 15:24       ` Caleb James DeLisle
  2025-10-29 18:41         ` [PATCH v2] wifi: mt76: mmio_*_copy fix byte order and alignment Caleb James DeLisle
  2025-10-29 20:12         ` [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian Jonas Gorski
  0 siblings, 2 replies; 12+ messages in thread
From: Caleb James DeLisle @ 2025-10-29 15:24 UTC (permalink / raw)
  To: Jonas Gorski
  Cc: nbd, lorenzo, ryder.lee, shayne.chen, sean.wang, matthias.bgg,
	angelogioacchino.delregno, linux-wireless, linux-kernel,
	linux-mediatek, Daniel Golle


On 29/10/2025 10:15, Jonas Gorski wrote:
> On Tue, Oct 28, 2025 at 10:42 PM Caleb James DeLisle <cjd@cjdns.fr> wrote:
>>
>> On 28/10/2025 21:19, Jonas Gorski wrote:
>>> Hi,
>>>
>>> On Mon, Oct 27, 2025 at 6:19 PM Caleb James DeLisle <cjd@cjdns.fr> wrote:
>>>> When on a Big Endian machine, PCI swaps words to/from LE when
>>>> reading/writing them. This presents a problem when we're trying
>>>> to copy an opaque byte array such as firmware or encryption key.
>>>>
>>>> Byte-swapping during copy results in two swaps, but solves the
>>>> problem.
>>>>
>>>> Fixes:
>>>> mt76x2e 0000:02:00.0: ROM patch build: 20141115060606a
>>>> mt76x2e 0000:02:00.0: Firmware Version: 0.0.00
>>>> mt76x2e 0000:02:00.0: Build: 1
>>>> mt76x2e 0000:02:00.0: Build Time: 201607111443____
>>>> mt76x2e 0000:02:00.0: Firmware failed to start
>>>> mt76x2e 0000:02:00.0: probe with driver mt76x2e failed with error -145
>>>>
>>>> Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
>>>> ---
>>>>    drivers/net/wireless/mediatek/mt76/mmio.c | 34 +++++++++++++++++++++++
>>>>    1 file changed, 34 insertions(+)
>>>>
>>>> diff --git a/drivers/net/wireless/mediatek/mt76/mmio.c b/drivers/net/wireless/mediatek/mt76/mmio.c
>>>> index cd2e9737c3bf..776dbaacc8a3 100644
>>>> --- a/drivers/net/wireless/mediatek/mt76/mmio.c
>>>> +++ b/drivers/net/wireless/mediatek/mt76/mmio.c
>>>> @@ -30,15 +30,49 @@ static u32 mt76_mmio_rmw(struct mt76_dev *dev, u32 offset, u32 mask, u32 val)
>>>>           return val;
>>>>    }
>>>>
>>>> +static void mt76_mmio_write_copy_portable(void __iomem *dst,
>>>> +                                         const u8 *src, int len)
>>>> +{
>>>> +       __le32 val;
>>>> +       int i = 0;
>>>> +
>>>> +       for (i = 0; i < ALIGN(len, 4); i += 4) {
>>>> +               memcpy(&val, src + i, sizeof(val));
>>>> +               writel(cpu_to_le32(val), dst + i);
>>>> +       }
>>>> +}
>>>> +
>>>>    static void mt76_mmio_write_copy(struct mt76_dev *dev, u32 offset,
>>>>                                    const void *data, int len)
>>>>    {
>>>> +       if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) {
>>>> +               mt76_mmio_write_copy_portable(dev->mmio.regs + offset, data,
>>>> +                                             len);
>>>> +               return;
>>>> +       }
>>>>           __iowrite32_copy(dev->mmio.regs + offset, data, DIV_ROUND_UP(len, 4));
>>> Maybe just replace this with memcpy_toio() which does no swapping at
>>> all instead of double swapping on BE?
>>
>> I'm not that informed about how PCI works so I had to test to confirm
>> my understanding, but I can confirm that memcpy_toio() does not solve
>> the problem.
> Ah, right, I misread _iowrite32_copy() to do conversion to LE, but it doesn't.
>
> What architecture is this you have? PowerPC? ARM? MIPS? 32 bit? 64 bit?


MIPS32 (EcoNet EN751221 34Kc)


>
> So the differences I see are:
>
> 1. __iowrite32_copy() uses __raw_writel(), which has different memory
> semantics than writel()
> 2. __iowrite32_copy() assumed src is aligned to 32 bit, while you
> explicitly align it
> 3. memcpy_toio() will handle unaligned src properly, but does 64 bit
> accesses on 64 bit systems (and uses __raw* again).
>
> Is src aligned? If not, then the issue might be 2. And if your system
> is 64 bit, it would explain why 3 didn't help.


I'm not a regular developer of mt76 so I wasn't sure if that was
guaranteed and I just wanted to code for safety.

After reviewing the code, I see that there are a few places where
mt76_mmio_write_copy is being called with stack-allocated u8 arrays
so it's pretty clear to me that this is being treated as a memcpy-like
function and we should be handling unaligned inputs.


>
> As a first step you could try to replace the writel(cpu_to_le32(val)
> with a iowrite32be(val, ...) which should do the same except avoiding
> the doubled byte swapping. If that works, you can try to replace it

This works.

These symbols are a bit of a nightmare to trace, so I ended up making
an .i file so I could confirm what's happening.

iowrite32be() uses the version in iomap.c so I understand that's using 
writel(swab32(val),port), so a writel with an unconditional byte swap.

writel() is more complicated, it's an inline function that is generated 
in a rat's nest of preprocessor macros in mips/include/asm/io.h

The preprocessed is this:

__mem = (void *)((unsigned long)(mem)); __val = (val); if (sizeof(u32) 
!= sizeof(u64) || sizeof(u64) == sizeof(long)) { *__mem = __val;

The source is this:

     __mem = (void *)__swizzle_addr_##bwlq((unsigned long)(mem));    \
     __val = pfx##ioswab##bwlq(__mem, val);                \
     if (sizeof(type) != sizeof(u64) || sizeof(u64) == sizeof(long)) \
         *__mem = __val;                        \

The line "pfx##ioswab##bwlq(__mem, val);" is ioswabl() and the source
of that explains the issue:

  * Sane hardware offers swapping of PCI/ISA I/O space accesses in hardware;
  * less sane hardware forces software to fiddle with this...

So this confirms my initial understanding, the PCI hardware is doing the
byte swapping unconditionally.


> with __raw_writel(), which then would make this the same as
> __iowrite32_copy, except making sure that src is aligned.


This fails.

Since I'm the maintainer of this SoC and it's still fairly new, I wrote
a trivial kmod to verify that unaligned access is not just silently
returning trash, it works as though it were aligned so alignment is
not the issue.


>
> Also you could replace your memcpy() with get_unaligned((u32 *)(src +
> i)); Should do the same but inline.
Good idea, I will do this.
>> The issue as I understand it is that rather than making every driver
>> carefully call cpu_to_le*() every MMIO write, someone decided to make
>> the PCI host bridge itself transparently byte-swap all MMIO on the
>> wire. Since most MMIO is hitting registers and most buffers are
>> transferred by DMA, for the most part everything works and nobody
>> notices.
>>
>> But in the rare case that we need to write a blob to MMIO, it gets
>> transparently swapped in hardware so you need to use cpu_to_le in that
>> case. Doing a search of ./drivers for write.*cpu_to_le I can see this
>> issue comes up a bit.
> Every (PCI) driver does conversion to LE implicitly by using
> writel/readl (or iowrite32/ioread32) etc do the conversion to/from LE.
> So writel(foo, dst )is a __raw_writel(cpu_to_le32(foo), dst) etc. PCI
> memory is assumed to be in LE. If you are on a little endian system,
> then no byte swapping happens, and on BE it will do byte swapping
> before writing the value.

Okay so it seems that in the case of MIPS, that's not always how it
works.

https://github.com/torvalds/linux/blob/e53642b87a4f4b03a8d7e5f8507fc3cd0c595ea6/arch/mips/include/asm/mach-generic/mangle-port.h#L17

Since we don't know if the swap will happen in hardware or software
and (AFAIK) this is not a hot enough path that double-swapping will
have a notable performance penalty, I think the most sane thing to
do is use writel(cpu_to_le32()) and not care if it's swapped back
in the kernel or hardware.

I've just tested writel(get_unaligned_le32(src + i), dst + i);
and confirmed this is working.

Lastly, I'd like to just move everybody over rather than
special-casing Big Endian, but I don't have a Little Endian board
with an mt75x2e on it and no matter how trivial it is, I loathe to
submit a patch that hasn't been tested.

Is there somebody here who can get me a Tested-by for LE ?


Thanks,

Caleb


>
> Best regards,
> Jonas


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] wifi: mt76: mmio_*_copy fix byte order and alignment
  2025-10-29 15:24       ` Caleb James DeLisle
@ 2025-10-29 18:41         ` Caleb James DeLisle
  2025-10-29 20:12         ` [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian Jonas Gorski
  1 sibling, 0 replies; 12+ messages in thread
From: Caleb James DeLisle @ 2025-10-29 18:41 UTC (permalink / raw)
  To: nbd, lorenzo, jonas.gorski
  Cc: ryder.lee, shayne.chen, sean.wang, matthias.bgg,
	angelogioacchino.delregno, linux-wireless, linux-kernel,
	linux-mediatek, daniel, Caleb James DeLisle

Update functions which copy to and from MMIO to load bytes as Little
Endian, and also support unaligned buffers.

PCI devices almost universally use Little Endian ordering for MMIO
registers, mt76 is no exception. PCI hardware that is designed to work
with Big Endian CPUs often (but not always) "helps" by transparently
byte-swapping MMIO reads and writes on the wire. If this is enabled
then it cannot be turned off for a single write. On hardware which does
not support this, writel() does the swap in software. When we are
transferring arbitrary bytes to MMIO space, we need them to arrive in
the same order they were in memory, so when the hardware swaps them
this is a problem. Rather than care about how our PCI host controller
works, we instead load bytes as Little Endian - so on a Big Endian
machine this will reverse them, then we use writel() which will put
them back in the right order again. This way we do not make it our
business whether the swapping is done in software or hardware.

Furthermore, inspection of the code shows that these functions are
often called with stack-allocated u8 arrays which have no alignment
guarantees so we now use (get|put)_unaligned_le32().

Fixes:
mt76x2e 0000:02:00.0: ROM patch build: 20141115060606a
mt76x2e 0000:02:00.0: Firmware Version: 0.0.00
mt76x2e 0000:02:00.0: Build: 1
mt76x2e 0000:02:00.0: Build Time: 201607111443____
mt76x2e 0000:02:00.0: Firmware failed to start
mt76x2e 0000:02:00.0: probe with driver mt76x2e failed with error -145

Tested on:
SmartFiber XP8421-B (Big Endian MIPS 34Kc)
  - MT7612 -> 5g / ap / psk2
  - MT7603 -> 2g / sta / psk2
  - MT7603 -> 2g / ap / psk2
TpLink Archer v1200v-v2 (Big Endian MIPS 34Kc)
  - MT7613 -> 5g / ap / psk2
  - MT7603 -> 2g / sta / psk2

Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
---
 drivers/net/wireless/mediatek/mt76/mmio.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mmio.c b/drivers/net/wireless/mediatek/mt76/mmio.c
index cd2e9737c3bf..9f7e64f05b15 100644
--- a/drivers/net/wireless/mediatek/mt76/mmio.c
+++ b/drivers/net/wireless/mediatek/mt76/mmio.c
@@ -33,13 +33,23 @@ static u32 mt76_mmio_rmw(struct mt76_dev *dev, u32 offset, u32 mask, u32 val)
 static void mt76_mmio_write_copy(struct mt76_dev *dev, u32 offset,
 				 const void *data, int len)
 {
-	__iowrite32_copy(dev->mmio.regs + offset, data, DIV_ROUND_UP(len, 4));
+	int i;
+
+	for (i = 0; i < ALIGN(len, 4); i += 4) {
+		writel(get_unaligned_le32(data + i),
+		       dev->mmio.regs + offset + i);
+	}
 }
 
 static void mt76_mmio_read_copy(struct mt76_dev *dev, u32 offset,
 				void *data, int len)
 {
-	__ioread32_copy(data, dev->mmio.regs + offset, DIV_ROUND_UP(len, 4));
+	int i;
+
+	for (i = 0; i < ALIGN(len, 4); i += 4) {
+		put_unaligned_le32(readl(dev->mmio.regs + offset + i),
+				   data + i);
+	}
 }
 
 static int mt76_mmio_wr_rp(struct mt76_dev *dev, u32 base,
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian
  2025-10-29 15:24       ` Caleb James DeLisle
  2025-10-29 18:41         ` [PATCH v2] wifi: mt76: mmio_*_copy fix byte order and alignment Caleb James DeLisle
@ 2025-10-29 20:12         ` Jonas Gorski
  2025-10-29 20:40           ` Caleb James DeLisle
  1 sibling, 1 reply; 12+ messages in thread
From: Jonas Gorski @ 2025-10-29 20:12 UTC (permalink / raw)
  To: Caleb James DeLisle
  Cc: nbd, lorenzo, ryder.lee, shayne.chen, sean.wang, matthias.bgg,
	angelogioacchino.delregno, linux-wireless, linux-kernel,
	linux-mediatek, Daniel Golle

On Wed, Oct 29, 2025 at 4:24 PM Caleb James DeLisle <cjd@cjdns.fr> wrote:
>
>
> On 29/10/2025 10:15, Jonas Gorski wrote:
> > On Tue, Oct 28, 2025 at 10:42 PM Caleb James DeLisle <cjd@cjdns.fr> wrote:
> >>
> >> On 28/10/2025 21:19, Jonas Gorski wrote:
> >>> Hi,
> >>>
> >>> On Mon, Oct 27, 2025 at 6:19 PM Caleb James DeLisle <cjd@cjdns.fr> wrote:
> >>>> When on a Big Endian machine, PCI swaps words to/from LE when
> >>>> reading/writing them. This presents a problem when we're trying
> >>>> to copy an opaque byte array such as firmware or encryption key.
> >>>>
> >>>> Byte-swapping during copy results in two swaps, but solves the
> >>>> problem.
> >>>>
> >>>> Fixes:
> >>>> mt76x2e 0000:02:00.0: ROM patch build: 20141115060606a
> >>>> mt76x2e 0000:02:00.0: Firmware Version: 0.0.00
> >>>> mt76x2e 0000:02:00.0: Build: 1
> >>>> mt76x2e 0000:02:00.0: Build Time: 201607111443____
> >>>> mt76x2e 0000:02:00.0: Firmware failed to start
> >>>> mt76x2e 0000:02:00.0: probe with driver mt76x2e failed with error -145
> >>>>
> >>>> Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
> >>>> ---
> >>>>    drivers/net/wireless/mediatek/mt76/mmio.c | 34 +++++++++++++++++++++++
> >>>>    1 file changed, 34 insertions(+)
> >>>>
> >>>> diff --git a/drivers/net/wireless/mediatek/mt76/mmio.c b/drivers/net/wireless/mediatek/mt76/mmio.c
> >>>> index cd2e9737c3bf..776dbaacc8a3 100644
> >>>> --- a/drivers/net/wireless/mediatek/mt76/mmio.c
> >>>> +++ b/drivers/net/wireless/mediatek/mt76/mmio.c
> >>>> @@ -30,15 +30,49 @@ static u32 mt76_mmio_rmw(struct mt76_dev *dev, u32 offset, u32 mask, u32 val)
> >>>>           return val;
> >>>>    }
> >>>>
> >>>> +static void mt76_mmio_write_copy_portable(void __iomem *dst,
> >>>> +                                         const u8 *src, int len)
> >>>> +{
> >>>> +       __le32 val;
> >>>> +       int i = 0;
> >>>> +
> >>>> +       for (i = 0; i < ALIGN(len, 4); i += 4) {
> >>>> +               memcpy(&val, src + i, sizeof(val));
> >>>> +               writel(cpu_to_le32(val), dst + i);
> >>>> +       }
> >>>> +}
> >>>> +
> >>>>    static void mt76_mmio_write_copy(struct mt76_dev *dev, u32 offset,
> >>>>                                    const void *data, int len)
> >>>>    {
> >>>> +       if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) {
> >>>> +               mt76_mmio_write_copy_portable(dev->mmio.regs + offset, data,
> >>>> +                                             len);
> >>>> +               return;
> >>>> +       }
> >>>>           __iowrite32_copy(dev->mmio.regs + offset, data, DIV_ROUND_UP(len, 4));
> >>> Maybe just replace this with memcpy_toio() which does no swapping at
> >>> all instead of double swapping on BE?
> >>
> >> I'm not that informed about how PCI works so I had to test to confirm
> >> my understanding, but I can confirm that memcpy_toio() does not solve
> >> the problem.
> > Ah, right, I misread _iowrite32_copy() to do conversion to LE, but it doesn't.
> >
> > What architecture is this you have? PowerPC? ARM? MIPS? 32 bit? 64 bit?
>
>
> MIPS32 (EcoNet EN751221 34Kc)
>
>
> >
> > So the differences I see are:
> >
> > 1. __iowrite32_copy() uses __raw_writel(), which has different memory
> > semantics than writel()
> > 2. __iowrite32_copy() assumed src is aligned to 32 bit, while you
> > explicitly align it
> > 3. memcpy_toio() will handle unaligned src properly, but does 64 bit
> > accesses on 64 bit systems (and uses __raw* again).
> >
> > Is src aligned? If not, then the issue might be 2. And if your system
> > is 64 bit, it would explain why 3 didn't help.
>
>
> I'm not a regular developer of mt76 so I wasn't sure if that was
> guaranteed and I just wanted to code for safety.
>
> After reviewing the code, I see that there are a few places where
> mt76_mmio_write_copy is being called with stack-allocated u8 arrays
> so it's pretty clear to me that this is being treated as a memcpy-like
> function and we should be handling unaligned inputs.
>
>
> >
> > As a first step you could try to replace the writel(cpu_to_le32(val)
> > with a iowrite32be(val, ...) which should do the same except avoiding
> > the doubled byte swapping. If that works, you can try to replace it
>
> This works.
>
> These symbols are a bit of a nightmare to trace, so I ended up making
> an .i file so I could confirm what's happening.
>
> iowrite32be() uses the version in iomap.c so I understand that's using
> writel(swab32(val),port), so a writel with an unconditional byte swap.

>
> writel() is more complicated, it's an inline function that is generated
> in a rat's nest of preprocessor macros in mips/include/asm/io.h
>
> The preprocessed is this:
>
> __mem = (void *)((unsigned long)(mem)); __val = (val); if (sizeof(u32)
> != sizeof(u64) || sizeof(u64) == sizeof(long)) { *__mem = __val;
>
> The source is this:
>
>      __mem = (void *)__swizzle_addr_##bwlq((unsigned long)(mem));    \
>      __val = pfx##ioswab##bwlq(__mem, val);                \
>      if (sizeof(type) != sizeof(u64) || sizeof(u64) == sizeof(long)) \
>          *__mem = __val;                        \
>
> The line "pfx##ioswab##bwlq(__mem, val);" is ioswabl() and the source
> of that explains the issue:
>
>   * Sane hardware offers swapping of PCI/ISA I/O space accesses in hardware;
>   * less sane hardware forces software to fiddle with this...
>
> So this confirms my initial understanding, the PCI hardware is doing the
> byte swapping unconditionally.
>
>
> > with __raw_writel(), which then would make this the same as
> > __iowrite32_copy, except making sure that src is aligned.
>
>
> This fails.
>
> Since I'm the maintainer of this SoC and it's still fairly new, I wrote
> a trivial kmod to verify that unaligned access is not just silently
> returning trash, it works as though it were aligned so alignment is
> not the issue.
>
>
> >
> > Also you could replace your memcpy() with get_unaligned((u32 *)(src +
> > i)); Should do the same but inline.
> Good idea, I will do this.
> >> The issue as I understand it is that rather than making every driver
> >> carefully call cpu_to_le*() every MMIO write, someone decided to make
> >> the PCI host bridge itself transparently byte-swap all MMIO on the
> >> wire. Since most MMIO is hitting registers and most buffers are
> >> transferred by DMA, for the most part everything works and nobody
> >> notices.
> >>
> >> But in the rare case that we need to write a blob to MMIO, it gets
> >> transparently swapped in hardware so you need to use cpu_to_le in that
> >> case. Doing a search of ./drivers for write.*cpu_to_le I can see this
> >> issue comes up a bit.
> > Every (PCI) driver does conversion to LE implicitly by using
> > writel/readl (or iowrite32/ioread32) etc do the conversion to/from LE.
> > So writel(foo, dst )is a __raw_writel(cpu_to_le32(foo), dst) etc. PCI
> > memory is assumed to be in LE. If you are on a little endian system,
> > then no byte swapping happens, and on BE it will do byte swapping
> > before writing the value.
>
> Okay so it seems that in the case of MIPS, that's not always how it
> works.
>
> https://github.com/torvalds/linux/blob/e53642b87a4f4b03a8d7e5f8507fc3cd0c595ea6/arch/mips/include/asm/mach-generic/mangle-port.h#L17
>
> Since we don't know if the swap will happen in hardware or software
> and (AFAIK) this is not a hot enough path that double-swapping will
> have a notable performance penalty, I think the most sane thing to
> do is use writel(cpu_to_le32()) and not care if it's swapped back
> in the kernel or hardware.

Oh, I think I see what it happening here. ECONET is a Big Endian MIPS
platform, but does not select SWAP_IO_SPACE (most other BE platforms
do).

Does that mean the PCI space is swapped in hardware there?

I guess that means that anything that uses __raw accessors to PCI
space directly or indirectly is broken, as the raw data is now
actually in the wrong order and needs to be swab'd.

I don't know if it is a good idea to change this in __iowrite32_copy()
/ __ioread32_copy() (and other helpers), or if there are drivers that
use it on non-PCI spaces and would be broken by that.

If there is a way, I would suggest disabling hardware conversion and
selecting SWAP_IO_SPACE, but that will affect a lot of your code that
assumes that writel() etc don't convert to/from little endian.

Best regards,
Jonas


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian
  2025-10-29 20:12         ` [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian Jonas Gorski
@ 2025-10-29 20:40           ` Caleb James DeLisle
  2025-10-29 23:06             ` Caleb James DeLisle
  0 siblings, 1 reply; 12+ messages in thread
From: Caleb James DeLisle @ 2025-10-29 20:40 UTC (permalink / raw)
  To: Jonas Gorski
  Cc: nbd, lorenzo, ryder.lee, shayne.chen, sean.wang, matthias.bgg,
	angelogioacchino.delregno, linux-wireless, linux-kernel,
	linux-mediatek, Daniel Golle


On 29/10/2025 21:12, Jonas Gorski wrote:
> On Wed, Oct 29, 2025 at 4:24 PM Caleb James DeLisle <cjd@cjdns.fr> wrote:
>>
>> On 29/10/2025 10:15, Jonas Gorski wrote:
>>> On Tue, Oct 28, 2025 at 10:42 PM Caleb James DeLisle <cjd@cjdns.fr> wrote:
>>>> On 28/10/2025 21:19, Jonas Gorski wrote:
>>>>> Hi,
>>>>>
>>>>> On Mon, Oct 27, 2025 at 6:19 PM Caleb James DeLisle <cjd@cjdns.fr> wrote:
>>>>>> When on a Big Endian machine, PCI swaps words to/from LE when
>>>>>> reading/writing them. This presents a problem when we're trying
>>>>>> to copy an opaque byte array such as firmware or encryption key.
>>>>>>
>>>>>> Byte-swapping during copy results in two swaps, but solves the
>>>>>> problem.
>>>>>>
>>>>>> Fixes:
>>>>>> mt76x2e 0000:02:00.0: ROM patch build: 20141115060606a
>>>>>> mt76x2e 0000:02:00.0: Firmware Version: 0.0.00
>>>>>> mt76x2e 0000:02:00.0: Build: 1
>>>>>> mt76x2e 0000:02:00.0: Build Time: 201607111443____
>>>>>> mt76x2e 0000:02:00.0: Firmware failed to start
>>>>>> mt76x2e 0000:02:00.0: probe with driver mt76x2e failed with error -145
>>>>>>
>>>>>> Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
>>>>>> ---
>>>>>>     drivers/net/wireless/mediatek/mt76/mmio.c | 34 +++++++++++++++++++++++
>>>>>>     1 file changed, 34 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/net/wireless/mediatek/mt76/mmio.c b/drivers/net/wireless/mediatek/mt76/mmio.c
>>>>>> index cd2e9737c3bf..776dbaacc8a3 100644
>>>>>> --- a/drivers/net/wireless/mediatek/mt76/mmio.c
>>>>>> +++ b/drivers/net/wireless/mediatek/mt76/mmio.c
>>>>>> @@ -30,15 +30,49 @@ static u32 mt76_mmio_rmw(struct mt76_dev *dev, u32 offset, u32 mask, u32 val)
>>>>>>            return val;
>>>>>>     }
>>>>>>
>>>>>> +static void mt76_mmio_write_copy_portable(void __iomem *dst,
>>>>>> +                                         const u8 *src, int len)
>>>>>> +{
>>>>>> +       __le32 val;
>>>>>> +       int i = 0;
>>>>>> +
>>>>>> +       for (i = 0; i < ALIGN(len, 4); i += 4) {
>>>>>> +               memcpy(&val, src + i, sizeof(val));
>>>>>> +               writel(cpu_to_le32(val), dst + i);
>>>>>> +       }
>>>>>> +}
>>>>>> +
>>>>>>     static void mt76_mmio_write_copy(struct mt76_dev *dev, u32 offset,
>>>>>>                                     const void *data, int len)
>>>>>>     {
>>>>>> +       if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) {
>>>>>> +               mt76_mmio_write_copy_portable(dev->mmio.regs + offset, data,
>>>>>> +                                             len);
>>>>>> +               return;
>>>>>> +       }
>>>>>>            __iowrite32_copy(dev->mmio.regs + offset, data, DIV_ROUND_UP(len, 4));
>>>>> Maybe just replace this with memcpy_toio() which does no swapping at
>>>>> all instead of double swapping on BE?
>>>> I'm not that informed about how PCI works so I had to test to confirm
>>>> my understanding, but I can confirm that memcpy_toio() does not solve
>>>> the problem.
>>> Ah, right, I misread _iowrite32_copy() to do conversion to LE, but it doesn't.
>>>
>>> What architecture is this you have? PowerPC? ARM? MIPS? 32 bit? 64 bit?
>>
>> MIPS32 (EcoNet EN751221 34Kc)
>>
>>
>>> So the differences I see are:
>>>
>>> 1. __iowrite32_copy() uses __raw_writel(), which has different memory
>>> semantics than writel()
>>> 2. __iowrite32_copy() assumed src is aligned to 32 bit, while you
>>> explicitly align it
>>> 3. memcpy_toio() will handle unaligned src properly, but does 64 bit
>>> accesses on 64 bit systems (and uses __raw* again).
>>>
>>> Is src aligned? If not, then the issue might be 2. And if your system
>>> is 64 bit, it would explain why 3 didn't help.
>>
>> I'm not a regular developer of mt76 so I wasn't sure if that was
>> guaranteed and I just wanted to code for safety.
>>
>> After reviewing the code, I see that there are a few places where
>> mt76_mmio_write_copy is being called with stack-allocated u8 arrays
>> so it's pretty clear to me that this is being treated as a memcpy-like
>> function and we should be handling unaligned inputs.
>>
>>
>>> As a first step you could try to replace the writel(cpu_to_le32(val)
>>> with a iowrite32be(val, ...) which should do the same except avoiding
>>> the doubled byte swapping. If that works, you can try to replace it
>> This works.
>>
>> These symbols are a bit of a nightmare to trace, so I ended up making
>> an .i file so I could confirm what's happening.
>>
>> iowrite32be() uses the version in iomap.c so I understand that's using
>> writel(swab32(val),port), so a writel with an unconditional byte swap.
>> writel() is more complicated, it's an inline function that is generated
>> in a rat's nest of preprocessor macros in mips/include/asm/io.h
>>
>> The preprocessed is this:
>>
>> __mem = (void *)((unsigned long)(mem)); __val = (val); if (sizeof(u32)
>> != sizeof(u64) || sizeof(u64) == sizeof(long)) { *__mem = __val;
>>
>> The source is this:
>>
>>       __mem = (void *)__swizzle_addr_##bwlq((unsigned long)(mem));    \
>>       __val = pfx##ioswab##bwlq(__mem, val);                \
>>       if (sizeof(type) != sizeof(u64) || sizeof(u64) == sizeof(long)) \
>>           *__mem = __val;                        \
>>
>> The line "pfx##ioswab##bwlq(__mem, val);" is ioswabl() and the source
>> of that explains the issue:
>>
>>    * Sane hardware offers swapping of PCI/ISA I/O space accesses in hardware;
>>    * less sane hardware forces software to fiddle with this...
>>
>> So this confirms my initial understanding, the PCI hardware is doing the
>> byte swapping unconditionally.
>>
>>
>>> with __raw_writel(), which then would make this the same as
>>> __iowrite32_copy, except making sure that src is aligned.
>>
>> This fails.
>>
>> Since I'm the maintainer of this SoC and it's still fairly new, I wrote
>> a trivial kmod to verify that unaligned access is not just silently
>> returning trash, it works as though it were aligned so alignment is
>> not the issue.
>>
>>
>>> Also you could replace your memcpy() with get_unaligned((u32 *)(src +
>>> i)); Should do the same but inline.
>> Good idea, I will do this.
>>>> The issue as I understand it is that rather than making every driver
>>>> carefully call cpu_to_le*() every MMIO write, someone decided to make
>>>> the PCI host bridge itself transparently byte-swap all MMIO on the
>>>> wire. Since most MMIO is hitting registers and most buffers are
>>>> transferred by DMA, for the most part everything works and nobody
>>>> notices.
>>>>
>>>> But in the rare case that we need to write a blob to MMIO, it gets
>>>> transparently swapped in hardware so you need to use cpu_to_le in that
>>>> case. Doing a search of ./drivers for write.*cpu_to_le I can see this
>>>> issue comes up a bit.
>>> Every (PCI) driver does conversion to LE implicitly by using
>>> writel/readl (or iowrite32/ioread32) etc do the conversion to/from LE.
>>> So writel(foo, dst )is a __raw_writel(cpu_to_le32(foo), dst) etc. PCI
>>> memory is assumed to be in LE. If you are on a little endian system,
>>> then no byte swapping happens, and on BE it will do byte swapping
>>> before writing the value.
>> Okay so it seems that in the case of MIPS, that's not always how it
>> works.
>>
>> https://github.com/torvalds/linux/blob/e53642b87a4f4b03a8d7e5f8507fc3cd0c595ea6/arch/mips/include/asm/mach-generic/mangle-port.h#L17
>>
>> Since we don't know if the swap will happen in hardware or software
>> and (AFAIK) this is not a hot enough path that double-swapping will
>> have a notable performance penalty, I think the most sane thing to
>> do is use writel(cpu_to_le32()) and not care if it's swapped back
>> in the kernel or hardware.
> Oh, I think I see what it happening here. ECONET is a Big Endian MIPS
> platform, but does not select SWAP_IO_SPACE (most other BE platforms
> do).
>
> Does that mean the PCI space is swapped in hardware there?
>
> I guess that means that anything that uses __raw accessors to PCI
> space directly or indirectly is broken, as the raw data is now
> actually in the wrong order and needs to be swab'd.
>
> I don't know if it is a good idea to change this in __iowrite32_copy()
> / __ioread32_copy() (and other helpers), or if there are drivers that
> use it on non-PCI spaces and would be broken by that.
>
> If there is a way, I would suggest disabling hardware conversion and
> selecting SWAP_IO_SPACE, but that will affect a lot of your code that
> assumes that writel() etc don't convert to/from little endian.


I can look around in the hardware registers and see if I can shut it
off for EcoNet, but if you're saying MT76 should not support BE unless
they disable hardware swapping and use SWAP_IO_SPACE, that means the
majority of BE hardware on OpenWrt is not going to be supported. If
that's the decision then it at least warrants clear documentation.

Thanks,
Caleb


user@cjd-dev:~/en7526/openwrt$ find ./ -name 'config-6.12' | while read 
x; do grep -q 'CPU_BIG_ENDIAN=y' "$x" && ( grep -q 'SWAP_IO_SPACE=y' 
"$x" || echo "$x
  does not use SWAP_IO_SPACE" ) ; done
./target/linux/apm821xx/config-6.12 does not use SWAP_IO_SPACE
./target/linux/realtek/rtl931x/config-6.12 does not use SWAP_IO_SPACE
./target/linux/realtek/rtl930x_nand/config-6.12 does not use SWAP_IO_SPACE
./target/linux/realtek/rtl839x/config-6.12 does not use SWAP_IO_SPACE
./target/linux/realtek/rtl931x_nand/config-6.12 does not use SWAP_IO_SPACE
./target/linux/realtek/rtl930x/config-6.12 does not use SWAP_IO_SPACE
./target/linux/realtek/rtl838x/config-6.12 does not use SWAP_IO_SPACE
./target/linux/ath79/config-6.12 does not use SWAP_IO_SPACE
./target/linux/octeon/config-6.12 does not use SWAP_IO_SPACE
./target/linux/ixp4xx/config-6.12 does not use SWAP_IO_SPACE
./target/linux/econet/en751221/config-6.12 does not use SWAP_IO_SPACE
user@cjd-dev:~/en7526/openwrt$ find ./ -name 'config-6.12' | while read 
x; do grep -q 'CPU_BIG_ENDIAN=y' "$x" && ( grep -q 'SWAP_IO_SPACE=y' 
"$x" && echo "$x
  does use SWAP_IO_SPACE" ) ; done
./target/linux/bmips/bcm6358/config-6.12 does use SWAP_IO_SPACE
./target/linux/bmips/bcm6328/config-6.12 does use SWAP_IO_SPACE
./target/linux/bmips/bcm6318/config-6.12 does use SWAP_IO_SPACE
./target/linux/bmips/bcm6368/config-6.12 does use SWAP_IO_SPACE
./target/linux/bmips/bcm6362/config-6.12 does use SWAP_IO_SPACE
./target/linux/bmips/bcm63268/config-6.12 does use SWAP_IO_SPACE
./target/linux/lantiq/config-6.12 does use SWAP_IO_SPACE
user@cjd-dev:~/en7526/openwrt$


>
> Best regards,
> Jonas
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian
  2025-10-29 20:40           ` Caleb James DeLisle
@ 2025-10-29 23:06             ` Caleb James DeLisle
  0 siblings, 0 replies; 12+ messages in thread
From: Caleb James DeLisle @ 2025-10-29 23:06 UTC (permalink / raw)
  To: Jonas Gorski
  Cc: nbd, lorenzo, ryder.lee, shayne.chen, sean.wang, matthias.bgg,
	angelogioacchino.delregno, linux-wireless, linux-kernel,
	linux-mediatek, Daniel Golle


On 29/10/2025 21:40, Caleb James DeLisle wrote:
>
> On 29/10/2025 21:12, Jonas Gorski wrote:
>> On Wed, Oct 29, 2025 at 4:24 PM Caleb James DeLisle <cjd@cjdns.fr> 
>> wrote:
>>>
>>> On 29/10/2025 10:15, Jonas Gorski wrote:
>>>> On Tue, Oct 28, 2025 at 10:42 PM Caleb James DeLisle <cjd@cjdns.fr> 
>>>> wrote:
>>>>> On 28/10/2025 21:19, Jonas Gorski wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On Mon, Oct 27, 2025 at 6:19 PM Caleb James DeLisle 
>>>>>> <cjd@cjdns.fr> wrote:
>>>>>>> When on a Big Endian machine, PCI swaps words to/from LE when
>>>>>>> reading/writing them. This presents a problem when we're trying
>>>>>>> to copy an opaque byte array such as firmware or encryption key.
>>>>>>>
>>>>>>> Byte-swapping during copy results in two swaps, but solves the
>>>>>>> problem.
>>>>>>>
>>>>>>> Fixes:
>>>>>>> mt76x2e 0000:02:00.0: ROM patch build: 20141115060606a
>>>>>>> mt76x2e 0000:02:00.0: Firmware Version: 0.0.00
>>>>>>> mt76x2e 0000:02:00.0: Build: 1
>>>>>>> mt76x2e 0000:02:00.0: Build Time: 201607111443____
>>>>>>> mt76x2e 0000:02:00.0: Firmware failed to start
>>>>>>> mt76x2e 0000:02:00.0: probe with driver mt76x2e failed with 
>>>>>>> error -145
>>>>>>>
>>>>>>> Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
>>>>>>> ---
>>>>>>>     drivers/net/wireless/mediatek/mt76/mmio.c | 34 
>>>>>>> +++++++++++++++++++++++
>>>>>>>     1 file changed, 34 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/net/wireless/mediatek/mt76/mmio.c 
>>>>>>> b/drivers/net/wireless/mediatek/mt76/mmio.c
>>>>>>> index cd2e9737c3bf..776dbaacc8a3 100644
>>>>>>> --- a/drivers/net/wireless/mediatek/mt76/mmio.c
>>>>>>> +++ b/drivers/net/wireless/mediatek/mt76/mmio.c
>>>>>>> @@ -30,15 +30,49 @@ static u32 mt76_mmio_rmw(struct mt76_dev 
>>>>>>> *dev, u32 offset, u32 mask, u32 val)
>>>>>>>            return val;
>>>>>>>     }
>>>>>>>
>>>>>>> +static void mt76_mmio_write_copy_portable(void __iomem *dst,
>>>>>>> +                                         const u8 *src, int len)
>>>>>>> +{
>>>>>>> +       __le32 val;
>>>>>>> +       int i = 0;
>>>>>>> +
>>>>>>> +       for (i = 0; i < ALIGN(len, 4); i += 4) {
>>>>>>> +               memcpy(&val, src + i, sizeof(val));
>>>>>>> +               writel(cpu_to_le32(val), dst + i);
>>>>>>> +       }
>>>>>>> +}
>>>>>>> +
>>>>>>>     static void mt76_mmio_write_copy(struct mt76_dev *dev, u32 
>>>>>>> offset,
>>>>>>>                                     const void *data, int len)
>>>>>>>     {
>>>>>>> +       if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) {
>>>>>>> + mt76_mmio_write_copy_portable(dev->mmio.regs + offset, data,
>>>>>>> +                                             len);
>>>>>>> +               return;
>>>>>>> +       }
>>>>>>>            __iowrite32_copy(dev->mmio.regs + offset, data, 
>>>>>>> DIV_ROUND_UP(len, 4));
>>>>>> Maybe just replace this with memcpy_toio() which does no swapping at
>>>>>> all instead of double swapping on BE?
>>>>> I'm not that informed about how PCI works so I had to test to confirm
>>>>> my understanding, but I can confirm that memcpy_toio() does not solve
>>>>> the problem.
>>>> Ah, right, I misread _iowrite32_copy() to do conversion to LE, but 
>>>> it doesn't.
>>>>
>>>> What architecture is this you have? PowerPC? ARM? MIPS? 32 bit? 64 
>>>> bit?
>>>
>>> MIPS32 (EcoNet EN751221 34Kc)
>>>
>>>
>>>> So the differences I see are:
>>>>
>>>> 1. __iowrite32_copy() uses __raw_writel(), which has different memory
>>>> semantics than writel()
>>>> 2. __iowrite32_copy() assumed src is aligned to 32 bit, while you
>>>> explicitly align it
>>>> 3. memcpy_toio() will handle unaligned src properly, but does 64 bit
>>>> accesses on 64 bit systems (and uses __raw* again).
>>>>
>>>> Is src aligned? If not, then the issue might be 2. And if your system
>>>> is 64 bit, it would explain why 3 didn't help.
>>>
>>> I'm not a regular developer of mt76 so I wasn't sure if that was
>>> guaranteed and I just wanted to code for safety.
>>>
>>> After reviewing the code, I see that there are a few places where
>>> mt76_mmio_write_copy is being called with stack-allocated u8 arrays
>>> so it's pretty clear to me that this is being treated as a memcpy-like
>>> function and we should be handling unaligned inputs.
>>>
>>>
>>>> As a first step you could try to replace the writel(cpu_to_le32(val)
>>>> with a iowrite32be(val, ...) which should do the same except avoiding
>>>> the doubled byte swapping. If that works, you can try to replace it
>>> This works.
>>>
>>> These symbols are a bit of a nightmare to trace, so I ended up making
>>> an .i file so I could confirm what's happening.
>>>
>>> iowrite32be() uses the version in iomap.c so I understand that's using
>>> writel(swab32(val),port), so a writel with an unconditional byte swap.
>>> writel() is more complicated, it's an inline function that is generated
>>> in a rat's nest of preprocessor macros in mips/include/asm/io.h
>>>
>>> The preprocessed is this:
>>>
>>> __mem = (void *)((unsigned long)(mem)); __val = (val); if (sizeof(u32)
>>> != sizeof(u64) || sizeof(u64) == sizeof(long)) { *__mem = __val;
>>>
>>> The source is this:
>>>
>>>       __mem = (void *)__swizzle_addr_##bwlq((unsigned long)(mem));    \
>>>       __val = pfx##ioswab##bwlq(__mem, val);                \
>>>       if (sizeof(type) != sizeof(u64) || sizeof(u64) == sizeof(long)) \
>>>           *__mem = __val;                        \
>>>
>>> The line "pfx##ioswab##bwlq(__mem, val);" is ioswabl() and the source
>>> of that explains the issue:
>>>
>>>    * Sane hardware offers swapping of PCI/ISA I/O space accesses in 
>>> hardware;
>>>    * less sane hardware forces software to fiddle with this...
>>>
>>> So this confirms my initial understanding, the PCI hardware is doing 
>>> the
>>> byte swapping unconditionally.
>>>
>>>
>>>> with __raw_writel(), which then would make this the same as
>>>> __iowrite32_copy, except making sure that src is aligned.
>>>
>>> This fails.
>>>
>>> Since I'm the maintainer of this SoC and it's still fairly new, I wrote
>>> a trivial kmod to verify that unaligned access is not just silently
>>> returning trash, it works as though it were aligned so alignment is
>>> not the issue.
>>>
>>>
>>>> Also you could replace your memcpy() with get_unaligned((u32 *)(src +
>>>> i)); Should do the same but inline.
>>> Good idea, I will do this.
>>>>> The issue as I understand it is that rather than making every driver
>>>>> carefully call cpu_to_le*() every MMIO write, someone decided to make
>>>>> the PCI host bridge itself transparently byte-swap all MMIO on the
>>>>> wire. Since most MMIO is hitting registers and most buffers are
>>>>> transferred by DMA, for the most part everything works and nobody
>>>>> notices.
>>>>>
>>>>> But in the rare case that we need to write a blob to MMIO, it gets
>>>>> transparently swapped in hardware so you need to use cpu_to_le in 
>>>>> that
>>>>> case. Doing a search of ./drivers for write.*cpu_to_le I can see this
>>>>> issue comes up a bit.
>>>> Every (PCI) driver does conversion to LE implicitly by using
>>>> writel/readl (or iowrite32/ioread32) etc do the conversion to/from LE.
>>>> So writel(foo, dst )is a __raw_writel(cpu_to_le32(foo), dst) etc. PCI
>>>> memory is assumed to be in LE. If you are on a little endian system,
>>>> then no byte swapping happens, and on BE it will do byte swapping
>>>> before writing the value.
>>> Okay so it seems that in the case of MIPS, that's not always how it
>>> works.
>>>
>>> https://github.com/torvalds/linux/blob/e53642b87a4f4b03a8d7e5f8507fc3cd0c595ea6/arch/mips/include/asm/mach-generic/mangle-port.h#L17 
>>>
>>>
>>> Since we don't know if the swap will happen in hardware or software
>>> and (AFAIK) this is not a hot enough path that double-swapping will
>>> have a notable performance penalty, I think the most sane thing to
>>> do is use writel(cpu_to_le32()) and not care if it's swapped back
>>> in the kernel or hardware.
>> Oh, I think I see what it happening here. ECONET is a Big Endian MIPS
>> platform, but does not select SWAP_IO_SPACE (most other BE platforms
>> do).
>>
>> Does that mean the PCI space is swapped in hardware there?
>>
>> I guess that means that anything that uses __raw accessors to PCI
>> space directly or indirectly is broken, as the raw data is now
>> actually in the wrong order and needs to be swab'd.
>>
>> I don't know if it is a good idea to change this in __iowrite32_copy()
>> / __ioread32_copy() (and other helpers), or if there are drivers that
>> use it on non-PCI spaces and would be broken by that.
>>
>> If there is a way, I would suggest disabling hardware conversion and
>> selecting SWAP_IO_SPACE, but that will affect a lot of your code that
>> assumes that writel() etc don't convert to/from little endian.
>
>
> I can look around in the hardware registers and see if I can shut it
> off for EcoNet, but if you're saying MT76 should not support BE unless
> they disable hardware swapping and use SWAP_IO_SPACE, that means the
> majority of BE hardware on OpenWrt is not going to be supported. If
> that's the decision then it at least warrants clear documentation.


Update: I tested an MT7921 USB and it works fine with the patched
driver, I checked the codepath and it is never used for USB anyway,
it's only for PCI and mt7628-wmac, a direct-wired wlan on the MT7628
which is Little Endian.

Also there's no way I can switch this to use SWAP_IO_SPACE because it
uses mtk-xhci for USB and that expects writel() to not swap bytes.

I don't see why this patch should be controversial - it fixes a bug
that breaks 6 OpenWrt platforms and there's no evidence that it breaks
anything. Not only that but even if these platforms are considered low
priority because Big Endian is a historical artifact, that's all the
more reason why nobody should mind the addition of a cpu_to_le32()
call which on Little Endian is a no-op.

In any case, I would appreciate if you could look at my v2 because
I switched to using (get|put)_unaligned_le32() which is much nicer.
Thank you for the advice on that point.

Thanks,

Caleb



>
> Thanks,
> Caleb
>
>
> user@cjd-dev:~/en7526/openwrt$ find ./ -name 'config-6.12' | while 
> read x; do grep -q 'CPU_BIG_ENDIAN=y' "$x" && ( grep -q 
> 'SWAP_IO_SPACE=y' "$x" || echo "$x
>  does not use SWAP_IO_SPACE" ) ; done
> ./target/linux/apm821xx/config-6.12 does not use SWAP_IO_SPACE
> ./target/linux/realtek/rtl931x/config-6.12 does not use SWAP_IO_SPACE
> ./target/linux/realtek/rtl930x_nand/config-6.12 does not use 
> SWAP_IO_SPACE
> ./target/linux/realtek/rtl839x/config-6.12 does not use SWAP_IO_SPACE
> ./target/linux/realtek/rtl931x_nand/config-6.12 does not use 
> SWAP_IO_SPACE
> ./target/linux/realtek/rtl930x/config-6.12 does not use SWAP_IO_SPACE
> ./target/linux/realtek/rtl838x/config-6.12 does not use SWAP_IO_SPACE
> ./target/linux/ath79/config-6.12 does not use SWAP_IO_SPACE
> ./target/linux/octeon/config-6.12 does not use SWAP_IO_SPACE
> ./target/linux/ixp4xx/config-6.12 does not use SWAP_IO_SPACE
> ./target/linux/econet/en751221/config-6.12 does not use SWAP_IO_SPACE
> user@cjd-dev:~/en7526/openwrt$ find ./ -name 'config-6.12' | while 
> read x; do grep -q 'CPU_BIG_ENDIAN=y' "$x" && ( grep -q 
> 'SWAP_IO_SPACE=y' "$x" && echo "$x
>  does use SWAP_IO_SPACE" ) ; done
> ./target/linux/bmips/bcm6358/config-6.12 does use SWAP_IO_SPACE
> ./target/linux/bmips/bcm6328/config-6.12 does use SWAP_IO_SPACE
> ./target/linux/bmips/bcm6318/config-6.12 does use SWAP_IO_SPACE
> ./target/linux/bmips/bcm6368/config-6.12 does use SWAP_IO_SPACE
> ./target/linux/bmips/bcm6362/config-6.12 does use SWAP_IO_SPACE
> ./target/linux/bmips/bcm63268/config-6.12 does use SWAP_IO_SPACE
> ./target/linux/lantiq/config-6.12 does use SWAP_IO_SPACE
> user@cjd-dev:~/en7526/openwrt$
>
>
>>
>> Best regards,
>> Jonas
>>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian
  2025-10-27 17:17 [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian Caleb James DeLisle
  2025-10-28 20:19 ` Jonas Gorski
@ 2025-10-31  1:18 ` kernel test robot
  2025-10-31  8:22   ` Caleb James DeLisle
  2025-11-01 16:25 ` kernel test robot
  2 siblings, 1 reply; 12+ messages in thread
From: kernel test robot @ 2025-10-31  1:18 UTC (permalink / raw)
  To: Caleb James DeLisle, nbd, lorenzo
  Cc: oe-kbuild-all, ryder.lee, shayne.chen, sean.wang, matthias.bgg,
	angelogioacchino.delregno, linux-wireless, linux-kernel,
	linux-mediatek, Caleb James DeLisle

Hi Caleb,

kernel test robot noticed the following build warnings:

[auto build test WARNING on wireless-next/main]
[also build test WARNING on wireless/main linus/master v6.18-rc3 next-20251030]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Caleb-James-DeLisle/wifi-mt76-mmio_-read-write-_copy-byte-swap-when-on-Big-Endian/20251028-012349
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
patch link:    https://lore.kernel.org/r/20251027171759.1484844-1-cjd%40cjdns.fr
patch subject: [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian
config: i386-randconfig-061-20251031 (https://download.01.org/0day-ci/archive/20251031/202510310816.kyDHJNiS-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251031/202510310816.kyDHJNiS-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510310816.kyDHJNiS-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/net/wireless/mediatek/mt76/mmio.c:41:24: sparse: sparse: cast from restricted __le32
>> drivers/net/wireless/mediatek/mt76/mmio.c:41:24: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int val @@     got restricted __le32 [usertype] @@
   drivers/net/wireless/mediatek/mt76/mmio.c:41:24: sparse:     expected unsigned int val
   drivers/net/wireless/mediatek/mt76/mmio.c:41:24: sparse:     got restricted __le32 [usertype]
>> drivers/net/wireless/mediatek/mt76/mmio.c:63:23: sparse: sparse: cast to restricted __le32

vim +41 drivers/net/wireless/mediatek/mt76/mmio.c

    32	
    33	static void mt76_mmio_write_copy_portable(void __iomem *dst,
    34						  const u8 *src, int len)
    35	{
    36		__le32 val;
    37		int i = 0;
    38	
    39		for (i = 0; i < ALIGN(len, 4); i += 4) {
    40			memcpy(&val, src + i, sizeof(val));
  > 41			writel(cpu_to_le32(val), dst + i);
    42		}
    43	}
    44	
    45	static void mt76_mmio_write_copy(struct mt76_dev *dev, u32 offset,
    46					 const void *data, int len)
    47	{
    48		if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) {
    49			mt76_mmio_write_copy_portable(dev->mmio.regs + offset, data,
    50						      len);
    51			return;
    52		}
    53		__iowrite32_copy(dev->mmio.regs + offset, data, DIV_ROUND_UP(len, 4));
    54	}
    55	
    56	static void mt76_mmio_read_copy_portable(u8 *dst,
    57						 const void __iomem *src, int len)
    58	{
    59		u32 val;
    60		int i;
    61	
    62		for (i = 0; i < ALIGN(len, 4); i += 4) {
  > 63			val = le32_to_cpu(readl(src + i));
    64			memcpy(dst + i, &val, sizeof(val));
    65		}
    66	}
    67	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian
  2025-10-31  1:18 ` kernel test robot
@ 2025-10-31  8:22   ` Caleb James DeLisle
  0 siblings, 0 replies; 12+ messages in thread
From: Caleb James DeLisle @ 2025-10-31  8:22 UTC (permalink / raw)
  To: kernel test robot, nbd, lorenzo
  Cc: oe-kbuild-all, ryder.lee, shayne.chen, sean.wang, matthias.bgg,
	angelogioacchino.delregno, linux-wireless, linux-kernel,
	linux-mediatek


On 31/10/2025 02:18, kernel test robot wrote:
> Hi Caleb,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on wireless-next/main]
> [also build test WARNING on wireless/main linus/master v6.18-rc3 next-20251030]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Caleb-James-DeLisle/wifi-mt76-mmio_-read-write-_copy-byte-swap-when-on-Big-Endian/20251028-012349
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
> patch link:    https://lore.kernel.org/r/20251027171759.1484844-1-cjd%40cjdns.fr
> patch subject: [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian
> config: i386-randconfig-061-20251031 (https://download.01.org/0day-ci/archive/20251031/202510310816.kyDHJNiS-lkp@intel.com/config)
> compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251031/202510310816.kyDHJNiS-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202510310816.kyDHJNiS-lkp@intel.com/
>
> sparse warnings: (new ones prefixed by >>)
>>> drivers/net/wireless/mediatek/mt76/mmio.c:41:24: sparse: sparse: cast from restricted __le32
>>> drivers/net/wireless/mediatek/mt76/mmio.c:41:24: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int val @@     got restricted __le32 [usertype] @@


This should not be an issue in PATCH v2 because it no longer uses a 
temporary variable.

Thanks,

Caleb


>     drivers/net/wireless/mediatek/mt76/mmio.c:41:24: sparse:     expected unsigned int val
>     drivers/net/wireless/mediatek/mt76/mmio.c:41:24: sparse:     got restricted __le32 [usertype]
>>> drivers/net/wireless/mediatek/mt76/mmio.c:63:23: sparse: sparse: cast to restricted __le32
> vim +41 drivers/net/wireless/mediatek/mt76/mmio.c
>
>      32	
>      33	static void mt76_mmio_write_copy_portable(void __iomem *dst,
>      34						  const u8 *src, int len)
>      35	{
>      36		__le32 val;
>      37		int i = 0;
>      38	
>      39		for (i = 0; i < ALIGN(len, 4); i += 4) {
>      40			memcpy(&val, src + i, sizeof(val));
>    > 41			writel(cpu_to_le32(val), dst + i);
>      42		}
>      43	}
>      44	
>      45	static void mt76_mmio_write_copy(struct mt76_dev *dev, u32 offset,
>      46					 const void *data, int len)
>      47	{
>      48		if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) {
>      49			mt76_mmio_write_copy_portable(dev->mmio.regs + offset, data,
>      50						      len);
>      51			return;
>      52		}
>      53		__iowrite32_copy(dev->mmio.regs + offset, data, DIV_ROUND_UP(len, 4));
>      54	}
>      55	
>      56	static void mt76_mmio_read_copy_portable(u8 *dst,
>      57						 const void __iomem *src, int len)
>      58	{
>      59		u32 val;
>      60		int i;
>      61	
>      62		for (i = 0; i < ALIGN(len, 4); i += 4) {
>    > 63			val = le32_to_cpu(readl(src + i));
>      64			memcpy(dst + i, &val, sizeof(val));
>      65		}
>      66	}
>      67	
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian
  2025-10-27 17:17 [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian Caleb James DeLisle
  2025-10-28 20:19 ` Jonas Gorski
  2025-10-31  1:18 ` kernel test robot
@ 2025-11-01 16:25 ` kernel test robot
  2 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2025-11-01 16:25 UTC (permalink / raw)
  To: Caleb James DeLisle, nbd, lorenzo
  Cc: oe-kbuild-all, ryder.lee, shayne.chen, sean.wang, matthias.bgg,
	angelogioacchino.delregno, linux-wireless, linux-kernel,
	linux-mediatek, Caleb James DeLisle

Hi Caleb,

kernel test robot noticed the following build warnings:

[auto build test WARNING on wireless-next/main]
[also build test WARNING on wireless/main linus/master v6.18-rc3 next-20251031]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Caleb-James-DeLisle/wifi-mt76-mmio_-read-write-_copy-byte-swap-when-on-Big-Endian/20251028-012349
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
patch link:    https://lore.kernel.org/r/20251027171759.1484844-1-cjd%40cjdns.fr
patch subject: [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian
config: loongarch-randconfig-r111-20251101 (https://download.01.org/0day-ci/archive/20251102/202511020028.smeiQhNk-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251102/202511020028.smeiQhNk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511020028.smeiQhNk-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
   drivers/net/wireless/mediatek/mt76/mmio.c:41:24: sparse: sparse: cast from restricted __le32
>> drivers/net/wireless/mediatek/mt76/mmio.c:41:24: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] value @@     got restricted __le32 [usertype] @@
   drivers/net/wireless/mediatek/mt76/mmio.c:41:24: sparse:     expected unsigned int [usertype] value
   drivers/net/wireless/mediatek/mt76/mmio.c:41:24: sparse:     got restricted __le32 [usertype]
   drivers/net/wireless/mediatek/mt76/mmio.c:63:23: sparse: sparse: cast to restricted __le32

vim +41 drivers/net/wireless/mediatek/mt76/mmio.c

    32	
    33	static void mt76_mmio_write_copy_portable(void __iomem *dst,
    34						  const u8 *src, int len)
    35	{
    36		__le32 val;
    37		int i = 0;
    38	
    39		for (i = 0; i < ALIGN(len, 4); i += 4) {
    40			memcpy(&val, src + i, sizeof(val));
  > 41			writel(cpu_to_le32(val), dst + i);
    42		}
    43	}
    44	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-11-01 16:26 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-27 17:17 [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian Caleb James DeLisle
2025-10-28 20:19 ` Jonas Gorski
2025-10-28 21:41   ` Caleb James DeLisle
2025-10-29  9:15     ` Jonas Gorski
2025-10-29 15:24       ` Caleb James DeLisle
2025-10-29 18:41         ` [PATCH v2] wifi: mt76: mmio_*_copy fix byte order and alignment Caleb James DeLisle
2025-10-29 20:12         ` [PATCH] wifi: mt76: mmio_(read|write)_copy byte swap when on Big Endian Jonas Gorski
2025-10-29 20:40           ` Caleb James DeLisle
2025-10-29 23:06             ` Caleb James DeLisle
2025-10-31  1:18 ` kernel test robot
2025-10-31  8:22   ` Caleb James DeLisle
2025-11-01 16:25 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox