All of lore.kernel.org
 help / color / mirror / Atom feed
From: Geliang Tang <geliangtang@gmail.com>
To: Marek Vasut <marek.vasut@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>,
	Brian Norris <computersforpeace@gmail.com>,
	Boris Brezillon <boris.brezillon@free-electrons.com>,
	Richard Weinberger <richard@nod.at>,
	Cyrille Pitchen <cyrille.pitchen@atmel.com>,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mtd: mtdswap: use MTDSWAP_ECNT_MIN/MAX
Date: Sat, 24 Dec 2016 23:18:54 +0800	[thread overview]
Message-ID: <20161224151854.a3xxd6jdobjviuu2@ThinkPad> (raw)
In-Reply-To: <252d763e-1767-9bf9-be1a-af0c02d4078f@gmail.com>

On Fri, Dec 23, 2016 at 07:51:56PM +0100, Marek Vasut wrote:
> On 12/20/2016 02:54 PM, Geliang Tang wrote:
> > Since macros MTDSWAP_ECNT_MIN() and MTDSWAP_ECNT_MAX() have been
> > defined in mtdswap.c, use them instead of open-coding.
> > 
> > Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> 
> Sorry for getting to this so late. The patch is fine, but if you grep
> for 'rb_first' in the mtdswap driver , you'll see you can also use the
> MTDSWAP_ECNT_MIN macro in mtdswap_map_free_block() and in
> mtdswap_pick_gc_blk() . Can you fix those too ?

Hi Marek,

Thanks for your review. I double checked the code and found out that we can't
use MTDSWAP_ECNT_MIN macro in mtdswap_map_free_block() and mtdswap_pick_gc_eblk().
Since the MTDSWAP_ECNT_MIN macro points to erase_count field of swap_eb, and
erase_count field are not used in these two functions. Isn't it?

-Geliang

> 
> Otherwise:
> Acked-by: Marek Vasut <marek.vasut@gmail.com>
> 
> > ---
> >  drivers/mtd/mtdswap.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/mtd/mtdswap.c b/drivers/mtd/mtdswap.c
> > index c40e2c9..f12879a 100644
> > --- a/drivers/mtd/mtdswap.c
> > +++ b/drivers/mtd/mtdswap.c
> > @@ -1235,10 +1235,8 @@ static int mtdswap_show(struct seq_file *s, void *data)
> >  
> >  		if (root->rb_node) {
> >  			count[i] = d->trees[i].count;
> > -			min[i] = rb_entry(rb_first(root), struct swap_eb,
> > -					rb)->erase_count;
> > -			max[i] = rb_entry(rb_last(root), struct swap_eb,
> > -					rb)->erase_count;
> > +			min[i] = MTDSWAP_ECNT_MIN(root);
> > +			max[i] = MTDSWAP_ECNT_MAX(root);
> >  		} else
> >  			count[i] = 0;
> >  	}
> > 
> 
> 
> -- 
> Best regards,
> Marek Vasut

  reply	other threads:[~2016-12-24 15:19 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-20 13:54 [PATCH] mtd: mtdswap: use MTDSWAP_ECNT_MIN/MAX Geliang Tang
2016-12-23 18:51 ` Marek Vasut
2016-12-24 15:18   ` Geliang Tang [this message]
2016-12-25 13:49     ` Marek Vasut
2017-04-19 20:14 ` Brian Norris

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=20161224151854.a3xxd6jdobjviuu2@ThinkPad \
    --to=geliangtang@gmail.com \
    --cc=boris.brezillon@free-electrons.com \
    --cc=computersforpeace@gmail.com \
    --cc=cyrille.pitchen@atmel.com \
    --cc=dwmw2@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marek.vasut@gmail.com \
    --cc=richard@nod.at \
    /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.