[coreboot] flashrom: Probe for up to 3 flash chips. [was: Fix ambiguity, if a board is equipped with more than one chip]

Peter Stuge peter at stuge.se
Mon May 5 18:29:38 CEST 2008


On Mon, May 05, 2008 at 06:00:13PM +0200, Carl-Daniel Hailfinger wrote:
> > flashrom assumes that flash chips are top-aligned at 4GB so there
> > will not be very many flash chips actually connected in a way that
> > flashrom understands today.
> 
> Yes, but it would be more future-proof(TM).

That future will require many other changes, so I prefer keeping it
simple for now.


> >> What happens if there are multiple flash chips of the same
> >> technology?
> >
> > If same name, then same size and at same address => larger problem.
> 
> Same name, same size, different address... unsupported by flashrom
> right now, but we could change that.

When we do, let's revisit the "unlimited" flash chip count.


> > -	if ((flash = probe_flash(flashchips)) == NULL) {
> > +	for (i = 0; i < ARRAY_SIZE(flashes); i++) {
> > +		flashes[i] = probe_flash(i ? flashes[i - 1] + 1 : flashchips);
> > +		if (!flashes[i])
> > +			for (i++; i < ARRAY_SIZE(flashes); i++)
> > +				flashes[i] = NULL;
> >   
> 
> This for loop could be replaced by an early memset, making the code clearer.

Sure. Matter of taste.


> >  	printf("Flash part is %s (%d KB).\n", flash->name, flash->total_size);
> 
> Move that statement into the loop above, please.

No need since the probe() function always prints the name of the
found flash chip.


> Otherwise, this is
> Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Thanks, but I'm still waiting for Claus to test that it works on his
hardware.


On Mon, May 05, 2008 at 06:06:53PM +0200, Carl-Daniel Hailfinger wrote:
> >> +	for (i = 0; i < ARRAY_SIZE(flashes); i++) {
> >> +		flashes[i] = probe_flash(i ? flashes[i - 1] + 1 : flashchips);
> >> +		if (!flashes[i])
> >>     
> 
> Missing break. This loop runs ARRAY_SIZE(flashchips) times even if the
> first probe failed.
> 
> >> +			for (i++; i < ARRAY_SIZE(flashes); i++)
> >> +				flashes[i] = NULL;

The same counter is used to clear remaining entries so that the outer
loop also is finished after the first failed probe.


//Peter




More information about the coreboot mailing list