Re: [flashrom] [PATCH] Make struct flashchip a field in struct flashctx instead of a complete copy.

[Carl-Daniel Hailfinger]

Am 26.02.2012 02:05 schrieb Stefan Tauner:
> This is not tested very thoroughly, but it compiles and writing the dummy works.
> Most of the transformation is quite obvious and painless (well, it was no pleasure,
> so i would rather not rebase this often. :)
> I have introduced dedicated struct flashchip variables where the functions use the
> chip field a lot and some of those (not) introduced variables may be debatable,
> but one gets at least a feeling how the code would look like with this change.
>
> One major FIXME is: the code allocates "the new field" while probing and copies
> the data from the *const* struct flashchip in the flashchips.c array into it.
> We need to free that sometime... it is probably obvious (scope of fill_flash), but i
> have not looked into it yet and this is my reminder.

I have answered that in annotations.
> Signed-off-by: Stefan Tauner

Good idea. It was too dangerous to do this directly before 0.9.5, but
now that the tree has reopened, I think we can merge such a patch early
in the cycle (now) and have enough time to get it tested.
> diff --git a/cli_classic.c b/cli_classic.c
> index 972043b..4bce7d7 100644
> --- a/cli_classic.c
> +++ b/cli_classic.c
> @@ -166,6 +166,7 @@ int main(int argc, char *argv[])
>    const struct flashchip *flash;
>    struct flashctx flashes[3];
>    struct flashctx *fill_flash;
> const struct flashchip *chip;
> const char *name;
> int namelen, opt, i, j;
> int startchip = -1, chipcount = 0, option_index = 0, force = 0;
>
> @@ -441,11 +442,13 @@ int main(int argc, char *argv[])
>    	}
>    }
> fill_flash = &flashes[0];

Can you move this assignment back to where it was?
> chip = fill_flash->chip;

And move this assignment together with the other one?
> if (chipcount > 1) {
>    	printf("Multiple flash chips were detected: "%s"",
> 	flashes[0].name);
> 	chip->name);

Can you please use flashes[0].chip->name here to keep the code
consistent? Besides that, do you have any idea why the current code is
so screwed up? Moving a printf for flashes[0] outside a perfectly
working for loop seems odd to me. It would be nice if you could just let
the loop below start at i=0 and remove the chip name part from the
printf above.
> for (i = 1; i < chipcount; i++)
> 	printf(", \"%s\"", flashes[i].name);
> 	printf(", \"%s\"", flashes[i].chip->name);
> printf("\nPlease specify which chip to use with the "
>       "-c <chipname> option.\n");
> ret = 1;
>
> @@ -464,7 +467,7 @@ int main(int argc, char *argv[])
>    		/* This loop just counts compatible controllers. */
>    		for (j = 0; j < registered_programmer_count; j++) {
>    			pgm = &registered_programmers[j];
> 		if (pgm->buses_supported & flashes[0].bustype)
> 		if (pgm->buses_supported & chip->bustype)

flashes[0].chip->bustype please.
Note to self: This code needs to be audited again if it should really
run against flashes[0] here. It looks odd, at least when looking only at
the patch.
> 			compatible_programmers++;
> 	}
> 	if (compatible_programmers > 1)
>
> @@ -491,19 +494,17 @@ int main(int argc, char *argv[])
>    	goto out_shutdown;
>    } else if (!chip_to_probe) {
>    	/* repeat for convenience when looking at foreign logs */
> tempstr = flashbuses_to_text(flashes[0].bustype);
> tempstr = flashbuses_to_text(chip->bustype);
> msg_gdbg("Found %s flash chip "%s" (%d kB, %s).\n",
> 	 flashes[0].vendor, flashes[0].name,
> 	 flashes[0].total_size, tempstr);
> 	 chip->vendor, chip->name,
> 	 chip->total_size, tempstr);

