* [PATCH] staging: qlge: qlge_main.c: convert do-while loops to for loops @ 2022-06-09 15:15 Srivathsan Sivakumar 2022-06-09 15:26 ` Dan Carpenter 0 siblings, 1 reply; 5+ messages in thread From: Srivathsan Sivakumar @ 2022-06-09 15:15 UTC (permalink / raw) To: Manish Chopra, GR-Linux-NIC-Dev, Coiby Xu, Greg Kroah-Hartman Cc: netdev, linux-staging, linux-kernel simplify do-while loops to for loops Signed-off-by: Srivathsan Sivakumar <sri.skumar05@gmail.com> --- drivers/staging/qlge/qlge_main.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c index 8c35d4c4b851..308e8b621185 100644 --- a/drivers/staging/qlge/qlge_main.c +++ b/drivers/staging/qlge/qlge_main.c @@ -3006,13 +3006,13 @@ static int qlge_start_rx_ring(struct qlge_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 { - *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)); + + for (page_entries = 0; page_entries < + MAX_DB_PAGES_PER_BQ(QLGE_BQ_LEN); page_entries++) { + *base_indirect_ptr = cpu_to_le64(tmp); + tmp += DB_PAGE_SIZE; + base_indirect_ptr++; + } 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)); @@ -3023,13 +3023,13 @@ static int qlge_start_rx_ring(struct qlge_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 { - *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)); + + for (page_entries = 0; page_entries < + MAX_DB_PAGES_PER_BQ(QLGE_BQ_LEN); page_entries++) { + *base_indirect_ptr = cpu_to_le64(tmp); + tmp += DB_PAGE_SIZE; + base_indirect_ptr++; + } cqicb->sbq_addr = cpu_to_le64(rx_ring->sbq.base_indirect_dma); cqicb->sbq_buf_size = cpu_to_le16(SMALL_BUFFER_SIZE); -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: qlge: qlge_main.c: convert do-while loops to for loops 2022-06-09 15:15 [PATCH] staging: qlge: qlge_main.c: convert do-while loops to for loops Srivathsan Sivakumar @ 2022-06-09 15:26 ` Dan Carpenter 2022-06-09 17:01 ` Srivathsan Sivakumar 0 siblings, 1 reply; 5+ messages in thread From: Dan Carpenter @ 2022-06-09 15:26 UTC (permalink / raw) To: Srivathsan Sivakumar Cc: Manish Chopra, GR-Linux-NIC-Dev, Coiby Xu, Greg Kroah-Hartman, netdev, linux-staging, linux-kernel On Thu, Jun 09, 2022 at 11:15:51AM -0400, Srivathsan Sivakumar wrote: > diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c > index 8c35d4c4b851..308e8b621185 100644 > --- a/drivers/staging/qlge/qlge_main.c > +++ b/drivers/staging/qlge/qlge_main.c > @@ -3006,13 +3006,13 @@ static int qlge_start_rx_ring(struct qlge_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 { > - *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)); > + > + for (page_entries = 0; page_entries < > + MAX_DB_PAGES_PER_BQ(QLGE_BQ_LEN); page_entries++) { > + *base_indirect_ptr = cpu_to_le64(tmp); > + tmp += DB_PAGE_SIZE; > + base_indirect_ptr++; > + } It's better than the original, but wouldn't it be better yet to write something like this (untested): for (i = 0; i < MAX_DB_PAGES_PER_BQ(QLGE_BQ_LEN); i++) base_indirect_ptr[i] = cpu_to_le64(tmp + (i * DB_PAGE_SIZE)); Same with the other as well, obviously. regards, dan carpenter ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] staging: qlge: qlge_main.c: convert do-while loops to for loops 2022-06-09 15:26 ` Dan Carpenter @ 2022-06-09 17:01 ` Srivathsan Sivakumar 2022-06-09 18:34 ` Dan Carpenter 0 siblings, 1 reply; 5+ messages in thread From: Srivathsan Sivakumar @ 2022-06-09 17:01 UTC (permalink / raw) To: Dan Carpenter Cc: Manish Chopra, GR-Linux-NIC-Dev, Coiby Xu, Greg Kroah-Hartman, netdev, linux-staging, linux-kernel On Thu, Jun 09, 2022 at 06:26:53PM +0300, Dan Carpenter wrote: > On Thu, Jun 09, 2022 at 11:15:51AM -0400, Srivathsan Sivakumar wrote: > > diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c > > index 8c35d4c4b851..308e8b621185 100644 > > --- a/drivers/staging/qlge/qlge_main.c > > +++ b/drivers/staging/qlge/qlge_main.c > > @@ -3006,13 +3006,13 @@ static int qlge_start_rx_ring(struct qlge_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 { > > - *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)); > > + > > + for (page_entries = 0; page_entries < > > + MAX_DB_PAGES_PER_BQ(QLGE_BQ_LEN); page_entries++) { > > + *base_indirect_ptr = cpu_to_le64(tmp); > > + tmp += DB_PAGE_SIZE; > > + base_indirect_ptr++; > > + } > > It's better than the original, but wouldn't it be better yet to write > something like this (untested): > > for (i = 0; i < MAX_DB_PAGES_PER_BQ(QLGE_BQ_LEN); i++) > base_indirect_ptr[i] = cpu_to_le64(tmp + (i * DB_PAGE_SIZE)); > > Same with the other as well, obviously. > > regards, > dan carpenter > Hello Dan, Thanks for your input wouldn't base_indirect_ptr point at a different endian value if tmp is added with (i * DB_PAGE_SIZE)? Thanks, Srivathsan ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: qlge: qlge_main.c: convert do-while loops to for loops 2022-06-09 17:01 ` Srivathsan Sivakumar @ 2022-06-09 18:34 ` Dan Carpenter 2022-06-09 19:39 ` Srivathsan Sivakumar 0 siblings, 1 reply; 5+ messages in thread From: Dan Carpenter @ 2022-06-09 18:34 UTC (permalink / raw) To: Srivathsan Sivakumar Cc: Manish Chopra, GR-Linux-NIC-Dev, Coiby Xu, Greg Kroah-Hartman, netdev, linux-staging, linux-kernel On Thu, Jun 09, 2022 at 01:01:27PM -0400, Srivathsan Sivakumar wrote: > On Thu, Jun 09, 2022 at 06:26:53PM +0300, Dan Carpenter wrote: > > On Thu, Jun 09, 2022 at 11:15:51AM -0400, Srivathsan Sivakumar wrote: > > > diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c > > > index 8c35d4c4b851..308e8b621185 100644 > > > --- a/drivers/staging/qlge/qlge_main.c > > > +++ b/drivers/staging/qlge/qlge_main.c > > > @@ -3006,13 +3006,13 @@ static int qlge_start_rx_ring(struct qlge_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 { > > > - *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)); > > > + > > > + for (page_entries = 0; page_entries < > > > + MAX_DB_PAGES_PER_BQ(QLGE_BQ_LEN); page_entries++) { > > > + *base_indirect_ptr = cpu_to_le64(tmp); > > > + tmp += DB_PAGE_SIZE; > > > + base_indirect_ptr++; > > > + } > > > > It's better than the original, but wouldn't it be better yet to write > > something like this (untested): > > > > for (i = 0; i < MAX_DB_PAGES_PER_BQ(QLGE_BQ_LEN); i++) > > base_indirect_ptr[i] = cpu_to_le64(tmp + (i * DB_PAGE_SIZE)); > > > > Same with the other as well, obviously. > > > > regards, > > dan carpenter > > > > Hello Dan, > > Thanks for your input > > wouldn't base_indirect_ptr point at a different endian value if tmp is > added with (i * DB_PAGE_SIZE)? tmp is cpu endian so we can do math on it. Then we convert the result to le64. This is how it works before and after. What isn't allowed (doesn't make sense) is to do math on endian data so "cpu_to_le64(tmp) + i * DB_PAGE_SIZE" is wrong. Sparse can detect endian bugs like that: https://lwn.net/Articles/205624/ regards, dan carpenter ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] staging: qlge: qlge_main.c: convert do-while loops to for loops 2022-06-09 18:34 ` Dan Carpenter @ 2022-06-09 19:39 ` Srivathsan Sivakumar 0 siblings, 0 replies; 5+ messages in thread From: Srivathsan Sivakumar @ 2022-06-09 19:39 UTC (permalink / raw) To: Dan Carpenter Cc: Manish Chopra, GR-Linux-NIC-Dev, Coiby Xu, Greg Kroah-Hartman, netdev, linux-staging, linux-kernel On Thu, Jun 09, 2022 at 09:34:19PM +0300, Dan Carpenter wrote: > On Thu, Jun 09, 2022 at 01:01:27PM -0400, Srivathsan Sivakumar wrote: > > On Thu, Jun 09, 2022 at 06:26:53PM +0300, Dan Carpenter wrote: > > > On Thu, Jun 09, 2022 at 11:15:51AM -0400, Srivathsan Sivakumar wrote: > > > > diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c > > > > index 8c35d4c4b851..308e8b621185 100644 > > > > --- a/drivers/staging/qlge/qlge_main.c > > > > +++ b/drivers/staging/qlge/qlge_main.c > > > > @@ -3006,13 +3006,13 @@ static int qlge_start_rx_ring(struct qlge_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 { > > > > - *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)); > > > > + > > > > + for (page_entries = 0; page_entries < > > > > + MAX_DB_PAGES_PER_BQ(QLGE_BQ_LEN); page_entries++) { > > > > + *base_indirect_ptr = cpu_to_le64(tmp); > > > > + tmp += DB_PAGE_SIZE; > > > > + base_indirect_ptr++; > > > > + } > > > > > > It's better than the original, but wouldn't it be better yet to write > > > something like this (untested): > > > > > > for (i = 0; i < MAX_DB_PAGES_PER_BQ(QLGE_BQ_LEN); i++) > > > base_indirect_ptr[i] = cpu_to_le64(tmp + (i * DB_PAGE_SIZE)); > > > > > > Same with the other as well, obviously. > > > > > > regards, > > > dan carpenter > > > > > > > Hello Dan, > > > > Thanks for your input > > > > wouldn't base_indirect_ptr point at a different endian value if tmp is > > added with (i * DB_PAGE_SIZE)? > > tmp is cpu endian so we can do math on it. Then we convert the result > to le64. This is how it works before and after. What isn't allowed > (doesn't make sense) is to do math on endian data so "cpu_to_le64(tmp) + > i * DB_PAGE_SIZE" is wrong. > > Sparse can detect endian bugs like that: > https://lwn.net/Articles/205624/ > > regards, > dan carpenter > I understand now, I'll send the revised patch soon thanks, Srivathsan ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-06-09 19:40 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-06-09 15:15 [PATCH] staging: qlge: qlge_main.c: convert do-while loops to for loops Srivathsan Sivakumar 2022-06-09 15:26 ` Dan Carpenter 2022-06-09 17:01 ` Srivathsan Sivakumar 2022-06-09 18:34 ` Dan Carpenter 2022-06-09 19:39 ` Srivathsan Sivakumar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).