Skip to contentSkip to conversation

remove mmio dependency from mcu event code

PATCH In review Lorenzo Bianconi 3 participants 8 messages Feb 5, 2020 – Feb 7, 2020

Subsystems (2)
Arm MediaTek MediaTek MT76 wireless LAN driver
lore
Share & tools
lore

Lorenzo Bianconi

author maintainer
#0 lore raw

cover remove mmio dependency from mcu event code

Remove mmio dependency from mt76_mcu_rx_event and mt76_mcu_get_response in order to reuse them in usb code and remove duplicated code

Lorenzo Bianconi (3):
mt76: reuse mt76_mcu in mt76u_mcu
mt76: generalize mt76_mcu_rx_event routine
mt76: generalize mt76_mcu_get_response routine

 drivers/net/wireless/mediatek/mt76/mcu.c      | 15 ++++---
 drivers/net/wireless/mediatek/mt76/mt76.h     | 43 ++++++++++---------
 .../net/wireless/mediatek/mt76/mt7603/dma.c   |  4 +-
 .../net/wireless/mediatek/mt76/mt7615/mcu.c   |  2 +-
 .../net/wireless/mediatek/mt76/mt76x02_txrx.c |  3 +-
 .../wireless/mediatek/mt76/mt76x02_usb_mcu.c  | 22 ++++++----
 drivers/net/wireless/mediatek/mt76/usb.c      |  6 ++-
 7 files changed, 52 insertions(+), 43 deletions(-)
Signature · 1 line
2.21.1
author maintainer
#1 lore raw

PATCH 1/3 mt76: reuse mt76_mcu in mt76u_mcu

Introduce mt76_mcu data structure to contain common fields between mt76u_mcu and mt76e_mcu.
Move mt76u_mcu at the beginning of mt76_usb in order to rely on mt76_mcu to access mt76u_mcu common fields

Signed-off-by Lorenzo Bianconi <[email protected]>
---
 drivers/net/wireless/mediatek/mt76/mt76.h     | 41 ++++++++++---------
 .../wireless/mediatek/mt76/mt76x02_usb_mcu.c  | 22 ++++++----
 drivers/net/wireless/mediatek/mt76/usb.c      |  6 ++-
 3 files changed, 39 insertions(+), 30 deletions(-)
drivers/net/wireless/mediatek/mt76/mt76.h +21 -20
index 0956785ad6f8..c09f929c2d31 100644
@@ -388,12 +388,31 @@ enum mt76u_out_ep {
 	__MT_EP_OUT_MAX,
 };
 
