mt7601u: make write with mask access atomic
authorLorenzo Bianconi <lorenzo.bianconi@redhat.com>
Fri, 16 Feb 2018 22:30:01 +0000 (23:30 +0100)
committerKalle Valo <kvalo@codeaurora.org>
Wed, 28 Feb 2018 14:58:42 +0000 (16:58 +0200)
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 <lorenzo.bianconi@redhat.com>
Acked-by: Jakub Kicinski <kubakici@wp.pl>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
drivers/net/wireless/mediatek/mt7601u/mt7601u.h
drivers/net/wireless/mediatek/mt7601u/usb.c

index c7ec40475a5f3a52e485f333ea86f7b0ad8ea9ed..9233744451a936162e1b069b9877d382eb930851 100644 (file)
@@ -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
index b9e4f679313852736ff2f5a2b103cf7ab494d711..d8b7863f79261a3275b6641ffdf7607e23fdd206 100644 (file)
@@ -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;
 }