Re: [flashrom] [PATCH] fix chip size limiting in atapromise

[Urja Rannikko]

Hi,

Seems fine (i built it etc) except for one tiny nitpick noticed by git
(or i think a default commit hook, but whatever) below.

On Sun, Jan 17, 2016 at 2:57 AM, Carl-Daniel Hailfinger
<[e-mail]> wrote:
> Atapromise chip size limiting has two bugs I didn't catch during review,
> but only in the moment of commit.
>
> The current code is checking model_id to remember if a chip has already
> been limited, but if flashchips.c contains two subsequent chips with
> different vendor_id but identical model_id the adjustment will not be
> done. Switch to checking the chip size instead.
>
> If a chip has multiple whole-chip erase functions, only one will be
> modified. Fix that.
>
> Signed-off-by: Carl-Daniel Hailfinger <[e-mail]>
>
> Index: flashrom-atapromise_fix_limit_chip/atapromise.c
> ===================================================================
> --- flashrom-atapromise_fix_limit_chip/atapromise.c     (Revision 1916)
> +++ flashrom-atapromise_fix_limit_chip/atapromise.c     (Arbeitskopie)
> @@ -79,38 +79,35 @@
>
> static void atapromise_limit_chip(struct flashchip *chip)
>  {
> -       static uint32_t last_model_id = 0;
>         unsigned int i, size;
> +       unsigned int usable_erasers = 0;
>
> -       if (chip->model_id == last_model_id)
> -               return;
> -
>         size = chip->total_size * 1024;
> -       if (size > rom_size) {
> -               /* Undefine all block_erasers that don't operate on the whole chip,
> -                * and adjust the eraseblock size of the one that does.
> -                */
> -               for (i = 0; i < NUM_ERASEFUNCTIONS; ++i) {
> -                       if (chip->block_erasers[i].eraseblocks[0].size != size) {
> -                               chip->block_erasers[i].eraseblocks[0].count = 0;
> -                               chip->block_erasers[i].block_erase = NULL;
> -                       } else {
> -                               chip->block_erasers[i].eraseblocks[0].size = rom_size;
> -                               break;
> -                       }
> -               }
>
> -               if (i != NUM_ERASEFUNCTIONS) {
> -                       chip->total_size = rom_size / 1024;
> -                       if (chip->page_size > rom_size)
> -                               chip->page_size = rom_size;
> +       /* Chip is small enough or already limited. */
> +       if (size <= rom_size)
> +                       return;
> ^ That line has a mix of spaces (first) and a tab... rest of the
> function seems to be tabs, so fix with spaces=>tab seems appropriate.

So with that fixed, this is:
Acked-by: Urja Rannikko <[e-mail]>


-- 
Urja Rannikko