diff mbox series

[4/6] staging: qlge: qlge_main: Simplify while statements.

Message ID 1bb472c5595d832221fd142dddb68907feeeecbe.1594642213.git.usuraj35@gmail.com
State Not Applicable
Delegated to: David Miller
Headers show
Series staging: qlge: General cleanup and refactor. | expand

Commit Message

Suraj Upadhyay July 13, 2020, 12:20 p.m. UTC 
Simplify while loops into more readable and simple for loops.

Signed-off-by: Suraj Upadhyay <usuraj35@gmail.com>
---
 drivers/staging/qlge/qlge_main.c | 49 ++++++++++++++------------------
 1 file changed, 22 insertions(+), 27 deletions(-)

Comments

gregkh@linuxfoundation.org July 13, 2020, 1:38 p.m. UTC | #1 
On Mon, Jul 13, 2020 at 05:50:14PM +0530, Suraj Upadhyay wrote:
> Simplify while loops into more readable and simple for loops.
> 
> Signed-off-by: Suraj Upadhyay <usuraj35@gmail.com>
> ---
>  drivers/staging/qlge/qlge_main.c | 49 ++++++++++++++------------------
>  1 file changed, 22 insertions(+), 27 deletions(-)

This patch did not apply for some odd reason :(

Please rebase and resend

thanks,

greg k-h
Dan Carpenter July 13, 2020, 2:12 p.m. UTC | #2 
On Mon, Jul 13, 2020 at 05:50:14PM +0530, Suraj Upadhyay wrote:
> Simplify while loops into more readable and simple for loops.
> 

I don't think either is more clear that the other.  Walter Harms hates
count down loops and he's not entirely wrong...

regards,
dan carpenter
Benjamin Poirier July 14, 2020, 5:41 a.m. UTC | #3 
On 2020-07-13 17:50 +0530, Suraj Upadhyay wrote:
> Simplify while loops into more readable and simple for loops.
> 
> Signed-off-by: Suraj Upadhyay <usuraj35@gmail.com>
> ---
[...]
> @@ -1824,7 +1821,7 @@ static struct sk_buff *ql_build_rx_skb(struct ql_adapter *qdev,
>  			sbq_desc->p.skb = NULL;
>  			skb_reserve(skb, NET_IP_ALIGN);
>  		}
> -		do {
> +		for (; length > 0; length -= size, i++) {
>  			lbq_desc = ql_get_curr_lchunk(qdev, rx_ring);
>  			size = min(length, qdev->lbq_buf_size);
>  
> @@ -1839,7 +1836,7 @@ static struct sk_buff *ql_build_rx_skb(struct ql_adapter *qdev,
>  			skb->truesize += size;
>  			length -= size;
>  			i++;
> -		} while (length > 0);
> +		}

Looks like length and i modification should be removed from here. But in
this instance, maybe the original was better anyways.

Agreed with Dan. At least some of those loops can be converted to "count
up" loops for a more familiar appearance.
diff mbox series

Patch

diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
index f7e26defb844..98710d3d4429 100644
--- a/drivers/staging/qlge/qlge_main.c
+++ b/drivers/staging/qlge/qlge_main.c
@@ -138,13 +138,11 @@  static int ql_sem_trylock(struct ql_adapter *qdev, u32 sem_mask)
 
 int ql_sem_spinlock(struct ql_adapter *qdev, u32 sem_mask)
 {
-	unsigned int wait_count = 30;
+	unsigned int wait_count;

-	do {
+	for (wait_count = 30; wait_count; wait_count--) {
 		if (!ql_sem_trylock(qdev, sem_mask))
 			return 0;
 		udelay(100);
-	} while (--wait_count);
+	}
 	return -ETIMEDOUT;
 }
 
@@ -1101,7 +1099,7 @@  static int qlge_refill_bq(struct qlge_bq *bq, gfp_t gfp)
 	i = bq->next_to_use;
 	bq_desc = &bq->queue[i];
 	i -= QLGE_BQ_LEN;
-	do {
+	for (; refill_count; refill_count--) {
 		netif_printk(qdev, rx_status, KERN_DEBUG, qdev->ndev,
 			     "ring %u %s: try cleaning idx %d\n",
 			     rx_ring->cq_id, bq_type_name[bq->type], i);
@@ -1123,8 +1121,7 @@  static int qlge_refill_bq(struct qlge_bq *bq, gfp_t gfp)
 			bq_desc = &bq->queue[0];
 			i -= QLGE_BQ_LEN;
 		}
-		refill_count--;
-	} while (refill_count);
+	}
 	i += QLGE_BQ_LEN;
 
 	if (bq->next_to_use != i) {
@@ -1824,7 +1821,7 @@  static struct sk_buff *ql_build_rx_skb(struct ql_adapter *qdev,
 			sbq_desc->p.skb = NULL;
 			skb_reserve(skb, NET_IP_ALIGN);
 		}
-		do {
+		for (; length > 0; length -= size, i++) {
 			lbq_desc = ql_get_curr_lchunk(qdev, rx_ring);
 			size = min(length, qdev->lbq_buf_size);
 
@@ -1839,7 +1836,7 @@  static struct sk_buff *ql_build_rx_skb(struct ql_adapter *qdev,
 			skb->truesize += size;
 			length -= size;
 			i++;
-		} while (length > 0);
+		}
 		ql_update_mac_hdr_len(qdev, ib_mac_rsp, lbq_desc->p.pg_chunk.va,
 				      &hlen);
 		__pskb_pull_tail(skb, hlen);
@@ -2098,11 +2095,11 @@  static int ql_clean_outbound_rx_ring(struct rx_ring *rx_ring)
 	struct ql_adapter *qdev = rx_ring->qdev;
 	u32 prod = ql_read_sh_reg(rx_ring->prod_idx_sh_reg);
 	struct ob_mac_iocb_rsp *net_rsp = NULL;
-	int count = 0;
+	int count;
 
 	struct tx_ring *tx_ring;
 	/* While there are entries in the completion queue. */
-	while (prod != rx_ring->cnsmr_idx) {
+	for (count = 0; prod != rx_ring->cnsmr_idx; count++) {
 
 		netif_printk(qdev, rx_status, KERN_DEBUG, qdev->ndev,
 			     "cq_id = %d, prod = %d, cnsmr = %d\n",
@@ -2121,7 +2118,6 @@  static int ql_clean_outbound_rx_ring(struct rx_ring *rx_ring)
 				     "Hit default case, not handled! dropping the packet, opcode = %x.\n",
 				     net_rsp->opcode);
 		}
-		count++;
 		ql_update_cq(rx_ring);
 		prod = ql_read_sh_reg(rx_ring->prod_idx_sh_reg);
 	}
@@ -2146,10 +2142,10 @@  static int ql_clean_inbound_rx_ring(struct rx_ring *rx_ring, int budget)
 	struct ql_adapter *qdev = rx_ring->qdev;
 	u32 prod = ql_read_sh_reg(rx_ring->prod_idx_sh_reg);
 	struct ql_net_rsp_iocb *net_rsp;
-	int count = 0;
+	int count;
 
 	/* While there are entries in the completion queue. */
-	while (prod != rx_ring->cnsmr_idx) {
+	for (count = 0; prod != rx_ring->cnsmr_idx; count++) {
 
 		netif_printk(qdev, rx_status, KERN_DEBUG, qdev->ndev,
 			     "cq_id = %d, prod = %d, cnsmr = %d\n",
@@ -2174,7 +2170,6 @@  static int ql_clean_inbound_rx_ring(struct rx_ring *rx_ring, int budget)
 				     net_rsp->opcode);
 			break;
 		}
-		count++;
 		ql_update_cq(rx_ring);
 		prod = ql_read_sh_reg(rx_ring->prod_idx_sh_reg);
 		if (count == budget)
@@ -3026,13 +3021,12 @@  static int ql_start_rx_ring(struct ql_adapter *qdev, struct rx_ring *rx_ring)
 		cqicb->flags |= FLAGS_LL;	/* Load lbq values */
 		tmp = (u64)rx_ring->lbq.base_dma;
 		base_indirect_ptr = rx_ring->lbq.base_indirect;
-		page_entries = 0;
-		do {
+
+		for (page_entries = 0; page_entries < MAX_DB_PAGES_PER_BQ(QLGE_BQ_LEN);
+		     page_entries++, base_indirect_ptr++) {
 			*base_indirect_ptr = cpu_to_le64(tmp);
 			tmp += DB_PAGE_SIZE;
-			base_indirect_ptr++;
-			page_entries++;
-		} while (page_entries < MAX_DB_PAGES_PER_BQ(QLGE_BQ_LEN));
+		}
 		cqicb->lbq_addr = cpu_to_le64(rx_ring->lbq.base_indirect_dma);
 		cqicb->lbq_buf_size =
 			cpu_to_le16(QLGE_FIT16(qdev->lbq_buf_size));
@@ -3043,13 +3037,12 @@  static int ql_start_rx_ring(struct ql_adapter *qdev, struct rx_ring *rx_ring)
 		cqicb->flags |= FLAGS_LS;	/* Load sbq values */
 		tmp = (u64)rx_ring->sbq.base_dma;
 		base_indirect_ptr = rx_ring->sbq.base_indirect;
-		page_entries = 0;
-		do {
+
+		for (page_entries = 0; page_entries < MAX_DB_PAGES_PER_BQ(QLGE_BQ_LEN);
+		     page_entries++, base_indirect_ptr++) {
 			*base_indirect_ptr = cpu_to_le64(tmp);
 			tmp += DB_PAGE_SIZE;
-			base_indirect_ptr++;
-			page_entries++;
-		} while (page_entries < MAX_DB_PAGES_PER_BQ(QLGE_BQ_LEN));
+		}
 		cqicb->sbq_addr =
 		    cpu_to_le64(rx_ring->sbq.base_indirect_dma);
 		cqicb->sbq_buf_size = cpu_to_le16(SMALL_BUFFER_SIZE);
@@ -4036,9 +4029,11 @@  static int ql_change_rx_buffers(struct ql_adapter *qdev)
 
 	/* Wait for an outstanding reset to complete. */
 	if (!test_bit(QL_ADAPTER_UP, &qdev->flags)) {
-		int i = 4;
+		int i;
 
-		while (--i && !test_bit(QL_ADAPTER_UP, &qdev->flags)) {
+		for (i = 3; i; i--) {
+			if test_bit(QL_ADAPTER_UP, &qdev->flags)
+				break;
 			netif_err(qdev, ifup, qdev->ndev,
 				  "Waiting for adapter UP...\n");
 			ssleep(1);