diff mbox series

[RFC,05/12] mt76usb: remove mt76u_buf redundant fileds

Message ID 1552403166-3821-6-git-send-email-sgruszka@redhat.com (mailing list archive)
State RFC
Delegated to: Felix Fietkau
Headers show
Series mt76usb: some cleanups and optimizations | expand

Commit Message

Stanislaw Gruszka March 12, 2019, 3:05 p.m. UTC 
Remove mt76u_buf->{len, buf} fields and operate on corresponding
urb fields directly.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/mediatek/mt76/mt76.h |  2 --
 drivers/net/wireless/mediatek/mt76/usb.c  | 40 +++++++++++++++++--------------
 2 files changed, 22 insertions(+), 20 deletions(-)

Comments

Lorenzo Bianconi March 12, 2019, 4:25 p.m. UTC | #1 
> Remove mt76u_buf->{len, buf} fields and operate on corresponding
> urb fields directly.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/mediatek/mt76/mt76.h |  2 --
>  drivers/net/wireless/mediatek/mt76/usb.c  | 40 +++++++++++++++++--------------
>  2 files changed, 22 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
> index 89cdfe4abf33..476cb39c99b7 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> @@ -85,8 +85,6 @@ struct mt76_queue_buf {
>  

[...]

> @@ -377,7 +378,6 @@ static void mt76u_buf_free(struct mt76u_buf *buf)
>  		    void *context)
>  {
>  	struct usb_device *udev = to_usb_device(dev->dev);
> -	u8 *data = buf->urb->num_sgs ? NULL : buf->buf;
>  	unsigned int pipe;
>  
>  	if (dir == USB_DIR_IN)
> @@ -385,8 +385,10 @@ static void mt76u_buf_free(struct mt76u_buf *buf)
>  	else
>  		pipe = usb_sndbulkpipe(udev, dev->usb.out_ep[index]);
>  
> -	usb_fill_bulk_urb(buf->urb, udev, pipe, data, buf->len,
> -			  complete_fn, context);
> +	buf->urb->dev = udev;
> +	buf->urb->pipe = pipe;
> +	buf->urb->complete = complete_fn;
> +	buf->urb->context = context;
>  }
>  
>  static inline struct mt76u_buf
> @@ -426,7 +428,7 @@ static int mt76u_get_rx_entry_len(u8 *data, u32 data_len)
>  {
>  	struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
>  	struct urb *urb = buf->urb;
> -	u8 *data = urb->num_sgs ? sg_virt(&urb->sg[0]) : buf->buf;
> +	u8 *data = urb->num_sgs ? sg_virt(&urb->sg[0]) : urb->transfer_buffer;
>  	int data_len, len, nsgs = 1;
>  	struct sk_buff *skb;
>  
> @@ -437,7 +439,7 @@ static int mt76u_get_rx_entry_len(u8 *data, u32 data_len)
>  	if (len < 0)
>  		return 0;
>  
> -	data_len = urb->num_sgs ? urb->sg[0].length : buf->len;
> +	data_len = urb->num_sgs ? urb->sg[0].length : INT_MAX;

In this way we only use len, maybe better to do:
	data_len = urb->num_sgs ? urb->sg[0].length :
				  SKB_WITH_OVERHEAD(q->buf_size)

>  	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;
> @@ -731,14 +733,16 @@ static void mt76u_complete_tx(struct urb *urb)
>  		return err;
>  
>  	buf = &q->entry[idx].ubuf;
> -	buf->buf = skb->data;
> -	buf->len = skb->len;
> +	if (!dev->usb.sg_en) {
> +		buf->urb->transfer_buffer = skb->data;

I think you can move this in mt76u_tx_build_sg or remove the if condition in
mt76u_tx_build_sg()

Regards,
Lorenzo

> +	} else {
> +		err = mt76u_tx_build_sg(dev, skb, buf->urb);
> +		if (err < 0)
> +			return err;
> +	}
> +	buf->urb->transfer_buffer_length = skb->len;
>  	buf->done = false;
>  
> -	err = mt76u_tx_build_sg(dev, skb, buf->urb);
> -	if (err < 0)
> -		return err;
> -
>  	mt76u_fill_bulk_urb(dev, USB_DIR_OUT, q2ep(q->hw_idx),
>  			    buf, mt76u_complete_tx, buf);
>  
> -- 
> 1.9.3
>
Stanislaw Gruszka March 13, 2019, 12:53 p.m. UTC | #2 
On Tue, Mar 12, 2019 at 05:25:50PM +0100, Lorenzo Bianconi wrote:
> >  	buf = &q->entry[idx].ubuf;
> > -	buf->buf = skb->data;
> > -	buf->len = skb->len;
> > +	if (!dev->usb.sg_en) {
> > +		buf->urb->transfer_buffer = skb->data;
> 
> I think you can move this in mt76u_tx_build_sg or remove the if condition in
> mt76u_tx_build_sg()

I'll rename mt76u_tx_build_sg() to mt76u_tx_setup_bufffers() then,
which will more corresponding the what the function do.

Stanislaw
diff mbox series

Patch

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index 89cdfe4abf33..476cb39c99b7 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -85,8 +85,6 @@  struct mt76_queue_buf {
 
 struct mt76u_buf {
 	struct urb *urb;
-	size_t len;
-	void *buf;
 	bool done;
 };
 
diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index 954feb311620..86bf852c6e3d 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -315,7 +315,7 @@  static bool mt76u_check_sg(struct mt76_dev *dev)
 	}
 
 	urb->num_sgs = max_t(int, i, urb->num_sgs);
-	buf->len = urb->num_sgs * sglen,
+	urb->transfer_buffer_length = urb->num_sgs * sglen,
 	sg_init_marker(urb->sg, urb->num_sgs);
 
 	return i ? : -ENOMEM;
@@ -328,8 +328,11 @@  static bool mt76u_check_sg(struct mt76_dev *dev)
 	if (dev->usb.sg_en) {
 		return mt76u_fill_rx_sg(dev, buf, nsgs, gfp);
 	} else {
-		buf->buf = page_frag_alloc(&q->rx_page, q->buf_size, gfp);
-		return buf->buf ? 0 : -ENOMEM;
+		buf->urb->transfer_buffer_length =
+			SKB_WITH_OVERHEAD(q->buf_size);
+		buf->urb->transfer_buffer =
+			page_frag_alloc(&q->rx_page, q->buf_size, gfp);
+		return buf->urb->transfer_buffer ? 0 : -ENOMEM;
 	}
 }
 
@@ -338,8 +341,6 @@  static bool mt76u_check_sg(struct mt76_dev *dev)
 {
 	struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
 
-	buf->len = SKB_WITH_OVERHEAD(q->buf_size);
-
 	buf->urb = usb_alloc_urb(0, GFP_KERNEL);
 	if (!buf->urb)
 		return -ENOMEM;
@@ -365,8 +366,8 @@  static void mt76u_buf_free(struct mt76u_buf *buf)
 	for (i = 0; i < urb->num_sgs; i++)
 		skb_free_frag(sg_virt(&urb->sg[i]));
 
-	if (buf->buf)
-		skb_free_frag(buf->buf);
+	if (urb->transfer_buffer)
+		skb_free_frag(urb->transfer_buffer);
 
 	usb_free_urb(buf->urb);
 }
@@ -377,7 +378,6 @@  static void mt76u_buf_free(struct mt76u_buf *buf)
 		    void *context)
 {
 	struct usb_device *udev = to_usb_device(dev->dev);
-	u8 *data = buf->urb->num_sgs ? NULL : buf->buf;
 	unsigned int pipe;
 
 	if (dir == USB_DIR_IN)
@@ -385,8 +385,10 @@  static void mt76u_buf_free(struct mt76u_buf *buf)
 	else
 		pipe = usb_sndbulkpipe(udev, dev->usb.out_ep[index]);
 
-	usb_fill_bulk_urb(buf->urb, udev, pipe, data, buf->len,
-			  complete_fn, context);
+	buf->urb->dev = udev;
+	buf->urb->pipe = pipe;
+	buf->urb->complete = complete_fn;
+	buf->urb->context = context;
 }
 
 static inline struct mt76u_buf
