Introduce __mt7601u_rr and __mt7601u_vendor_single_wr routines in order
to make mt7601u_rmw and mt7601u_rmc atomic. This patch does not fix a
reported issue but makes the usb access more robust to concurrent
operations on the same register since it is theoretically possible that
read and write accesses of mt7601u_rmw/mt7601u_rmc can be interleaved with
a different write operation on the same register.
Moreover using __mt7601u_rr and __mt7601u_vendor_single_wr in
mt7601u_rmw/mt7601u_rmc allows to grab vendor_req_mutex mutex once
instead of twice
Signed-off-by: Lorenzo Bianconi <[email protected]>
---
Changes since v1:
- improve commit log
---
drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 3 +-
drivers/net/wireless/mediatek/mt7601u/usb.c | 52 ++++++++++++++++++-------
2 files changed, 39 insertions(+), 16 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
index c7ec40475a5f..9233744451a9 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
+++ b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
@@ -147,7 +147,8 @@ enum {
* @rx_lock: protects @rx_q.
* @con_mon_lock: protects @ap_bssid, @bcn_*, @avg_rssi.
* @mutex: ensures exclusive access from mac80211 callbacks.
- * @vendor_req_mutex: protects @vend_buf, ensures atomicity of split writes.
+ * @vendor_req_mutex: protects @vend_buf, ensures atomicity of read/write
+ * accesses
* @reg_atomic_mutex: ensures atomicity of indirect register accesses
* (accesses to RF and BBP).
* @hw_atomic_mutex: ensures exclusive access to HW during critical
diff --git a/drivers/net/wireless/mediatek/mt7601u/usb.c b/drivers/net/wireless/mediatek/mt7601u/usb.c
index b9e4f6793138..d8b7863f7926 100644
--- a/drivers/net/wireless/mediatek/mt7601u/usb.c
+++ b/drivers/net/wireless/mediatek/mt7601u/usb.c
@@ -129,15 +129,14 @@ void mt7601u_vendor_reset(struct mt7601u_dev *dev)
MT_VEND_DEV_MODE_RESET, 0, NULL, 0);
}
-u32 mt7601u_rr(struct mt7601u_dev *dev, u32 offset)
+/* should be called with vendor_req_mutex held */
+static u32 __mt7601u_rr(struct mt7601u_dev *dev, u32 offset)
{
int ret;
u32 val = ~0;
WARN_ONCE(offset > USHRT_MAX, "read high off:%08x", offset);
- mutex_lock(&dev->vendor_req_mutex);
-
ret = mt7601u_vendor_request(dev, MT_VEND_MULTI_READ, USB_DIR_IN,
0, offset, dev->vend_buf, MT_VEND_BUF);
if (ret == MT_VEND_BUF)
@@ -146,25 +145,41 @@ u32 mt7601u_rr(struct mt7601u_dev *dev, u32 offset)
dev_err(dev->dev, "Error: wrong size read:%d off:%08x\n",
ret, offset);
- mutex_unlock(&dev->vendor_req_mutex);
-
trace_reg_read(dev, offset, val);
return val;
}
-int mt7601u_vendor_single_wr(struct mt7601u_dev *dev, const u8 req,
- const u16 offset, const u32 val)
+u32 mt7601u_rr(struct mt7601u_dev *dev, u32 offset)
{
- int ret;
+ u32 ret;
mutex_lock(&dev->vendor_req_mutex);
+ ret = __mt7601u_rr(dev, offset);
+ mutex_unlock(&dev->vendor_req_mutex);
- ret = mt7601u_vendor_request(dev, req, USB_DIR_OUT,
- val & 0xffff, offset, NULL, 0);
+ return ret;
+}
+
+/* should be called with vendor_req_mutex held */
+static int __mt7601u_vendor_single_wr(struct mt7601u_dev *dev, const u8 req,
+ const u16 offset, const u32 val)
+{
+ int ret = mt7601u_vendor_request(dev, req, USB_DIR_OUT,
+ val & 0xffff, offset, NULL, 0);
if (!ret)
ret = mt7601u_vendor_request(dev, req, USB_DIR_OUT,
val >> 16, offset + 2, NULL, 0);
+ trace_reg_write(dev, offset, val);
+ return ret;
+}
+
+int mt7601u_vendor_single_wr(struct mt7601u_dev *dev, const u8 req,
+ const u16 offset, const u32 val)
+{
+ int ret;
+ mutex_lock(&dev->vendor_req_mutex);
+ ret = __mt7601u_vendor_single_wr(dev, req, offset, val);
mutex_unlock(&dev->vendor_req_mutex);
return ret;
@@ -175,23 +190,30 @@ void mt7601u_wr(struct mt7601u_dev *dev, u32 offset, u32 val)
WARN_ONCE(offset > USHRT_MAX, "write high off:%08x", offset);
mt7601u_vendor_single_wr(dev, MT_VEND_WRITE, offset, val);
- trace_reg_write(dev, offset, val);
}
u32 mt7601u_rmw(struct mt7601u_dev *dev, u32 offset, u32 mask, u32 val)
{
- val |= mt7601u_rr(dev, offset) & ~mask;
- mt7601u_wr(dev, offset, val);
+ mutex_lock(&dev->vendor_req_mutex);
+ val |= __mt7601u_rr(dev, offset) & ~mask;
+ __mt7601u_vendor_single_wr(dev, MT_VEND_WRITE, offset, val);
+ mutex_unlock(&dev->vendor_req_mutex);
+
return val;
}
u32 mt7601u_rmc(struct mt7601u_dev *dev, u32 offset, u32 mask, u32 val)
{
- u32 reg = mt7601u_rr(dev, offset);
+ u32 reg;
+ mutex_lock(&dev->vendor_req_mutex);
+ reg = __mt7601u_rr(dev, offset);
val |= reg & ~mask;
if (reg != val)
- mt7601u_wr(dev, offset, val);
+ __mt7601u_vendor_single_wr(dev, MT_VEND_WRITE,
+ offset, val);
+ mutex_unlock(&dev->vendor_req_mutex);
+
return val;
}
--
2.14.3
Lorenzo Bianconi <[email protected]> wrote:
> Introduce __mt7601u_rr and __mt7601u_vendor_single_wr routines in order
> to make mt7601u_rmw and mt7601u_rmc atomic. This patch does not fix a
> reported issue but makes the usb access more robust to concurrent
> operations on the same register since it is theoretically possible that
> read and write accesses of mt7601u_rmw/mt7601u_rmc can be interleaved with
> a different write operation on the same register.
> Moreover using __mt7601u_rr and __mt7601u_vendor_single_wr in
> mt7601u_rmw/mt7601u_rmc allows to grab vendor_req_mutex mutex once
> instead of twice
>
> Signed-off-by: Lorenzo Bianconi <[email protected]>
> Acked-by: Jakub Kicinski <[email protected]>
Patch applied to wireless-drivers-next.git, thanks.
fee05843801c mt7601u: make write with mask access atomic
--
https://patchwork.kernel.org/patch/10225849/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On Fri, 16 Feb 2018 23:30:01 +0100, Lorenzo Bianconi wrote:
> Introduce __mt7601u_rr and __mt7601u_vendor_single_wr routines in order
> to make mt7601u_rmw and mt7601u_rmc atomic. This patch does not fix a
> reported issue but makes the usb access more robust to concurrent
> operations on the same register since it is theoretically possible that
> read and write accesses of mt7601u_rmw/mt7601u_rmc can be interleaved with
> a different write operation on the same register.
> Moreover using __mt7601u_rr and __mt7601u_vendor_single_wr in
> mt7601u_rmw/mt7601u_rmc allows to grab vendor_req_mutex mutex once
> instead of twice
>
> Signed-off-by: Lorenzo Bianconi <[email protected]>
Acked-by: Jakub Kicinski <[email protected]>
Thanks!