+struct mt76_mcu {
+	struct mutex mutex;
+	u32 msg_seq;
+
+	struct sk_buff_head res_q;
+	wait_queue_head_t wait;
+};
+
 #define MT_TX_SG_MAX_SIZE	8
 #define MT_RX_SG_MAX_SIZE	1
 #define MT_NUM_TX_ENTRIES	256
 #define MT_NUM_RX_ENTRIES	128
 #define MCU_RESP_URB_SIZE	1024
 struct mt76_usb {
+	struct mt76u_mcu {
+		struct mt76_mcu common; /* first */
+		u8 *data;
+
+		/* multiple reads */
+		struct mt76_reg_pair *rp;
+		int rp_len;
+		u32 base;
+		bool burst;
+	} mcu;
+
 	struct mutex usb_ctrl_mtx;
 	__le32 reg_val;
 	u8 *data;

@@ -406,29 +425,10 @@ struct mt76_usb {
 	u8 out_ep[__MT_EP_OUT_MAX];
 	u8 in_ep[__MT_EP_IN_MAX];
 	bool sg_en;
-
-	struct mt76u_mcu {
-		struct mutex mutex;
-		u8 *data;
-		u32 msg_seq;
-
-		/* multiple reads */
-		struct mt76_reg_pair *rp;
-		int rp_len;
-		u32 base;
-		bool burst;
-	} mcu;
 };
 
 struct mt76_mmio {
-	struct mt76e_mcu {
-		struct mutex mutex;
-
-		wait_queue_head_t wait;
-		struct sk_buff_head res_q;
-
-		u32 msg_seq;
-	} mcu;
+	struct mt76_mcu mcu;
 	void __iomem *regs;
 	spinlock_t irq_lock;
 	u32 irqmask;

@@ -561,6 +561,7 @@ struct mt76_dev {
 	union {
 		struct mt76_mmio mmio;
 		struct mt76_usb usb;
+		struct mt76_mcu mcu;
 	};
 };
drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c +13 -9
index c58282baee46..5055fea3a382 100644
@@ -84,17 +84,18 @@ __mt76x02u_mcu_send_msg(struct mt76_dev *dev, struct sk_buff *skb,
 			int cmd, bool wait_resp)
 {
 	struct mt76_usb *usb = &dev->usb;
-	int ret;
+	struct mt76_mcu *mcu = &usb->mcu.common;
 	u8 seq = 0;
 	u32 info;
+	int ret;
 
 	if (test_bit(MT76_REMOVED, &dev->phy.state))
 		return 0;
 
 	if (wait_resp) {
-		seq = ++usb->mcu.msg_seq & 0xf;
+		seq = ++mcu->msg_seq & 0xf;
 		if (!seq)
-			seq = ++usb->mcu.msg_seq & 0xf;
+			seq = ++mcu->msg_seq & 0xf;
 	}
 
 	info = FIELD_PREP(MT_MCU_MSG_CMD_SEQ, seq) |

@@ -122,6 +123,7 @@ mt76x02u_mcu_send_msg(struct mt76_dev *dev, int cmd, const void *data,
 		      int len, bool wait_resp)
 {
 	struct mt76_usb *usb = &dev->usb;
+	struct mt76_mcu *mcu = &usb->mcu.common;
 	struct sk_buff *skb;
 	int err;
 

@@ -129,9 +131,9 @@ mt76x02u_mcu_send_msg(struct mt76_dev *dev, int cmd, const void *data,
 	if (!skb)
 		return -ENOMEM;
 
-	mutex_lock(&usb->mcu.mutex);
+	mutex_lock(&mcu->mutex);
 	err = __mt76x02u_mcu_send_msg(dev, skb, cmd, wait_resp);
-	mutex_unlock(&usb->mcu.mutex);
+	mutex_unlock(&mcu->mutex);
 
 	return err;
 }

@@ -148,6 +150,7 @@ mt76x02u_mcu_wr_rp(struct mt76_dev *dev, u32 base,
 	const int CMD_RANDOM_WRITE = 12;
 	const int max_vals_per_cmd = MT_INBAND_PACKET_MAX_LEN / 8;
 	struct mt76_usb *usb = &dev->usb;
+	struct mt76_mcu *mcu = &usb->mcu.common;
 	struct sk_buff *skb;
 	int cnt, i, ret;
 

@@ -166,9 +169,9 @@ mt76x02u_mcu_wr_rp(struct mt76_dev *dev, u32 base,
 		skb_put_le32(skb, data[i].value);
 	}
 
-	mutex_lock(&usb->mcu.mutex);
+	mutex_lock(&mcu->mutex);
 	ret = __mt76x02u_mcu_send_msg(dev, skb, CMD_RANDOM_WRITE, cnt == n);
-	mutex_unlock(&usb->mcu.mutex);
+	mutex_unlock(&mcu->mutex);
 	if (ret)
 		return ret;
 

@@ -182,6 +185,7 @@ mt76x02u_mcu_rd_rp(struct mt76_dev *dev, u32 base,
 	const int CMD_RANDOM_READ = 10;
 	const int max_vals_per_cmd = MT_INBAND_PACKET_MAX_LEN / 8;
 	struct mt76_usb *usb = &dev->usb;
+	struct mt76_mcu *mcu = &usb->mcu.common;
 	struct sk_buff *skb;
 	int cnt, i, ret;
 

@@ -202,7 +206,7 @@ mt76x02u_mcu_rd_rp(struct mt76_dev *dev, u32 base,
 		skb_put_le32(skb, data[i].value);
 	}
 
-	mutex_lock(&usb->mcu.mutex);
+	mutex_lock(&mcu->mutex);
 
 	usb->mcu.rp = data;
 	usb->mcu.rp_len = n;

@@ -213,7 +217,7 @@ mt76x02u_mcu_rd_rp(struct mt76_dev *dev, u32 base,
 
 	usb->mcu.rp = NULL;
 
-	mutex_unlock(&usb->mcu.mutex);
+	mutex_unlock(&mcu->mutex);
 
 	return ret;
 }
drivers/net/wireless/mediatek/mt76/usb.c +5 -1
index 981d8a985557..5510baa22ff2 100644
@@ -1150,6 +1150,7 @@ int mt76u_init(struct mt76_dev *dev,
 	};
 	struct usb_device *udev = interface_to_usbdev(intf);
 	struct mt76_usb *usb = &dev->usb;
+	struct mt76_mcu *mcu = &usb->mcu.common;
 
 	mt76u_ops.rr = ext ? mt76u_rr_ext : mt76u_rr;
 	mt76u_ops.wr = ext ? mt76u_wr_ext : mt76u_wr;

@@ -1167,13 +1168,16 @@ int mt76u_init(struct mt76_dev *dev,
 	usb->data_len = usb_maxpacket(udev, usb_sndctrlpipe(udev, 0), 1);
 	if (usb->data_len < 32)
 		usb->data_len = 32;
+
 	usb->data = devm_kmalloc(dev->dev, usb->data_len, GFP_KERNEL);
 	if (!usb->data) {
 		mt76u_deinit(dev);
 		return -ENOMEM;
 	}
 
-	mutex_init(&usb->mcu.mutex);
+	skb_queue_head_init(&mcu->res_q);
+	init_waitqueue_head(&mcu->wait);
+	mutex_init(&mcu->mutex);
 
 	mutex_init(&usb->usb_ctrl_mtx);
 	dev->bus = &mt76u_ops;
Signature · 1 line
2.21.1
author maintainer
#4 lore raw

PATCH 2/3 mt76: generalize mt76_mcu_rx_event routine

In reply to Lorenzo Bianconi

Rely on mt76_mcu in mt76_mcu_rx_event signature in order to reuse it in usb code

Signed-off-by Lorenzo Bianconi <[email protected]>
---
 drivers/net/wireless/mediatek/mt76/mcu.c          | 8 +++++---
 drivers/net/wireless/mediatek/mt76/mt76.h         | 2 +-
 drivers/net/wireless/mediatek/mt76/mt7603/dma.c   | 4 ++--
 drivers/net/wireless/mediatek/mt76/mt7615/mcu.c   | 2 +-
 drivers/net/wireless/mediatek/mt76/mt76x02_txrx.c | 3 +--
 5 files changed, 10 insertions(+), 9 deletions(-)
drivers/net/wireless/mediatek/mt76/mcu.c +5 -3
index b0fb0830c9e1..a2936f8de915 100644
@@ -42,9 +42,11 @@ struct sk_buff *mt76_mcu_get_response(struct mt76_dev *dev,
 }
 EXPORT_SYMBOL_GPL(mt76_mcu_get_response);
 
-void mt76_mcu_rx_event(struct mt76_dev *dev, struct sk_buff *skb)
+void mt76_mcu_rx_event(void *data, struct sk_buff *skb)
 {
-	skb_queue_tail(&dev->mmio.mcu.res_q, skb);
-	wake_up(&dev->mmio.mcu.wait);
+	struct mt76_mcu *mcu = (struct mt76_mcu *)data;
+
+	skb_queue_tail(&mcu->res_q, skb);
+	wake_up(&mcu->wait);
 }
 EXPORT_SYMBOL_GPL(mt76_mcu_rx_event);
drivers/net/wireless/mediatek/mt76/mt76.h +1 -1
index c09f929c2d31..48d6101dc4ee 100644
@@ -903,7 +903,7 @@ void mt76u_queues_deinit(struct mt76_dev *dev);
 struct sk_buff *
 mt76_mcu_msg_alloc(const void *data, int head_len,
 		   int data_len, int tail_len);
-void mt76_mcu_rx_event(struct mt76_dev *dev, struct sk_buff *skb);
+void mt76_mcu_rx_event(void *data, struct sk_buff *skb);
 struct sk_buff *mt76_mcu_get_response(struct mt76_dev *dev,
 				      unsigned long expires);
drivers/net/wireless/mediatek/mt76/mt7603/dma.c +2 -2
index a08b85281170..a5e23b349999 100644
@@ -103,7 +103,7 @@ void mt7603_queue_rx_skb(struct mt76_dev *mdev, enum mt76_rxq_id q,
 
 	if (q == MT_RXQ_MCU) {
 		if (type == PKT_TYPE_RX_EVENT)
-			mt76_mcu_rx_event(&dev->mt76, skb);
+			mt76_mcu_rx_event(&dev->mt76.mcu, skb);
 		else
 			mt7603_rx_loopback_skb(dev, skb);
 		return;

@@ -116,7 +116,7 @@ void mt7603_queue_rx_skb(struct mt76_dev *mdev, enum mt76_rxq_id q,
 		dev_kfree_skb(skb);
 		break;
 	case PKT_TYPE_RX_EVENT:
-		mt76_mcu_rx_event(&dev->mt76, skb);
+		mt76_mcu_rx_event(&dev->mt76.mcu, skb);
 		return;
 	case PKT_TYPE_NORMAL:
 		if (mt7603_mac_fill_rx(dev, skb) == 0) {
drivers/net/wireless/mediatek/mt76/mt7615/mcu.c +1 -1
index c93a22110cf9..60cfa7366ea7 100644
@@ -301,7 +301,7 @@ void mt7615_mcu_rx_event(struct mt7615_dev *dev, struct sk_buff *skb)
 	    !rxd->seq)
 		mt7615_mcu_rx_unsolicited_event(dev, skb);
 	else
-		mt76_mcu_rx_event(&dev->mt76, skb);
+		mt76_mcu_rx_event(&dev->mt76.mcu, skb);
 }
 
 static int mt7615_mcu_init_download(struct mt7615_dev *dev, u32 addr,
drivers/net/wireless/mediatek/mt76/mt76x02_txrx.c +1 -2
index 039f96877787..d1aec50bccae 100644
@@ -39,8 +39,7 @@ void mt76x02_queue_rx_skb(struct mt76_dev *mdev, enum mt76_rxq_id q,
 	void *rxwi = skb->data;
 
 	if (q == MT_RXQ_MCU) {
-		/* this is used just by mmio code */
-		mt76_mcu_rx_event(&dev->mt76, skb);
+		mt76_mcu_rx_event(&mdev->mcu, skb);
 		return;
 	}
Signature · 1 line
2.21.1
author maintainer
#7 lore raw

PATCH 3/3 mt76: generalize mt76_mcu_get_response routine

In reply to Lorenzo Bianconi

Rely on mt76_mcu in mt76_mcu_get_response routine in order to reuse it in usb code

Signed-off-by Lorenzo Bianconi <[email protected]>
---
 drivers/net/wireless/mediatek/mt76/mcu.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/net/wireless/mediatek/mt76/mcu.c +3 -4
index a2936f8de915..644f396a58a6 100644
@@ -24,7 +24,6 @@ mt76_mcu_msg_alloc(const void *data, int head_len,
 }
 EXPORT_SYMBOL_GPL(mt76_mcu_msg_alloc);
 
-/* mmio */
 struct sk_buff *mt76_mcu_get_response(struct mt76_dev *dev,
 				      unsigned long expires)
 {

@@ -34,11 +33,11 @@ struct sk_buff *mt76_mcu_get_response(struct mt76_dev *dev,
 		return NULL;
 
 	timeout = expires - jiffies;
-	wait_event_timeout(dev->mmio.mcu.wait,
-			   (!skb_queue_empty(&dev->mmio.mcu.res_q) ||
+	wait_event_timeout(dev->mcu.wait,
+			   (!skb_queue_empty(&dev->mcu.res_q) ||
 			    test_bit(MT76_MCU_RESET, &dev->phy.state)),
 			   timeout);
-	return skb_dequeue(&dev->mmio.mcu.res_q);
+	return skb_dequeue(&dev->mcu.res_q);
 }
 EXPORT_SYMBOL_GPL(mt76_mcu_get_response);
Signature · 1 line
2.21.1
maintainer
#2 lore raw

Re: [PATCH 1/3] mt76: reuse mt76_mcu in mt76u_mcu

In reply to Lorenzo Bianconi
Quoted 4 lines · Lorenzo Bianconi

Introduce mt76_mcu data structure to contain common fields between mt76u_mcu and mt76e_mcu.
Move mt76u_mcu at the beginning of mt76_usb in order to rely on mt76_mcu to access mt76u_mcu common fields

Why not move it to struct mt76_dev directly and out of the union? I think that would be cleaner, and you can also initialize its fields in mt76_alloc_device().

- Felix

maintainer
#5 lore raw

Re: [PATCH 2/3] mt76: generalize mt76_mcu_rx_event routine

In reply to Lorenzo Bianconi
Quoted 17 lines · Lorenzo Bianconi

Rely on mt76_mcu in mt76_mcu_rx_event signature in order to reuse it in usb code

Signed-off-by Lorenzo Bianconi <[email protected]>
---
 drivers/net/wireless/mediatek/mt76/mcu.c          | 8 +++++---
 drivers/net/wireless/mediatek/mt76/mt76.h         | 2 +-
 drivers/net/wireless/mediatek/mt76/mt7603/dma.c   | 4 ++--
 drivers/net/wireless/mediatek/mt76/mt7615/mcu.c   | 2 +-
 drivers/net/wireless/mediatek/mt76/mt76x02_txrx.c | 3 +--
 5 files changed, 10 insertions(+), 9 deletions(-)
drivers/net/wireless/mediatek/mt76/mcu.c +0 -0
index b0fb0830c9e1..a2936f8de915 100644

drivers/net/wireless/mediatek/mt76/mcu.c struct sk_buff *mt76_mcu_get_response(struct mt76_dev *dev, · line 45

Quoted 5 lines
drivers/net/wireless/mediatek/mt76/mcu.c +1 -1
@@ -42,9 +42,11 @@ struct sk_buff *mt76_mcu_get_response(struct mt76_dev *dev,
 }
 EXPORT_SYMBOL_GPL(mt76_mcu_get_response);
 
-void mt76_mcu_rx_event(struct mt76_dev *dev, struct sk_buff *skb)
+void mt76_mcu_rx_event(void *data, struct sk_buff *skb)

Why the void* pointer if we have the mcu struct in a common place in struct mt76_dev anyway?

- Felix

#6 lore raw

Re: [PATCH 2/3] mt76: generalize mt76_mcu_rx_event routine

Quoted 21 lines · Felix Fietkau
Quoted 20 lines · Lorenzo Bianconi

Rely on mt76_mcu in mt76_mcu_rx_event signature in order to reuse it in usb code

Signed-off-by Lorenzo Bianconi <[email protected]>
---
 drivers/net/wireless/mediatek/mt76/mcu.c          | 8 +++++---
 drivers/net/wireless/mediatek/mt76/mt76.h         | 2 +-
 drivers/net/wireless/mediatek/mt76/mt7603/dma.c   | 4 ++--
 drivers/net/wireless/mediatek/mt76/mt7615/mcu.c   | 2 +-
 drivers/net/wireless/mediatek/mt76/mt76x02_txrx.c | 3 +--
 5 files changed, 10 insertions(+), 9 deletions(-)
drivers/net/wireless/mediatek/mt76/mcu.c +0 -0
index b0fb0830c9e1..a2936f8de915 100644
@@ -42,9 +42,11 @@ struct sk_buff *mt76_mcu_get_response(struct mt76_dev *dev,
 }
 EXPORT_SYMBOL_GPL(mt76_mcu_get_response);
Quoted 4 lines

drivers/net/wireless/mediatek/mt76/mcu.c struct sk_buff *mt76_mcu_get_response(struct mt76_dev *dev, · line 45

Quoted 2 lines
drivers/net/wireless/mediatek/mt76/mcu.c +1 -1
-void mt76_mcu_rx_event(struct mt76_dev *dev, struct sk_buff *skb)
+void mt76_mcu_rx_event(void *data, struct sk_buff *skb)

Why the void* pointer if we have the mcu struct in a common place in struct mt76_dev anyway?

ack, I will fix it in v2

Regards,
Lorenzo

Quoted 3 lines

- Felix

#3 lore raw

Re: [PATCH 1/3] mt76: reuse mt76_mcu in mt76u_mcu

In reply to Felix Fietkau
Quoted 8 lines · Felix Fietkau
Quoted 4 lines · Lorenzo Bianconi

Introduce mt76_mcu data structure to contain common fields between mt76u_mcu and mt76e_mcu.
Move mt76u_mcu at the beginning of mt76_usb in order to rely on mt76_mcu to access mt76u_mcu common fields

Why not move it to struct mt76_dev directly and out of the union? I think that would be cleaner, and you can also initialize its fields in mt76_alloc_device().

ack and I can maintain the usb specific fields (e.g mt76_reg_pair or burst) in mt76u_mcu. I will fix it in v2

Regards,
Lorenzo

Quoted 3 lines

- Felix