[PATCH RESEND 0/4] do not use sg if not properly supported by usb controller

[PATCH RESEND 0/4] do not use sg if not properly supported by usb controller

[lorenzo]

From: Lorenzo Bianconi <lorenzo@kernel.org>

Use linear fragment and not a single usb scatter-gather buffer in mt76u
{tx,rx} datapath if the usb controller has sg data length constraints.
Moreover add disable_usb_sg module parameter in order to explicitly
disable scatter-gather. SG I/O is not supported by all host drivers and
some users have reported sg issues on AMD IOMMU.
This series has been tested on AMD IOMMU cpus/motherboards and on rpi3+

Changes since RFC:
- rebased on top of 'fix multiple issues in mt76u error path'
  https://patchwork.kernel.org/cover/10804919/

I am resending the series since the first attempt seems to be rejected by
the ML

Lorenzo Bianconi (4):
  mt76: usb: move mt76u_check_sg in usb.c
  mt76: usb: do not use sg buffers for mcu messages
  mt76: usb: use a linear buffer for tx/rx datapath if sg is not
    supported
  mt76: usb: introduce disable_usb_sg parameter

 drivers/net/wireless/mediatek/mt76/mt76.h     |  14 +-
 .../net/wireless/mediatek/mt76/mt76x0/usb.c   |   2 +-
 .../wireless/mediatek/mt76/mt76x02_usb_mcu.c  |   3 +-
 .../wireless/mediatek/mt76/mt76x2/usb_init.c  |   2 +-
 drivers/net/wireless/mediatek/mt76/usb.c      | 133 +++++++++++++-----
 drivers/net/wireless/mediatek/mt76/usb_mcu.c  |   5 +-
 6 files changed, 105 insertions(+), 54 deletions(-)

-- 
2.20.1

[PATCH RESEND 1/4] mt76: usb: move mt76u_check_sg in usb.c

[lorenzo]

From: Lorenzo Bianconi <lorenzo@kernel.org>