And the flashes[0].chip dance again.
> free(tempstr);
>
> }
> fill_flash = &flashes[0];
> check_chip_supported(fill_flash);
> check_chip_supported(chip);
> size = fill_flash->total_size * 1024;
> if (check_max_decode(fill_flash->pgm->buses_supported & fill_flash->bustype, size) &&
> size = chip->total_size * 1024;
> if (check_max_decode(fill_flash->pgm->buses_supported & chip->bustype, size) &&
>   (!force)) {
> fprintf(stderr, "Chip is too big for this programmer "
> 	"(-V gives details). Use --force to override.\n");
>
> diff --git a/flash.h b/flash.h
> index 0dac13d..ed64512 100644
> --- a/flash.h
> +++ b/flash.h
> @@ -87,6 +87,7 @@ enum chipbustype {
>  #define FEATURE_WRSR_EITHER	(FEATURE_WRSR_EWSR | FEATURE_WRSR_WREN)
>
> struct flashctx;
> +typedef int (erasefunc_t)(struct flashctx *flash, unsigned int addr, unsigned int blocklen);
>
> struct flashchip {
>    const char *vendor;
> @@ -135,7 +136,7 @@ struct flashchip {
>    	} eraseblocks[NUM_ERASEREGIONS];
>    	/* a block_erase function should try to erase one block of size
>    	 * 'blocklen' at address 'blockaddr' and return 0 on success. */
> int (*block_erase) (struct flashctx *flash, unsigned int blockaddr, unsigned int blocklen);
> erasefunc_t *block_erase;

I really really hate that typedef. I wonder if we could keep the code
here and use typeof() instead of the typedef elsewhere.
> } block_erasers[NUM_ERASEFUNCTIONS];
>
> int (*printlock) (struct flashctx *flash);
> @@ -148,35 +149,14 @@ struct flashchip {
>    } voltage;
>  };
>
> -/* struct flashctx must always contain struct flashchip at the beginning. */
>  struct flashctx {
> const char *vendor;
> const char *name;
> enum chipbustype bustype;
> uint32_t manufacture_id;
> uint32_t model_id;
> int total_size;
> int page_size;
> int feature_bits;
> uint32_t tested;
> int (*probe) (struct flashctx *flash);
> int probe_timing;
> struct block_eraser block_erasers[NUM_ERASEFUNCTIONS];
> int (*printlock) (struct flashctx *flash);
> int (*unlock) (struct flashctx *flash);
> int (*write) (struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len);
> int (*read) (struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len);
> struct voltage voltage;
> /* struct flashchip ends here. */
> struct flashchip *chip;

Yay!
> chipaddr virtual_memory;
>    /* Some flash devices have an additional register space. */
>    chipaddr virtual_registers;
>    struct registered_programmer *pgm;
>  };
>
> -typedef int (erasefunc_t)(struct flashctx *flash, unsigned int addr, unsigned int blocklen);
>
> #define TEST_UNTESTED	0
>
> #define TEST_OK_PROBE	(1 << 0)
> diff --git a/flashrom.c b/flashrom.c
> index cad043b..0b6cca3 100644
> --- a/flashrom.c
> +++ b/flashrom.c
> @@ -522,7 +522,7 @@ char *extract_programmer_param(const char *param_name)
>  }
>
> /* Returns the number of well-defined erasers for a chip. */
> -static unsigned int count_usable_erasers(const struct flashctx *flash)
> +static unsigned int count_usable_erasers(const struct flashchip *flash)

Do we want to count usable erasers for the current programmer (for chips
with erase functions limited to certain buses)? If yes, please keep
struct flashctx.
> {
>    unsigned int usable_erasefunctions = 0;
>    int k;
> @@ -941,32 +942,38 @@ int check_max_decode(enum chipbustype buses, uint32_t size)
>  int probe_flash(struct registered_programmer *pgm, int startchip,
>    	struct flashctx *fill_flash, int force)
>  {
> const struct flashchip *flash;
> const struct flashchip *chip;
> unsigned long base = 0;
> char location[64];
> uint32_t size;
> enum chipbustype buses_common;
> char *tmp;
> for (flash = flashchips + startchip; flash && flash->name; flash++) {
> if (chip_to_probe && strcmp(flash->name, chip_to_probe) != 0)
> for (chip = flashchips + startchip; chip && chip->name; chip++) {
> if (chip_to_probe && strcmp(chip->name, chip_to_probe) != 0)
> continue;
> buses_common = pgm->buses_supported & flash->bustype;
> buses_common = pgm->buses_supported & chip->bustype;
> if (!buses_common)
> 	continue;
> msg_gdbg("Probing for %s %s, %d kB: ",
> 	     flash->vendor, flash->name, flash->total_size);
> if (!flash->probe && !force) {
> 	     chip->vendor, chip->name, chip->total_size);
> if (!chip->probe && !force) {
> msg_gdbg("failed! flashrom has no probe function for "
> 	 "this flash chip.\n");
> continue;
> }
> size = flash->total_size * 1024;
> size = chip->total_size * 1024;
>
> check_max_decode(buses_common, size);
>
> /* Start filling in the dynamic data. */
> memcpy(fill_flash, flash, sizeof(struct flashchip));
> /* FIXME: when to free? */

Tricky, but I think we are lucky here because there is only one path
which returns a non-match and that's the only path where we want to
free. I have annotated that place a few hunks below.
> fill_flash->chip = malloc(sizeof(struct flashchip));
> if (fill_flash->chip == NULL) {
> 	msg_gerr("%s: out of memory.\n", __func__);
> 	return -1;

Can we return an explicit code for OOM here or would that screw up the
caller?
> }
> memcpy(fill_flash->chip, chip, sizeof(struct flashchip));
>
> fill_flash->pgm = pgm;
>
> base = flashbase ? flashbase : (0xffffffff - size + 1);
>
> @@ -985,11 +992,11 @@ int probe_flash(struct registered_programmer *pgm, int startchip,
>    	 * one for this programmer interface and thus no other chip has
>    	 * been found on this interface.
>    	 */
> if (startchip == 0 && fill_flash->model_id == SFDP_DEVICE_ID) {
> if (startchip == 0 && chip->model_id == SFDP_DEVICE_ID) {

We used fill_flash for consistency (i.e. only use the rw copy of the
flashchip entry) here, and if we want to keep that consistency, it's
fill_flash->chip->model_id.
> 	msg_cinfo("===\n"
> 		  "SFDP has autodetected a flash chip which is "
> 		  "not natively supported by flashrom yet.\n");
> 	if (count_usable_erasers(fill_flash) == 0)
> 	if (count_usable_erasers(chip) == 0)

See the count_usable_erasers comment a few hunks above.
> 		msg_cinfo("The standard operations read and "
> 			  "verify should work, but to support "
> 			  "erase, write and all other "
>
> @@ -1010,15 +1017,15 @@ int probe_flash(struct registered_programmer *pgm, int startchip,
>    	}
> if (startchip == 0 ||
>     ((fill_flash->model_id != GENERIC_DEVICE_ID) &&
>      (fill_flash->model_id != SFDP_DEVICE_ID)))
>     ((chip->model_id != GENERIC_DEVICE_ID) &&
>      (chip->model_id != SFDP_DEVICE_ID)))

See above for fill_flash->chip->model_id.
> 	break;
>
> notfound:
>    	programmer_unmap_flash_region((void *)fill_flash->virtual_memory, size);
>    }
> if (!flash || !flash->name)
> if (!chip || !chip->name)

Here you have to free flash->chip. No other freeing is needed AFAICS.
> return -1;
>
> #if CONFIG_INTERNAL == 1
> @@ -1028,27 +1035,27 @@ notfound:
>  #endif
>    	snprintf(location, sizeof(location), "on %s", programmer_table[programmer].name);
> tmp = flashbuses_to_text(flash->bustype);
> tmp = flashbuses_to_text(chip->bustype);

fill_flash->chip->...
> msg_cinfo("%s %s flash chip "%s" (%d kB, %s) %s.\n",
>   force ? "Assuming" : "Found", fill_flash->vendor,
>   fill_flash->name, fill_flash->total_size, tmp, location);
>   force ? "Assuming" : "Found", chip->vendor,
>   chip->name, chip->total_size, tmp, location);

dito
> free(tmp);
>
> /* Flash registers will not be mapped if the chip was forced. Lock info
>     * may be stored in registers, so avoid lock info printing.
>     */
>    if (!force)
> if (fill_flash->printlock)
> 	fill_flash->printlock(fill_flash);
> if (chip->printlock)

dito
> 	chip->printlock(fill_flash);

dito
> /* Return position of matching chip. */
> return flash - flashchips;
> return chip - flashchips;
>
> }
>
> int verify_flash(struct flashctx *flash, uint8_t *buf)
> @@ -1308,7 +1315,7 @@ static int walk_eraseregions(struct flashctx *flash, int erasefunction,
>    return 0;
>  }
>
> -static int check_block_eraser(const struct flashctx *flash, int k, int log)
> +static int check_block_eraser(const struct flashchip *flash, int k, int log)

No conversion, please. See below.
> {
>    struct block_eraser eraser = flash->block_erasers[k];
>
> @@ -1357,7 +1366,7 @@ int erase_and_write_flash(struct flashctx *flash, uint8_t *oldcontents,
>    		break;
>    	}
>    	msg_cdbg("Trying erase function %i... ", k);
> if (check_block_eraser(flash, k, 1))
> if (check_block_eraser(chip, k, 1))

This one is debatable. We know that some chips support certain erase
functions only on some buses, e.g. Sharp LHF00L04 which can do a
full-chip erase only if it is accesses in A/A Mux mode, not in FWH mode.
My mid-term plan is to augment erase functions (and possibly also
read/write/probe) with a list of valid buses for any given function.
> 	continue;
> usable_erasefunctions--;
> ret = walk_eraseregions(flash, k, &erase_and_write_block_helper,
>
> @@ -1550,14 +1559,6 @@ int selfcheck(void)
>    	msg_gerr("Flashchips table miscompilation!\n");
>    	ret = 1;
>    }
> /* Check that virtual_memory in struct flashctx is placed directly
> * after the members copied from struct flashchip.
> */
> if (sizeof(struct flashchip) !=
>    offsetof(struct flashctx, virtual_memory)) {
> msg_gerr("struct flashctx broken!\n");
> ret = 1;
> }

Glad to lose this hunk!
> for (flash = flashchips; flash && flash->name; flash++)
>    	if (selfcheck_eraseblocks(flash))
>    		ret = 1;
> @@ -1642,7 +1643,7 @@ void check_chip_supported(const struct flashctx *flash)
>  /* FIXME: This function signature needs to be improved once doit() has a better
> function signature.
>
> */
> -int chip_safety_check(struct flashctx *flash, int force, int read_it,
> +int chip_safety_check(const struct flashchip *flash, int force, int read_it,

Can you please make sure that all pointers to struct flashchip are
called "chip"? Otherwise grepping for "chip->" won't return all accesses
to struct flashchip. The same comment applies to other hunks of the
patch as well.

However, chip_safety_check can not be converted to a different calling
convention because we need struct flashctx here. Admittedly we don't
need it right now, but we will need it once programmer_may_write is part
of the programmer struct in struct flashctx.
>       int write_it, int erase_it, int verify_it)
>
> {
>    if (!programmer_may_write && (write_it || erase_it)) {
> @@ -1710,9 +1711,10 @@ int doit(struct flashctx *flash, int force, const char *filename, int read_it,
>    uint8_t *oldcontents;
>    uint8_t *newcontents;
>    int ret = 0;
> unsigned long size = flash->total_size * 1024;
> const struct flashchip *chip = flash->chip;
> unsigned long size = chip->total_size * 1024;
> if (chip_safety_check(flash, force, read_it, write_it, erase_it, verify_it)) {
> if (chip_safety_check(chip, force, read_it, write_it, erase_it, verify_it)) {

No conversion, please. See above.
> msg_cerr("Aborting.\n");
> ret = 1;
> goto out_nofree;
>
> @@ -1791,7 +1793,7 @@ int doit(struct flashctx *flash, int force, const char *filename, int read_it,
>
> // This should be moved into each flash part's code to do it 
>    // cleanly. This does the job.
> handle_romentries(flash, oldcontents, newcontents);
> handle_romentries(chip, oldcontents, newcontents);

No conversion, please. See two hunks below.
> // ////////////////////////////////////////////////////////////
>
> diff --git a/jedec.c b/jedec.c
> index 69c0c0c..5c549a2 100644
> --- a/jedec.c
> +++ b/jedec.c
> @@ -93,7 +93,7 @@ void data_polling_jedec(const struct flashctx *flash, chipaddr dst,
>    	msg_cdbg("%s: excessive loops, i=0x%x\n", __func__, i);
>  }
>
> -static unsigned int getaddrmask(struct flashctx *flash)
> +static unsigned int getaddrmask(const struct flashchip *flash)

Hm yes, const makes sense. In fact, const makes sense in many more places.
> {
>    switch (flash->feature_bits & FEATURE_ADDR_MASK) {
>    case FEATURE_ADDR_FULL:
> diff --git a/layout.c b/layout.c
> index 90d3cce..f0f7a6f 100644
> --- a/layout.c
> +++ b/layout.c
> @@ -302,7 +302,7 @@ romlayout_t *get_next_included_romentry(unsigned int start)
>    return best_entry;
>  }
>
> -int handle_romentries(struct flashctx *flash, uint8_t *oldcontents, uint8_t *newcontents)
> +int handle_romentries(const struct flashchip *flash, uint8_t *oldcontents, uint8_t *newcontents)

Will we have to change this back once we try to merge the layout
patches? In that case, we need some way to get permissible regions from
the programmer, and that would probably be done via the programmer struct.
> {
>    unsigned int start = 0;
>    romlayout_t *entry;
> diff --git a/spi25.c b/spi25.c
> index b7e8189..2accb6d 100644
> --- a/spi25.c
> +++ b/spi25.c
> @@ -416,22 +416,23 @@ void spi_prettyprint_status_register_sst25vf040b(uint8_t status)
>
> int spi_prettyprint_status_register(struct flashctx *flash)
>  {
> const struct flashchip *chip = flash->chip;

const struct flashchip here, but just struct flashchip in the next hunk.
What's the reason? Same question applies to other hunks of the patch as
well.
> uint8_t status;
>
> status = spi_read_status_register(flash);
>    msg_cdbg("Chip status register is %02x\n", status);
> switch (flash->manufacture_id) {
> switch (chip->manufacture_id) {
> case ST_ID:
> if (((flash->model_id & 0xff00) == 0x2000) ||
>     ((flash->model_id & 0xff00) == 0x2500))
> if (((chip->model_id & 0xff00) == 0x2000) ||
>     ((chip->model_id & 0xff00) == 0x2500))
> spi_prettyprint_status_register_st_m25p(status);
> break;
> case MACRONIX_ID:
> if ((flash->model_id & 0xff00) == 0x2000)
> if ((chip->model_id & 0xff00) == 0x2000)
> spi_prettyprint_status_register_st_m25p(status);
> break;
> case SST_ID:
> switch (flash->model_id) {
> switch (chip->model_id) {
> case 0x2541:
> 	spi_prettyprint_status_register_sst25vf016(status);
> 	break;
>
> @@ -862,16 +863,17 @@ static int spi_write_status_register_wren(struct flashctx *flash, int status)
>
> int spi_write_status_register(struct flashctx *flash, int status)
>  {
> struct flashchip *chip = flash->chip;
> int ret = 1;
> if (!(flash->feature_bits & (FEATURE_WRSR_WREN | FEATURE_WRSR_EWSR))) {
> if (!(chip->feature_bits & (FEATURE_WRSR_WREN | FEATURE_WRSR_EWSR))) {
> msg_cdbg("Missing status register write definition, assuming "
> 	 "EWSR is needed\n");
> flash->feature_bits |= FEATURE_WRSR_EWSR;
> chip->feature_bits |= FEATURE_WRSR_EWSR;
> }
> if (flash->feature_bits & FEATURE_WRSR_WREN)
> if (chip->feature_bits & FEATURE_WRSR_WREN)
> ret = spi_write_status_register_wren(flash, status);
> if (ret && (flash->feature_bits & FEATURE_WRSR_EWSR))
> if (ret && (chip->feature_bits & FEATURE_WRSR_EWSR))
> ret = spi_write_status_register_ewsr(flash, status);
> return ret;
>
> }

I think the next iteration of this patch can go in.

Regards,
Carl-Daniel

[PATCH] Make struct flashchip a field in struct flashctx instead of a complete copy.

[Stefan Tauner]

On Sun, 11 Mar 2012 23:11:20 +0100
Carl-Daniel Hailfinger  wrote:
> Am 26.02.2012 02:05 schrieb Stefan Tauner:
> > This is not tested very thoroughly, but it compiles and writing the dummy works.
> > Most of the transformation is quite obvious and painless (well, it was no pleasure,
> > so i would rather not rebase this often. :)
> > I have introduced dedicated struct flashchip variables where the functions use the
> > chip field a lot and some of those (not) introduced variables may be debatable,
> > but one gets at least a feeling how the code would look like with this change.
> >
> > One major FIXME is: the code allocates "the new field" while probing and copies
> > the data from the *const* struct flashchip in the flashchips.c array into it.
> > We need to free that sometime... it is probably obvious (scope of fill_flash), but i
> > have not looked into it yet and this is my reminder.
>
> I have answered that in annotations.
> > Signed-off-by: Stefan Tauner
>
> Good idea. It was too dangerous to do this directly before 0.9.5, but
> now that the tree has reopened, I think we can merge such a patch early
> in the cycle (now) and have enough time to get it tested.

sorry for not touching it so long, but it is one of the *cough* less
exciting patches :)
the free() problem still needs to be addressed before this can be
merged, please see below.
i had to do parts of this twice because i made a mistake that not even
git was able to prevent... so the patch content may not be 100%
reflected by this mail and vice versa, sorry.
> > diff --git a/cli_classic.c b/cli_classic.c
> > index 972043b..4bce7d7 100644
> > --- a/cli_classic.c
> > +++ b/cli_classic.c
> > @@ -166,6 +166,7 @@ int main(int argc, char *argv[])
> >    const struct flashchip *flash;
> >    struct flashctx flashes[3];
> >    struct flashctx *fill_flash;
> > const struct flashchip *chip;
> > const char *name;
> > int namelen, opt, i, j;
> > int startchip = -1, chipcount = 0, option_index = 0, force = 0;
> >
> > @@ -441,11 +442,13 @@ int main(int argc, char *argv[])
> >    	}
> >    }
> > fill_flash = &flashes[0];
>
> Can you move this assignment back to where it was?
> > chip = fill_flash->chip;
>
> And move this assignment together with the other one?

done, although i am not sure i really like it...
> > if (chipcount > 1) {
> >    	printf("Multiple flash chips were detected: "%s"",
> > 	flashes[0].name);
> > 	chip->name);
>
> Can you please use flashes[0].chip->name here to keep the code
> consistent?

done against my will. one day i will create a patch that fixes probing
and persuade you... but not today :)
> Besides that, do you have any idea why the current code is
> so screwed up? Moving a printf for flashes[0] outside a perfectly
> working for loop seems odd to me. It would be nice if you could just let
> the loop below start at i=0 and remove the chip name part from the
> printf above.

due to ", " printing! the first chip does not have one in front.
alternative would be to have the last one as special case. left it
unchanged for now.
> > for (i = 1; i < chipcount; i++)
> > 	printf(", \"%s\"", flashes[i].name);
> > 	printf(", \"%s\"", flashes[i].chip->name);
> > printf("\nPlease specify which chip to use with the "
> >       "-c <chipname> option.\n");
> > ret = 1;
> >
> > @@ -464,7 +467,7 @@ int main(int argc, char *argv[])
> >    		/* This loop just counts compatible controllers. */
> >    		for (j = 0; j < registered_programmer_count; j++) {
> >    			pgm = &registered_programmers[j];
> > 		if (pgm->buses_supported & flashes[0].bustype)
> > 		if (pgm->buses_supported & chip->bustype)
>
> flashes[0].chip->bustype please.
> Note to self: This code needs to be audited again if it should really
> run against flashes[0] here. It looks odd, at least when looking only at
> the patch.

i think it is ok... this is the --force path where multiple chips have
been discovered. we want to just use the first one in that case i
think...
> > 			compatible_programmers++;
> > 	}
> > 	if (compatible_programmers > 1)
> >
> > @@ -491,19 +494,17 @@ int main(int argc, char *argv[])
> >    	goto out_shutdown;
> >    } else if (!chip_to_probe) {
> >    	/* repeat for convenience when looking at foreign logs */
> > tempstr = flashbuses_to_text(flashes[0].bustype);
> > tempstr = flashbuses_to_text(chip->bustype);
> > msg_gdbg("Found %s flash chip "%s" (%d kB, %s).\n",
> > 	 flashes[0].vendor, flashes[0].name,
> > 	 flashes[0].total_size, tempstr);
> > 	 chip->vendor, chip->name,
> > 	 chip->total_size, tempstr);
>
> And the flashes[0].chip dance again.

done (see above).
> > free(tempstr);
> >
> > }
> > fill_flash = &flashes[0];
> > check_chip_supported(fill_flash);
> > check_chip_supported(chip);
> > size = fill_flash->total_size * 1024;
> > if (check_max_decode(fill_flash->pgm->buses_supported & fill_flash->bustype, size) &&
> > size = chip->total_size * 1024;
> > if (check_max_decode(fill_flash->pgm->buses_supported & chip->bustype, size) &&
> >   (!force)) {
> > fprintf(stderr, "Chip is too big for this programmer "
> > 	"(-V gives details). Use --force to override.\n");
> >
> > diff --git a/flash.h b/flash.h
> > index 0dac13d..ed64512 100644
> > --- a/flash.h
> > +++ b/flash.h
> > @@ -87,6 +87,7 @@ enum chipbustype {
> >  #define FEATURE_WRSR_EITHER	(FEATURE_WRSR_EWSR | FEATURE_WRSR_WREN)
> >
> > struct flashctx;
> > +typedef int (erasefunc_t)(struct flashctx *flash, unsigned int addr, unsigned int blocklen);
> >
> > struct flashchip {
> >    const char *vendor;
> > @@ -135,7 +136,7 @@ struct flashchip {
> >    	} eraseblocks[NUM_ERASEREGIONS];
> >    	/* a block_erase function should try to erase one block of size
> >    	 * 'blocklen' at address 'blockaddr' and return 0 on success. */
> > int (*block_erase) (struct flashctx *flash, unsigned int blockaddr, unsigned int blocklen);
> > erasefunc_t *block_erase;
>
> I really really hate that typedef. I wonder if we could keep the code
> here and use typeof() instead of the typedef elsewhere.

dunno. i like it :)
if you care, please look into it yourself and tell me what i should
change and where, or better add a patch on top of this one please.
> > diff --git a/flashrom.c b/flashrom.c
> > index cad043b..0b6cca3 100644
> > --- a/flashrom.c
> > +++ b/flashrom.c
> > @@ -522,7 +522,7 @@ char *extract_programmer_param(const char *param_name)
> >  }
> >
> > /* Returns the number of well-defined erasers for a chip. */
> > -static unsigned int count_usable_erasers(const struct flashctx *flash)
> > +static unsigned int count_usable_erasers(const struct flashchip *flash)
>
> Do we want to count usable erasers for the current programmer (for chips
> with erase functions limited to certain buses)? If yes, please keep
> struct flashctx.

changed back to flashctx
> > @@ -941,32 +942,38 @@ int check_max_decode(enum chipbustype buses, uint32_t size)
> >  int probe_flash(struct registered_programmer *pgm, int startchip,
> >    	struct flashctx *fill_flash, int force)
> >  {
> > const struct flashchip *flash;
> > const struct flashchip *chip;
> > unsigned long base = 0;
> > char location[64];
> > uint32_t size;
> > enum chipbustype buses_common;
> > char *tmp;
> > for (flash = flashchips + startchip; flash && flash->name; flash++) {
> > if (chip_to_probe && strcmp(flash->name, chip_to_probe) != 0)
> > for (chip = flashchips + startchip; chip && chip->name; chip++) {
> > if (chip_to_probe && strcmp(chip->name, chip_to_probe) != 0)
> > continue;
> > buses_common = pgm->buses_supported & flash->bustype;
> > buses_common = pgm->buses_supported & chip->bustype;
> > if (!buses_common)
> > 	continue;
> > msg_gdbg("Probing for %s %s, %d kB: ",
> > 	     flash->vendor, flash->name, flash->total_size);
> > if (!flash->probe && !force) {
> > 	     chip->vendor, chip->name, chip->total_size);
> > if (!chip->probe && !force) {
> > msg_gdbg("failed! flashrom has no probe function for "
> > 	 "this flash chip.\n");
> > continue;
> > }
> > size = flash->total_size * 1024;
> > size = chip->total_size * 1024;
> >
> > check_max_decode(buses_common, size);
> >
> > /* Start filling in the dynamic data. */
> > memcpy(fill_flash, flash, sizeof(struct flashchip));
> > /* FIXME: when to free? */
>
> Tricky, but I think we are lucky here because there is only one path
> which returns a non-match and that's the only path where we want to
> free. I have annotated that place a few hunks below.

see there...
> > fill_flash->chip = malloc(sizeof(struct flashchip));
> > if (fill_flash->chip == NULL) {
> > 	msg_gerr("%s: out of memory.\n", __func__);
> > 	return -1;
>
> Can we return an explicit code for OOM here or would that screw up the
> caller?

we can. i would rather do this later, when we have defined the global
error codes. there are lots of other places like this anyway that need
changes after the defines are fixed.
-> unchanged
> > }
> > memcpy(fill_flash->chip, chip, sizeof(struct flashchip));
> >
> > fill_flash->pgm = pgm;
> >
> > base = flashbase ? flashbase : (0xffffffff - size + 1);
> >
> > @@ -985,11 +992,11 @@ int probe_flash(struct registered_programmer *pgm, int startchip,
> >    	 * one for this programmer interface and thus no other chip has
> >    	 * been found on this interface.
> >    	 */
> > if (startchip == 0 && fill_flash->model_id == SFDP_DEVICE_ID) {
> > if (startchip == 0 && chip->model_id == SFDP_DEVICE_ID) {
>
> We used fill_flash for consistency (i.e. only use the rw copy of the
> flashchip entry) here, and if we want to keep that consistency, it's
> fill_flash->chip->model_id.

after changing it everywhere you noted, it is makes sense. it would
probably have been easier for *both of us* if you would have changed it
yourself though. while this weakens the "capsulation" between the
reviewer and the author i think it would often be way better and faster.
> > 	msg_cinfo("===\n"
> > 		  "SFDP has autodetected a flash chip which is "
> > 		  "not natively supported by flashrom yet.\n");
> > 	if (count_usable_erasers(fill_flash) == 0)
> > 	if (count_usable_erasers(chip) == 0)
>
> See the count_usable_erasers comment a few hunks above.
> > 		msg_cinfo("The standard operations read and "
> > 			  "verify should work, but to support "
> > 			  "erase, write and all other "
> >
> > @@ -1010,15 +1017,15 @@ int probe_flash(struct registered_programmer *pgm, int startchip,
> >    	}
> > if (startchip == 0 ||
> >     ((fill_flash->model_id != GENERIC_DEVICE_ID) &&
> >      (fill_flash->model_id != SFDP_DEVICE_ID)))
> >     ((chip->model_id != GENERIC_DEVICE_ID) &&
> >      (chip->model_id != SFDP_DEVICE_ID)))
>
> See above for fill_flash->chip->model_id.
> > 	break;
> >
> > notfound:
> >    	programmer_unmap_flash_region((void *)fill_flash->virtual_memory, size);
> >    }
> > if (!flash || !flash->name)
> > if (!chip || !chip->name)
>
> Here you have to free flash->chip. No other freeing is needed AFAICS.

well yes i have to free it here, but if the function succeeds the
reference escapes inside the function's input parameter struct flashctx
*fill_flash.

i have not thought it through, but maybe we should free it before
mallocing it?
> > return -1;
> >
> > #if CONFIG_INTERNAL == 1
> > @@ -1028,27 +1035,27 @@ notfound:
> >  #endif
> >    	snprintf(location, sizeof(location), "on %s", programmer_table[programmer].name);
> > tmp = flashbuses_to_text(flash->bustype);
> > tmp = flashbuses_to_text(chip->bustype);
>
> fill_flash->chip->...
> […]

done
> > {
> >    struct block_eraser eraser = flash->block_erasers[k];
> >
> > @@ -1357,7 +1366,7 @@ int erase_and_write_flash(struct flashctx *flash, uint8_t *oldcontents,
> >    		break;
> >    	}
> >    	msg_cdbg("Trying erase function %i... ", k);
> > if (check_block_eraser(flash, k, 1))
> > if (check_block_eraser(chip, k, 1))
>
> This one is debatable. We know that some chips support certain erase
> functions only on some buses, e.g. Sharp LHF00L04 which can do a
> full-chip erase only if it is accesses in A/A Mux mode, not in FWH mode.
> My mid-term plan is to augment erase functions (and possibly also
> read/write/probe) with a list of valid buses for any given function.

reverted
> > for (flash = flashchips; flash && flash->name; flash++)
> >    	if (selfcheck_eraseblocks(flash))
> >    		ret = 1;
> > @@ -1642,7 +1643,7 @@ void check_chip_supported(const struct flashctx *flash)
> >  /* FIXME: This function signature needs to be improved once doit() has a better
> > function signature.
> >
> > */
> > -int chip_safety_check(struct flashctx *flash, int force, int read_it,
> > +int chip_safety_check(const struct flashchip *flash, int force, int read_it,
>
> Can you please make sure that all pointers to struct flashchip are
> called "chip"? Otherwise grepping for "chip->" won't return all accesses
> to struct flashchip. The same comment applies to other hunks of the
> patch as well.

hopefully done, this now includes print.c too.
> However, chip_safety_check can not be converted to a different calling
> convention because we need struct flashctx here. Admittedly we don't
> need it right now, but we will need it once programmer_may_write is part
> of the programmer struct in struct flashctx.

reverted
> > {
> >    switch (flash->feature_bits & FEATURE_ADDR_MASK) {
> >    case FEATURE_ADDR_FULL:
> > diff --git a/layout.c b/layout.c
> > index 90d3cce..f0f7a6f 100644
> > --- a/layout.c
> > +++ b/layout.c
> > @@ -302,7 +302,7 @@ romlayout_t *get_next_included_romentry(unsigned int start)
> >    return best_entry;
> >  }
> >
> > -int handle_romentries(struct flashctx *flash, uint8_t *oldcontents, uint8_t *newcontents)
> > +int handle_romentries(const struct flashchip *flash, uint8_t *oldcontents, uint8_t *newcontents)
>
> Will we have to change this back once we try to merge the layout
> patches? In that case, we need some way to get permissible regions from
> the programmer, and that would probably be done via the programmer struct.

with the current implementation this is not needed, but you are right
that his would definitely go into the context and not the flash chip
(although i dont think it will be in the programmer... i think our
region solution will have its own entry in the context because it is
not bound to either the programmer or the flash chip, but both (and
more (i.e. the user!))).
> > {
> >    unsigned int start = 0;
> >    romlayout_t *entry;
> > diff --git a/spi25.c b/spi25.c
> > index b7e8189..2accb6d 100644
> > --- a/spi25.c
> > +++ b/spi25.c
> > @@ -416,22 +416,23 @@ void spi_prettyprint_status_register_sst25vf040b(uint8_t status)
> >
> > int spi_prettyprint_status_register(struct flashctx *flash)
> >  {
> > const struct flashchip *chip = flash->chip;
>
> const struct flashchip here, but just struct flashchip in the next hunk.
> What's the reason? Same question applies to other hunks of the patch as
> well.
> […]

added const almost everywhere. const flashctx would also make a lot
of sense on many occasions, but this patch is too big already and i am
fed up already :)

[PATCH] Make struct flashchip a field in struct flashctx instead of a complete copy.

[Carl-Daniel Hailfinger]

Counter-proposal (conversion from scratch). AFAICS the only differences
to your version regarding code flow are in cli_classic.c and flashrom.c.

Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-flashctx_separate_struct_flashchip/flash.h
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/flash.h	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/flash.h	(Arbeitskopie)
@@ -148,27 +148,8 @@
    } voltage;
 };

-/* struct flashctx must always contain struct flashchip at the beginning. */
 struct flashctx {
-	const char *vendor;
-	const char *name;
-	enum chipbustype bustype;
-	uint32_t manufacture_id;
-	uint32_t model_id;
-	int total_size;
-	int page_size;
-	int feature_bits;
-	uint32_t tested;
-	int (*probe) (struct flashctx *flash);
-	int probe_timing;
-	struct block_eraser block_erasers[NUM_ERASEFUNCTIONS];
-	int (*printlock) (struct flashctx *flash);
-	int (*unlock) (struct flashctx *flash);
-	int (*write) (struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len);
-	int (*read) (struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len);
-	struct voltage voltage;
-	/* struct flashchip ends here. */
-	
+	struct flashchip *chip;
    chipaddr virtual_memory;
    /* Some flash devices have an additional register space. */
    chipaddr virtual_registers;
Index: flashrom-flashctx_separate_struct_flashchip/it87spi.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/it87spi.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/it87spi.c	(Arbeitskopie)
@@ -331,7 +331,7 @@
    /* FIXME: The command below seems to be redundant or wrong. */
    OUTB(0x06, it8716f_flashport + 1);
    OUTB(((2 + (fast_spi ? 1 : 0)) << 4), it8716f_flashport);
-	for (i = 0; i < flash->page_size; i++)
+	for (i = 0; i < flash->chip->page_size; i++)
    	mmio_writeb(buf[i], (void *)(bios + start + i));
    OUTB(0, it8716f_flashport);
    /* Wait until the Write-In-Progress bit is cleared.
@@ -355,7 +355,7 @@
     * the mainboard does not use IT87 SPI translation. This should be done
     * via a programmer parameter for the internal programmer.
     */
-	if ((flash->total_size * 1024 > 512 * 1024)) {
+	if ((flash->chip->total_size * 1024 > 512 * 1024)) {
    	spi_read_chunked(flash, buf, start, len, 3);
    } else {
    	mmio_readn((void *)(flash->virtual_memory + start), buf, len);
@@ -377,28 +377,28 @@
     * the mainboard does not use IT87 SPI translation. This should be done
     * via a programmer parameter for the internal programmer.
     */
-	if ((flash->total_size * 1024 > 512 * 1024) ||
-	    (flash->page_size > 256)) {
+	if ((flash->chip->total_size * 1024 > 512 * 1024) ||
+	    (flash->chip->page_size > 256)) {
    	spi_chip_write_1(flash, buf, start, len);
    } else {
    	unsigned int lenhere;

-		if (start % flash->page_size) {
+		if (start % flash->chip->page_size) {
    		/* start to the end of the page or to start + len,
    		 * whichever is smaller.
    		 */
-			lenhere = min(len, flash->page_size - start % flash->page_size);
+			lenhere = min(len, flash->chip->page_size - start % flash->chip->page_size);
    		spi_chip_write_1(flash, buf, start, lenhere);
    		start += lenhere;
    		len -= lenhere;
    		buf += lenhere;
    	}

-		while (len >= flash->page_size) {
+		while (len >= flash->chip->page_size) {
    		it8716f_spi_page_program(flash, buf, start);
-			start += flash->page_size;
-			len -= flash->page_size;
-			buf += flash->page_size;
+			start += flash->chip->page_size;
+			len -= flash->chip->page_size;
+			buf += flash->chip->page_size;
    	}
    	if (len)
    		spi_chip_write_1(flash, buf, start, len);
Index: flashrom-flashctx_separate_struct_flashchip/jedec.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/jedec.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/jedec.c	(Arbeitskopie)
@@ -93,9 +93,9 @@
    	msg_cdbg("%s: excessive loops, i=0x%x\n", __func__, i);
 }

-static unsigned int getaddrmask(struct flashctx *flash)
+static unsigned int getaddrmask(const struct flashchip *chip)
 {
-	switch (flash->feature_bits & FEATURE_ADDR_MASK) {
+	switch (chip->feature_bits & FEATURE_ADDR_MASK) {
    case FEATURE_ADDR_FULL:
    	return MASK_FULL;
    	break;
@@ -129,11 +129,11 @@
    uint32_t flashcontent1, flashcontent2;
    int probe_timing_enter, probe_timing_exit;

-	if (flash->probe_timing > 0) 
-		probe_timing_enter = probe_timing_exit = flash->probe_timing;
-	else if (flash->probe_timing == TIMING_ZERO) { /* No delay. */
+	if (flash->chip->probe_timing > 0) 
+		probe_timing_enter = probe_timing_exit = flash->chip->probe_timing;
+	else if (flash->chip->probe_timing == TIMING_ZERO) { /* No delay. */
    	probe_timing_enter = probe_timing_exit = 0;
-	} else if (flash->probe_timing == TIMING_FIXME) { /* == _IGNORED */
+	} else if (flash->chip->probe_timing == TIMING_FIXME) { /* == _IGNORED */
    	msg_cdbg("Chip lacks correct probe timing information, "
    		     "using default 10mS/40uS. ");
    	probe_timing_enter = 10000;
@@ -151,7 +151,7 @@
    if (probe_timing_enter)
    	programmer_delay(probe_timing_enter);
    /* Reset chip to a clean slate */
-	if ((flash->feature_bits & FEATURE_RESET_MASK) == FEATURE_LONG_RESET)
+	if ((flash->chip->feature_bits & FEATURE_RESET_MASK) == FEATURE_LONG_RESET)
    {
    	chip_writeb(flash, 0xAA, bios + (0x5555 & mask));
    	if (probe_timing_exit)
@@ -194,7 +194,7 @@
    }

/* Issue JEDEC Product ID Exit command */
-	if ((flash->feature_bits & FEATURE_RESET_MASK) == FEATURE_LONG_RESET)
+	if ((flash->chip->feature_bits & FEATURE_RESET_MASK) == FEATURE_LONG_RESET)
    {
    	chip_writeb(flash, 0xAA, bios + (0x5555 & mask));
    	if (probe_timing_exit)
@@ -231,10 +231,10 @@
    	msg_cdbg(", id2 is normal flash content");

msg_cdbg("\n");
-	if (largeid1 != flash->manufacture_id || largeid2 != flash->model_id)
+	if (largeid1 != flash->chip->manufacture_id || largeid2 != flash->chip->model_id)
    	return 0;

-	if (flash->feature_bits & FEATURE_REGISTERMAP)
+	if (flash->chip->feature_bits & FEATURE_REGISTERMAP)
    	map_flash_registers(flash);

return 1;
@@ -245,7 +245,7 @@
 {
    chipaddr bios = flash->virtual_memory;
    int delay_us = 0;
-	if(flash->probe_timing != TIMING_ZERO)
+	if(flash->chip->probe_timing != TIMING_ZERO)
            delay_us = 10;

/*  Issue the Sector Erase command   */
@@ -275,7 +275,7 @@
 {
    chipaddr bios = flash->virtual_memory;
    int delay_us = 0;
-	if(flash->probe_timing != TIMING_ZERO)
+	if(flash->chip->probe_timing != TIMING_ZERO)
            delay_us = 10;

/*  Issue the Sector Erase command   */
@@ -304,7 +304,7 @@
 {
    chipaddr bios = flash->virtual_memory;
    int delay_us = 0;
-	if(flash->probe_timing != TIMING_ZERO)
+	if(flash->chip->probe_timing != TIMING_ZERO)
            delay_us = 10;

/*  Issue the JEDEC Chip Erase command   */
@@ -366,7 +366,7 @@
    chipaddr olddst;
    unsigned int mask;

-	mask = getaddrmask(flash);
+	mask = getaddrmask(flash->chip);

olddst = dst;
    for (i = 0; i < len; i++) {
@@ -390,7 +390,7 @@
    chipaddr d = dst;
    unsigned int mask;

-	mask = getaddrmask(flash);
+	mask = getaddrmask(flash->chip);

retry:
    /* Issue JEDEC Start Program command */
@@ -438,7 +438,7 @@
     * write_jedec have page_size set to max_writechunk_size, so
     * we're OK for now.
     */
-	unsigned int page_size = flash->page_size;
+	unsigned int page_size = flash->chip->page_size;

/* Warning: This loop has a very unusual condition and body.
     * The loop needs to go through each page with at least one affected
@@ -469,8 +469,8 @@
 {
    unsigned int mask;

-	mask = getaddrmask(flash);
-	if ((addr != 0) || (blocksize != flash->total_size * 1024)) {
+	mask = getaddrmask(flash->chip);
+	if ((addr != 0) || (blocksize != flash->chip->total_size * 1024)) {
    	msg_cerr("%s called with incorrect arguments\n",
    		__func__);
    	return -1;
@@ -482,7 +482,7 @@
 {
    unsigned int mask;

-	mask = getaddrmask(flash);
+	mask = getaddrmask(flash->chip);
    return probe_jedec_common(flash, mask);
 }

@@ -491,7 +491,7 @@
 {
    unsigned int mask;

-	mask = getaddrmask(flash);
+	mask = getaddrmask(flash->chip);
    return erase_sector_jedec_common(flash, page, size, mask);
 }

@@ -500,7 +500,7 @@
 {
    unsigned int mask;

-	mask = getaddrmask(flash);
+	mask = getaddrmask(flash->chip);
    return erase_block_jedec_common(flash, page, size, mask);
 }

@@ -508,6 +508,6 @@
 {
    unsigned int mask;

-	mask = getaddrmask(flash);
+	mask = getaddrmask(flash->chip);
    return erase_chip_jedec_common(flash, mask);
 }
Index: flashrom-flashctx_separate_struct_flashchip/w39.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/w39.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/w39.c	(Arbeitskopie)
@@ -138,11 +138,11 @@

static int printlock_w39_fwh(struct flashctx *flash)
 {
-	unsigned int i, total_size = flash->total_size * 1024;
+	unsigned int i, total_size = flash->chip->total_size * 1024;
    int ret = 0;
    
    /* Print lock status of the complete chip */
-	for (i = 0; i < total_size; i += flash->page_size)
+	for (i = 0; i < total_size; i += flash->chip->page_size)
    	ret |= printlock_w39_fwh_block(flash, i);

return ret;
@@ -150,10 +150,10 @@

static int unlock_w39_fwh(struct flashctx *flash)
 {
-	unsigned int i, total_size = flash->total_size * 1024;
+	unsigned int i, total_size = flash->chip->total_size * 1024;
    
    /* Unlock the complete chip */
-	for (i = 0; i < total_size; i += flash->page_size)
+	for (i = 0; i < total_size; i += flash->chip->page_size)
    	if (unlock_w39_fwh_block(flash, i))
    		return -1;

Index: flashrom-flashctx_separate_struct_flashchip/sst49lfxxxc.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/sst49lfxxxc.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/sst49lfxxxc.c	(Arbeitskopie)
@@ -38,7 +38,7 @@
 static int write_lockbits_49lfxxxc(struct flashctx *flash, unsigned char bits)
 {
    chipaddr registers = flash->virtual_registers;
-	unsigned int i, left = flash->total_size * 1024;
+	unsigned int i, left = flash->chip->total_size * 1024;
    unsigned long address;

msg_cdbg("\nbios=0x%08lx\n", registers);
Index: flashrom-flashctx_separate_struct_flashchip/sst_fwhub.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/sst_fwhub.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/sst_fwhub.c	(Arbeitskopie)
@@ -31,7 +31,7 @@

blockstatus = chip_readb(flash, registers + offset + 2);
    msg_cdbg("Lock status for 0x%06x (size 0x%06x) is %02x, ",
-		     offset, flash->page_size, blockstatus);
+		     offset, flash->chip->page_size, blockstatus);
    switch (blockstatus & 0x3) {
    case 0x0:
    	msg_cdbg("full access\n");
@@ -72,7 +72,7 @@
 {
    int i;

-	for (i = 0; i < flash->total_size * 1024; i += flash->page_size)
+	for (i = 0; i < flash->chip->total_size * 1024; i += flash->chip->page_size)
    	check_sst_fwhub_block_lock(flash, i);

return 0;
@@ -82,7 +82,7 @@
 {
    int i, ret=0;

-	for (i = 0; i < flash->total_size * 1024; i += flash->page_size)
+	for (i = 0; i < flash->chip->total_size * 1024; i += flash->chip->page_size)
    {
    	if (clear_sst_fwhub_block_lock(flash, i))
    	{
Index: flashrom-flashctx_separate_struct_flashchip/cli_classic.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/cli_classic.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/cli_classic.c	(Arbeitskopie)
@@ -109,8 +109,8 @@
 {
    unsigned long size;
    /* Probe for up to three flash chips. */
-	const struct flashchip *flash;
-	struct flashctx flashes[3];
+	const struct flashchip *chip;
+	struct flashctx flashes[3] = {};
    struct flashctx *fill_flash;
    const char *name;
    int namelen, opt, i, j;
@@ -389,17 +389,16 @@
    }
    /* Does a chip with the requested name exist in the flashchips array? */
    if (chip_to_probe) {
-		for (flash = flashchips; flash && flash->name; flash++)
-			if (!strcmp(flash->name, chip_to_probe))
+		for (chip = flashchips; chip && chip->name; chip++)
+			if (!strcmp(chip->name, chip_to_probe))
    			break;
-		if (!flash || !flash->name) {
+		if (!chip || !chip->name) {
    		msg_cerr("Error: Unknown chip '%s' specified.\n", chip_to_probe);
    		msg_gerr("Run flashrom -L to view the hardware supported in this flashrom version.\n");
    		ret = 1;
    		goto out;
    	}
-		/* Clean up after the check. */
-		flash = NULL;
+		/* Keep chip around for later usage. */
    }

if (prog == PROGRAMMER_INVALID) {
@@ -419,16 +418,13 @@
    	goto out_shutdown;
    }
    tempstr = flashbuses_to_text(get_buses_supported());
-	msg_pdbg("The following protocols are supported: %s.\n",
-		 tempstr);
+	msg_pdbg("The following protocols are supported: %s.\n", tempstr);
    free(tempstr);

for (j = 0; j < registered_programmer_count; j++) {
    	startchip = 0;
    	while (chipcount < ARRAY_SIZE(flashes)) {
-			startchip = probe_flash(&registered_programmers[j],
-						startchip, 
-						&flashes[chipcount], 0);
+			startchip = probe_flash(&registered_programmers[j], startchip, &flashes[chipcount], 0);
    		if (startchip == -1)
    			break;
    		chipcount++;
@@ -437,9 +433,9 @@
    }

if (chipcount > 1) {
-		msg_cinfo("Multiple flash chips were detected: "%s"", flashes[0].name);
+		msg_cinfo("Multiple flash chips were detected: "%s"", flashes[0].chip->name);
    	for (i = 1; i < chipcount; i++)
-			msg_cinfo(", "%s"", flashes[i].name);
+			msg_cinfo(", "%s"", flashes[i].chip->name);
    	msg_cinfo("\nPlease specify which chip to use with the -c <chipname> option.\n");
    	ret = 1;
    	goto out_shutdown;
@@ -456,9 +452,15 @@
    		/* This loop just counts compatible controllers. */
    		for (j = 0; j < registered_programmer_count; j++) {
    			pgm = &registered_programmers[j];
-				if (pgm->buses_supported & flashes[0].bustype)
+				/* chip is still set from the chip_to_probe earlier in this function. */
+				if (pgm->buses_supported & chip->bustype)
    				compatible_programmers++;
    		}
+			if (!compatible_programmers) {
+				msg_cinfo("No compatible controller found for the requested flash chip.\n");
+				ret = 1;
+				goto out_shutdown;
+			}
    		if (compatible_programmers > 1)
    			msg_cinfo("More than one compatible controller found for the requested flash "
    				  "chip, using the first one.\n");
@@ -469,6 +471,7 @@
    				break;
    		}
    		if (startchip == -1) {
+				// FIXME: This should never happen! Ask for a bug report?
    			msg_cinfo("Probing for flash chip '%s' failed.\n", chip_to_probe);
    			ret = 1;
    			goto out_shutdown;
@@ -481,19 +484,18 @@
    	goto out_shutdown;
    } else if (!chip_to_probe) {
    	/* repeat for convenience when looking at foreign logs */
-		tempstr = flashbuses_to_text(flashes[0].bustype);
+		tempstr = flashbuses_to_text(flashes[0].chip->bustype);
    	msg_gdbg("Found %s flash chip "%s" (%d kB, %s).\n",
-			 flashes[0].vendor, flashes[0].name,
-			 flashes[0].total_size, tempstr);
+			 flashes[0].chip->vendor, flashes[0].chip->name, flashes[0].chip->total_size, tempstr);
    	free(tempstr);
    }

fill_flash = &flashes[0];

-	check_chip_supported(fill_flash);
+	check_chip_supported(fill_flash->chip);

-	size = fill_flash->total_size * 1024;
-	if (check_max_decode(fill_flash->pgm->buses_supported & fill_flash->bustype, size) && (!force)) {
+	size = fill_flash->chip->total_size * 1024;
+	if (check_max_decode(fill_flash->pgm->buses_supported & fill_flash->chip->bustype, size) && (!force)) {
    	msg_cerr("Chip is too big for this programmer (-V gives details). Use --force to override.\n");
    	ret = 1;
    	goto out_shutdown;
Index: flashrom-flashctx_separate_struct_flashchip/layout.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/layout.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/layout.c	(Arbeitskopie)
@@ -221,7 +221,7 @@
 {
    unsigned int start = 0;
    romlayout_t *entry;
-	unsigned int size = flash->total_size * 1024;
+	unsigned int size = flash->chip->total_size * 1024;

/* If no regions were specified for inclusion, assume
     * that the user wants to write the complete new image.
Index: flashrom-flashctx_separate_struct_flashchip/ichspi.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/ichspi.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/ichspi.c	(Arbeitskopie)
@@ -1193,9 +1193,9 @@
    else
    	msg_cdbg(" with a");
    msg_cdbg(" density of %d kB.\n", total_size / 1024);
-	flash->total_size = total_size / 1024;
+	flash->chip->total_size = total_size / 1024;

-	eraser = &(flash->block_erasers[0]);
+	eraser = &(flash->chip->block_erasers[0]);
    boundary = (REGREAD32(ICH9_REG_FPB) & FPB_FPBA) << 12;
    size_high = total_size - boundary;
    erase_size_high = ich_hwseq_get_erase_block_size(boundary);
@@ -1228,7 +1228,7 @@
    	msg_cdbg("In that range are %d erase blocks with %d B each.\n",
    		 size_high / erase_size_high, erase_size_high);
    }
-	flash->tested = TEST_OK_PREW;
+	flash->chip->tested = TEST_OK_PREW;
    return 1;
 }

@@ -1256,7 +1256,7 @@
    	return -1;
    }

-	if (addr + len > flash->total_size * 1024) {
+	if (addr + len > flash->chip->total_size * 1024) {
    	msg_perr("Request to erase some inaccessible memory address(es)"
    		 " (addr=0x%x, len=%d). "
    		 "Not erasing anything.\n", addr, len);
@@ -1288,7 +1288,7 @@
    uint16_t timeout = 100 * 60;
    uint8_t block_len;

-	if (addr + len > flash->total_size * 1024) {
+	if (addr + len > flash->chip->total_size * 1024) {
    	msg_perr("Request to read from an inaccessible memory address "
    		 "(addr=0x%x, len=%d).\n", addr, len);
    	return -1;
@@ -1326,7 +1326,7 @@
    uint16_t timeout = 100 * 60;
    uint8_t block_len;

-	if (addr + len > flash->total_size * 1024) {
+	if (addr + len > flash->chip->total_size * 1024) {
    	msg_perr("Request to write to an inaccessible memory address "
    		 "(addr=0x%x, len=%d).\n", addr, len);
    	return -1;
Index: flashrom-flashctx_separate_struct_flashchip/82802ab.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/82802ab.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/82802ab.c	(Arbeitskopie)
@@ -44,7 +44,7 @@
 {
    chipaddr bios = flash->virtual_memory;
    uint8_t id1, id2, flashcontent1, flashcontent2;
-	int shifted = (flash->feature_bits & FEATURE_ADDR_SHIFTED) != 0;
+	int shifted = (flash->chip->feature_bits & FEATURE_ADDR_SHIFTED) != 0;

/* Reset to get a clean state */
    chip_writeb(flash, 0xFF, bios);
@@ -80,10 +80,10 @@
    	msg_cdbg(", id2 is normal flash content");

msg_cdbg("\n");
-	if (id1 != flash->manufacture_id || id2 != flash->model_id)
+	if (id1 != flash->chip->manufacture_id || id2 != flash->chip->model_id)
    	return 0;

-	if (flash->feature_bits & FEATURE_REGISTERMAP)
+	if (flash->chip->feature_bits & FEATURE_REGISTERMAP)
    	map_flash_registers(flash);

return 1;
@@ -112,7 +112,7 @@
    int i;
    //chipaddr wrprotect = flash->virtual_registers + page + 2;

-	for (i = 0; i < flash->total_size * 1024; i+= flash->page_size)
+	for (i = 0; i < flash->chip->total_size * 1024; i+= flash->chip->page_size)
    	chip_writeb(flash, 0, flash->virtual_registers + i + 2);

return 0;
@@ -181,7 +181,7 @@
    }

/* Read block lock-bits */
-	for (i = 0; i < flash->total_size * 1024; i+= (64 * 1024)) {
+	for (i = 0; i < flash->chip->total_size * 1024; i+= (64 * 1024)) {
    	bcfg = chip_readb(flash, bios + i + 2); // read block lock config
    	msg_cdbg("block lock at %06x is %slocked!\n", i, bcfg ? "" : "un");
    	if (bcfg) {
@@ -234,7 +234,7 @@
    }

/* Read block lock-bits, 8 * 8 KB + 15 * 64 KB */
-	for (i = 0; i < flash->total_size * 1024;
+	for (i = 0; i < flash->chip->total_size * 1024;
         i += (i >= (64 * 1024) ? 64 * 1024 : 8 * 1024)) {
    	bcfg = chip_readb(flash, bios + i + 2); /* read block lock config */
    	msg_cdbg("block lock at %06x is %slocked!\n", i,
Index: flashrom-flashctx_separate_struct_flashchip/dediprog.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/dediprog.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/dediprog.c	(Arbeitskopie)
@@ -381,7 +381,7 @@
    		      unsigned int start, unsigned int len, uint8_t dedi_spi_cmd)
 {
    int ret;
-	const unsigned int chunksize = flash->page_size;
+	const unsigned int chunksize = flash->chip->page_size;
    unsigned int residue = start % chunksize ? chunksize - start % chunksize : 0;
    unsigned int bulklen;

Index: flashrom-flashctx_separate_struct_flashchip/spi25.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/spi25.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/spi25.c	(Arbeitskopie)
@@ -147,7 +147,7 @@

msg_cdbg("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1, id2);

-	if (id1 == flash->manufacture_id && id2 == flash->model_id) {
+	if (id1 == flash->chip->manufacture_id && id2 == flash->chip->model_id) {
    	/* Print the status register to tell the
    	 * user about possible write protection.
    	 */
@@ -157,12 +157,12 @@
    }

/* Test if this is a pure vendor match. */
-	if (id1 == flash->manufacture_id &&
-	    GENERIC_DEVICE_ID == flash->model_id)
+	if (id1 == flash->chip->manufacture_id &&
+	    GENERIC_DEVICE_ID == flash->chip->model_id)
    	return 1;

/* Test if there is any vendor ID. */
-	if (GENERIC_MANUF_ID == flash->manufacture_id &&
+	if (GENERIC_MANUF_ID == flash->chip->manufacture_id &&
        id1 != 0xff)
    	return 1;

@@ -210,7 +210,7 @@

msg_cdbg("%s: id1 0x%x, id2 0x%x\n", __func__, id1, id2);

-	if (id1 == flash->manufacture_id && id2 == flash->model_id) {
+	if (id1 == flash->chip->manufacture_id && id2 == flash->chip->model_id) {
    	/* Print the status register to tell the
    	 * user about possible write protection.
    	 */
@@ -220,12 +220,12 @@
    }

/* Test if this is a pure vendor match. */
-	if (id1 == flash->manufacture_id &&
-	    GENERIC_DEVICE_ID == flash->model_id)
+	if (id1 == flash->chip->manufacture_id &&
+	    GENERIC_DEVICE_ID == flash->chip->model_id)
    	return 1;

/* Test if there is any vendor ID. */
-	if (GENERIC_MANUF_ID == flash->manufacture_id &&
+	if (GENERIC_MANUF_ID == flash->chip->manufacture_id &&
        id1 != 0xff)
    	return 1;

@@ -267,7 +267,7 @@

msg_cdbg("%s: id 0x%x\n", __func__, id2);

-	if (id2 != flash->model_id)
+	if (id2 != flash->chip->model_id)
    	return 0;

/* Print the status register to tell the
@@ -291,7 +291,7 @@

msg_cdbg("%s: id1 0x%x, id2 0x%x\n", __func__, id1, id2);

-	if (id1 != flash->manufacture_id || id2 != flash->model_id)
+	if (id1 != flash->chip->manufacture_id || id2 != flash->chip->model_id)
    	return 0;

/* Print the status register to tell the
@@ -423,18 +423,18 @@

status = spi_read_status_register(flash);
    msg_cdbg("Chip status register is %02x\n", status);
-	switch (flash->manufacture_id) {
+	switch (flash->chip->manufacture_id) {
    case ST_ID:
-		if (((flash->model_id & 0xff00) == 0x2000) ||
-		    ((flash->model_id & 0xff00) == 0x2500))
+		if (((flash->chip->model_id & 0xff00) == 0x2000) ||
+		    ((flash->chip->model_id & 0xff00) == 0x2500))
    		spi_prettyprint_status_register_st_m25p(status);
    	break;
    case MACRONIX_ID:
-		if ((flash->model_id & 0xff00) == 0x2000)
+		if ((flash->chip->model_id & 0xff00) == 0x2000)
    		spi_prettyprint_status_register_st_m25p(status);
    	break;
    case SST_ID:
-		switch (flash->model_id) {
+		switch (flash->chip->model_id) {
    	case 0x2541:
    		spi_prettyprint_status_register_sst25vf016(status);
    		break;
@@ -704,7 +704,7 @@
 int spi_block_erase_60(struct flashctx *flash, unsigned int addr,
    	       unsigned int blocklen)
 {
-	if ((addr != 0) || (blocklen != flash->total_size * 1024)) {
+	if ((addr != 0) || (blocklen != flash->chip->total_size * 1024)) {
    	msg_cerr("%s called with incorrect arguments\n",
    		__func__);
    	return -1;
@@ -715,7 +715,7 @@
 int spi_block_erase_c7(struct flashctx *flash, unsigned int addr,
    	       unsigned int blocklen)
 {
-	if ((addr != 0) || (blocklen != flash->total_size * 1024)) {
+	if ((addr != 0) || (blocklen != flash->chip->total_size * 1024)) {
    	msg_cerr("%s called with incorrect arguments\n",
    		__func__);
    	return -1;
@@ -820,7 +820,7 @@

int spi_write_status_register(struct flashctx *flash, int status)
 {
-	int feature_bits = flash->feature_bits;
+	int feature_bits = flash->chip->feature_bits;
    int ret = 1;

if (!(feature_bits & (FEATURE_WRSR_WREN | FEATURE_WRSR_EWSR))) {
@@ -972,7 +972,7 @@
 {
    int rc = 0;
    unsigned int i, j, starthere, lenhere, toread;
-	unsigned int page_size = flash->page_size;
+	unsigned int page_size = flash->chip->page_size;

/* Warning: This loop has a very unusual condition and body.
     * The loop needs to go through each page with at least one affected
@@ -1017,7 +1017,7 @@
     * spi_chip_write_256 have page_size set to max_writechunk_size, so
     * we're OK for now.
     */
-	unsigned int page_size = flash->page_size;
+	unsigned int page_size = flash->chip->page_size;

/* Warning: This loop has a very unusual condition and body.
     * The loop needs to go through each page with at least one affected
Index: flashrom-flashctx_separate_struct_flashchip/pm49fl00x.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/pm49fl00x.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/pm49fl00x.c	(Arbeitskopie)
@@ -40,14 +40,14 @@

int unlock_49fl00x(struct flashctx *flash)
 {
-	write_lockbits_49fl00x(flash, flash->total_size * 1024, 0,
-			       flash->page_size);
+	write_lockbits_49fl00x(flash, flash->chip->total_size * 1024, 0,
+			       flash->chip->page_size);
    return 0;
 }

int lock_49fl00x(struct flashctx *flash)
 {
-	write_lockbits_49fl00x(flash, flash->total_size * 1024, 1,
-			       flash->page_size);
+	write_lockbits_49fl00x(flash, flash->chip->total_size * 1024, 1,
+			       flash->chip->page_size);
    return 0;
 }
Index: flashrom-flashctx_separate_struct_flashchip/en29lv640b.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/en29lv640b.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/en29lv640b.c	(Arbeitskopie)
@@ -81,7 +81,7 @@

msg_cdbg("%s: id1 0x%04x, id2 0x%04x\n", __func__, id1, id2);

-	if (id1 == flash->manufacture_id && id2 == flash->model_id)
+	if (id1 == flash->chip->manufacture_id && id2 == flash->chip->model_id)
    	return 1;

return 0;
@@ -130,7 +130,7 @@
 int block_erase_chip_en29lv640b(struct flashctx *flash, unsigned int address,
    		        unsigned int blocklen)
 {
-	if ((address != 0) || (blocklen != flash->total_size * 1024)) {
+	if ((address != 0) || (blocklen != flash->chip->total_size * 1024)) {
    	msg_cerr("%s called with incorrect arguments\n", __func__);
    	return -1;
    }
Index: flashrom-flashctx_separate_struct_flashchip/w29ee011.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/w29ee011.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/w29ee011.c	(Arbeitskopie)
@@ -29,11 +29,11 @@
    chipaddr bios = flash->virtual_memory;
    uint8_t id1, id2;

-	if (!chip_to_probe || strcmp(chip_to_probe, flash->name)) {
+	if (!chip_to_probe || strcmp(chip_to_probe, flash->chip->name)) {
    	msg_cdbg("Old Winbond W29* probe method disabled because "
    		 "the probing sequence puts the AMIC A49LF040A in "
    		 "a funky state. Use 'flashrom -c %s' if you "
-			 "have a board with such a chip.\n", flash->name);
+			 "have a board with such a chip.\n", flash->chip->name);
    	return 0;
    }

@@ -65,7 +65,7 @@

msg_cdbg("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1, id2);

-	if (id1 == flash->manufacture_id && id2 == flash->model_id)
+	if (id1 == flash->chip->manufacture_id && id2 == flash->chip->model_id)
    	return 1;

return 0;
Index: flashrom-flashctx_separate_struct_flashchip/spi.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/spi.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/spi.c	(Arbeitskopie)
@@ -111,16 +111,16 @@
     * means 0xffffff, the highest unsigned 24bit number.
     */
    addrbase = spi_get_valid_read_addr(flash);
-	if (addrbase + flash->total_size * 1024 > (1 << 24)) {
+	if (addrbase + flash->chip->total_size * 1024 > (1 << 24)) {
    	msg_perr("Flash chip size exceeds the allowed access window. ");
    	msg_perr("Read will probably fail.\n");
    	/* Try to get the best alignment subject to constraints. */
-		addrbase = (1 << 24) - flash->total_size * 1024;
+		addrbase = (1 << 24) - flash->chip->total_size * 1024;
    }
    /* Check if alignment is native (at least the largest power of two which
     * is a factor of the mapped size of the chip).
     */
-	if (ffs(flash->total_size * 1024) > (ffs(addrbase) ? : 33)) {
+	if (ffs(flash->chip->total_size * 1024) > (ffs(addrbase) ? : 33)) {
    	msg_perr("Flash chip is not aligned natively in the allowed "
    		 "access window.\n");
    	msg_perr("Read will probably return garbage.\n");
Index: flashrom-flashctx_separate_struct_flashchip/sfdp.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/sfdp.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/sfdp.c	(Arbeitskopie)
@@ -84,7 +84,7 @@
 static int sfdp_add_uniform_eraser(struct flashctx *flash, uint8_t opcode, uint32_t block_size)
 {
    int i;
-	uint32_t total_size = flash->total_size * 1024;
+	uint32_t total_size = flash->chip->total_size * 1024;
    erasefunc_t *erasefn = spi_get_erasefn_from_opcode(opcode);

if (erasefn == NULL || total_size == 0 || block_size == 0 ||
@@ -95,7 +95,7 @@
    }

for (i = 0; i < NUM_ERASEFUNCTIONS; i++) {
-		struct block_eraser *eraser = &flash->block_erasers[i];
+		struct block_eraser *eraser = &flash->chip->block_erasers[i];
    	/* Check for duplicates (including (some) non-uniform ones). */
    	if (eraser->eraseblocks[0].size == block_size &&
    	    eraser->block_erase == erasefn) {
@@ -170,28 +170,28 @@
    	msg_cdbg2("volatile and writes to the status register have to "
    		  "be enabled with ");
    	if (tmp32 & (1 << 4)) {
-			flash->feature_bits = FEATURE_WRSR_WREN;
+			flash->chip->feature_bits = FEATURE_WRSR_WREN;
    		msg_cdbg2("WREN (0x06).\n");
    	} else {
-			flash->feature_bits = FEATURE_WRSR_EWSR;
+			flash->chip->feature_bits = FEATURE_WRSR_EWSR;
    		msg_cdbg2("EWSR (0x50).\n");
    	}
    } else {
    	msg_cdbg2("non-volatile and the standard does not allow "
    		  "vendors to tell us whether EWSR/WREN is needed for "
    		  "status register writes - assuming EWSR.\n");
-			flash->feature_bits = FEATURE_WRSR_EWSR;
+			flash->chip->feature_bits = FEATURE_WRSR_EWSR;
    	}

msg_cdbg2("  Write chunk size is ");
    if (tmp32 & (1 << 2)) {
    	msg_cdbg2("at least 64 B.\n");
-		flash->page_size = 64;
-		flash->write = spi_chip_write_256;
+		flash->chip->page_size = 64;
+		flash->chip->write = spi_chip_write_256;
    } else {
    	msg_cdbg2("1 B only.\n");
-		flash->page_size = 256;
-		flash->write = spi_chip_write_1;
+		flash->chip->page_size = 256;
+		flash->chip->write = spi_chip_write_1;
    }

if ((tmp32 & 0x3) == 0x1) {
@@ -212,8 +212,8 @@
    	return 1;
    }
    total_size = ((tmp32 & 0x7FFFFFFF) + 1) / 8;
-	flash->total_size = total_size / 1024;
-	msg_cdbg2("  Flash chip size is %d kB.\n", flash->total_size);
+	flash->chip->total_size = total_size / 1024;
+	msg_cdbg2("  Flash chip size is %d kB.\n", flash->chip->total_size);
    if (total_size > (1 << 24)) {
    	msg_cdbg("Flash chip size is bigger than what 3-Byte addressing "
    		 "can access.\n");
Index: flashrom-flashctx_separate_struct_flashchip/sst28sf040.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/sst28sf040.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/sst28sf040.c	(Arbeitskopie)
@@ -119,7 +119,7 @@
 int erase_chip_28sf040(struct flashctx *flash, unsigned int addr,
    	       unsigned int blocklen)
 {
-	if ((addr != 0) || (blocklen != flash->total_size * 1024)) {
+	if ((addr != 0) || (blocklen != flash->chip->total_size * 1024)) {
    	msg_cerr("%s called with incorrect arguments\n",
    		__func__);
    	return -1;
Index: flashrom-flashctx_separate_struct_flashchip/stm50flw0x0x.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/stm50flw0x0x.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/stm50flw0x0x.c	(Arbeitskopie)
@@ -54,7 +54,7 @@
    /* Check, if it's is a top/bottom-block with 4k-sectors. */
    /* TODO: What about the other types? */
    if ((offset == 0) ||
-	    (offset == (flash->model_id == ST_M50FLW080A ? 0xE0000 : 0x10000))
+	    (offset == (flash->chip->model_id == ST_M50FLW080A ? 0xE0000 : 0x10000))
        || (offset == 0xF0000)) {

// unlock each 4k-sector
@@ -85,7 +85,7 @@
 {
    int i;

-	for (i = 0; i < flash->total_size * 1024; i+= flash->page_size) {
+	for (i = 0; i < flash->chip->total_size * 1024; i+= flash->chip->page_size) {
    	if(unlock_block_stm50flw0x0x(flash, i)) {
    		msg_cerr("UNLOCK FAILED!\n");
    		return -1;
Index: flashrom-flashctx_separate_struct_flashchip/flashrom.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/flashrom.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/flashrom.c	(Arbeitskopie)
@@ -416,7 +416,7 @@

void map_flash_registers(struct flashctx *flash)
 {
-	size_t size = flash->total_size * 1024;
+	size_t size = flash->chip->total_size * 1024;
    /* Flash registers live 4 MByte below the flash. */
    /* FIXME: This is incorrect for nonstandard flashbase. */
    flash->virtual_registers = (chipaddr)programmer_map_flash_region("flash chip registers", (0xFFFFFFFF - 0x400000 - size + 1), size);
@@ -580,7 +580,7 @@
    if (!len)
    	goto out_free;

-	if (!flash->read) {
+	if (!flash->chip->read) {
    	msg_cerr("ERROR: flashrom has no read function for this flash chip.\n");
    	return 1;
    }
@@ -589,17 +589,17 @@
    	exit(1);
    }

-	if (start + len > flash->total_size * 1024) {
+	if (start + len > flash->chip->total_size * 1024) {
    	msg_gerr("Error: %s called with start 0x%x + len 0x%x >"
    		" total_size 0x%x\n", __func__, start, len,
-			flash->total_size * 1024);
+			flash->chip->total_size * 1024);
    	ret = -1;
    	goto out_free;
    }
    if (!message)
    	message = "VERIFY";

-	ret = flash->read(flash, readbuf, start, len);
+	ret = flash->chip->read(flash, readbuf, start, len);
    if (ret) {
    	msg_gerr("Verification impossible because read failed "
    		 "at 0x%x (len 0x%x)\n", start, len);
@@ -950,44 +950,52 @@
    return 1;
 }

-int probe_flash(struct registered_programmer *pgm, int startchip,
-		struct flashctx *fill_flash, int force)
+int probe_flash(struct registered_programmer *pgm, int startchip, struct flashctx *flash, int force)
 {
-	const struct flashchip *flash;
+	const struct flashchip *chip;
    unsigned long base = 0;
    char location[64];
    uint32_t size;
    enum chipbustype buses_common;
    char *tmp;

-	for (flash = flashchips + startchip; flash && flash->name; flash++) {
-		if (chip_to_probe && strcmp(flash->name, chip_to_probe) != 0)
+	for (chip = flashchips + startchip; chip && chip->name; chip++) {
+		if (chip_to_probe && strcmp(chip->name, chip_to_probe) != 0)
    		continue;
-		buses_common = pgm->buses_supported & flash->bustype;
+		buses_common = pgm->buses_supported & chip->bustype;
    	if (!buses_common)
    		continue;
    	msg_gdbg("Probing for %s %s, %d kB: ",
-			     flash->vendor, flash->name, flash->total_size);
-		if (!flash->probe && !force) {
+			     chip->vendor, chip->name, chip->total_size);
+		if (!chip->probe && !force) {
    		msg_gdbg("failed! flashrom has no probe function for "
    			 "this flash chip.\n");
    		continue;
    	}

-		size = flash->total_size * 1024;
+		size = chip->total_size * 1024;
    	check_max_decode(buses_common, size);

/* Start filling in the dynamic data. */
-		memcpy(fill_flash, flash, sizeof(struct flashchip));
-		fill_flash->pgm = pgm;
+		flash->chip = calloc(1, sizeof(struct flashchip));
+		if (!flash->chip) {
+			msg_gerr("Out of memory!\n");
+			// FIXME: Is -1 the right return code?
+			return -1;
+		}
+		memcpy(flash->chip, chip, sizeof(struct flashchip));
+		flash->pgm = pgm;

base = flashbase ? flashbase : (0xffffffff - size + 1);
-		fill_flash->virtual_memory = (chipaddr)programmer_map_flash_region("flash chip", base, size);
+		flash->virtual_memory = (chipaddr)programmer_map_flash_region("flash chip", base, size);

+		/* We handle a forced match like a real match, we just avoid probing. Note that probe_flash()
+		 * is only called with force=1 after normal probing failed.
+		 */
    	if (force)
    		break;

-		if (fill_flash->probe(fill_flash) != 1)
+		if (flash->chip->probe(flash) != 1)
    		goto notfound;

/* If this is the first chip found, accept it.
@@ -997,11 +1005,11 @@
    	 * one for this programmer interface and thus no other chip has
    	 * been found on this interface.
    	 */
-		if (startchip == 0 && fill_flash->model_id == SFDP_DEVICE_ID) {
+		if (startchip == 0 && flash->chip->model_id == SFDP_DEVICE_ID) {
    		msg_cinfo("===\n"
    			  "SFDP has autodetected a flash chip which is "
    			  "not natively supported by flashrom yet.\n");
-			if (count_usable_erasers(fill_flash) == 0)
+			if (count_usable_erasers(flash) == 0)
    			msg_cinfo("The standard operations read and "
    				  "verify should work, but to support "
    				  "erase, write and all other "
@@ -1022,15 +1030,18 @@
    	}

if (startchip == 0 ||
-		    ((fill_flash->model_id != GENERIC_DEVICE_ID) &&
-		     (fill_flash->model_id != SFDP_DEVICE_ID)))
+		    ((flash->chip->model_id != GENERIC_DEVICE_ID) &&
+		     (flash->chip->model_id != SFDP_DEVICE_ID)))
    		break;

notfound:
-		programmer_unmap_flash_region((void *)fill_flash->virtual_memory, size);
+		programmer_unmap_flash_region((void *)flash->virtual_memory, size);
+		flash->virtual_memory = (chipaddr)NULL;
+		free(flash->chip);
+		flash->chip = NULL;
    }

-	if (!flash || !flash->name)
+	if (!flash->chip)
    	return -1;

#if CONFIG_INTERNAL == 1
@@ -1040,27 +1051,26 @@
 #endif
    	snprintf(location, sizeof(location), "on %s", programmer_table[programmer].name);

-	tmp = flashbuses_to_text(flash->bustype);
-	msg_cinfo("%s %s flash chip "%s" (%d kB, %s) %s.\n",
-		  force ? "Assuming" : "Found", fill_flash->vendor,
-		  fill_flash->name, fill_flash->total_size, tmp, location);
+	tmp = flashbuses_to_text(flash->chip->bustype);
+	msg_cinfo("%s %s flash chip "%s" (%d kB, %s) %s.\n", force ? "Assuming" : "Found",
+		  flash->chip->vendor, flash->chip->name, flash->chip->total_size, tmp, location);
    free(tmp);

/* Flash registers will not be mapped if the chip was forced. Lock info
     * may be stored in registers, so avoid lock info printing.
     */
    if (!force)
-		if (fill_flash->printlock)
-			fill_flash->printlock(fill_flash);
+		if (flash->chip->printlock)
+			flash->chip->printlock(flash);

/* Return position of matching chip. */
-	return flash - flashchips;
+	return chip - flashchips;
 }

int verify_flash(struct flashctx *flash, uint8_t *buf)
 {
    int ret;
-	unsigned int total_size = flash->total_size * 1024;
+	unsigned int total_size = flash->chip->total_size * 1024;

msg_cinfo("Verifying flash... ");

@@ -1133,7 +1143,7 @@

int read_flash_to_file(struct flashctx *flash, const char *filename)
 {
-	unsigned long size = flash->total_size * 1024;
+	unsigned long size = flash->chip->total_size * 1024;
    unsigned char *buf = calloc(size, sizeof(char));
    int ret = 0;

@@ -1143,12 +1153,12 @@
    	msg_cinfo("FAILED.\n");
    	return 1;
    }
-	if (!flash->read) {
+	if (!flash->chip->read) {
    	msg_cerr("No read function available for this flash chip.\n");
    	ret = 1;
    	goto out_free;
    }
-	if (flash->read(flash, buf, 0, size)) {
+	if (flash->chip->read(flash, buf, 0, size)) {
    	msg_cerr("Read operation failed!\n");
    	ret = 1;
    	goto out_free;
@@ -1165,14 +1175,14 @@
  * walk_eraseregions().
  * Even if an error is found, the function will keep going and check the rest.
  */
-static int selfcheck_eraseblocks(const struct flashchip *flash)
+static int selfcheck_eraseblocks(const struct flashchip *chip)
 {
    int i, j, k;
    int ret = 0;

for (k = 0; k < NUM_ERASEFUNCTIONS; k++) {
    	unsigned int done = 0;
-		struct block_eraser eraser = flash->block_erasers[k];
+		struct block_eraser eraser = chip->block_erasers[k];

for (i = 0; i < NUM_ERASEREGIONS; i++) {
    		/* Blocks with zero size are bugs in flashchips.c. */
@@ -1181,7 +1191,7 @@
    			msg_gerr("ERROR: Flash chip %s erase function "
    				"%i region %i has size 0. Please report"
    				" a bug at [e-mail]\n",
-					flash->name, k, i);
+					chip->name, k, i);
    			ret = 1;
    		}
    		/* Blocks with zero count are bugs in flashchips.c. */
@@ -1190,7 +1200,7 @@
    			msg_gerr("ERROR: Flash chip %s erase function "
    				"%i region %i has count 0. Please report"
    				" a bug at [e-mail]\n",
-					flash->name, k, i);
+					chip->name, k, i);
    			ret = 1;
    		}
    		done += eraser.eraseblocks[i].count *
@@ -1202,12 +1212,12 @@
    			  "non-empty erase function. Not an error.\n");
    	if (!done)
    		continue;
-		if (done != flash->total_size * 1024) {
+		if (done != chip->total_size * 1024) {
    		msg_gerr("ERROR: Flash chip %s erase function %i "
    			"region walking resulted in 0x%06x bytes total,"
    			" expected 0x%06x bytes. Please report a bug at"
-				" [e-mail]\n", flash->name, k,
-				done, flash->total_size * 1024);
+				" [e-mail]\n", chip->name, k,
+				done, chip->total_size * 1024);
    		ret = 1;
    	}
    	if (!eraser.block_erase)
@@ -1218,11 +1228,11 @@
    	 */
    	for (j = k + 1; j < NUM_ERASEFUNCTIONS; j++) {
    		if (eraser.block_erase ==
-			    flash->block_erasers[j].block_erase) {
+			    chip->block_erasers[j].block_erase) {
    			msg_gerr("ERROR: Flash chip %s erase function "
    				"%i and %i are identical. Please report"
    				" a bug at [e-mail]\n",
-					flash->name, k, j);
+					chip->name, k, j);
    			ret = 1;
    		}
    	}
@@ -1269,7 +1279,7 @@
    	if (!writecount++)
    		msg_cdbg("W");
    	/* Needs the partial write function signature. */
-		ret = flash->write(flash, newcontents + starthere,
+		ret = flash->chip->write(flash, newcontents + starthere,
    			   start + starthere, lenhere);
    	if (ret)
    		return ret;
@@ -1296,7 +1306,7 @@
    int i, j;
    unsigned int start = 0;
    unsigned int len;
-	struct block_eraser eraser = flash->block_erasers[erasefunction];
+	struct block_eraser eraser = flash->chip->block_erasers[erasefunction];

for (i = 0; i < NUM_ERASEREGIONS; i++) {
    	/* count==0 for all automatically initialized array
@@ -1322,7 +1332,7 @@

static int check_block_eraser(const struct flashctx *flash, int k, int log)
 {
-	struct block_eraser eraser = flash->block_erasers[k];
+	struct block_eraser eraser = flash->chip->block_erasers[k];

if (!eraser.block_erase && !eraser.eraseblocks[0].count) {
    	if (log)
@@ -1341,6 +1351,7 @@
    			 "eraseblock layout is not defined. ");
    	return 1;
    }
+	// TODO: Once erase functions are annotated with allowed buses, check that as well.
    return 0;
 }

@@ -1349,7 +1360,7 @@
 {
    int k, ret = 1;
    uint8_t *curcontents;
-	unsigned long size = flash->total_size * 1024;
+	unsigned long size = flash->chip->total_size * 1024;
    unsigned int usable_erasefunctions = count_usable_erasers(flash);

msg_cinfo("Erasing and writing flash chip... ");
@@ -1387,7 +1398,7 @@
    	 * in non-verbose mode.
    	 */
    	msg_cinfo("Reading current flash chip contents... ");
-		if (flash->read(flash, curcontents, 0, size)) {
+		if (flash->chip->read(flash, curcontents, 0, size)) {
    		/* Now we are truly screwed. Read failed as well. */
    		msg_cerr("Can't read anymore! Aborting.\n");
    		/* We have no idea about the flash chip contents, so
@@ -1576,7 +1587,7 @@
 int selfcheck(void)
 {
    int ret = 0;
-	const struct flashchip *flash;
+	const struct flashchip *chip;

/* Safety check. Instead of aborting after the first error, check
     * if more errors exist.
@@ -1594,16 +1605,8 @@
    	msg_gerr("Flashchips table miscompilation!\n");
    	ret = 1;
    }
-	/* Check that virtual_memory in struct flashctx is placed directly
-	 * after the members copied from struct flashchip.
-	 */
-	if (sizeof(struct flashchip) !=
-	    offsetof(struct flashctx, virtual_memory)) {
-		msg_gerr("struct flashctx broken!\n");
-		ret = 1;
-	}
-	for (flash = flashchips; flash && flash->name; flash++)
-		if (selfcheck_eraseblocks(flash))
+	for (chip = flashchips; chip && chip->name; chip++)
+		if (selfcheck_eraseblocks(chip))
    		ret = 1;

#if CONFIG_INTERNAL == 1
@@ -1627,41 +1630,41 @@
    return ret;
 }

-void check_chip_supported(const struct flashctx *flash)
+void check_chip_supported(const struct flashchip *chip)
 {
-	if (flash->feature_bits & FEATURE_OTP) {
+	if (chip->feature_bits & FEATURE_OTP) {
    	msg_cdbg("This chip may contain one-time programmable memory. "
    		 "flashrom cannot read\nand may never be able to write "
    		 "it, hence it may not be able to completely\n"
    		 "clone the contents of this chip (see man page for "
    		 "details).\n");
    }
-	if (TEST_OK_MASK != (flash->tested & TEST_OK_MASK)) {
+	if (TEST_OK_MASK != (chip->tested & TEST_OK_MASK)) {
    	msg_cinfo("===\n");
-		if (flash->tested & TEST_BAD_MASK) {
+		if (chip->tested & TEST_BAD_MASK) {
    		msg_cinfo("This flash part has status NOT WORKING for operations:");
-			if (flash->tested & TEST_BAD_PROBE)
+			if (chip->tested & TEST_BAD_PROBE)
    			msg_cinfo(" PROBE");
-			if (flash->tested & TEST_BAD_READ)
+			if (chip->tested & TEST_BAD_READ)
    			msg_cinfo(" READ");
-			if (flash->tested & TEST_BAD_ERASE)
+			if (chip->tested & TEST_BAD_ERASE)
    			msg_cinfo(" ERASE");
-			if (flash->tested & TEST_BAD_WRITE)
+			if (chip->tested & TEST_BAD_WRITE)
    			msg_cinfo(" WRITE");
    		msg_cinfo("\n");
    	}
-		if ((!(flash->tested & TEST_BAD_PROBE) && !(flash->tested & TEST_OK_PROBE)) ||
-		    (!(flash->tested & TEST_BAD_READ) && !(flash->tested & TEST_OK_READ)) ||
-		    (!(flash->tested & TEST_BAD_ERASE) && !(flash->tested & TEST_OK_ERASE)) ||
-		    (!(flash->tested & TEST_BAD_WRITE) && !(flash->tested & TEST_OK_WRITE))) {
+		if ((!(chip->tested & TEST_BAD_PROBE) && !(chip->tested & TEST_OK_PROBE)) ||
+		    (!(chip->tested & TEST_BAD_READ) && !(chip->tested & TEST_OK_READ)) ||
+		    (!(chip->tested & TEST_BAD_ERASE) && !(chip->tested & TEST_OK_ERASE)) ||
+		    (!(chip->tested & TEST_BAD_WRITE) && !(chip->tested & TEST_OK_WRITE))) {
    		msg_cinfo("This flash part has status UNTESTED for operations:");
-			if (!(flash->tested & TEST_BAD_PROBE) && !(flash->tested & TEST_OK_PROBE))
+			if (!(chip->tested & TEST_BAD_PROBE) && !(chip->tested & TEST_OK_PROBE))
    			msg_cinfo(" PROBE");
-			if (!(flash->tested & TEST_BAD_READ) && !(flash->tested & TEST_OK_READ))
+			if (!(chip->tested & TEST_BAD_READ) && !(chip->tested & TEST_OK_READ))
    			msg_cinfo(" READ");
-			if (!(flash->tested & TEST_BAD_ERASE) && !(flash->tested & TEST_OK_ERASE))
+			if (!(chip->tested & TEST_BAD_ERASE) && !(chip->tested & TEST_OK_ERASE))
    			msg_cinfo(" ERASE");
-			if (!(flash->tested & TEST_BAD_WRITE) && !(flash->tested & TEST_OK_WRITE))
+			if (!(chip->tested & TEST_BAD_WRITE) && !(chip->tested & TEST_OK_WRITE))
    			msg_cinfo(" WRITE");
    		msg_cinfo("\n");
    	}
@@ -1702,13 +1705,13 @@

if (read_it || erase_it || write_it || verify_it) {
    	/* Everything needs read. */
-		if (flash->tested & TEST_BAD_READ) {
+		if (flash->chip->tested & TEST_BAD_READ) {
    		msg_cerr("Read is not working on this chip. ");
    		if (!force)
    			return 1;
    		msg_cerr("Continuing anyway.\n");
    	}
-		if (!flash->read) {
+		if (!flash->chip->read) {
    		msg_cerr("flashrom has no read function for this "
    			 "flash chip.\n");
    		return 1;
@@ -1716,7 +1719,7 @@
    }
    if (erase_it || write_it) {
    	/* Write needs erase. */
-		if (flash->tested & TEST_BAD_ERASE) {
+		if (flash->chip->tested & TEST_BAD_ERASE) {
    		msg_cerr("Erase is not working on this chip. ");
    		if (!force)
    			return 1;
@@ -1729,13 +1732,13 @@
    	}
    }
    if (write_it) {
-		if (flash->tested & TEST_BAD_WRITE) {
+		if (flash->chip->tested & TEST_BAD_WRITE) {
    		msg_cerr("Write is not working on this chip. ");
    		if (!force)
    			return 1;
    		msg_cerr("Continuing anyway.\n");
    	}
-		if (!flash->write) {
+		if (!flash->chip->write) {
    		msg_cerr("flashrom has no write function for this "
    			 "flash chip.\n");
    		return 1;
@@ -1754,7 +1757,7 @@
    uint8_t *oldcontents;
    uint8_t *newcontents;
    int ret = 0;
-	unsigned long size = flash->total_size * 1024;
+	unsigned long size = flash->chip->total_size * 1024;

if (chip_safety_check(flash, force, read_it, write_it, erase_it, verify_it)) {
    	msg_cerr("Aborting.\n");
@@ -1765,8 +1768,8 @@
    /* Given the existence of read locks, we want to unlock for read,
     * erase and write.
     */
-	if (flash->unlock)
-		flash->unlock(flash);
+	if (flash->chip->unlock)
+		flash->chip->unlock(flash);

if (read_it) {
    	ret = read_flash_to_file(flash, filename);
@@ -1829,7 +1832,7 @@
     * takes time as well.
     */
    msg_cinfo("Reading old flash chip contents... ");
-	if (flash->read(flash, oldcontents, 0, size)) {
+	if (flash->chip->read(flash, oldcontents, 0, size)) {
    	ret = 1;
    	msg_cinfo("FAILED.\n");
    	goto out;
@@ -1846,7 +1849,7 @@
    	if (erase_and_write_flash(flash, oldcontents, newcontents)) {
    		msg_cerr("Uh oh. Erase/write failed. Checking if "
    			 "anything changed.\n");
-			if (!flash->read(flash, newcontents, 0, size)) {
+			if (!flash->chip->read(flash, newcontents, 0, size)) {
    			if (!memcmp(oldcontents, newcontents, size)) {
    				msg_cinfo("Good. It seems nothing was "
    					  "changed.\n");
Index: flashrom-flashctx_separate_struct_flashchip/programmer.h
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/programmer.h	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/programmer.h	(Arbeitskopie)
@@ -475,7 +475,7 @@
 extern struct decode_sizes max_rom_decode;
 extern int programmer_may_write;
 extern unsigned long flashbase;
-void check_chip_supported(const struct flashctx *flash);
+void check_chip_supported(const struct flashchip *chip);
 int check_max_decode(enum chipbustype buses, uint32_t size);
 char *extract_programmer_param(const char *param_name);

Index: flashrom-flashctx_separate_struct_flashchip/m29f400bt.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/m29f400bt.c	(Revision 1576)
+++ flashrom-flashctx_separate_struct_flashchip/m29f400bt.c	(Arbeitskopie)
@@ -81,7 +81,7 @@

msg_cdbg("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1, id2);

-	if (id1 == flash->manufacture_id && id2 == flash->model_id)
+	if (id1 == flash->chip->manufacture_id && id2 == flash->chip->model_id)
    	return 1;

return 0;
@@ -130,7 +130,7 @@
 int block_erase_chip_m29f400bt(struct flashctx *flash, unsigned int address,
    		       unsigned int blocklen)
 {
-	if ((address != 0) || (blocklen != flash->total_size * 1024)) {
+	if ((address != 0) || (blocklen != flash->chip->total_size * 1024)) {
    	msg_cerr("%s called with incorrect arguments\n",
    		__func__);
    	return -1;

[PATCH] Make struct flashchip a field in struct flashctx instead of a complete copy.

[Stefan Tauner]

On Sun, 19 Aug 2012 01:51:16 +0200
Carl-Daniel Hailfinger  wrote:
> Counter-proposal (conversion from scratch). AFAICS the only differences
> to your version regarding code flow are in cli_classic.c and flashrom.c.

this is not a full review, i just looked at flashrom.c and cli_classic.c
> Index: flashrom-flashctx_separate_struct_flashchip/cli_classic.c
>
> --- flashrom-flashctx_separate_struct_flashchip/cli_classic.c	(Revision 1576)
> +++ flashrom-flashctx_separate_struct_flashchip/cli_classic.c	(Arbeitskopie)
> @@ -109,8 +109,8 @@
>  {
>    unsigned long size;
>    /* Probe for up to three flash chips. */
> const struct flashchip *flash;
> struct flashctx flashes[3];
> const struct flashchip *chip;
> struct flashctx flashes[3] = {};

this is not standard C afaik, but gcc does not accept (the standard) {0}
without warning either, hence my {{0}} suggestion.
http://stackoverflow.com/questions/1352370/c-static-array-initialization-how...
> struct flashctx *fill_flash;
>    const char *name;
>    int namelen, opt, i, j;
> @@ -389,17 +389,16 @@
>    }
>    /* Does a chip with the requested name exist in the flashchips array? */
>    if (chip_to_probe) {
> for (flash = flashchips; flash && flash->name; flash++)
> 	if (!strcmp(flash->name, chip_to_probe))
> for (chip = flashchips; chip && chip->name; chip++)
> 	if (!strcmp(chip->name, chip_to_probe))
> 	break;
> if (!flash || !flash->name) {
> if (!chip || !chip->name) {
> msg_cerr("Error: Unknown chip '%s' specified.\n", chip_to_probe);
> msg_gerr("Run flashrom -L to view the hardware supported in this flashrom version.\n");
> ret = 1;
> goto out;
> }
> /* Clean up after the check. */
> flash = NULL;
> /* Keep chip around for later usage. */
> }

nasty hack for the evil hack named forced reads. :)
> @@ -456,9 +452,15 @@
>    		/* This loop just counts compatible controllers. */
>    		for (j = 0; j < registered_programmer_count; j++) {
>    			pgm = &registered_programmers[j];
> 		if (pgm->buses_supported & flashes[0].bustype)
> 		/* chip is still set from the chip_to_probe earlier in this function. */
> 		if (pgm->buses_supported & chip->bustype)
> 		compatible_programmers++;
> }
> 	if (!compatible_programmers) {
> 		msg_cinfo("No compatible controller found for the requested flash chip.\n");
> 		ret = 1;
> 		goto out_shutdown;
> 	}

good catch!
> Index: flashrom-flashctx_separate_struct_flashchip/flashrom.c
>
> --- flashrom-flashctx_separate_struct_flashchip/flashrom.c	(Revision 1576)
> +++ flashrom-flashctx_separate_struct_flashchip/flashrom.c	(Arbeitskopie)
> @@ -950,44 +950,52 @@
>    return 1;
>  }
>
> -int probe_flash(struct registered_programmer *pgm, int startchip,
> struct flashctx *fill_flash, int force)
>
> +int probe_flash(struct registered_programmer *pgm, int startchip, struct flashctx *flash, int force)
>  {
> const struct flashchip *flash;
> const struct flashchip *chip;
> unsigned long base = 0;
> char location[64];
> uint32_t size;
> enum chipbustype buses_common;
> char *tmp;
> for (flash = flashchips + startchip; flash && flash->name; flash++) {
> if (chip_to_probe && strcmp(flash->name, chip_to_probe) != 0)
> for (chip = flashchips + startchip; chip && chip->name; chip++) {
> if (chip_to_probe && strcmp(chip->name, chip_to_probe) != 0)
> continue;
> buses_common = pgm->buses_supported & flash->bustype;
> buses_common = pgm->buses_supported & chip->bustype;
> if (!buses_common)
> 	continue;
> msg_gdbg("Probing for %s %s, %d kB: ",
> 	     flash->vendor, flash->name, flash->total_size);
> if (!flash->probe && !force) {
> 	     chip->vendor, chip->name, chip->total_size);

this can be combined with the line before easily.
> if (!chip->probe && !force) {
> msg_gdbg("failed! flashrom has no probe function for "
> 	 "this flash chip.\n");
> continue;
> }
> size = flash->total_size * 1024;
> size = chip->total_size * 1024;
> check_max_decode(buses_common, size);

hm.. return value of check_max_decode is ignored here, and checked
later in main()... quite odd, but not related to flashctx.
> /* Start filling in the dynamic data. */
> memcpy(fill_flash, flash, sizeof(struct flashchip));
> fill_flash->pgm = pgm;
> flash->chip = calloc(1, sizeof(struct flashchip));

right. my patch has the potential to explode spectacularly due to a
normal malloc here and the code expecting 0 to recognize end of lists
etc :/
> if (!flash->chip) {
> 	msg_gerr("Out of memory!\n");
> 	// FIXME: Is -1 the right return code?
> 	return -1;

hm... well it breaks the outer loop, but there is no real error handling
there. better add an exit(1) here (yes, really :) for now and fix it
when the error definitions are merged (which has rather high priority
for me). just returning with the current code would postpone the
explosion to doit(). NB: i havent checked that too thoroughly.
> }
> memcpy(flash->chip, chip, sizeof(struct flashchip));
> flash->pgm = pgm;
>
> base = flashbase ? flashbase : (0xffffffff - size + 1);
> fill_flash->virtual_memory = (chipaddr)programmer_map_flash_region("flash chip", base, size);
> flash->virtual_memory = (chipaddr)programmer_map_flash_region("flash chip", base, size);
>
> /* We handle a forced match like a real match, we just avoid probing. Note that probe_flash()
>  * is only called with force=1 after normal probing failed.
>  */

yay more comments in the hard parts of flashrom :)
> if (force)
> 	break;
>
> if (fill_flash->probe(fill_flash) != 1)
> if (flash->chip->probe(flash) != 1)
> goto notfound;
>
> /* If this is the first chip found, accept it.
>
> @@ -1022,15 +1030,18 @@
>    	}
> if (startchip == 0 ||
>     ((fill_flash->model_id != GENERIC_DEVICE_ID) &&
>      (fill_flash->model_id != SFDP_DEVICE_ID)))
>     ((flash->chip->model_id != GENERIC_DEVICE_ID) &&
>      (flash->chip->model_id != SFDP_DEVICE_ID)))

i like that bit better in my patch, because it groups together what
belongs together.
> 	break;
>
> notfound:
> programmer_unmap_flash_region((void *)fill_flash->virtual_memory, size);
> programmer_unmap_flash_region((void *)flash->virtual_memory, size);
> flash->virtual_memory = (chipaddr)NULL;
> free(flash->chip);
> flash->chip = NULL;

pretty clear indication that this function has issues imho :)
those lines are a candidate to form a function flashctx_init or
something like that, if we need this more often (i really hope we dont.)
> }
> if (!flash || !flash->name)
> if (!flash->chip)
> return -1;

i wondered too why that ->name check was there, any ideas?

[PATCH] Make struct flashchip a field in struct flashctx instead of a complete copy.

[Carl-Daniel Hailfinger]

Am 19.08.2012 03:29 schrieb Stefan Tauner:
> On Sun, 19 Aug 2012 01:51:16 +0200
> Carl-Daniel Hailfinger  wrote:
> > Counter-proposal (conversion from scratch). AFAICS the only differences
> > to your version regarding code flow are in cli_classic.c and flashrom.c.
>
> this is not a full review, i just looked at flashrom.c and cli_classic.c
> > Index: flashrom-flashctx_separate_struct_flashchip/cli_classic.c
> >
> > --- flashrom-flashctx_separate_struct_flashchip/cli_classic.c	(Revision 1576)
> > +++ flashrom-flashctx_separate_struct_flashchip/cli_classic.c	(Arbeitskopie)
> > @@ -109,8 +109,8 @@
> >  {
> >    unsigned long size;
> >    /* Probe for up to three flash chips. */
> > const struct flashchip *flash;
> > struct flashctx flashes[3];
> > const struct flashchip *chip;
> > struct flashctx flashes[3] = {};
>
> this is not standard C afaik, but gcc does not accept (the standard) {0}
> without warning either, hence my {{0}} suggestion.
> http://stackoverflow.com/questions/1352370/c-static-array-initialization-how...

Thanks for the hint. We'll take your version.
> > struct flashctx *fill_flash;
> >    const char *name;
> >    int namelen, opt, i, j;
> > @@ -389,17 +389,16 @@
> >    }
> >    /* Does a chip with the requested name exist in the flashchips array? */
> >    if (chip_to_probe) {
> > for (flash = flashchips; flash && flash->name; flash++)
> > 	if (!strcmp(flash->name, chip_to_probe))
> > for (chip = flashchips; chip && chip->name; chip++)
> > 	if (!strcmp(chip->name, chip_to_probe))
> > 	break;
> > if (!flash || !flash->name) {
> > if (!chip || !chip->name) {
> > msg_cerr("Error: Unknown chip '%s' specified.\n", chip_to_probe);
> > msg_gerr("Run flashrom -L to view the hardware supported in this flashrom version.\n");
> > ret = 1;
> > goto out;
> > }
> > /* Clean up after the check. */
> > flash = NULL;
> > /* Keep chip around for later usage. */
> > }
>
> nasty hack for the evil hack named forced reads. :)

Hm yes, I should have mentioned forced reads in that comment.
> > @@ -456,9 +452,15 @@
> >    		/* This loop just counts compatible controllers. */
> >    		for (j = 0; j < registered_programmer_count; j++) {
> >    			pgm = &registered_programmers[j];
> > 		if (pgm->buses_supported & flashes[0].bustype)
> > 		/* chip is still set from the chip_to_probe earlier in this function. */
> > 		if (pgm->buses_supported & chip->bustype)
> > 		compatible_programmers++;
> > }
> > 	if (!compatible_programmers) {
> > 		msg_cinfo("No compatible controller found for the requested flash chip.\n");
> > 		ret = 1;
> > 		goto out_shutdown;
> > 	}
>
> good catch!

Thanks!
> > Index: flashrom-flashctx_separate_struct_flashchip/flashrom.c
> >
> > --- flashrom-flashctx_separate_struct_flashchip/flashrom.c	(Revision 1576)
> > +++ flashrom-flashctx_separate_struct_flashchip/flashrom.c	(Arbeitskopie)
> > @@ -950,44 +950,52 @@
> >    return 1;
> >  }
> >
> > -int probe_flash(struct registered_programmer *pgm, int startchip,
> > struct flashctx *fill_flash, int force)
> >
> > +int probe_flash(struct registered_programmer *pgm, int startchip, struct flashctx *flash, int force)
> >  {
> > const struct flashchip *flash;
> > const struct flashchip *chip;
> > unsigned long base = 0;
> > char location[64];
> > uint32_t size;
> > enum chipbustype buses_common;
> > char *tmp;
> > for (flash = flashchips + startchip; flash && flash->name; flash++) {
> > if (chip_to_probe && strcmp(flash->name, chip_to_probe) != 0)
> > for (chip = flashchips + startchip; chip && chip->name; chip++) {
> > if (chip_to_probe && strcmp(chip->name, chip_to_probe) != 0)
> > continue;
> > buses_common = pgm->buses_supported & flash->bustype;
> > buses_common = pgm->buses_supported & chip->bustype;
> > if (!buses_common)
> > 	continue;
> > msg_gdbg("Probing for %s %s, %d kB: ",
> > 	     flash->vendor, flash->name, flash->total_size);
> > if (!flash->probe && !force) {
> > 	     chip->vendor, chip->name, chip->total_size);
>
> this can be combined with the line before easily.

Right.
> > if (!flash->chip) {
> > 	msg_gerr("Out of memory!\n");
> > 	// FIXME: Is -1 the right return code?
> > 	return -1;
>
> hm... well it breaks the outer loop, but there is no real error handling
> there. better add an exit(1) here (yes, really :) for now and fix it
> when the error definitions are merged (which has rather high priority
> for me). just returning with the current code would postpone the
> explosion to doit(). NB: i havent checked that too thoroughly.

Hm... while I think that we shouldn't introduce new exit(1), I agree
that this case needs careful audit, and the patch is already complicated
enough as is.
> > @@ -1022,15 +1030,18 @@
> >    	}
> > if (startchip == 0 ||
> >     ((fill_flash->model_id != GENERIC_DEVICE_ID) &&
> >      (fill_flash->model_id != SFDP_DEVICE_ID)))
> >     ((flash->chip->model_id != GENERIC_DEVICE_ID) &&
> >      (flash->chip->model_id != SFDP_DEVICE_ID)))
>
> i like that bit better in my patch, because it groups together what
> belongs together.

You mean having both related checks on one line? Or do you mean using
chip-> instead of flash->chip-> ?
> > 	break;
> >
> > notfound:
> > programmer_unmap_flash_region((void *)fill_flash->virtual_memory, size);
> > programmer_unmap_flash_region((void *)flash->virtual_memory, size);
> > flash->virtual_memory = (chipaddr)NULL;
> > free(flash->chip);
> > flash->chip = NULL;
>
> pretty clear indication that this function has issues imho :)

_Had_ issues. I really like the new code, because it not only
frees/unmaps stuff, it also leaves no dangling pointers around.
> those lines are a candidate to form a function flashctx_init or
> something like that, if we need this more often (i really hope we dont.)
> > }
> > if (!flash || !flash->name)
> > if (!flash->chip)
> > return -1;
>
> i wondered too why that ->name check was there, any ideas?

Yes. The old !flash check would only have triggered in very odd
circumstances (empty array in flashchips.c), and !flash->name was the
detection for end-of-array (last array member was zeroed).
The new !flash->chip check is way more readable and doesn't rely on
implicit properties of an array in another file.

How do we proceed? Should I repost a fixed version of my patch so you
can merge yours and mine? Can you do the missing dediprog conversion
locally or do you want to take mine?

Regards,
Carl-Daniel

[PATCH] Make struct flashchip a field in struct flashctx instead of a complete copy.

[Stefan Tauner]

On Mon, 20 Aug 2012 00:39:19 +0200
Carl-Daniel Hailfinger  wrote:
> > > notfound:
> > > programmer_unmap_flash_region((void *)fill_flash->virtual_memory, size);
> > > programmer_unmap_flash_region((void *)flash->virtual_memory, size);
> > > flash->virtual_memory = (chipaddr)NULL;
> > > free(flash->chip);
> > > flash->chip = NULL;  
> >
> > pretty clear indication that this function has issues imho :)
>
> _Had_ issues. I really like the new code, because it not only
> frees/unmaps stuff, it also leaves no dangling pointers around.

in a function that should actually just probe for a chip. my definition
of code issues seems to be different. :)

but it got better, and it is certainly ready for merge.

i would prefer if you would take the bits of my patch that you like, add
them to your patch and then repost or commit right away.

[PATCH] Make struct flashchip a field in struct flashctx instead of a complete copy.

[Carl-Daniel Hailfinger]

New version.

All the driver conversion work and cleanup has been done by Stefan.
flashrom.c and cli_classic.c are a joint work of Stefan and Carl-Daniel.

It works, has survived various build tests with gcc and clang, and this
is the fourth(?) iteration, hopefully satisfying everyone.

Signed-off-by: Carl-Daniel Hailfinger 
Signed-off-by: Stefan Tauner

Index: flashrom-flashctx_separate_struct_flashchip/flash.h
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/flash.h	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/flash.h	(Arbeitskopie)
@@ -87,6 +87,7 @@
 #define FEATURE_WRSR_EITHER	(FEATURE_WRSR_EWSR | FEATURE_WRSR_WREN)

struct flashctx;
+typedef int (erasefunc_t)(struct flashctx *flash, unsigned int addr, unsigned int blocklen);

struct flashchip {
    const char *vendor;
@@ -148,35 +149,14 @@
    } voltage;
 };

-/* struct flashctx must always contain struct flashchip at the beginning. */
 struct flashctx {
-	const char *vendor;
-	const char *name;
-	enum chipbustype bustype;
-	uint32_t manufacture_id;
-	uint32_t model_id;
-	int total_size;
-	int page_size;
-	int feature_bits;
-	uint32_t tested;
-	int (*probe) (struct flashctx *flash);
-	int probe_timing;
-	struct block_eraser block_erasers[NUM_ERASEFUNCTIONS];
-	int (*printlock) (struct flashctx *flash);
-	int (*unlock) (struct flashctx *flash);
-	int (*write) (struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len);
-	int (*read) (struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len);
-	struct voltage voltage;
-	/* struct flashchip ends here. */
-	
+	struct flashchip *chip;
    chipaddr virtual_memory;
    /* Some flash devices have an additional register space. */
    chipaddr virtual_registers;
    struct registered_programmer *pgm;
 };

-typedef int (erasefunc_t)(struct flashctx *flash, unsigned int addr, unsigned int blocklen);
-
 #define TEST_UNTESTED	0

#define TEST_OK_PROBE	(1 << 0)
@@ -307,7 +287,7 @@
 int register_include_arg(char *name);
 int process_include_args(void);
 int read_romlayout(char *name);
-int handle_romentries(struct flashctx *flash, uint8_t *oldcontents, uint8_t *newcontents);
+int handle_romentries(const struct flashctx *flash, uint8_t *oldcontents, uint8_t *newcontents);

/* spi.c */
 struct spi_command {
Index: flashrom-flashctx_separate_struct_flashchip/it87spi.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/it87spi.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/it87spi.c	(Arbeitskopie)
@@ -331,7 +331,7 @@
    /* FIXME: The command below seems to be redundant or wrong. */
    OUTB(0x06, it8716f_flashport + 1);
    OUTB(((2 + (fast_spi ? 1 : 0)) << 4), it8716f_flashport);
-	for (i = 0; i < flash->page_size; i++)
+	for (i = 0; i < flash->chip->page_size; i++)
    	mmio_writeb(buf[i], (void *)(bios + start + i));
    OUTB(0, it8716f_flashport);
    /* Wait until the Write-In-Progress bit is cleared.
@@ -355,7 +355,7 @@
     * the mainboard does not use IT87 SPI translation. This should be done
     * via a programmer parameter for the internal programmer.
     */
-	if ((flash->total_size * 1024 > 512 * 1024)) {
+	if ((flash->chip->total_size * 1024 > 512 * 1024)) {
    	spi_read_chunked(flash, buf, start, len, 3);
    } else {
    	mmio_readn((void *)(flash->virtual_memory + start), buf, len);
@@ -367,6 +367,7 @@
 static int it8716f_spi_chip_write_256(struct flashctx *flash, uint8_t *buf,
    			      unsigned int start, unsigned int len)
 {
+	const struct flashchip *chip = flash->chip;
    /*
     * IT8716F only allows maximum of 512 kb SPI chip size for memory
     * mapped access. It also can't write more than 1+3+256 bytes at once,
@@ -377,28 +378,27 @@
     * the mainboard does not use IT87 SPI translation. This should be done
     * via a programmer parameter for the internal programmer.
     */
-	if ((flash->total_size * 1024 > 512 * 1024) ||
-	    (flash->page_size > 256)) {
+	if ((chip->total_size * 1024 > 512 * 1024) || (chip->page_size > 256)) {
    	spi_chip_write_1(flash, buf, start, len);
    } else {
    	unsigned int lenhere;

-		if (start % flash->page_size) {
+		if (start % chip->page_size) {
    		/* start to the end of the page or to start + len,
    		 * whichever is smaller.
    		 */
-			lenhere = min(len, flash->page_size - start % flash->page_size);
+			lenhere = min(len, chip->page_size - start % chip->page_size);
    		spi_chip_write_1(flash, buf, start, lenhere);
    		start += lenhere;
    		len -= lenhere;
    		buf += lenhere;
    	}

-		while (len >= flash->page_size) {
+		while (len >= chip->page_size) {
    		it8716f_spi_page_program(flash, buf, start);
-			start += flash->page_size;
-			len -= flash->page_size;
-			buf += flash->page_size;
+			start += chip->page_size;
+			len -= chip->page_size;
+			buf += chip->page_size;
    	}
    	if (len)
    		spi_chip_write_1(flash, buf, start, len);
Index: flashrom-flashctx_separate_struct_flashchip/jedec.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/jedec.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/jedec.c	(Arbeitskopie)
@@ -93,9 +93,9 @@
    	msg_cdbg("%s: excessive loops, i=0x%x\n", __func__, i);
 }

-static unsigned int getaddrmask(struct flashctx *flash)
+static unsigned int getaddrmask(const struct flashchip *chip)
 {
-	switch (flash->feature_bits & FEATURE_ADDR_MASK) {
+	switch (chip->feature_bits & FEATURE_ADDR_MASK) {
    case FEATURE_ADDR_FULL:
    	return MASK_FULL;
    	break;
@@ -124,16 +124,17 @@
 static int probe_jedec_common(struct flashctx *flash, unsigned int mask)
 {
    chipaddr bios = flash->virtual_memory;
+	const struct flashchip *chip = flash->chip;
    uint8_t id1, id2;
    uint32_t largeid1, largeid2;
    uint32_t flashcontent1, flashcontent2;
    int probe_timing_enter, probe_timing_exit;

-	if (flash->probe_timing > 0) 
-		probe_timing_enter = probe_timing_exit = flash->probe_timing;
-	else if (flash->probe_timing == TIMING_ZERO) { /* No delay. */
+	if (chip->probe_timing > 0)
+		probe_timing_enter = probe_timing_exit = chip->probe_timing;
+	else if (chip->probe_timing == TIMING_ZERO) { /* No delay. */
    	probe_timing_enter = probe_timing_exit = 0;
-	} else if (flash->probe_timing == TIMING_FIXME) { /* == _IGNORED */
+	} else if (chip->probe_timing == TIMING_FIXME) { /* == _IGNORED */
    	msg_cdbg("Chip lacks correct probe timing information, "
    		     "using default 10mS/40uS. ");
    	probe_timing_enter = 10000;
@@ -151,7 +152,7 @@
    if (probe_timing_enter)
    	programmer_delay(probe_timing_enter);
    /* Reset chip to a clean slate */
-	if ((flash->feature_bits & FEATURE_RESET_MASK) == FEATURE_LONG_RESET)
+	if ((chip->feature_bits & FEATURE_RESET_MASK) == FEATURE_LONG_RESET)
    {
    	chip_writeb(flash, 0xAA, bios + (0x5555 & mask));
    	if (probe_timing_exit)
@@ -194,7 +195,7 @@
    }

/* Issue JEDEC Product ID Exit command */
-	if ((flash->feature_bits & FEATURE_RESET_MASK) == FEATURE_LONG_RESET)
+	if ((chip->feature_bits & FEATURE_RESET_MASK) == FEATURE_LONG_RESET)
    {
    	chip_writeb(flash, 0xAA, bios + (0x5555 & mask));
    	if (probe_timing_exit)
@@ -231,10 +232,10 @@
    	msg_cdbg(", id2 is normal flash content");

msg_cdbg("\n");
-	if (largeid1 != flash->manufacture_id || largeid2 != flash->model_id)
+	if (largeid1 != chip->manufacture_id || largeid2 != chip->model_id)
    	return 0;

-	if (flash->feature_bits & FEATURE_REGISTERMAP)
+	if (chip->feature_bits & FEATURE_REGISTERMAP)
    	map_flash_registers(flash);

return 1;
@@ -245,7 +246,7 @@
 {
    chipaddr bios = flash->virtual_memory;
    int delay_us = 0;
-	if(flash->probe_timing != TIMING_ZERO)
+	if(flash->chip->probe_timing != TIMING_ZERO)
            delay_us = 10;

/*  Issue the Sector Erase command   */
@@ -275,7 +276,7 @@
 {
    chipaddr bios = flash->virtual_memory;
    int delay_us = 0;
-	if(flash->probe_timing != TIMING_ZERO)
+	if(flash->chip->probe_timing != TIMING_ZERO)
            delay_us = 10;

/*  Issue the Sector Erase command   */
@@ -304,7 +305,7 @@
 {
    chipaddr bios = flash->virtual_memory;
    int delay_us = 0;
-	if(flash->probe_timing != TIMING_ZERO)
+	if(flash->chip->probe_timing != TIMING_ZERO)
            delay_us = 10;

/*  Issue the JEDEC Chip Erase command   */
@@ -366,7 +367,7 @@
    chipaddr olddst;
    unsigned int mask;

-	mask = getaddrmask(flash);
+	mask = getaddrmask(flash->chip);

olddst = dst;
    for (i = 0; i < len; i++) {
@@ -390,7 +391,7 @@
    chipaddr d = dst;
    unsigned int mask;

-	mask = getaddrmask(flash);
+	mask = getaddrmask(flash->chip);

retry:
    /* Issue JEDEC Start Program command */
@@ -438,7 +439,7 @@
     * write_jedec have page_size set to max_writechunk_size, so
     * we're OK for now.
     */
-	unsigned int page_size = flash->page_size;
+	unsigned int page_size = flash->chip->page_size;

/* Warning: This loop has a very unusual condition and body.
     * The loop needs to go through each page with at least one affected
@@ -469,8 +470,8 @@
 {
    unsigned int mask;

-	mask = getaddrmask(flash);
-	if ((addr != 0) || (blocksize != flash->total_size * 1024)) {
+	mask = getaddrmask(flash->chip);
+	if ((addr != 0) || (blocksize != flash->chip->total_size * 1024)) {
    	msg_cerr("%s called with incorrect arguments\n",
    		__func__);
    	return -1;
@@ -482,7 +483,7 @@
 {
    unsigned int mask;

-	mask = getaddrmask(flash);
+	mask = getaddrmask(flash->chip);
    return probe_jedec_common(flash, mask);
 }

@@ -491,7 +492,7 @@
 {
    unsigned int mask;

-	mask = getaddrmask(flash);
+	mask = getaddrmask(flash->chip);
    return erase_sector_jedec_common(flash, page, size, mask);
 }

@@ -500,7 +501,7 @@
 {
    unsigned int mask;

-	mask = getaddrmask(flash);
+	mask = getaddrmask(flash->chip);
    return erase_block_jedec_common(flash, page, size, mask);
 }

@@ -508,6 +509,6 @@
 {
    unsigned int mask;

-	mask = getaddrmask(flash);
+	mask = getaddrmask(flash->chip);
    return erase_chip_jedec_common(flash, mask);
 }
Index: flashrom-flashctx_separate_struct_flashchip/w39.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/w39.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/w39.c	(Arbeitskopie)
@@ -138,11 +138,11 @@

static int printlock_w39_fwh(struct flashctx *flash)
 {
-	unsigned int i, total_size = flash->total_size * 1024;
+	unsigned int i, total_size = flash->chip->total_size * 1024;
    int ret = 0;
    
    /* Print lock status of the complete chip */
-	for (i = 0; i < total_size; i += flash->page_size)
+	for (i = 0; i < total_size; i += flash->chip->page_size)
    	ret |= printlock_w39_fwh_block(flash, i);

return ret;
@@ -150,10 +150,10 @@

static int unlock_w39_fwh(struct flashctx *flash)
 {
-	unsigned int i, total_size = flash->total_size * 1024;
+	unsigned int i, total_size = flash->chip->total_size * 1024;
    
    /* Unlock the complete chip */
-	for (i = 0; i < total_size; i += flash->page_size)
+	for (i = 0; i < total_size; i += flash->chip->page_size)
    	if (unlock_w39_fwh_block(flash, i))
    		return -1;

Index: flashrom-flashctx_separate_struct_flashchip/sst49lfxxxc.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/sst49lfxxxc.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/sst49lfxxxc.c	(Arbeitskopie)
@@ -38,7 +38,7 @@
 static int write_lockbits_49lfxxxc(struct flashctx *flash, unsigned char bits)
 {
    chipaddr registers = flash->virtual_registers;
-	unsigned int i, left = flash->total_size * 1024;
+	unsigned int i, left = flash->chip->total_size * 1024;
    unsigned long address;

msg_cdbg("\nbios=0x%08lx\n", registers);
Index: flashrom-flashctx_separate_struct_flashchip/sst_fwhub.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/sst_fwhub.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/sst_fwhub.c	(Arbeitskopie)
@@ -31,7 +31,7 @@

blockstatus = chip_readb(flash, registers + offset + 2);
    msg_cdbg("Lock status for 0x%06x (size 0x%06x) is %02x, ",
-		     offset, flash->page_size, blockstatus);
+		     offset, flash->chip->page_size, blockstatus);
    switch (blockstatus & 0x3) {
    case 0x0:
    	msg_cdbg("full access\n");
@@ -72,7 +72,7 @@
 {
    int i;

-	for (i = 0; i < flash->total_size * 1024; i += flash->page_size)
+	for (i = 0; i < flash->chip->total_size * 1024; i += flash->chip->page_size)
    	check_sst_fwhub_block_lock(flash, i);

return 0;
@@ -82,7 +82,7 @@
 {
    int i, ret=0;

-	for (i = 0; i < flash->total_size * 1024; i += flash->page_size)
+	for (i = 0; i < flash->chip->total_size * 1024; i += flash->chip->page_size)
    {
    	if (clear_sst_fwhub_block_lock(flash, i))
    	{
Index: flashrom-flashctx_separate_struct_flashchip/cli_classic.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/cli_classic.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/cli_classic.c	(Arbeitskopie)
@@ -109,8 +109,8 @@
 {
    unsigned long size;
    /* Probe for up to three flash chips. */
-	const struct flashchip *flash;
-	struct flashctx flashes[3];
+	const struct flashchip *chip = NULL;
+	struct flashctx flashes[3] = {{0}};
    struct flashctx *fill_flash;
    const char *name;
    int namelen, opt, i, j;
@@ -389,17 +389,16 @@
    }
    /* Does a chip with the requested name exist in the flashchips array? */
    if (chip_to_probe) {
-		for (flash = flashchips; flash && flash->name; flash++)
-			if (!strcmp(flash->name, chip_to_probe))
+		for (chip = flashchips; chip && chip->name; chip++)
+			if (!strcmp(chip->name, chip_to_probe))
    			break;
-		if (!flash || !flash->name) {
+		if (!chip || !chip->name) {
    		msg_cerr("Error: Unknown chip '%s' specified.\n", chip_to_probe);
    		msg_gerr("Run flashrom -L to view the hardware supported in this flashrom version.\n");
    		ret = 1;
    		goto out;
    	}
-		/* Clean up after the check. */
-		flash = NULL;
+		/* Keep chip around for later usage in case a forced read is requested. */
    }

if (prog == PROGRAMMER_INVALID) {
@@ -419,16 +418,13 @@
    	goto out_shutdown;
    }
    tempstr = flashbuses_to_text(get_buses_supported());
-	msg_pdbg("The following protocols are supported: %s.\n",
-		 tempstr);
+	msg_pdbg("The following protocols are supported: %s.\n", tempstr);
    free(tempstr);

for (j = 0; j < registered_programmer_count; j++) {
    	startchip = 0;
    	while (chipcount < ARRAY_SIZE(flashes)) {
-			startchip = probe_flash(&registered_programmers[j],
-						startchip, 
-						&flashes[chipcount], 0);
+			startchip = probe_flash(&registered_programmers[j], startchip, &flashes[chipcount], 0);
    		if (startchip == -1)
    			break;
    		chipcount++;
@@ -437,9 +433,9 @@
    }

if (chipcount > 1) {
-		msg_cinfo("Multiple flash chips were detected: "%s"", flashes[0].name);
+		msg_cinfo("Multiple flash chips were detected: "%s"", flashes[0].chip->name);
    	for (i = 1; i < chipcount; i++)
-			msg_cinfo(", "%s"", flashes[i].name);
+			msg_cinfo(", "%s"", flashes[i].chip->name);
    	msg_cinfo("\nPlease specify which chip to use with the -c <chipname> option.\n");
    	ret = 1;
    	goto out_shutdown;
@@ -456,9 +452,15 @@
    		/* This loop just counts compatible controllers. */
    		for (j = 0; j < registered_programmer_count; j++) {
    			pgm = &registered_programmers[j];
-				if (pgm->buses_supported & flashes[0].bustype)
+				/* chip is still set from the chip_to_probe earlier in this function. */
+				if (pgm->buses_supported & chip->bustype)
    				compatible_programmers++;
    		}
+			if (!compatible_programmers) {
+				msg_cinfo("No compatible controller found for the requested flash chip.\n");
+				ret = 1;
+				goto out_shutdown;
+			}
    		if (compatible_programmers > 1)
    			msg_cinfo("More than one compatible controller found for the requested flash "
    				  "chip, using the first one.\n");
@@ -469,6 +471,7 @@
    				break;
    		}
    		if (startchip == -1) {
+				// FIXME: This should never happen! Ask for a bug report?
    			msg_cinfo("Probing for flash chip '%s' failed.\n", chip_to_probe);
    			ret = 1;
    			goto out_shutdown;
@@ -481,19 +484,18 @@
    	goto out_shutdown;
    } else if (!chip_to_probe) {
    	/* repeat for convenience when looking at foreign logs */
-		tempstr = flashbuses_to_text(flashes[0].bustype);
+		tempstr = flashbuses_to_text(flashes[0].chip->bustype);
    	msg_gdbg("Found %s flash chip "%s" (%d kB, %s).\n",
-			 flashes[0].vendor, flashes[0].name,
-			 flashes[0].total_size, tempstr);
+			 flashes[0].chip->vendor, flashes[0].chip->name, flashes[0].chip->total_size, tempstr);
    	free(tempstr);
    }

fill_flash = &flashes[0];

-	check_chip_supported(fill_flash);
+	check_chip_supported(fill_flash->chip);

-	size = fill_flash->total_size * 1024;
-	if (check_max_decode(fill_flash->pgm->buses_supported & fill_flash->bustype, size) && (!force)) {
+	size = fill_flash->chip->total_size * 1024;
+	if (check_max_decode(fill_flash->pgm->buses_supported & fill_flash->chip->bustype, size) && (!force)) {
    	msg_cerr("Chip is too big for this programmer (-V gives details). Use --force to override.\n");
    	ret = 1;
    	goto out_shutdown;
Index: flashrom-flashctx_separate_struct_flashchip/layout.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/layout.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/layout.c	(Arbeitskopie)
@@ -217,11 +217,11 @@
    return best_entry;
 }

-int handle_romentries(struct flashctx *flash, uint8_t *oldcontents, uint8_t *newcontents)
+int handle_romentries(const struct flashctx *flash, uint8_t *oldcontents, uint8_t *newcontents)
 {
    unsigned int start = 0;
    romlayout_t *entry;
-	unsigned int size = flash->total_size * 1024;
+	unsigned int size = flash->chip->total_size * 1024;

/* If no regions were specified for inclusion, assume
     * that the user wants to write the complete new image.
Index: flashrom-flashctx_separate_struct_flashchip/ichspi.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/ichspi.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/ichspi.c	(Arbeitskopie)
@@ -1193,9 +1193,9 @@
    else
    	msg_cdbg(" with a");
    msg_cdbg(" density of %d kB.\n", total_size / 1024);
-	flash->total_size = total_size / 1024;
+	flash->chip->total_size = total_size / 1024;

-	eraser = &(flash->block_erasers[0]);
+	eraser = &(flash->chip->block_erasers[0]);
    boundary = (REGREAD32(ICH9_REG_FPB) & FPB_FPBA) << 12;
    size_high = total_size - boundary;
    erase_size_high = ich_hwseq_get_erase_block_size(boundary);
@@ -1228,7 +1228,7 @@
    	msg_cdbg("In that range are %d erase blocks with %d B each.\n",
    		 size_high / erase_size_high, erase_size_high);
    }
-	flash->tested = TEST_OK_PREW;
+	flash->chip->tested = TEST_OK_PREW;
    return 1;
 }

@@ -1256,7 +1256,7 @@
    	return -1;
    }

-	if (addr + len > flash->total_size * 1024) {
+	if (addr + len > flash->chip->total_size * 1024) {
    	msg_perr("Request to erase some inaccessible memory address(es)"
    		 " (addr=0x%x, len=%d). "
    		 "Not erasing anything.\n", addr, len);
@@ -1288,7 +1288,7 @@
    uint16_t timeout = 100 * 60;
    uint8_t block_len;

-	if (addr + len > flash->total_size * 1024) {
+	if (addr + len > flash->chip->total_size * 1024) {
    	msg_perr("Request to read from an inaccessible memory address "
    		 "(addr=0x%x, len=%d).\n", addr, len);
    	return -1;
@@ -1326,7 +1326,7 @@
    uint16_t timeout = 100 * 60;
    uint8_t block_len;

-	if (addr + len > flash->total_size * 1024) {
+	if (addr + len > flash->chip->total_size * 1024) {
    	msg_perr("Request to write to an inaccessible memory address "
    		 "(addr=0x%x, len=%d).\n", addr, len);
    	return -1;
Index: flashrom-flashctx_separate_struct_flashchip/82802ab.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/82802ab.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/82802ab.c	(Arbeitskopie)
@@ -44,7 +44,7 @@
 {
    chipaddr bios = flash->virtual_memory;
    uint8_t id1, id2, flashcontent1, flashcontent2;
-	int shifted = (flash->feature_bits & FEATURE_ADDR_SHIFTED) != 0;
+	int shifted = (flash->chip->feature_bits & FEATURE_ADDR_SHIFTED) != 0;

/* Reset to get a clean state */
    chip_writeb(flash, 0xFF, bios);
@@ -80,10 +80,10 @@
    	msg_cdbg(", id2 is normal flash content");

msg_cdbg("\n");
-	if (id1 != flash->manufacture_id || id2 != flash->model_id)
+	if (id1 != flash->chip->manufacture_id || id2 != flash->chip->model_id)
    	return 0;

-	if (flash->feature_bits & FEATURE_REGISTERMAP)
+	if (flash->chip->feature_bits & FEATURE_REGISTERMAP)
    	map_flash_registers(flash);

return 1;
@@ -112,7 +112,7 @@
    int i;
    //chipaddr wrprotect = flash->virtual_registers + page + 2;

-	for (i = 0; i < flash->total_size * 1024; i+= flash->page_size)
+	for (i = 0; i < flash->chip->total_size * 1024; i+= flash->chip->page_size)
    	chip_writeb(flash, 0, flash->virtual_registers + i + 2);

return 0;
@@ -181,7 +181,7 @@
    }

/* Read block lock-bits */
-	for (i = 0; i < flash->total_size * 1024; i+= (64 * 1024)) {
+	for (i = 0; i < flash->chip->total_size * 1024; i+= (64 * 1024)) {
    	bcfg = chip_readb(flash, bios + i + 2); // read block lock config
    	msg_cdbg("block lock at %06x is %slocked!\n", i, bcfg ? "" : "un");
    	if (bcfg) {
@@ -234,7 +234,7 @@
    }

/* Read block lock-bits, 8 * 8 KB + 15 * 64 KB */
-	for (i = 0; i < flash->total_size * 1024;
+	for (i = 0; i < flash->chip->total_size * 1024;
         i += (i >= (64 * 1024) ? 64 * 1024 : 8 * 1024)) {
    	bcfg = chip_readb(flash, bios + i + 2); /* read block lock config */
    	msg_cdbg("block lock at %06x is %slocked!\n", i,
Index: flashrom-flashctx_separate_struct_flashchip/dediprog.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/dediprog.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/dediprog.c	(Arbeitskopie)
@@ -381,7 +381,7 @@
    		      unsigned int start, unsigned int len, uint8_t dedi_spi_cmd)
 {
    int ret;
-	const unsigned int chunksize = flash->page_size;
+	const unsigned int chunksize = flash->chip->page_size;
    unsigned int residue = start % chunksize ? chunksize - start % chunksize : 0;
    unsigned int bulklen;

Index: flashrom-flashctx_separate_struct_flashchip/spi25.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/spi25.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/spi25.c	(Arbeitskopie)
@@ -117,6 +117,7 @@

static int probe_spi_rdid_generic(struct flashctx *flash, int bytes)
 {
+	const struct flashchip *chip = flash->chip;
    unsigned char readarr[4];
    uint32_t id1;
    uint32_t id2;
@@ -147,7 +148,7 @@

msg_cdbg("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1, id2);

-	if (id1 == flash->manufacture_id && id2 == flash->model_id) {
+	if (id1 == chip->manufacture_id && id2 == chip->model_id) {
    	/* Print the status register to tell the
    	 * user about possible write protection.
    	 */
@@ -157,13 +158,11 @@
    }

/* Test if this is a pure vendor match. */
-	if (id1 == flash->manufacture_id &&
-	    GENERIC_DEVICE_ID == flash->model_id)
+	if (id1 == chip->manufacture_id && GENERIC_DEVICE_ID == chip->model_id)
    	return 1;

/* Test if there is any vendor ID. */
-	if (GENERIC_MANUF_ID == flash->manufacture_id &&
-	    id1 != 0xff)
+	if (GENERIC_MANUF_ID == chip->manufacture_id && id1 != 0xff)
    	return 1;

return 0;
@@ -198,6 +197,7 @@

int probe_spi_rems(struct flashctx *flash)
 {
+	const struct flashchip *chip = flash->chip;
    unsigned char readarr[JEDEC_REMS_INSIZE];
    uint32_t id1, id2;

@@ -210,7 +210,7 @@

msg_cdbg("%s: id1 0x%x, id2 0x%x\n", __func__, id1, id2);

-	if (id1 == flash->manufacture_id && id2 == flash->model_id) {
+	if (id1 == chip->manufacture_id && id2 == chip->model_id) {
    	/* Print the status register to tell the
    	 * user about possible write protection.
    	 */
@@ -220,13 +220,11 @@
    }

/* Test if this is a pure vendor match. */
-	if (id1 == flash->manufacture_id &&
-	    GENERIC_DEVICE_ID == flash->model_id)
+	if (id1 == chip->manufacture_id && GENERIC_DEVICE_ID == chip->model_id)
    	return 1;

/* Test if there is any vendor ID. */
-	if (GENERIC_MANUF_ID == flash->manufacture_id &&
-	    id1 != 0xff)
+	if (GENERIC_MANUF_ID == chip->manufacture_id && id1 != 0xff)
    	return 1;

return 0;
@@ -267,7 +265,7 @@

msg_cdbg("%s: id 0x%x\n", __func__, id2);

-	if (id2 != flash->model_id)
+	if (id2 != flash->chip->model_id)
    	return 0;

/* Print the status register to tell the
@@ -291,7 +289,7 @@

msg_cdbg("%s: id1 0x%x, id2 0x%x\n", __func__, id1, id2);

-	if (id1 != flash->manufacture_id || id2 != flash->model_id)
+	if (id1 != flash->chip->manufacture_id || id2 != flash->chip->model_id)
    	return 0;

/* Print the status register to tell the
@@ -419,22 +417,23 @@

int spi_prettyprint_status_register(struct flashctx *flash)
 {
+	const struct flashchip *chip = flash->chip;
    uint8_t status;

status = spi_read_status_register(flash);
    msg_cdbg("Chip status register is %02x\n", status);
-	switch (flash->manufacture_id) {
+	switch (chip->manufacture_id) {
    case ST_ID:
-		if (((flash->model_id & 0xff00) == 0x2000) ||
-		    ((flash->model_id & 0xff00) == 0x2500))
+		if (((chip->model_id & 0xff00) == 0x2000) ||
+		    ((chip->model_id & 0xff00) == 0x2500))
    		spi_prettyprint_status_register_st_m25p(status);
    	break;
    case MACRONIX_ID:
-		if ((flash->model_id & 0xff00) == 0x2000)
+		if ((chip->model_id & 0xff00) == 0x2000)
    		spi_prettyprint_status_register_st_m25p(status);
    	break;
    case SST_ID:
-		switch (flash->model_id) {
+		switch (chip->model_id) {
    	case 0x2541:
    		spi_prettyprint_status_register_sst25vf016(status);
    		break;
@@ -704,7 +703,7 @@
 int spi_block_erase_60(struct flashctx *flash, unsigned int addr,
    	       unsigned int blocklen)
 {
-	if ((addr != 0) || (blocklen != flash->total_size * 1024)) {
+	if ((addr != 0) || (blocklen != flash->chip->total_size * 1024)) {
    	msg_cerr("%s called with incorrect arguments\n",
    		__func__);
    	return -1;
@@ -715,7 +714,7 @@
 int spi_block_erase_c7(struct flashctx *flash, unsigned int addr,
    	       unsigned int blocklen)
 {
-	if ((addr != 0) || (blocklen != flash->total_size * 1024)) {
+	if ((addr != 0) || (blocklen != flash->chip->total_size * 1024)) {
    	msg_cerr("%s called with incorrect arguments\n",
    		__func__);
    	return -1;
@@ -820,7 +819,7 @@

int spi_write_status_register(struct flashctx *flash, int status)
 {
-	int feature_bits = flash->feature_bits;
+	int feature_bits = flash->chip->feature_bits;
    int ret = 1;

if (!(feature_bits & (FEATURE_WRSR_WREN | FEATURE_WRSR_EWSR))) {
@@ -972,7 +971,7 @@
 {
    int rc = 0;
    unsigned int i, j, starthere, lenhere, toread;
-	unsigned int page_size = flash->page_size;
+	unsigned int page_size = flash->chip->page_size;

/* Warning: This loop has a very unusual condition and body.
     * The loop needs to go through each page with at least one affected
@@ -1017,7 +1016,7 @@
     * spi_chip_write_256 have page_size set to max_writechunk_size, so
     * we're OK for now.
     */
-	unsigned int page_size = flash->page_size;
+	unsigned int page_size = flash->chip->page_size;

/* Warning: This loop has a very unusual condition and body.
     * The loop needs to go through each page with at least one affected
Index: flashrom-flashctx_separate_struct_flashchip/pm49fl00x.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/pm49fl00x.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/pm49fl00x.c	(Arbeitskopie)
@@ -40,14 +40,14 @@

int unlock_49fl00x(struct flashctx *flash)
 {
-	write_lockbits_49fl00x(flash, flash->total_size * 1024, 0,
-			       flash->page_size);
+	write_lockbits_49fl00x(flash, flash->chip->total_size * 1024, 0,
+			       flash->chip->page_size);
    return 0;
 }

int lock_49fl00x(struct flashctx *flash)
 {
-	write_lockbits_49fl00x(flash, flash->total_size * 1024, 1,
-			       flash->page_size);
+	write_lockbits_49fl00x(flash, flash->chip->total_size * 1024, 1,
+			       flash->chip->page_size);
    return 0;
 }
Index: flashrom-flashctx_separate_struct_flashchip/en29lv640b.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/en29lv640b.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/en29lv640b.c	(Arbeitskopie)
@@ -81,7 +81,7 @@

msg_cdbg("%s: id1 0x%04x, id2 0x%04x\n", __func__, id1, id2);

-	if (id1 == flash->manufacture_id && id2 == flash->model_id)
+	if (id1 == flash->chip->manufacture_id && id2 == flash->chip->model_id)
    	return 1;

return 0;
@@ -130,7 +130,7 @@
 int block_erase_chip_en29lv640b(struct flashctx *flash, unsigned int address,
    		        unsigned int blocklen)
 {
-	if ((address != 0) || (blocklen != flash->total_size * 1024)) {
+	if ((address != 0) || (blocklen != flash->chip->total_size * 1024)) {
    	msg_cerr("%s called with incorrect arguments\n", __func__);
    	return -1;
    }
Index: flashrom-flashctx_separate_struct_flashchip/w29ee011.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/w29ee011.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/w29ee011.c	(Arbeitskopie)
@@ -29,11 +29,11 @@
    chipaddr bios = flash->virtual_memory;
    uint8_t id1, id2;

-	if (!chip_to_probe || strcmp(chip_to_probe, flash->name)) {
+	if (!chip_to_probe || strcmp(chip_to_probe, flash->chip->name)) {
    	msg_cdbg("Old Winbond W29* probe method disabled because "
    		 "the probing sequence puts the AMIC A49LF040A in "
    		 "a funky state. Use 'flashrom -c %s' if you "
-			 "have a board with such a chip.\n", flash->name);
+			 "have a board with such a chip.\n", flash->chip->name);
    	return 0;
    }

@@ -65,7 +65,7 @@

msg_cdbg("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1, id2);

-	if (id1 == flash->manufacture_id && id2 == flash->model_id)
+	if (id1 == flash->chip->manufacture_id && id2 == flash->chip->model_id)
    	return 1;

return 0;
Index: flashrom-flashctx_separate_struct_flashchip/spi.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/spi.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/spi.c	(Arbeitskopie)
@@ -111,16 +111,16 @@
     * means 0xffffff, the highest unsigned 24bit number.
     */
    addrbase = spi_get_valid_read_addr(flash);
-	if (addrbase + flash->total_size * 1024 > (1 << 24)) {
+	if (addrbase + flash->chip->total_size * 1024 > (1 << 24)) {
    	msg_perr("Flash chip size exceeds the allowed access window. ");
    	msg_perr("Read will probably fail.\n");
    	/* Try to get the best alignment subject to constraints. */
-		addrbase = (1 << 24) - flash->total_size * 1024;
+		addrbase = (1 << 24) - flash->chip->total_size * 1024;
    }
    /* Check if alignment is native (at least the largest power of two which
     * is a factor of the mapped size of the chip).
     */
-	if (ffs(flash->total_size * 1024) > (ffs(addrbase) ? : 33)) {
+	if (ffs(flash->chip->total_size * 1024) > (ffs(addrbase) ? : 33)) {
    	msg_perr("Flash chip is not aligned natively in the allowed "
    		 "access window.\n");
    	msg_perr("Read will probably return garbage.\n");
Index: flashrom-flashctx_separate_struct_flashchip/sfdp.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/sfdp.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/sfdp.c	(Arbeitskopie)
@@ -81,10 +81,10 @@
    uint32_t ptp; /* 24b pointer */
 };

-static int sfdp_add_uniform_eraser(struct flashctx *flash, uint8_t opcode, uint32_t block_size)
+static int sfdp_add_uniform_eraser(struct flashchip *chip, uint8_t opcode, uint32_t block_size)
 {
    int i;
-	uint32_t total_size = flash->total_size * 1024;
+	uint32_t total_size = chip->total_size * 1024;
    erasefunc_t *erasefn = spi_get_erasefn_from_opcode(opcode);

if (erasefn == NULL || total_size == 0 || block_size == 0 ||
@@ -95,7 +95,7 @@
    }

for (i = 0; i < NUM_ERASEFUNCTIONS; i++) {
-		struct block_eraser *eraser = &flash->block_erasers[i];
+		struct block_eraser *eraser = &chip->block_erasers[i];
    	/* Check for duplicates (including (some) non-uniform ones). */
    	if (eraser->eraseblocks[0].size == block_size &&
    	    eraser->block_erase == erasefn) {
@@ -125,7 +125,7 @@
    return 1;
 }

-static int sfdp_fill_flash(struct flashctx *flash, uint8_t *buf, uint16_t len)
+static int sfdp_fill_flash(struct flashchip *chip, uint8_t *buf, uint16_t len)
 {
    uint8_t opcode_4k_erase = 0xFF;
    uint32_t tmp32;
@@ -170,28 +170,28 @@
    	msg_cdbg2("volatile and writes to the status register have to "
    		  "be enabled with ");
    	if (tmp32 & (1 << 4)) {
-			flash->feature_bits = FEATURE_WRSR_WREN;
+			chip->feature_bits = FEATURE_WRSR_WREN;
    		msg_cdbg2("WREN (0x06).\n");
    	} else {
-			flash->feature_bits = FEATURE_WRSR_EWSR;
+			chip->feature_bits = FEATURE_WRSR_EWSR;
    		msg_cdbg2("EWSR (0x50).\n");
    	}
    } else {
    	msg_cdbg2("non-volatile and the standard does not allow "
    		  "vendors to tell us whether EWSR/WREN is needed for "
    		  "status register writes - assuming EWSR.\n");
-			flash->feature_bits = FEATURE_WRSR_EWSR;
+			chip->feature_bits = FEATURE_WRSR_EWSR;
    	}

msg_cdbg2("  Write chunk size is ");
    if (tmp32 & (1 << 2)) {
    	msg_cdbg2("at least 64 B.\n");
-		flash->page_size = 64;
-		flash->write = spi_chip_write_256;
+		chip->page_size = 64;
+		chip->write = spi_chip_write_256;
    } else {
    	msg_cdbg2("1 B only.\n");
-		flash->page_size = 256;
-		flash->write = spi_chip_write_1;
+		chip->page_size = 256;
+		chip->write = spi_chip_write_1;
    }

if ((tmp32 & 0x3) == 0x1) {
@@ -212,8 +212,8 @@
    	return 1;
    }
    total_size = ((tmp32 & 0x7FFFFFFF) + 1) / 8;
-	flash->total_size = total_size / 1024;
-	msg_cdbg2("  Flash chip size is %d kB.\n", flash->total_size);
+	chip->total_size = total_size / 1024;
+	msg_cdbg2("  Flash chip size is %d kB.\n", chip->total_size);
    if (total_size > (1 << 24)) {
    	msg_cdbg("Flash chip size is bigger than what 3-Byte addressing "
    		 "can access.\n");
@@ -221,7 +221,7 @@
    }

if (opcode_4k_erase != 0xFF)
-		sfdp_add_uniform_eraser(flash, opcode_4k_erase, 4 * 1024);
+		sfdp_add_uniform_eraser(chip, opcode_4k_erase, 4 * 1024);

/* FIXME: double words 3-7 contain unused fast read information */

@@ -252,7 +252,7 @@
    	tmp8 = buf[(4 * 7) + (j * 2) + 1];
    	msg_cspew("   Erase Sector Type %d Opcode: 0x%02x\n", j + 1,
    		  tmp8);
-		sfdp_add_uniform_eraser(flash, tmp8, block_size);
+		sfdp_add_uniform_eraser(chip, tmp8, block_size);
    }

done:
@@ -381,7 +381,7 @@
    			msg_cdbg("Length of the mandatory JEDEC SFDP "
    				 "parameter table is wrong (%d B), "
    				 "skipping it.\n", len);
-			} else if (sfdp_fill_flash(flash, tbuf, len) == 0)
+			} else if (sfdp_fill_flash(flash->chip, tbuf, len) == 0)
    			ret = 1;
    	}
    	free(tbuf);
Index: flashrom-flashctx_separate_struct_flashchip/sst28sf040.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/sst28sf040.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/sst28sf040.c	(Arbeitskopie)
@@ -119,7 +119,7 @@
 int erase_chip_28sf040(struct flashctx *flash, unsigned int addr,
    	       unsigned int blocklen)
 {
-	if ((addr != 0) || (blocklen != flash->total_size * 1024)) {
+	if ((addr != 0) || (blocklen != flash->chip->total_size * 1024)) {
    	msg_cerr("%s called with incorrect arguments\n",
    		__func__);
    	return -1;
Index: flashrom-flashctx_separate_struct_flashchip/stm50flw0x0x.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/stm50flw0x0x.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/stm50flw0x0x.c	(Arbeitskopie)
@@ -54,7 +54,7 @@
    /* Check, if it's is a top/bottom-block with 4k-sectors. */
    /* TODO: What about the other types? */
    if ((offset == 0) ||
-	    (offset == (flash->model_id == ST_M50FLW080A ? 0xE0000 : 0x10000))
+	    (offset == (flash->chip->model_id == ST_M50FLW080A ? 0xE0000 : 0x10000))
        || (offset == 0xF0000)) {

// unlock each 4k-sector
@@ -85,7 +85,7 @@
 {
    int i;

-	for (i = 0; i < flash->total_size * 1024; i+= flash->page_size) {
+	for (i = 0; i < flash->chip->total_size * 1024; i+= flash->chip->page_size) {
    	if(unlock_block_stm50flw0x0x(flash, i)) {
    		msg_cerr("UNLOCK FAILED!\n");
    		return -1;
Index: flashrom-flashctx_separate_struct_flashchip/flashrom.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/flashrom.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/flashrom.c	(Arbeitskopie)
@@ -416,7 +416,7 @@

void map_flash_registers(struct flashctx *flash)
 {
-	size_t size = flash->total_size * 1024;
+	size_t size = flash->chip->total_size * 1024;
    /* Flash registers live 4 MByte below the flash. */
    /* FIXME: This is incorrect for nonstandard flashbase. */
    flash->virtual_registers = (chipaddr)programmer_map_flash_region("flash chip registers", (0xFFFFFFFF - 0x400000 - size + 1), size);
@@ -580,7 +580,7 @@
    if (!len)
    	goto out_free;

-	if (!flash->read) {
+	if (!flash->chip->read) {
    	msg_cerr("ERROR: flashrom has no read function for this flash chip.\n");
    	return 1;
    }
@@ -589,17 +589,17 @@
    	exit(1);
    }

-	if (start + len > flash->total_size * 1024) {
+	if (start + len > flash->chip->total_size * 1024) {
    	msg_gerr("Error: %s called with start 0x%x + len 0x%x >"
    		" total_size 0x%x\n", __func__, start, len,
-			flash->total_size * 1024);
+			flash->chip->total_size * 1024);
    	ret = -1;
    	goto out_free;
    }
    if (!message)
    	message = "VERIFY";

-	ret = flash->read(flash, readbuf, start, len);
+	ret = flash->chip->read(flash, readbuf, start, len);
    if (ret) {
    	msg_gerr("Verification impossible because read failed "
    		 "at 0x%x (len 0x%x)\n", start, len);
@@ -950,44 +950,49 @@
    return 1;
 }

-int probe_flash(struct registered_programmer *pgm, int startchip,
-		struct flashctx *fill_flash, int force)
+int probe_flash(struct registered_programmer *pgm, int startchip, struct flashctx *flash, int force)
 {
-	const struct flashchip *flash;
+	const struct flashchip *chip;
    unsigned long base = 0;
    char location[64];
    uint32_t size;
    enum chipbustype buses_common;
    char *tmp;

-	for (flash = flashchips + startchip; flash && flash->name; flash++) {
-		if (chip_to_probe && strcmp(flash->name, chip_to_probe) != 0)
+	for (chip = flashchips + startchip; chip && chip->name; chip++) {
+		if (chip_to_probe && strcmp(chip->name, chip_to_probe) != 0)
    		continue;
-		buses_common = pgm->buses_supported & flash->bustype;
+		buses_common = pgm->buses_supported & chip->bustype;
    	if (!buses_common)
    		continue;
-		msg_gdbg("Probing for %s %s, %d kB: ",
-			     flash->vendor, flash->name, flash->total_size);
-		if (!flash->probe && !force) {
-			msg_gdbg("failed! flashrom has no probe function for "
-				 "this flash chip.\n");
+		msg_gdbg("Probing for %s %s, %d kB: ", chip->vendor, chip->name, chip->total_size);
+		if (!chip->probe && !force) {
+			msg_gdbg("failed! flashrom has no probe function for this flash chip.\n");
    		continue;
    	}

-		size = flash->total_size * 1024;
+		size = chip->total_size * 1024;
    	check_max_decode(buses_common, size);

/* Start filling in the dynamic data. */
-		memcpy(fill_flash, flash, sizeof(struct flashchip));
-		fill_flash->pgm = pgm;
+		flash->chip = calloc(1, sizeof(struct flashchip));
+		if (!flash->chip) {
+			msg_gerr("Out of memory!\n");
+			exit(1);
+		}
+		memcpy(flash->chip, chip, sizeof(struct flashchip));
+		flash->pgm = pgm;

base = flashbase ? flashbase : (0xffffffff - size + 1);
-		fill_flash->virtual_memory = (chipaddr)programmer_map_flash_region("flash chip", base, size);
+		flash->virtual_memory = (chipaddr)programmer_map_flash_region("flash chip", base, size);

+		/* We handle a forced match like a real match, we just avoid probing. Note that probe_flash()
+		 * is only called with force=1 after normal probing failed.
+		 */
    	if (force)
    		break;

-		if (fill_flash->probe(fill_flash) != 1)
+		if (flash->chip->probe(flash) != 1)
    		goto notfound;

/* If this is the first chip found, accept it.
@@ -997,11 +1002,11 @@
    	 * one for this programmer interface and thus no other chip has
    	 * been found on this interface.
    	 */
-		if (startchip == 0 && fill_flash->model_id == SFDP_DEVICE_ID) {
+		if (startchip == 0 && flash->chip->model_id == SFDP_DEVICE_ID) {
    		msg_cinfo("===\n"
    			  "SFDP has autodetected a flash chip which is "
    			  "not natively supported by flashrom yet.\n");
-			if (count_usable_erasers(fill_flash) == 0)
+			if (count_usable_erasers(flash) == 0)
    			msg_cinfo("The standard operations read and "
    				  "verify should work, but to support "
    				  "erase, write and all other "
@@ -1020,16 +1025,21 @@
    			  "===\n");
    	}

-		if (startchip == 0 ||
-		    ((fill_flash->model_id != GENERIC_DEVICE_ID) &&
-		     (fill_flash->model_id != SFDP_DEVICE_ID)))
+		/* First flash chip detected on this bus. */
+		if (startchip == 0)
    		break;
-
+		/* Not the first flash chip detected on this bus, but not a generic match either. */
+		if ((flash->chip->model_id != GENERIC_DEVICE_ID) && (flash->chip->model_id != SFDP_DEVICE_ID))
+			break;
+		/* Not the first flash chip detected on this bus, and it's just a generic match. Ignore it. */
 notfound:
-		programmer_unmap_flash_region((void *)fill_flash->virtual_memory, size);
+		programmer_unmap_flash_region((void *)flash->virtual_memory, size);
+		flash->virtual_memory = (chipaddr)NULL;
+		free(flash->chip);
+		flash->chip = NULL;
    }

-	if (!flash || !flash->name)
+	if (!flash->chip)
    	return -1;

#if CONFIG_INTERNAL == 1
@@ -1039,27 +1049,26 @@
 #endif
    	snprintf(location, sizeof(location), "on %s", programmer_table[programmer].name);

-	tmp = flashbuses_to_text(flash->bustype);
-	msg_cinfo("%s %s flash chip "%s" (%d kB, %s) %s.\n",
-		  force ? "Assuming" : "Found", fill_flash->vendor,
-		  fill_flash->name, fill_flash->total_size, tmp, location);
+	tmp = flashbuses_to_text(flash->chip->bustype);
+	msg_cinfo("%s %s flash chip "%s" (%d kB, %s) %s.\n", force ? "Assuming" : "Found",
+		  flash->chip->vendor, flash->chip->name, flash->chip->total_size, tmp, location);
    free(tmp);

/* Flash registers will not be mapped if the chip was forced. Lock info
     * may be stored in registers, so avoid lock info printing.
     */
    if (!force)
-		if (fill_flash->printlock)
-			fill_flash->printlock(fill_flash);
+		if (flash->chip->printlock)
+			flash->chip->printlock(flash);

/* Return position of matching chip. */
-	return flash - flashchips;
+	return chip - flashchips;
 }

int verify_flash(struct flashctx *flash, uint8_t *buf)
 {
    int ret;
-	unsigned int total_size = flash->total_size * 1024;
+	unsigned int total_size = flash->chip->total_size * 1024;

msg_cinfo("Verifying flash... ");

@@ -1132,7 +1141,7 @@

int read_flash_to_file(struct flashctx *flash, const char *filename)
 {
-	unsigned long size = flash->total_size * 1024;
+	unsigned long size = flash->chip->total_size * 1024;
    unsigned char *buf = calloc(size, sizeof(char));
    int ret = 0;

@@ -1142,12 +1151,12 @@
    	msg_cinfo("FAILED.\n");
    	return 1;
    }
-	if (!flash->read) {
+	if (!flash->chip->read) {
    	msg_cerr("No read function available for this flash chip.\n");
    	ret = 1;
    	goto out_free;
    }
-	if (flash->read(flash, buf, 0, size)) {
+	if (flash->chip->read(flash, buf, 0, size)) {
    	msg_cerr("Read operation failed!\n");
    	ret = 1;
    	goto out_free;
@@ -1164,14 +1173,14 @@
  * walk_eraseregions().
  * Even if an error is found, the function will keep going and check the rest.
  */
-static int selfcheck_eraseblocks(const struct flashchip *flash)
+static int selfcheck_eraseblocks(const struct flashchip *chip)
 {
    int i, j, k;
    int ret = 0;

for (k = 0; k < NUM_ERASEFUNCTIONS; k++) {
    	unsigned int done = 0;
-		struct block_eraser eraser = flash->block_erasers[k];
+		struct block_eraser eraser = chip->block_erasers[k];

for (i = 0; i < NUM_ERASEREGIONS; i++) {
    		/* Blocks with zero size are bugs in flashchips.c. */
@@ -1180,7 +1189,7 @@
    			msg_gerr("ERROR: Flash chip %s erase function "
    				"%i region %i has size 0. Please report"
    				" a bug at [e-mail]\n",
-					flash->name, k, i);
+					chip->name, k, i);
    			ret = 1;
    		}
    		/* Blocks with zero count are bugs in flashchips.c. */
@@ -1189,7 +1198,7 @@
    			msg_gerr("ERROR: Flash chip %s erase function "
    				"%i region %i has count 0. Please report"
    				" a bug at [e-mail]\n",
-					flash->name, k, i);
+					chip->name, k, i);
    			ret = 1;
    		}
    		done += eraser.eraseblocks[i].count *
@@ -1201,12 +1210,12 @@
    			  "non-empty erase function. Not an error.\n");
    	if (!done)
    		continue;
-		if (done != flash->total_size * 1024) {
+		if (done != chip->total_size * 1024) {
    		msg_gerr("ERROR: Flash chip %s erase function %i "
    			"region walking resulted in 0x%06x bytes total,"
    			" expected 0x%06x bytes. Please report a bug at"
-				" [e-mail]\n", flash->name, k,
-				done, flash->total_size * 1024);
+				" [e-mail]\n", chip->name, k,
+				done, chip->total_size * 1024);
    		ret = 1;
    	}
    	if (!eraser.block_erase)
@@ -1217,11 +1226,11 @@
    	 */
    	for (j = k + 1; j < NUM_ERASEFUNCTIONS; j++) {
    		if (eraser.block_erase ==
-			    flash->block_erasers[j].block_erase) {
+			    chip->block_erasers[j].block_erase) {
    			msg_gerr("ERROR: Flash chip %s erase function "
    				"%i and %i are identical. Please report"
    				" a bug at [e-mail]\n",
-					flash->name, k, j);
+					chip->name, k, j);
    			ret = 1;
    		}
    	}
@@ -1268,7 +1277,7 @@
    	if (!writecount++)
    		msg_cdbg("W");
    	/* Needs the partial write function signature. */
-		ret = flash->write(flash, newcontents + starthere,
+		ret = flash->chip->write(flash, newcontents + starthere,
    			   start + starthere, lenhere);
    	if (ret)
    		return ret;
@@ -1295,7 +1304,7 @@
    int i, j;
    unsigned int start = 0;
    unsigned int len;
-	struct block_eraser eraser = flash->block_erasers[erasefunction];
+	struct block_eraser eraser = flash->chip->block_erasers[erasefunction];

for (i = 0; i < NUM_ERASEREGIONS; i++) {
    	/* count==0 for all automatically initialized array
@@ -1321,7 +1330,7 @@

static int check_block_eraser(const struct flashctx *flash, int k, int log)
 {
-	struct block_eraser eraser = flash->block_erasers[k];
+	struct block_eraser eraser = flash->chip->block_erasers[k];

if (!eraser.block_erase && !eraser.eraseblocks[0].count) {
    	if (log)
@@ -1340,6 +1349,7 @@
    			 "eraseblock layout is not defined. ");
    	return 1;
    }
+	// TODO: Once erase functions are annotated with allowed buses, check that as well.
    return 0;
 }

@@ -1348,7 +1358,7 @@
 {
    int k, ret = 1;
    uint8_t *curcontents;
-	unsigned long size = flash->total_size * 1024;
+	unsigned long size = flash->chip->total_size * 1024;
    unsigned int usable_erasefunctions = count_usable_erasers(flash);

msg_cinfo("Erasing and writing flash chip... ");
@@ -1386,7 +1396,7 @@
    	 * in non-verbose mode.
    	 */
    	msg_cinfo("Reading current flash chip contents... ");
-		if (flash->read(flash, curcontents, 0, size)) {
+		if (flash->chip->read(flash, curcontents, 0, size)) {
    		/* Now we are truly screwed. Read failed as well. */
    		msg_cerr("Can't read anymore! Aborting.\n");
    		/* We have no idea about the flash chip contents, so
@@ -1575,7 +1585,7 @@
 int selfcheck(void)
 {
    int ret = 0;
-	const struct flashchip *flash;
+	const struct flashchip *chip;

/* Safety check. Instead of aborting after the first error, check
     * if more errors exist.
@@ -1593,16 +1603,8 @@
    	msg_gerr("Flashchips table miscompilation!\n");
    	ret = 1;
    }
-	/* Check that virtual_memory in struct flashctx is placed directly
-	 * after the members copied from struct flashchip.
-	 */
-	if (sizeof(struct flashchip) !=
-	    offsetof(struct flashctx, virtual_memory)) {
-		msg_gerr("struct flashctx broken!\n");
-		ret = 1;
-	}
-	for (flash = flashchips; flash && flash->name; flash++)
-		if (selfcheck_eraseblocks(flash))
+	for (chip = flashchips; chip && chip->name; chip++)
+		if (selfcheck_eraseblocks(chip))
    		ret = 1;

#if CONFIG_INTERNAL == 1
@@ -1626,41 +1628,41 @@
    return ret;
 }

-void check_chip_supported(const struct flashctx *flash)
+void check_chip_supported(const struct flashchip *chip)
 {
-	if (flash->feature_bits & FEATURE_OTP) {
+	if (chip->feature_bits & FEATURE_OTP) {
    	msg_cdbg("This chip may contain one-time programmable memory. "
    		 "flashrom cannot read\nand may never be able to write "
    		 "it, hence it may not be able to completely\n"
    		 "clone the contents of this chip (see man page for "
    		 "details).\n");
    }
-	if (TEST_OK_MASK != (flash->tested & TEST_OK_MASK)) {
+	if (TEST_OK_MASK != (chip->tested & TEST_OK_MASK)) {
    	msg_cinfo("===\n");
-		if (flash->tested & TEST_BAD_MASK) {
+		if (chip->tested & TEST_BAD_MASK) {
    		msg_cinfo("This flash part has status NOT WORKING for operations:");
-			if (flash->tested & TEST_BAD_PROBE)
+			if (chip->tested & TEST_BAD_PROBE)
    			msg_cinfo(" PROBE");
-			if (flash->tested & TEST_BAD_READ)
+			if (chip->tested & TEST_BAD_READ)
    			msg_cinfo(" READ");
-			if (flash->tested & TEST_BAD_ERASE)
+			if (chip->tested & TEST_BAD_ERASE)
    			msg_cinfo(" ERASE");
-			if (flash->tested & TEST_BAD_WRITE)
+			if (chip->tested & TEST_BAD_WRITE)
    			msg_cinfo(" WRITE");
    		msg_cinfo("\n");
    	}
-		if ((!(flash->tested & TEST_BAD_PROBE) && !(flash->tested & TEST_OK_PROBE)) ||
-		    (!(flash->tested & TEST_BAD_READ) && !(flash->tested & TEST_OK_READ)) ||
-		    (!(flash->tested & TEST_BAD_ERASE) && !(flash->tested & TEST_OK_ERASE)) ||
-		    (!(flash->tested & TEST_BAD_WRITE) && !(flash->tested & TEST_OK_WRITE))) {
+		if ((!(chip->tested & TEST_BAD_PROBE) && !(chip->tested & TEST_OK_PROBE)) ||
+		    (!(chip->tested & TEST_BAD_READ) && !(chip->tested & TEST_OK_READ)) ||
+		    (!(chip->tested & TEST_BAD_ERASE) && !(chip->tested & TEST_OK_ERASE)) ||
+		    (!(chip->tested & TEST_BAD_WRITE) && !(chip->tested & TEST_OK_WRITE))) {
    		msg_cinfo("This flash part has status UNTESTED for operations:");
-			if (!(flash->tested & TEST_BAD_PROBE) && !(flash->tested & TEST_OK_PROBE))
+			if (!(chip->tested & TEST_BAD_PROBE) && !(chip->tested & TEST_OK_PROBE))
    			msg_cinfo(" PROBE");
-			if (!(flash->tested & TEST_BAD_READ) && !(flash->tested & TEST_OK_READ))
+			if (!(chip->tested & TEST_BAD_READ) && !(chip->tested & TEST_OK_READ))
    			msg_cinfo(" READ");
-			if (!(flash->tested & TEST_BAD_ERASE) && !(flash->tested & TEST_OK_ERASE))
+			if (!(chip->tested & TEST_BAD_ERASE) && !(chip->tested & TEST_OK_ERASE))
    			msg_cinfo(" ERASE");
-			if (!(flash->tested & TEST_BAD_WRITE) && !(flash->tested & TEST_OK_WRITE))
+			if (!(chip->tested & TEST_BAD_WRITE) && !(chip->tested & TEST_OK_WRITE))
    			msg_cinfo(" WRITE");
    		msg_cinfo("\n");
    	}
@@ -1685,9 +1687,11 @@
 /* FIXME: This function signature needs to be improved once doit() has a better
  * function signature.
  */
-int chip_safety_check(struct flashctx *flash, int force, int read_it,
-		      int write_it, int erase_it, int verify_it)
+int chip_safety_check(const struct flashctx *flash, int force, int read_it, int write_it, int erase_it,
+		      int verify_it)
 {
+	const struct flashchip *chip = flash->chip;
+
    if (!programmer_may_write && (write_it || erase_it)) {
    	msg_perr("Write/erase is not working yet on your programmer in "
    		 "its current configuration.\n");
@@ -1701,13 +1705,13 @@

if (read_it || erase_it || write_it || verify_it) {
    	/* Everything needs read. */
-		if (flash->tested & TEST_BAD_READ) {
+		if (chip->tested & TEST_BAD_READ) {
    		msg_cerr("Read is not working on this chip. ");
    		if (!force)
    			return 1;
    		msg_cerr("Continuing anyway.\n");
    	}
-		if (!flash->read) {
+		if (!chip->read) {
    		msg_cerr("flashrom has no read function for this "
    			 "flash chip.\n");
    		return 1;
@@ -1715,7 +1719,7 @@
    }
    if (erase_it || write_it) {
    	/* Write needs erase. */
-		if (flash->tested & TEST_BAD_ERASE) {
+		if (chip->tested & TEST_BAD_ERASE) {
    		msg_cerr("Erase is not working on this chip. ");
    		if (!force)
    			return 1;
@@ -1728,13 +1732,13 @@
    	}
    }
    if (write_it) {
-		if (flash->tested & TEST_BAD_WRITE) {
+		if (chip->tested & TEST_BAD_WRITE) {
    		msg_cerr("Write is not working on this chip. ");
    		if (!force)
    			return 1;
    		msg_cerr("Continuing anyway.\n");
    	}
-		if (!flash->write) {
+		if (!chip->write) {
    		msg_cerr("flashrom has no write function for this "
    			 "flash chip.\n");
    		return 1;
@@ -1753,7 +1757,7 @@
    uint8_t *oldcontents;
    uint8_t *newcontents;
    int ret = 0;
-	unsigned long size = flash->total_size * 1024;
+	unsigned long size = flash->chip->total_size * 1024;

if (chip_safety_check(flash, force, read_it, write_it, erase_it, verify_it)) {
    	msg_cerr("Aborting.\n");
@@ -1764,8 +1768,8 @@
    /* Given the existence of read locks, we want to unlock for read,
     * erase and write.
     */
-	if (flash->unlock)
-		flash->unlock(flash);
+	if (flash->chip->unlock)
+		flash->chip->unlock(flash);

if (read_it) {
    	ret = read_flash_to_file(flash, filename);
@@ -1833,7 +1837,7 @@
     * takes time as well.
     */
    msg_cinfo("Reading old flash chip contents... ");
-	if (flash->read(flash, oldcontents, 0, size)) {
+	if (flash->chip->read(flash, oldcontents, 0, size)) {
    	ret = 1;
    	msg_cinfo("FAILED.\n");
    	goto out;
@@ -1850,7 +1854,7 @@
    	if (erase_and_write_flash(flash, oldcontents, newcontents)) {
    		msg_cerr("Uh oh. Erase/write failed. Checking if "
    			 "anything changed.\n");
-			if (!flash->read(flash, newcontents, 0, size)) {
+			if (!flash->chip->read(flash, newcontents, 0, size)) {
    			if (!memcmp(oldcontents, newcontents, size)) {
    				msg_cinfo("Good. It seems nothing was "
    					  "changed.\n");
Index: flashrom-flashctx_separate_struct_flashchip/programmer.h
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/programmer.h	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/programmer.h	(Arbeitskopie)
@@ -473,7 +473,7 @@
 extern struct decode_sizes max_rom_decode;
 extern int programmer_may_write;
 extern unsigned long flashbase;
-void check_chip_supported(const struct flashctx *flash);
+void check_chip_supported(const struct flashchip *chip);
 int check_max_decode(enum chipbustype buses, uint32_t size);
 char *extract_programmer_param(const char *param_name);

Index: flashrom-flashctx_separate_struct_flashchip/m29f400bt.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/m29f400bt.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/m29f400bt.c	(Arbeitskopie)
@@ -81,7 +81,7 @@

msg_cdbg("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1, id2);

-	if (id1 == flash->manufacture_id && id2 == flash->model_id)
+	if (id1 == flash->chip->manufacture_id && id2 == flash->chip->model_id)
    	return 1;

return 0;
@@ -130,7 +130,7 @@
 int block_erase_chip_m29f400bt(struct flashctx *flash, unsigned int address,
    		       unsigned int blocklen)
 {
-	if ((address != 0) || (blocklen != flash->total_size * 1024)) {
+	if ((address != 0) || (blocklen != flash->chip->total_size * 1024)) {
    	msg_cerr("%s called with incorrect arguments\n",
    		__func__);
    	return -1;
Index: flashrom-flashctx_separate_struct_flashchip/print.c
===================================================================
--- flashrom-flashctx_separate_struct_flashchip/print.c	(Revision 1577)
+++ flashrom-flashctx_separate_struct_flashchip/print.c	(Arbeitskopie)
@@ -67,22 +67,22 @@
    int maxvendorlen = strlen("Vendor") + 1;
    int maxchiplen = strlen("Device") + 1;
    int maxtypelen = strlen("Type") + 1;
-	const struct flashchip *f;
+	const struct flashchip *chip;
    char *s;
    char *tmpven, *tmpdev;
    int tmpvenlen, tmpdevlen, curvenlen, curdevlen;

/* calculate maximum column widths and by iterating over all chips */
-	for (f = flashchips; f->name != NULL; f++) {
+	for (chip = flashchips; chip->name != NULL; chip++) {
    	/* Ignore generic entries. */
-		if (!strncmp(f->vendor, "Unknown", 7) ||
-		    !strncmp(f->vendor, "Programmer", 10) ||
-		    !strncmp(f->name, "unknown", 7))
+		if (!strncmp(chip->vendor, "Unknown", 7) ||
+		    !strncmp(chip->vendor, "Programmer", 10) ||
+		    !strncmp(chip->name, "unknown", 7))
    		continue;
    	chipcount++;

/* Find maximum vendor length (respecting line splitting). */
-		tmpven = (char *)f->vendor;
+		tmpven = (char *)chip->vendor;
    	do {
    		/* and take minimum token lengths into account */
    		tmpvenlen = 0;
@@ -100,7 +100,7 @@
    	} while (1);

/* same for device name */
-		tmpdev = (char *)f->name;
+		tmpdev = (char *)chip->name;
    	do {
    		tmpdevlen = 0;
    		do {
@@ -115,7 +115,7 @@
    			break;
    	} while (1);

-		s = flashbuses_to_text(f->bustype);
+		s = flashbuses_to_text(chip->bustype);
    	maxtypelen = max(maxtypelen, strlen(s));
    	free(s);
    }
@@ -162,11 +162,11 @@
    msg_ginfo("\n\n");
    msg_ginfo("(P = PROBE, R = READ, E = ERASE, W = WRITE)\n\n");

-	for (f = flashchips; f->name != NULL; f++) {
+	for (chip = flashchips; chip->name != NULL; chip++) {
    	/* Don't print generic entries. */
-		if (!strncmp(f->vendor, "Unknown", 7) ||
-		    !strncmp(f->vendor, "Programmer", 10) ||
-		    !strncmp(f->name, "unknown", 7))
+		if (!strncmp(chip->vendor, "Unknown", 7) ||
+		    !strncmp(chip->vendor, "Programmer", 10) ||
+		    !strncmp(chip->name, "unknown", 7))
    		continue;

/* support for multiline vendor names:
@@ -179,12 +179,12 @@
    	 * - after all other values are printed print the surplus tokens
    	 *   on fresh lines
    	 */
-		tmpven = malloc(strlen(f->vendor) + 1);
+		tmpven = malloc(strlen(chip->vendor) + 1);
    	if (tmpven == NULL) {
    		msg_gerr("Out of memory!\n");
    		exit(1);
    	}
-		strcpy(tmpven, f->vendor);
+		strcpy(tmpven, chip->vendor);

tmpven = strtok(tmpven, delim);
    	msg_ginfo("%s", tmpven);
@@ -203,12 +203,12 @@
    		msg_ginfo(" ");

/* support for multiline device names as above */
-		tmpdev = malloc(strlen(f->name) + 1);
+		tmpdev = malloc(strlen(chip->name) + 1);
    	if (tmpdev == NULL) {
    		msg_gerr("Out of memory!\n");
    		exit(1);
    	}
-		strcpy(tmpdev, f->name);
+		strcpy(tmpdev, chip->name);

tmpdev = strtok(tmpdev, delim);
    	msg_ginfo("%s", tmpdev);
@@ -226,60 +226,60 @@
    	for (i = curdevlen; i < maxchiplen; i++)
    		msg_ginfo(" ");

-		if ((f->tested & TEST_OK_PROBE))
+		if ((chip->tested & TEST_OK_PROBE))
    		msg_ginfo("P");
    	else
    		msg_ginfo(" ");
-		if ((f->tested & TEST_OK_READ))
+		if ((chip->tested & TEST_OK_READ))
    		msg_ginfo("R");
    	else
    		msg_ginfo(" ");
-		if ((f->tested & TEST_OK_ERASE))
+		if ((chip->tested & TEST_OK_ERASE))
    		msg_ginfo("E");
    	else
    		msg_ginfo(" ");
-		if ((f->tested & TEST_OK_WRITE))
+		if ((chip->tested & TEST_OK_WRITE))
    		msg_ginfo("W");
    	else
    		msg_ginfo(" ");
    	for (i = 0; i < border; i++)
    		msg_ginfo(" ");

-		if ((f->tested & TEST_BAD_PROBE))
+		if ((chip->tested & TEST_BAD_PROBE))
    		msg_ginfo("P");
    	else
    		msg_ginfo(" ");
-		if ((f->tested & TEST_BAD_READ))
+		if ((chip->tested & TEST_BAD_READ))
    		msg_ginfo("R");
    	else
    		msg_ginfo(" ");
-		if ((f->tested & TEST_BAD_ERASE))
+		if ((chip->tested & TEST_BAD_ERASE))
    		msg_ginfo("E");
    	else
    		msg_ginfo(" ");
-		if ((f->tested & TEST_BAD_WRITE))
+		if ((chip->tested & TEST_BAD_WRITE))
    		msg_ginfo("W");
    	else
    		msg_ginfo(" ");
    	for (i = 0; i < border + 1; i++)
    		msg_ginfo(" ");

-		msg_ginfo("%5d", f->total_size);
+		msg_ginfo("%5d", chip->total_size);
    	for (i = 0; i < border; i++)
    		msg_ginfo(" ");

-		s = flashbuses_to_text(f->bustype);
+		s = flashbuses_to_text(chip->bustype);
    	msg_ginfo("%s", s);
    	for (i = strlen(s); i < maxtypelen; i++)
    		msg_ginfo(" ");
    	free(s);

-		if (f->voltage.min == 0 && f->voltage.max == 0)
+		if (chip->voltage.min == 0 && chip->voltage.max == 0)
    		msg_gdbg("no info");
    	else
    		msg_gdbg("%0.02f;%0.02f",
-				 f->voltage.min/(double)1000,
-				 f->voltage.max/(double)1000);
+				 chip->voltage.min/(double)1000,
+				 chip->voltage.max/(double)1000);

/* print surplus vendor and device name tokens */
    	while (tmpven != NULL || tmpdev != NULL) {

[PATCH] Make struct flashchip a field in struct flashctx instead of a complete copy.

[Carl-Daniel Hailfinger]

Am 25.08.2012 00:14 schrieb Carl-Daniel Hailfinger:
> New version.
>
> All the driver conversion work and cleanup has been done by Stefan.
> flashrom.c and cli_classic.c are a joint work of Stefan and Carl-Daniel.
>
> It works, has survived various build tests with gcc and clang, and this
> is the fourth(?) iteration, hopefully satisfying everyone.
>
> Signed-off-by: Carl-Daniel Hailfinger 
> Signed-off-by: Stefan Tauner

Acked-by: Stefan Tauner 
(given on IRC)

Acked-by: Carl-Daniel Hailfinger

and committed in r1579.

Regards,
Carl-Daniel