All of lore.kernel.org
 help / color / mirror / Atom feed
From: Srivathsan Sivakumar <sri.skumar05@gmail.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Manish Chopra <manishc@marvell.com>,
	GR-Linux-NIC-Dev@marvell.com, Coiby Xu <coiby.xu@gmail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	netdev@vger.kernel.org, linux-staging@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: [PATCH] staging: qlge: qlge_main.c: convert do-while loops to for loops
Date: Thu, 9 Jun 2022 15:39:53 -0400	[thread overview]
Message-ID: <YqJMieQ3Hf8qc6nj@Sassy> (raw)
In-Reply-To: <20220609183419.GZ2168@kadam>

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

      reply	other threads:[~2022-06-09 19:39 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YqJMieQ3Hf8qc6nj@Sassy \
    --to=sri.skumar05@gmail.com \
    --cc=GR-Linux-NIC-Dev@marvell.com \
    --cc=coiby.xu@gmail.com \
    --cc=dan.carpenter@oracle.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=manishc@marvell.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.