Move mt76u_check_sg routine in usb.c and introduce sg_en variable
in mt76_usb in order to check if scatter-gather is supported by
mt76u layer

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/wireless/mediatek/mt76/mt76.h          | 11 +----------
 drivers/net/wireless/mediatek/mt76/mt76x0/usb.c    |  2 +-
 .../net/wireless/mediatek/mt76/mt76x2/usb_init.c   |  2 +-
 drivers/net/wireless/mediatek/mt76/usb.c           | 14 +++++++++++++-
 4 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index 13f6febc9b0f..0eb9152c5d18 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -379,6 +379,7 @@ struct mt76_usb {
 	u16 out_max_packet;
 	u8 in_ep[__MT_EP_IN_MAX];
 	u16 in_max_packet;
+	bool sg_en;
 
 	struct mt76u_mcu {
 		struct mutex mutex;
@@ -726,16 +727,6 @@ static inline u8 q2ep(u8 qid)
 	return qid + 1;
 }
 
-static inline bool mt76u_check_sg(struct mt76_dev *dev)
-{
-	struct usb_interface *intf = to_usb_interface(dev->dev);
-	struct usb_device *udev = interface_to_usbdev(intf);
-
-	return (udev->bus->sg_tablesize > 0 &&
-		(udev->bus->no_sg_constraint ||
-		 udev->speed == USB_SPEED_WIRELESS));
-}
-
 static inline int
 mt76u_bulk_msg(struct mt76_dev *dev, void *data, int len, int timeout)
 {
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c b/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
index 2f259bc0ad9e..da9d05f6074d 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
@@ -206,7 +206,7 @@ static int mt76x0u_register_device(struct mt76x02_dev *dev)
 		goto out_err;
 
 	/* check hw sg support in order to enable AMSDU */
-	if (mt76u_check_sg(&dev->mt76))
+	if (dev->mt76.usb.sg_en)
 		hw->max_tx_fragments = MT_SG_MAX_SIZE;
 	else
 		hw->max_tx_fragments = 1;
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_init.c b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_init.c
index 006e430e374e..090aaf71b3ef 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_init.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_init.c
@@ -228,7 +228,7 @@ int mt76x2u_register_device(struct mt76x02_dev *dev)
 		goto fail;
 
 	/* check hw sg support in order to enable AMSDU */
-	if (mt76u_check_sg(&dev->mt76))
+	if (dev->mt76.usb.sg_en)
 		hw->max_tx_fragments = MT_SG_MAX_SIZE;
 	else
 		hw->max_tx_fragments = 1;
diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index c7db8a9f6acc..829128a07701 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -241,6 +241,16 @@ mt76u_rd_rp(struct mt76_dev *dev, u32 base,
 		return mt76u_req_rd_rp(dev, base, data, n);
 }
 
+static bool mt76u_check_sg(struct mt76_dev *dev)
+{
+	struct usb_interface *intf = to_usb_interface(dev->dev);
+	struct usb_device *udev = interface_to_usbdev(intf);
+
+	return (udev->bus->sg_tablesize > 0 &&
+		(udev->bus->no_sg_constraint ||
+		 udev->speed == USB_SPEED_WIRELESS));
+}
+
 static int
 mt76u_set_endpoints(struct usb_interface *intf,
 		    struct mt76_usb *usb)
@@ -536,7 +546,7 @@ static int mt76u_alloc_rx(struct mt76_dev *dev)
 	if (!q->entry)
 		return -ENOMEM;
 
-	if (mt76u_check_sg(dev)) {
+	if (dev->usb.sg_en) {
 		q->buf_size = MT_RX_BUF_SIZE;
 		nsgs = MT_SG_MAX_SIZE;
 	} else {
@@ -881,6 +891,8 @@ int mt76u_init(struct mt76_dev *dev,
 	dev->bus = &mt76u_ops;
 	dev->queue_ops = &usb_queue_ops;
 
+	usb->sg_en = mt76u_check_sg(dev);
+
 	return mt76u_set_endpoints(intf, usb);
 }
 EXPORT_SYMBOL_GPL(mt76u_init);
-- 
2.20.1

[PATCH RESEND 2/4] mt76: usb: do not use sg buffers for mcu messages

[lorenzo]

From: Lorenzo Bianconi <lorenzo@kernel.org>

Do not use scatter-gather buffers for mcu commands.
Introduce mt76u_buf_alloc and mt76u_buf_alloc_sg routines.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/wireless/mediatek/mt76/mt76.h     |  3 +-
 .../wireless/mediatek/mt76/mt76x02_usb_mcu.c  |  3 +-
 drivers/net/wireless/mediatek/mt76/usb.c      | 38 +++++++++++++++----
 drivers/net/wireless/mediatek/mt76/usb_mcu.c  |  5 +--
 4 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index 0eb9152c5d18..f55dc621e060 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -87,6 +87,7 @@ struct mt76u_buf {
 	struct mt76_dev *dev;
 	struct urb *urb;
 	size_t len;
+	void *buf;
 	bool done;
 };
 
@@ -748,7 +749,7 @@ void mt76u_single_wr(struct mt76_dev *dev, const u8 req,
 int mt76u_init(struct mt76_dev *dev, struct usb_interface *intf);
 void mt76u_deinit(struct mt76_dev *dev);
 int mt76u_buf_alloc(struct mt76_dev *dev, struct mt76u_buf *buf,
-		    int nsgs, int len, int sglen, gfp_t gfp);
+		    int len, int data_len, gfp_t gfp);
 void mt76u_buf_free(struct mt76u_buf *buf);
 int mt76u_submit_buf(struct mt76_dev *dev, int dir, int index,
 		     struct mt76u_buf *buf, gfp_t gfp,
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
index 64c777298cee..e469e383cb88 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
@@ -63,9 +63,9 @@ static int mt76x02u_mcu_wait_resp(struct mt76_dev *dev, u8 seq)
 	struct mt76_usb *usb = &dev->usb;
 	struct mt76u_buf *buf = &usb->mcu.res;
 	struct urb *urb = buf->urb;
+	u8 *data = buf->buf;
 	int i, ret;
 	u32 rxfce;
-	u8 *data;
 
 	for (i = 0; i < 5; i++) {
 		if (!wait_for_completion_timeout(&usb->mcu.cmpl,
@@ -75,7 +75,6 @@ static int mt76x02u_mcu_wait_resp(struct mt76_dev *dev, u8 seq)
 		if (urb->status)
 			return -EIO;
 
-		data = sg_virt(&urb->sg[0]);
 		if (usb->mcu.rp)
 			mt76x02u_multiple_mcu_reads(dev, data + 4,
 						    urb->actual_length - 8);
diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index 829128a07701..66c9451cb6f3 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -319,8 +319,9 @@ mt76u_fill_rx_sg(struct mt76_dev *dev, struct mt76u_buf *buf,
 	return i ? : -ENOMEM;
 }
 
-int mt76u_buf_alloc(struct mt76_dev *dev, struct mt76u_buf *buf,
-		    int nsgs, int len, int sglen, gfp_t gfp)
+static int
+mt76u_buf_alloc_sg(struct mt76_dev *dev, struct mt76u_buf *buf,
+		   int nsgs, int len, int sglen, gfp_t gfp)
 {
 	buf->urb = usb_alloc_urb(0, gfp);
 	if (!buf->urb)
@@ -337,6 +338,25 @@ int mt76u_buf_alloc(struct mt76_dev *dev, struct mt76u_buf *buf,
 	return mt76u_fill_rx_sg(dev, buf, nsgs, len, sglen);
 }
 
+int mt76u_buf_alloc(struct mt76_dev *dev, struct mt76u_buf *buf,
+		    int len, int data_len, gfp_t gfp)
+{
+	struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
+
+	buf->urb = usb_alloc_urb(0, gfp);
+	if (!buf->urb)
+		return -ENOMEM;
+
+	buf->buf = page_frag_alloc(&q->rx_page, len, gfp);
+	if (!buf->buf)
+		return -ENOMEM;
+
+	buf->len = data_len;
+	buf->dev = dev;
+
+	return 0;
+}
+
 void mt76u_buf_free(struct mt76u_buf *buf)
 {
 	struct urb *urb = buf->urb;
@@ -350,6 +370,9 @@ void mt76u_buf_free(struct mt76u_buf *buf)
 
 		skb_free_frag(sg_virt(sg));
 	}
+	if (buf->buf)
+		skb_free_frag(buf->buf);
+
 	usb_free_urb(buf->urb);
 }
 EXPORT_SYMBOL_GPL(mt76u_buf_free);
@@ -360,6 +383,7 @@ int mt76u_submit_buf(struct mt76_dev *dev, int dir, int index,
 {
 	struct usb_interface *intf = to_usb_interface(dev->dev);
 	struct usb_device *udev = interface_to_usbdev(intf);
+	u8 *data = buf->urb->num_sgs ? NULL : buf->buf;
 	unsigned int pipe;
 
 	if (dir == USB_DIR_IN)
@@ -367,7 +391,7 @@ int mt76u_submit_buf(struct mt76_dev *dev, int dir, int index,
 	else
 		pipe = usb_sndbulkpipe(udev, dev->usb.out_ep[index]);
 
-	usb_fill_bulk_urb(buf->urb, udev, pipe, NULL, buf->len,
+	usb_fill_bulk_urb(buf->urb, udev, pipe, data, buf->len,
 			  complete_fn, context);
 	trace_submit_urb(dev, buf->urb);
 
@@ -556,10 +580,10 @@ static int mt76u_alloc_rx(struct mt76_dev *dev)
 
 	q->ndesc = MT_NUM_RX_ENTRIES;
 	for (i = 0; i < q->ndesc; i++) {
-		err = mt76u_buf_alloc(dev, &q->entry[i].ubuf,
-				      nsgs, q->buf_size,
-				      SKB_WITH_OVERHEAD(q->buf_size),
-				      GFP_KERNEL);
+		err = mt76u_buf_alloc_sg(dev, &q->entry[i].ubuf,
+					 nsgs, q->buf_size,
+					 SKB_WITH_OVERHEAD(q->buf_size),
+					 GFP_KERNEL);
 		if (err < 0)
 			return err;
 	}
diff --git a/drivers/net/wireless/mediatek/mt76/usb_mcu.c b/drivers/net/wireless/mediatek/mt76/usb_mcu.c
index 9527e1216f3d..72c8607da4b4 100644
--- a/drivers/net/wireless/mediatek/mt76/usb_mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/usb_mcu.c
@@ -29,9 +29,8 @@ int mt76u_mcu_init_rx(struct mt76_dev *dev)
 	struct mt76_usb *usb = &dev->usb;
 	int err;
 
-	err = mt76u_buf_alloc(dev, &usb->mcu.res, 1,
-			      MCU_RESP_URB_SIZE, MCU_RESP_URB_SIZE,
-			      GFP_KERNEL);
+	err = mt76u_buf_alloc(dev, &usb->mcu.res, MCU_RESP_URB_SIZE,
+			      MCU_RESP_URB_SIZE, GFP_KERNEL);
 	if (err < 0)
 		return err;
 
-- 
2.20.1

[PATCH RESEND 3/4] mt76: usb: use a linear buffer for tx/rx datapath if sg is not supported

[lorenzo]

From: Lorenzo Bianconi <lorenzo@kernel.org>

Use linear fragment and not a single usb scatter-gather buffer in mt76u
{tx,rx} datapath if the usb controller has sg data length constraints

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/wireless/mediatek/mt76/usb.c | 87 +++++++++++++++---------
 1 file changed, 54 insertions(+), 33 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index 66c9451cb6f3..7e7da6b49a88 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -432,10 +432,11 @@ static int mt76u_get_rx_entry_len(u8 *data, u32 data_len)
 }
 
 static int
-mt76u_process_rx_entry(struct mt76_dev *dev, struct urb *urb)
+mt76u_process_rx_entry(struct mt76_dev *dev, struct mt76u_buf *buf)
 {
 	struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
-	u8 *data = sg_virt(&urb->sg[0]);
+	struct urb *urb = buf->urb;
+	u8 *data = urb->num_sgs ? sg_virt(&urb->sg[0]) : buf->buf;
 	int data_len, len, nsgs = 1;
 	struct sk_buff *skb;
 
@@ -446,7 +447,8 @@ mt76u_process_rx_entry(struct mt76_dev *dev, struct urb *urb)
 	if (len < 0)
 		return 0;
 
-	data_len = min_t(int, len, urb->sg[0].length - MT_DMA_HDR_LEN);
+	data_len = urb->num_sgs ? urb->sg[0].length : buf->len;
+	data_len = min_t(int, len, data_len - MT_DMA_HDR_LEN);
 	if (MT_DMA_HDR_LEN + data_len > SKB_WITH_OVERHEAD(q->buf_size))
 		return 0;
 
@@ -458,7 +460,7 @@ mt76u_process_rx_entry(struct mt76_dev *dev, struct urb *urb)
 	__skb_put(skb, data_len);
 	len -= data_len;
 
-	while (len > 0) {
+	while (len > 0 && urb->num_sgs) {
 		data_len = min_t(int, len, urb->sg[nsgs].length);
 		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
 				sg_page(&urb->sg[nsgs]),
@@ -504,12 +506,26 @@ static void mt76u_complete_rx(struct urb *urb)
 	spin_unlock_irqrestore(&q->lock, flags);
 }
 
+static int
+mt76u_refill_rx(struct mt76_dev *dev, struct mt76_queue *q,
+		struct mt76u_buf *buf, int nsgs)
+{
+	if (dev->usb.sg_en) {
+		return mt76u_fill_rx_sg(dev, buf, nsgs, q->buf_size,
+					SKB_WITH_OVERHEAD(q->buf_size));
+	} else {
+		buf->buf = page_frag_alloc(&q->rx_page, q->buf_size,
+					   GFP_ATOMIC);
+		return buf->buf ? 0 : -ENOMEM;
+	}
+}
+
 static void mt76u_rx_tasklet(unsigned long data)
 {
 	struct mt76_dev *dev = (struct mt76_dev *)data;
 	struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
-	int err, nsgs, buf_len = q->buf_size;
 	struct mt76u_buf *buf;
+	int err, count;
 
 	rcu_read_lock();
 
@@ -518,11 +534,9 @@ static void mt76u_rx_tasklet(unsigned long data)
 		if (!buf)
 			break;
 
-		nsgs = mt76u_process_rx_entry(dev, buf->urb);
-		if (nsgs > 0) {
-			err = mt76u_fill_rx_sg(dev, buf, nsgs,
-					       buf_len,
-					       SKB_WITH_OVERHEAD(buf_len));
+		count = mt76u_process_rx_entry(dev, buf);
+		if (count > 0) {
+			err = mt76u_refill_rx(dev, q, buf, count);
 			if (err < 0)
 				break;
 		}
@@ -560,7 +574,7 @@ EXPORT_SYMBOL_GPL(mt76u_submit_rx_buffers);
 static int mt76u_alloc_rx(struct mt76_dev *dev)
 {
 	struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
-	int i, err, nsgs;
+	int i, err;
 
 	spin_lock_init(&q->rx_page_lock);
 	spin_lock_init(&q->lock);
@@ -570,20 +584,19 @@ static int mt76u_alloc_rx(struct mt76_dev *dev)
 	if (!q->entry)
 		return -ENOMEM;
 
-	if (dev->usb.sg_en) {
-		q->buf_size = MT_RX_BUF_SIZE;
-		nsgs = MT_SG_MAX_SIZE;
-	} else {
-		q->buf_size = PAGE_SIZE;
-		nsgs = 1;
-	}
-
+	q->buf_size = dev->usb.sg_en ? MT_RX_BUF_SIZE : PAGE_SIZE;
 	q->ndesc = MT_NUM_RX_ENTRIES;
 	for (i = 0; i < q->ndesc; i++) {
-		err = mt76u_buf_alloc_sg(dev, &q->entry[i].ubuf,
-					 nsgs, q->buf_size,
-					 SKB_WITH_OVERHEAD(q->buf_size),
-					 GFP_KERNEL);
+		if (dev->usb.sg_en)
+			err = mt76u_buf_alloc_sg(dev, &q->entry[i].ubuf,
+					MT_SG_MAX_SIZE, q->buf_size,
+					SKB_WITH_OVERHEAD(q->buf_size),
+					GFP_KERNEL);
+		else
+			err = mt76u_buf_alloc(dev, &q->entry[i].ubuf,
+					      q->buf_size,
+					      SKB_WITH_OVERHEAD(q->buf_size),
+					      GFP_KERNEL);
 		if (err < 0)
 			return err;
 	}
@@ -731,7 +744,7 @@ mt76u_tx_queue_skb(struct mt76_dev *dev, struct mt76_queue *q,
 {
 	struct usb_interface *intf = to_usb_interface(dev->dev);
 	struct usb_device *udev = interface_to_usbdev(intf);
-	u8 ep = q2ep(q->hw_idx);
+	u8 *data = NULL, ep = q2ep(q->hw_idx);
 	struct mt76u_buf *buf;
 	u16 idx = q->tail;
 	unsigned int pipe;
@@ -748,12 +761,16 @@ mt76u_tx_queue_skb(struct mt76_dev *dev, struct mt76_queue *q,
 	buf = &q->entry[idx].ubuf;
 	buf->done = false;
 
-	err = mt76u_tx_build_sg(skb, buf->urb);
-	if (err < 0)
-		return err;
+	if (dev->usb.sg_en) {
+		err = mt76u_tx_build_sg(skb, buf->urb);
+		if (err < 0)
+			return err;
+	} else {
+		data = skb->data;
+	}
 
 	pipe = usb_sndbulkpipe(udev, dev->usb.out_ep[ep]);
-	usb_fill_bulk_urb(buf->urb, udev, pipe, NULL, skb->len,
+	usb_fill_bulk_urb(buf->urb, udev, pipe, data, skb->len,
 			  mt76u_complete_tx, buf);
 
 	q->tail = (q->tail + 1) % q->ndesc;
@@ -789,10 +806,8 @@ static int mt76u_alloc_tx(struct mt76_dev *dev)
 {
 	struct mt76u_buf *buf;
 	struct mt76_queue *q;
-	size_t size;
 	int i, j;
 
-	size = MT_SG_MAX_SIZE * sizeof(struct scatterlist);
 	for (i = 0; i < IEEE80211_NUM_ACS; i++) {
 		q = &dev->q_tx[i];
 		spin_lock_init(&q->lock);
@@ -814,9 +829,15 @@ static int mt76u_alloc_tx(struct mt76_dev *dev)
 			if (!buf->urb)
 				return -ENOMEM;
 
-			buf->urb->sg = devm_kzalloc(dev->dev, size, GFP_KERNEL);
-			if (!buf->urb->sg)
-				return -ENOMEM;
+			if (dev->usb.sg_en) {
+				size_t size = MT_SG_MAX_SIZE *
+					      sizeof(struct scatterlist);
+
+				buf->urb->sg = devm_kzalloc(dev->dev, size,
+							    GFP_KERNEL);
+				if (!buf->urb->sg)
+					return -ENOMEM;
+			}
 		}
 	}
 	return 0;
-- 
2.20.1

[PATCH RESEND 4/4] mt76: usb: introduce disable_usb_sg parameter

[lorenzo]

From: Lorenzo Bianconi <lorenzo@kernel.org>

Add disable_usb_sg module parameter to disable scatter-gather on demand

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/wireless/mediatek/mt76/usb.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index 7e7da6b49a88..78191968b4fa 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -22,6 +22,10 @@
 #define MT_VEND_REQ_MAX_RETRY	10
 #define MT_VEND_REQ_TOUT_MS	300
 
+static bool disable_usb_sg;
+module_param_named(disable_usb_sg, disable_usb_sg, bool, 0644);
+MODULE_PARM_DESC(disable_usb_sg, "Disable usb scatter-gather support");
+
 /* should be called with usb_ctrl_mtx locked */
 static int __mt76u_vendor_request(struct mt76_dev *dev, u8 req,
 				  u8 req_type, u16 val, u16 offset,
@@ -246,7 +250,7 @@ static bool mt76u_check_sg(struct mt76_dev *dev)
 	struct usb_interface *intf = to_usb_interface(dev->dev);
 	struct usb_device *udev = interface_to_usbdev(intf);
 
-	return (udev->bus->sg_tablesize > 0 &&
+	return (!disable_usb_sg && udev->bus->sg_tablesize > 0 &&
 		(udev->bus->no_sg_constraint ||
 		 udev->speed == USB_SPEED_WIRELESS));
 }
-- 
2.20.1

Re: [PATCH 0/4] do not use sg if not properly supported by usb controller

[Stanislaw Gruszka]

On Tue, Feb 12, 2019 at 02:24:47PM +0100, LorenzoBianconilorenzo@kernel.org wrote:
> From: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> Use linear fragment and not a single usb scatter-gather buffer in mt76u
> {tx,rx} datapath if the usb controller has sg data length constraints.
> Moreover add disable_usb_sg module parameter in order to explicitly
> disable scatter-gather. SG I/O is not supported by all host drivers and
> some users have reported sg issues on AMD IOMMU.

Again. This is not right approach. SG issues should be fixed
not workarounded.

> This series has been tested on AMD IOMMU cpus/motherboards and on rpi3+

> - rebased on top of 'fix multiple issues in mt76u error path'
>   https://patchwork.kernel.org/cover/10804919/
> 
> Lorenzo Bianconi (4):
>   mt76: usb: move mt76u_check_sg in usb.c
>   mt76: usb: do not use sg buffers for mcu messages
>   mt76: usb: use a linear buffer for tx/rx datapath if sg is not
>     supported
>   mt76: usb: introduce disable_usb_sg parameter
> 
>  drivers/net/wireless/mediatek/mt76/mt76.h     |  14 +-
>  .../net/wireless/mediatek/mt76/mt76x0/usb.c   |   2 +-
>  .../wireless/mediatek/mt76/mt76x02_usb_mcu.c  |   3 +-
>  .../wireless/mediatek/mt76/mt76x2/usb_init.c  |   2 +-
>  drivers/net/wireless/mediatek/mt76/usb.c      | 133 +++++++++++++-----
>  drivers/net/wireless/mediatek/mt76/usb_mcu.c  |   5 +-
>  6 files changed, 105 insertions(+), 54 deletions(-)

I really think my approach is simpler. Diffstat from my proposed patch is:

  drivers/net/wireless/mediatek/mt76/mt76.h |  1 +
  drivers/net/wireless/mediatek/mt76/usb.c  | 55 ++++++++++++++---------
  2 files changed, 36 insertions(+), 20 deletions(-)

Also this fix bug(s), presumably regression for mt76x0, should be
backported.

Stanislaw

Re: [PATCH 0/4] do not use sg if not properly supported by usb controller

[Stanislaw Gruszka]

(repost with corrected Lorenzo email)

On Tue, Feb 12, 2019 at 02:45:50PM +0100, Stanislaw Gruszka wrote:
> On Tue, Feb 12, 2019 at 02:24:47PM +0100, LorenzoBianconilorenzo@kernel.org wrote:
> > From: Lorenzo Bianconi <lorenzo@kernel.org>
> > 
> > Use linear fragment and not a single usb scatter-gather buffer in mt76u
> > {tx,rx} datapath if the usb controller has sg data length constraints.
> > Moreover add disable_usb_sg module parameter in order to explicitly
> > disable scatter-gather. SG I/O is not supported by all host drivers and
> > some users have reported sg issues on AMD IOMMU.
> 
> Again. This is not right approach. SG issues should be fixed
> not workarounded.
> 
> > This series has been tested on AMD IOMMU cpus/motherboards and on rpi3+
> 
> > - rebased on top of 'fix multiple issues in mt76u error path'
> >   https://patchwork.kernel.org/cover/10804919/
> > 
> > Lorenzo Bianconi (4):
> >   mt76: usb: move mt76u_check_sg in usb.c
> >   mt76: usb: do not use sg buffers for mcu messages
> >   mt76: usb: use a linear buffer for tx/rx datapath if sg is not
> >     supported
> >   mt76: usb: introduce disable_usb_sg parameter
> > 
> >  drivers/net/wireless/mediatek/mt76/mt76.h     |  14 +-
> >  .../net/wireless/mediatek/mt76/mt76x0/usb.c   |   2 +-
> >  .../wireless/mediatek/mt76/mt76x02_usb_mcu.c  |   3 +-
> >  .../wireless/mediatek/mt76/mt76x2/usb_init.c  |   2 +-
> >  drivers/net/wireless/mediatek/mt76/usb.c      | 133 +++++++++++++-----
> >  drivers/net/wireless/mediatek/mt76/usb_mcu.c  |   5 +-
> >  6 files changed, 105 insertions(+), 54 deletions(-)
> 
> I really think my approach is simpler. Diffstat from my proposed patch is:
> 
>   drivers/net/wireless/mediatek/mt76/mt76.h |  1 +
>   drivers/net/wireless/mediatek/mt76/usb.c  | 55 ++++++++++++++---------
>   2 files changed, 36 insertions(+), 20 deletions(-)
> 
> Also this fix bug(s), presumably regression for mt76x0, should be
> backported.
> 
> Stanislaw
> 

Re: [PATCH 0/4] do not use sg if not properly supported by usb controller

[Lorenzo Bianconi]

>
> (repost with corrected Lorenzo email)
>
> On Tue, Feb 12, 2019 at 02:45:50PM +0100, Stanislaw Gruszka wrote:
> > On Tue, Feb 12, 2019 at 02:24:47PM +0100, LorenzoBianconilorenzo@kernel.org wrote:
> > > From: Lorenzo Bianconi <lorenzo@kernel.org>
> > >
> > > Use linear fragment and not a single usb scatter-gather buffer in mt76u
> > > {tx,rx} datapath if the usb controller has sg data length constraints.
> > > Moreover add disable_usb_sg module parameter in order to explicitly
> > > disable scatter-gather. SG I/O is not supported by all host drivers and
> > > some users have reported sg issues on AMD IOMMU.
> >
> > Again. This is not right approach. SG issues should be fixed
> > not workarounded.

Hi Stanislaw,

here we do not use SG, so num_sg is 0 and we use transfer_buffer. I do
not see how I am working around the issue.
Moreover with this approach we avoid some unnecessary operation in the hotpath

Regards,
Lorenzo

> >
> > > This series has been tested on AMD IOMMU cpus/motherboards and on rpi3+
> >
> > > - rebased on top of 'fix multiple issues in mt76u error path'
> > >   https://patchwork.kernel.org/cover/10804919/
> > >
> > > Lorenzo Bianconi (4):
> > >   mt76: usb: move mt76u_check_sg in usb.c
> > >   mt76: usb: do not use sg buffers for mcu messages
> > >   mt76: usb: use a linear buffer for tx/rx datapath if sg is not
> > >     supported
> > >   mt76: usb: introduce disable_usb_sg parameter
> > >
> > >  drivers/net/wireless/mediatek/mt76/mt76.h     |  14 +-
> > >  .../net/wireless/mediatek/mt76/mt76x0/usb.c   |   2 +-
> > >  .../wireless/mediatek/mt76/mt76x02_usb_mcu.c  |   3 +-
> > >  .../wireless/mediatek/mt76/mt76x2/usb_init.c  |   2 +-
> > >  drivers/net/wireless/mediatek/mt76/usb.c      | 133 +++++++++++++-----
> > >  drivers/net/wireless/mediatek/mt76/usb_mcu.c  |   5 +-
> > >  6 files changed, 105 insertions(+), 54 deletions(-)
> >
> > I really think my approach is simpler. Diffstat from my proposed patch is:
> >
> >   drivers/net/wireless/mediatek/mt76/mt76.h |  1 +
> >   drivers/net/wireless/mediatek/mt76/usb.c  | 55 ++++++++++++++---------
> >   2 files changed, 36 insertions(+), 20 deletions(-)
> >
> > Also this fix bug(s), presumably regression for mt76x0, should be
> > backported.
> >
> > Stanislaw
> >

Re: [PATCH 0/4] do not use sg if not properly supported by usb controller

[Stanislaw Gruszka]

On Tue, Feb 12, 2019 at 03:09:53PM +0100, Lorenzo Bianconi wrote:
> >
> > (repost with corrected Lorenzo email)
> >
> > On Tue, Feb 12, 2019 at 02:45:50PM +0100, Stanislaw Gruszka wrote:
> > > On Tue, Feb 12, 2019 at 02:24:47PM +0100, LorenzoBianconilorenzo@kernel.org wrote:
> > > > From: Lorenzo Bianconi <lorenzo@kernel.org>
> > > >
> > > > Use linear fragment and not a single usb scatter-gather buffer in mt76u
> > > > {tx,rx} datapath if the usb controller has sg data length constraints.
> > > > Moreover add disable_usb_sg module parameter in order to explicitly
> > > > disable scatter-gather. SG I/O is not supported by all host drivers and
> > > > some users have reported sg issues on AMD IOMMU.
> > >
> > > Again. This is not right approach. SG issues should be fixed
> > > not workarounded.
> 
> Hi Stanislaw,
> 
> here we do not use SG, so num_sg is 0 and we use transfer_buffer. I do
> not see how I am working around the issue.

By avoiding SG buffer allocation and configuration which most likely
need to be fixed.

> Moreover with this approach we avoid some unnecessary operation in the hotpath

What unnecessary operation ?

Stanislaw

Re: [PATCH 0/4] do not use sg if not properly supported by usb controller

[Lorenzo Bianconi]

> On Tue, Feb 12, 2019 at 03:09:53PM +0100, Lorenzo Bianconi wrote:
> > >
> > > (repost with corrected Lorenzo email)
> > >
> > > On Tue, Feb 12, 2019 at 02:45:50PM +0100, Stanislaw Gruszka wrote:
> > > > On Tue, Feb 12, 2019 at 02:24:47PM +0100, LorenzoBianconilorenzo@kernel.org wrote:
> > > > > From: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > >
> > > > > Use linear fragment and not a single usb scatter-gather buffer in mt76u
> > > > > {tx,rx} datapath if the usb controller has sg data length constraints.
> > > > > Moreover add disable_usb_sg module parameter in order to explicitly
> > > > > disable scatter-gather. SG I/O is not supported by all host drivers and
> > > > > some users have reported sg issues on AMD IOMMU.
> > > >
> > > > Again. This is not right approach. SG issues should be fixed
> > > > not workarounded.
> > 
> > Hi Stanislaw,
> > 
> > here we do not use SG, so num_sg is 0 and we use transfer_buffer. I do
> > not see how I am working around the issue.
> 
> By avoiding SG buffer allocation and configuration which most likely
> need to be fixed.

In my series I:
1- set num_sg to 0
2- use transfer_buffer

please correct me if I am wrong but in your solution you did the same since AFAIK
PageHighMem is always 0 so you end up setting num_sg to 0 and using
transfer_buffer as well. Is my understanding correct?

> 
> > Moreover with this approach we avoid some unnecessary operation in the hotpath
> 
> What unnecessary operation ?

the ones in mt76u_fill_bulk_urb()

Regards,
Lorenzo

> 
> Stanislaw

Re: [PATCH 0/4] do not use sg if not properly supported by usb controller

[Stanislaw Gruszka]

On Tue, Feb 12, 2019 at 03:25:30PM +0100, Lorenzo Bianconi wrote:
> > On Tue, Feb 12, 2019 at 03:09:53PM +0100, Lorenzo Bianconi wrote:
> > > >
> > > > (repost with corrected Lorenzo email)
> > > >
> > > > On Tue, Feb 12, 2019 at 02:45:50PM +0100, Stanislaw Gruszka wrote:
> > > > > On Tue, Feb 12, 2019 at 02:24:47PM +0100, LorenzoBianconilorenzo@kernel.org wrote:
> > > > > > From: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > > >
> > > > > > Use linear fragment and not a single usb scatter-gather buffer in mt76u
> > > > > > {tx,rx} datapath if the usb controller has sg data length constraints.
> > > > > > Moreover add disable_usb_sg module parameter in order to explicitly
> > > > > > disable scatter-gather. SG I/O is not supported by all host drivers and
> > > > > > some users have reported sg issues on AMD IOMMU.
> > > > >
> > > > > Again. This is not right approach. SG issues should be fixed
> > > > > not workarounded.
> > > 
> > > Hi Stanislaw,
> > > 
> > > here we do not use SG, so num_sg is 0 and we use transfer_buffer. I do
> > > not see how I am working around the issue.
> > 
> > By avoiding SG buffer allocation and configuration which most likely
> > need to be fixed.
> 
> In my series I:
> 1- set num_sg to 0
> 2- use transfer_buffer
>
> please correct me if I am wrong but in your solution you did the same since AFAIK
> PageHighMem is always 0 so you end up setting num_sg to 0 and using
> transfer_buffer as well. Is my understanding correct?

Yes. But it still using all existing SG allocation and setup code and
buffer is tracked in urb->sg[0].

> > > Moreover with this approach we avoid some unnecessary operation in the hotpath
> > 
> > What unnecessary operation ?
> 
> the ones in mt76u_fill_bulk_urb()

Your patches also add extra operations on hotpath due to urb->num_sgs and
dev->usb.sg_en checks.

Stanislaw

Re: [PATCH 0/4] do not use sg if not properly supported by usb controller

[Lorenzo Bianconi]

> On Tue, Feb 12, 2019 at 03:25:30PM +0100, Lorenzo Bianconi wrote:
> > > On Tue, Feb 12, 2019 at 03:09:53PM +0100, Lorenzo Bianconi wrote:
> > > > >
> > > > > (repost with corrected Lorenzo email)
> > > > >
> > > > > On Tue, Feb 12, 2019 at 02:45:50PM +0100, Stanislaw Gruszka wrote:
> > > > > > On Tue, Feb 12, 2019 at 02:24:47PM +0100, LorenzoBianconilorenzo@kernel.org wrote:
> > > > > > > From: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > > > >
> > > > > > > Use linear fragment and not a single usb scatter-gather buffer in mt76u
> > > > > > > {tx,rx} datapath if the usb controller has sg data length constraints.
> > > > > > > Moreover add disable_usb_sg module parameter in order to explicitly
> > > > > > > disable scatter-gather. SG I/O is not supported by all host drivers and
> > > > > > > some users have reported sg issues on AMD IOMMU.
> > > > > >
> > > > > > Again. This is not right approach. SG issues should be fixed
> > > > > > not workarounded.
> > > > 
> > > > Hi Stanislaw,
> > > > 
> > > > here we do not use SG, so num_sg is 0 and we use transfer_buffer. I do
> > > > not see how I am working around the issue.
> > > 
> > > By avoiding SG buffer allocation and configuration which most likely
> > > need to be fixed.
> > 
> > In my series I:
> > 1- set num_sg to 0
> > 2- use transfer_buffer
> >
> > please correct me if I am wrong but in your solution you did the same since AFAIK
> > PageHighMem is always 0 so you end up setting num_sg to 0 and using
> > transfer_buffer as well. Is my understanding correct?
> 
> Yes. But it still using all existing SG allocation and setup code and
> buffer is tracked in urb->sg[0].

both of them are from page_frag_alloc()

> 
> > > > Moreover with this approach we avoid some unnecessary operation in the hotpath
> > > 
> > > What unnecessary operation ?
> > 
> > the ones in mt76u_fill_bulk_urb()
> 
> Your patches also add extra operations on hotpath due to urb->num_sgs and
> dev->usb.sg_en checks.

here I guess you should use mt76u_check_sg() instead of
udev->bus->sg_tablesize > 0 so I think you need to precompute it since it is in
the hotpath

Moreover the RFC series has been tested by multiple users and on multiple
devices

Regards,
Lorenzo

> 
> Stanislaw

Re: [PATCH 0/4] do not use sg if not properly supported by usb controller

[Stanislaw Gruszka]

On Tue, Feb 12, 2019 at 04:08:37PM +0100, Lorenzo Bianconi wrote:
> > > > What unnecessary operation ?
> > > 
> > > the ones in mt76u_fill_bulk_urb()
> > 
> > Your patches also add extra operations on hotpath due to urb->num_sgs and
> > dev->usb.sg_en checks.
> 
> here I guess you should use mt76u_check_sg() instead of
> udev->bus->sg_tablesize > 0 so I think you need to precompute it since it is in
> the hotpath

It's not necessary. If udev->bus->sg_tablesize > 0 is true and rest
of mt76u_check_sg() is not, we will submit urb with urb->num_sgs = 1 to
usb host driver, what is fine. 

> Moreover the RFC series has been tested by multiple users and on multiple
> devices

I know. Perhaps you could test my patch on rpi ?

Stanislaw 

Re: [PATCH 0/4] do not use sg if not properly supported by usb controller

[Lorenzo Bianconi]

> On Tue, Feb 12, 2019 at 04:08:37PM +0100, Lorenzo Bianconi wrote:
> > > > > What unnecessary operation ?
> > > > 
> > > > the ones in mt76u_fill_bulk_urb()
> > > 
> > > Your patches also add extra operations on hotpath due to urb->num_sgs and
> > > dev->usb.sg_en checks.
> > 
> > here I guess you should use mt76u_check_sg() instead of
> > udev->bus->sg_tablesize > 0 so I think you need to precompute it since it is in
> > the hotpath
> 
> It's not necessary. If udev->bus->sg_tablesize > 0 is true and rest
> of mt76u_check_sg() is not, we will submit urb with urb->num_sgs = 1 to
> usb host driver, what is fine. 
> 
> > Moreover the RFC series has been tested by multiple users and on multiple
> > devices
> 
> I know. Perhaps you could test my patch on rpi ?

sure, I will do it later

Regards,
Lorenzo

> 
> Stanislaw 
> 
> 

Re: [PATCH 0/4] do not use sg if not properly supported by usb controller

[Lorenzo Bianconi]

> > On Tue, Feb 12, 2019 at 04:08:37PM +0100, Lorenzo Bianconi wrote:
> > > > > > What unnecessary operation ?
> > > > > 
> > > > > the ones in mt76u_fill_bulk_urb()
> > > > 
> > > > Your patches also add extra operations on hotpath due to urb->num_sgs and
> > > > dev->usb.sg_en checks.
> > > 
> > > here I guess you should use mt76u_check_sg() instead of
> > > udev->bus->sg_tablesize > 0 so I think you need to precompute it since it is in
> > > the hotpath
> > 
> > It's not necessary. If udev->bus->sg_tablesize > 0 is true and rest
> > of mt76u_check_sg() is not, we will submit urb with urb->num_sgs = 1 to
> > usb host driver, what is fine. 
> > 
> > > Moreover the RFC series has been tested by multiple users and on multiple
> > > devices
> > 
> > I know. Perhaps you could test my patch on rpi ?
> 
> sure, I will do it later

I confirm that even with Stanislaw's patch the usb dongle is properly working
on rpi3+

Regards,
Lorenzo

> 
> Regards,
> Lorenzo
> 
> > 
> > Stanislaw 
> > 
> > 

Re: [PATCH 0/4] do not use sg if not properly supported by usb controller

[Stanislaw Gruszka]

On Tue, Feb 12, 2019 at 11:09:03PM +0100, Lorenzo Bianconi wrote:
> > > On Tue, Feb 12, 2019 at 04:08:37PM +0100, Lorenzo Bianconi wrote:
> > > > > > > What unnecessary operation ?
> > > > > > 
> > > > > > the ones in mt76u_fill_bulk_urb()
> > > > > 
> > > > > Your patches also add extra operations on hotpath due to urb->num_sgs and
> > > > > dev->usb.sg_en checks.
> > > > 
> > > > here I guess you should use mt76u_check_sg() instead of
> > > > udev->bus->sg_tablesize > 0 so I think you need to precompute it since it is in
> > > > the hotpath
> > > 
> > > It's not necessary. If udev->bus->sg_tablesize > 0 is true and rest
> > > of mt76u_check_sg() is not, we will submit urb with urb->num_sgs = 1 to
> > > usb host driver, what is fine. 
> > > 
> > > > Moreover the RFC series has been tested by multiple users and on multiple
> > > > devices
> > > 
> > > I know. Perhaps you could test my patch on rpi ?
> > 
> > sure, I will do it later
> 
> I confirm that even with Stanislaw's patch the usb dongle is properly working
> on rpi3+

Thanks for testing. Would be ok to you to post my patch against
wireless-drivers tree and cc stable as fix for non-SG usb hosts,
drop your set for -next and work for fix for SG issue on AMD IOMMU?

Stanislaw

Re: [PATCH 0/4] do not use sg if not properly supported by usb controller

[Lorenzo Bianconi]

On Feb 13, Stanislaw Gruszka wrote:
> On Tue, Feb 12, 2019 at 11:09:03PM +0100, Lorenzo Bianconi wrote:
> > > > On Tue, Feb 12, 2019 at 04:08:37PM +0100, Lorenzo Bianconi wrote:
> > > > > > > > What unnecessary operation ?
> > > > > > > 
> > > > > > > the ones in mt76u_fill_bulk_urb()
> > > > > > 
> > > > > > Your patches also add extra operations on hotpath due to urb->num_sgs and
> > > > > > dev->usb.sg_en checks.
> > > > > 
> > > > > here I guess you should use mt76u_check_sg() instead of
> > > > > udev->bus->sg_tablesize > 0 so I think you need to precompute it since it is in
> > > > > the hotpath
> > > > 
> > > > It's not necessary. If udev->bus->sg_tablesize > 0 is true and rest
> > > > of mt76u_check_sg() is not, we will submit urb with urb->num_sgs = 1 to
> > > > usb host driver, what is fine. 
> > > > 
> > > > > Moreover the RFC series has been tested by multiple users and on multiple
> > > > > devices
> > > > 
> > > > I know. Perhaps you could test my patch on rpi ?
> > > 
> > > sure, I will do it later
> > 
> > I confirm that even with Stanislaw's patch the usb dongle is properly working
> > on rpi3+
> 
> Thanks for testing. Would be ok to you to post my patch against
> wireless-drivers tree and cc stable as fix for non-SG usb hosts,
> drop your set for -next and work for fix for SG issue on AMD IOMMU?

I agree that your patch works (since it does not use SG I/O :)) but
I think it is more clear (and manageable) to have two separated
routines for memory allocation.
Moreover I think that this check has to be done in the control plane instead
of the data plane, so I would like to spend some more time in order to see if it is
possible to remove some checks in the hot-path

Regards,
Lorenzo

> 
> Stanislaw

Re: [PATCH 0/4] do not use sg if not properly supported by usb controller

[Stanislaw Gruszka]

On Wed, Feb 13, 2019 at 12:00:42PM +0100, Lorenzo Bianconi wrote:
> On Feb 13, Stanislaw Gruszka wrote:
> > On Tue, Feb 12, 2019 at 11:09:03PM +0100, Lorenzo Bianconi wrote:
> > > > > On Tue, Feb 12, 2019 at 04:08:37PM +0100, Lorenzo Bianconi wrote:
> > > > > > > > > What unnecessary operation ?
> > > > > > > > 
> > > > > > > > the ones in mt76u_fill_bulk_urb()
> > > > > > > 
> > > > > > > Your patches also add extra operations on hotpath due to urb->num_sgs and
> > > > > > > dev->usb.sg_en checks.
> > > > > > 
> > > > > > here I guess you should use mt76u_check_sg() instead of
> > > > > > udev->bus->sg_tablesize > 0 so I think you need to precompute it since it is in
> > > > > > the hotpath
> > > > > 
> > > > > It's not necessary. If udev->bus->sg_tablesize > 0 is true and rest
> > > > > of mt76u_check_sg() is not, we will submit urb with urb->num_sgs = 1 to
> > > > > usb host driver, what is fine. 
> > > > > 
> > > > > > Moreover the RFC series has been tested by multiple users and on multiple
> > > > > > devices
> > > > > 
> > > > > I know. Perhaps you could test my patch on rpi ?
> > > > 
> > > > sure, I will do it later
> > > 
> > > I confirm that even with Stanislaw's patch the usb dongle is properly working
> > > on rpi3+
> > 
> > Thanks for testing. Would be ok to you to post my patch against
> > wireless-drivers tree and cc stable as fix for non-SG usb hosts,
> > drop your set for -next and work for fix for SG issue on AMD IOMMU?
> 
> I agree that your patch works (since it does not use SG I/O :)) but

It still use SG in mt76usb driver (urb->sg, sg_set_page(),
sg_init_marker(), etc ...) for all usb host controllers. It just
not submit urb->num_sgs > 1 to USB host driver if it does not
support SG.

> I think it is more clear (and manageable) to have two separated
> routines for memory allocation.

Hmm, I disagree on that and don't understand how you consider clearer
and manageable design with two separate buffer allocation methods
and bunch of extra ->num_sgs and ->sg_en checks, compered to solution
with one allocation method and without those checks.

Also some additional cleanups/simplification can be done after
applying my patch in mt76usb not possible after applying your set.

> Moreover I think that this check has to be done in the control plane instead
> of the data plane, so I would like to spend some more time in order to see if it is
> possible to remove some checks in the hot-path

I'm not sure what you mean by data-pane and control-plane in this context.

I considered to do mt76u_fill_buk_urb:

urb->num_sgs = (buf->num_sgs > 1) ? buf->num_sgs : 0;

Instead of usb->bus->sg_tablesize check. It should work, but it's not
what usb_sg_init() does and as pointed by Alan that function always
do the right thing, so I just copied code from there.

Beside that I don't think this check in mt76u_fill_buk_urb()
affects performance whatsoever.

Stanislaw  

Re: [PATCH RESEND 0/4] do not use sg if not properly supported by usb controller

[Felix Fietkau]

On 2019-02-12 14:42, lorenzo@kernel.org wrote:
> From: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> Use linear fragment and not a single usb scatter-gather buffer in mt76u
> {tx,rx} datapath if the usb controller has sg data length constraints.
> Moreover add disable_usb_sg module parameter in order to explicitly
> disable scatter-gather. SG I/O is not supported by all host drivers and
> some users have reported sg issues on AMD IOMMU.
> This series has been tested on AMD IOMMU cpus/motherboards and on rpi3+
> 
> Changes since RFC:
> - rebased on top of 'fix multiple issues in mt76u error path'
>   https://patchwork.kernel.org/cover/10804919/
> 
> I am resending the series since the first attempt seems to be rejected by
> the ML
Applied, thanks.

- Felix