2019-07-02 15:06:37

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH 0/3] mt76: usb: alignment and endianes improvements

Fix endian bug and do some minor optimizations in mt76u_{copy,rr,wr} .

I do not mark cc stable endian fix as noboby reported this issue,
i.e. use the driver on big endian machine, but make it a separate patch,
so it can be backported if needed.

This is on top of:
[PATCH] mt76: round up length on mt76_wr_copy

Stanislaw Gruszka (3):
mt76: usb: fix endian in mt76u_copy
mt76: usb: remove unneeded {put,get}_unaligned
mt76: usb: use full intermediate buffer in mt76u_copy

drivers/net/wireless/mediatek/mt76/mt76.h | 5 ++++-
drivers/net/wireless/mediatek/mt76/usb.c | 27 ++++++++++++++++-----------
2 files changed, 20 insertions(+), 12 deletions(-)

--
1.9.3


2019-07-02 15:08:23

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH 2/3] mt76: usb: remove unneeded {put,get}_unaligned

Compiler give us guaranties on variables alignment, so use
an variable as buffer when read/write registers and remove
unneeded {put,get}_unaligned.

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
drivers/net/wireless/mediatek/mt76/mt76.h | 5 ++++-
drivers/net/wireless/mediatek/mt76/usb.c | 8 ++++----
2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index 56bf93a8988e..094e6e543542 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -389,7 +389,10 @@ enum mt76u_out_ep {
#define MCU_RESP_URB_SIZE 1024
struct mt76_usb {
struct mutex usb_ctrl_mtx;
- u8 data[32];
+ union {
+ u8 data[32];
+ __le32 reg_val;
+ };

struct tasklet_struct rx_tasklet;
struct delayed_work stat_work;
diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index db90ec6b8775..a61bb8171557 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -95,9 +95,9 @@ static u32 __mt76u_rr(struct mt76_dev *dev, u32 addr)

ret = __mt76u_vendor_request(dev, req,
USB_DIR_IN | USB_TYPE_VENDOR,
- 0, offset, usb->data, sizeof(__le32));
+ 0, offset, &usb->reg_val, sizeof(__le32));
if (ret == sizeof(__le32))
- data = get_unaligned_le32(usb->data);
+ data = le32_to_cpu(usb->reg_val);
trace_usb_reg_rr(dev, addr, data);

return data;
@@ -131,10 +131,10 @@ static void __mt76u_wr(struct mt76_dev *dev, u32 addr, u32 val)
}
offset = addr & ~MT_VEND_TYPE_MASK;

- put_unaligned_le32(val, usb->data);
+ usb->reg_val = cpu_to_le32(val);
__mt76u_vendor_request(dev, req,
USB_DIR_OUT | USB_TYPE_VENDOR, 0,
- offset, usb->data, sizeof(__le32));
+ offset, &usb->reg_val, sizeof(__le32));
trace_usb_reg_wr(dev, addr, val);
}

--
1.9.3

2019-07-02 15:08:24

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH 3/3] mt76: usb: use full intermediate buffer in mt76u_copy

Instead of use several 4 bytes usb requests, use full 32 bytes buffer
to copy data to device. With the change we use less requests and copy
exact data size to the device regardless size is multiple of 4 or not.

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
drivers/net/wireless/mediatek/mt76/usb.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index a61bb8171557..7c564cc68c7c 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -160,19 +160,24 @@ static void mt76u_copy(struct mt76_dev *dev, u32 offset,
const void *data, int len)
{
struct mt76_usb *usb = &dev->usb;
- const u32 *val = data;
- int i, ret;
+ int ret, req_len;

mutex_lock(&usb->usb_ctrl_mtx);
- for (i = 0; i < DIV_ROUND_UP(len, 4); i++) {
- put_unaligned(val[i], usb->data);
+ do {
+ req_len = min_t(int, len, sizeof(usb->data));
+
+ memcpy(usb->data, data, req_len);
+
ret = __mt76u_vendor_request(dev, MT_VEND_MULTI_WRITE,
USB_DIR_OUT | USB_TYPE_VENDOR,
- 0, offset + i * 4, usb->data,
- sizeof(u32));
+ 0, offset, usb->data, req_len);
if (ret < 0)
break;
- }
+
+ data += req_len;
+ offset += req_len;
+ len -= req_len;
+ } while (len > 0);
mutex_unlock(&usb->usb_ctrl_mtx);
}

--
1.9.3

2019-07-03 22:15:47

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [PATCH 0/3] mt76: usb: alignment and endianes improvements

> Fix endian bug and do some minor optimizations in mt76u_{copy,rr,wr} .
>
> I do not mark cc stable endian fix as noboby reported this issue,
> i.e. use the driver on big endian machine, but make it a separate patch,
> so it can be backported if needed.
>

Tested-by: Lorenzo Bianconi <[email protected]>

> This is on top of:
> [PATCH] mt76: round up length on mt76_wr_copy
>
> Stanislaw Gruszka (3):
> mt76: usb: fix endian in mt76u_copy
> mt76: usb: remove unneeded {put,get}_unaligned
> mt76: usb: use full intermediate buffer in mt76u_copy
>
> drivers/net/wireless/mediatek/mt76/mt76.h | 5 ++++-
> drivers/net/wireless/mediatek/mt76/usb.c | 27 ++++++++++++++++-----------
> 2 files changed, 20 insertions(+), 12 deletions(-)
>
> --
> 1.9.3
>


Attachments:
(No filename) (792.00 B)
signature.asc (235.00 B)
Download all attachments

2019-07-09 10:13:51

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH 3/3] mt76: usb: use full intermediate buffer in mt76u_copy

On Tue, Jul 02, 2019 at 05:06:01PM +0200, Stanislaw Gruszka wrote:
> Instead of use several 4 bytes usb requests, use full 32 bytes buffer
> to copy data to device. With the change we use less requests and copy
> exact data size to the device regardless size is multiple of 4 or not.

And this does not work correctly on some usb hosts, request which
are not multiple of 4 do not ended being written to hardware,
what results in original problem of having last part of beacon
corrupted.

I would prefer to drop this set - and I would post 2 patches
(first patch fixed and third dropped). But since this is now in
Felix's wireless tree I guess I need to post fixes on top of this
set?

Stanislaw

2019-07-09 13:46:55

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH 3/3] mt76: usb: use full intermediate buffer in mt76u_copy

On 2019-07-09 12:12, Stanislaw Gruszka wrote:
> On Tue, Jul 02, 2019 at 05:06:01PM +0200, Stanislaw Gruszka wrote:
>> Instead of use several 4 bytes usb requests, use full 32 bytes buffer
>> to copy data to device. With the change we use less requests and copy
>> exact data size to the device regardless size is multiple of 4 or not.
>
> And this does not work correctly on some usb hosts, request which
> are not multiple of 4 do not ended being written to hardware,
> what results in original problem of having last part of beacon
> corrupted.
>
> I would prefer to drop this set - and I would post 2 patches
> (first patch fixed and third dropped). But since this is now in
> Felix's wireless tree I guess I need to post fixes on top of this
> set?
I haven't sent out a pull request yet, so I can still drop those patches
and apply replacements.

- Felix