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.bianc...@redhat.com> --- 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