Re: [flashrom] [PATCH 2/3] Refine handling chips that exceed maximum programmer sizes.

[Carl-Daniel Hailfinger]

Am 08.08.2014 16:25 schrieb Stefan Tauner:
> Change check_max_decode() to return the number of (common) busses
> where the flash chip exceeds the supported size of the programmer.
> Refine its signature to use a flashctx pointer only.
> Move CLI-related bits to cli_classic.c.
> Rename check_max_decode() to count_max_decode_exceedings() to
> better reflect what it (now) really does.
> Refine the messages printed by the caller to better integrate with the new
> setup, and simplify them.
>
> Signed-off-by: Stefan Tauner

I like it. One small comment below.
Acked-by: Carl-Daniel Hailfinger
> diff --git a/flashrom.c b/flashrom.c
> index eeed90b..0ff6b63 100644
> --- a/flashrom.c
> +++ b/flashrom.c
> @@ -1079,9 +1073,6 @@ int probe_flash(struct registered_master *mst, int startchip, struct flashctx *f
>    		continue;
>    	}
> size = chip->total_size * 1024;
> check_max_decode(buses_common, size);
> /* Start filling in the dynamic data. */
>   flash->chip = calloc(1, sizeof(struct flashchip));
>   if (!flash->chip) {
>
> @@ -1091,6 +1082,7 @@ int probe_flash(struct registered_master *mst, int startchip, struct flashctx *f
>    	memcpy(flash->chip, chip, sizeof(struct flashchip));
>    	flash->mst = mst;
> size = chip->total_size * 1024;

At this point, it might be better to use "flash->chip->total_size *
1024" given that flash->chip is now valid (it wasn't valid at the old
location).
> base = flashbase ? flashbase : (0xffffffff - size + 1);
> flash->virtual_memory = (chipaddr)programmer_map_flash_region("flash chip", base, size);

Regards,
Carl-Daniel

[PATCH 2/3] Refine handling chips that exceed maximum programmer sizes.

[Stefan Tauner]

On Fri, 15 Aug 2014 00:46:17 +0200
Carl-Daniel Hailfinger  wrote:
> Am 08.08.2014 16:25 schrieb Stefan Tauner:
> > Change check_max_decode() to return the number of (common) busses
> > where the flash chip exceeds the supported size of the programmer.
> > Refine its signature to use a flashctx pointer only.
> > Move CLI-related bits to cli_classic.c.
> > Rename check_max_decode() to count_max_decode_exceedings() to
> > better reflect what it (now) really does.
> > Refine the messages printed by the caller to better integrate with the new
> > setup, and simplify them.
> >
> > Signed-off-by: Stefan Tauner
>
> I like it. One small comment below.
> Acked-by: Carl-Daniel Hailfinger

Thanks.
> > diff --git a/flashrom.c b/flashrom.c
> > index eeed90b..0ff6b63 100644
> > --- a/flashrom.c
> > +++ b/flashrom.c
> > @@ -1079,9 +1073,6 @@ int probe_flash(struct registered_master *mst, int startchip, struct flashctx *f
> >    		continue;
> >    	}
> > size = chip->total_size * 1024;
> > check_max_decode(buses_common, size);
> > /* Start filling in the dynamic data. */
> >   flash->chip = calloc(1, sizeof(struct flashchip));
> >   if (!flash->chip) {
> >
> > @@ -1091,6 +1082,7 @@ int probe_flash(struct registered_master *mst, int startchip, struct flashctx *f
> >    	memcpy(flash->chip, chip, sizeof(struct flashchip));
> >    	flash->mst = mst;
> > size = chip->total_size * 1024;
>
> At this point, it might be better to use "flash->chip->total_size *
> 1024" given that flash->chip is now valid (it wasn't valid at the old
> location).

Mh'kay :)
Committed in r1842 after constifying the argument (and adding chip->).