All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joseph East <eastyjr@gmail.com>
To: Jonas Gorski <jogo@openwrt.org>
Cc: MTD Maling List <linux-mtd@lists.infradead.org>
Subject: Re: [PATCH] mtd: bcm47xxpart.c: Extra TRX magics, NVRAM part handling
Date: Sun, 26 Jul 2015 00:03:56 +0930	[thread overview]
Message-ID: <55B39E54.4080305@gmail.com> (raw)
In-Reply-To: <CAOiHx=na3_WYK1rvo8oO=2rKs5EzTMEcxGT-fccxoOyskfKrQg@mail.gmail.com>

On 25/07/2015 10:08 PM, Jonas Gorski wrote:
> If it's based on four patches, it should be also submitted as four
> patches, especially as these four changes only have in common that
> they touch the same file.

Will re-send patches as separate items shortly. I thought that as each patch only impacted this one file, squashing them would be the better course of action.

>>
>> +       if (!found_nvram) {
>> +               pr_err("can not find a nvram partition reserve last block\n");
> 
> I have trouble parsing this sentence, do you mean somethign like
> "Cannot find an nvram partition, therefore reserve the last block as
> one."? Also this probably should be a pr_warn, not a pr_err, as you
> don't abort here.
> 
> 
>> +               bcm47xxpart_add_part(&parts[curr_part++], "nvram_guess",
>> +                                    master->size - blocksize * 2, MTD_WRITEABLE);
>> +               for (i = 0; i < curr_part; i++) {
>> +                       if (parts[i].size + parts[i].offset == master->size)
>> +                               parts[i].offset -= blocksize * 2;
> 
> ... and you reserve the last *two* blocks here, not just the last block.
> 

Have corrected the description to reflect reservation of the last two blocks in V2.

Cheers,
Joseph

      reply	other threads:[~2015-07-25 14:35 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-25 12:13 [PATCH] mtd: bcm47xxpart.c: Extra TRX magics, NVRAM part handling Joseph East
2015-07-25 12:38 ` Jonas Gorski
2015-07-25 14:33   ` Joseph East [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=55B39E54.4080305@gmail.com \
    --to=eastyjr@gmail.com \
    --cc=jogo@openwrt.org \
    --cc=linux-mtd@lists.infradead.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.