@@ -426,7 +428,7 @@  static int mt76u_get_rx_entry_len(u8 *data, u32 data_len)
 {
 	struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
 	struct urb *urb = buf->urb;
-	u8 *data = urb->num_sgs ? sg_virt(&urb->sg[0]) : buf->buf;
+	u8 *data = urb->num_sgs ? sg_virt(&urb->sg[0]) : urb->transfer_buffer;
 	int data_len, len, nsgs = 1;
 	struct sk_buff *skb;
 
@@ -437,7 +439,7 @@  static int mt76u_get_rx_entry_len(u8 *data, u32 data_len)
 	if (len < 0)
 		return 0;
 
-	data_len = urb->num_sgs ? urb->sg[0].length : buf->len;
+	data_len = urb->num_sgs ? urb->sg[0].length : INT_MAX;
 	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;
@@ -731,14 +733,16 @@  static void mt76u_complete_tx(struct urb *urb)
 		return err;
 
 	buf = &q->entry[idx].ubuf;
-	buf->buf = skb->data;
-	buf->len = skb->len;
+	if (!dev->usb.sg_en) {
+		buf->urb->transfer_buffer = skb->data;
+	} else {
+		err = mt76u_tx_build_sg(dev, skb, buf->urb);
+		if (err < 0)
+			return err;
+	}
+	buf->urb->transfer_buffer_length = skb->len;
 	buf->done = false;
 
-	err = mt76u_tx_build_sg(dev, skb, buf->urb);
-	if (err < 0)
-		return err;
-
 	mt76u_fill_bulk_urb(dev, USB_DIR_OUT, q2ep(q->hw_idx),
 			    buf, mt76u_complete_tx, buf);