[coreboot] [RFC] Adding struct flashchip * to all chip functions

[Urja Rannikko]

>
> I think we have too many words for too many things.
> "bus protocol" in my mail above refers to the bus tye/protocol between
> flasher and flash chip.

I know - i was meaning that the serial (as in RS-232) protocol i have
doesnt currently have a way to say to the flasher/programmer which
bustype (parallel/fwh/lpc/spi) it is to use - in case somebody makes a
programmer that has sockets/support for more than one of the 4
bustypes.

>>> struct flashchip * as parameter to chip_read* and chip_write*
>>> [...]
>>> using global variables for current flash chip [...]
>>>
>>
>> I think that having a simple global current_flashchip pointer would
>> be simpler in many ways:
>> - it would only need a single assign to operation to the loop at
>> probe_flash to support it
>>
>
> IMHO the loop would get less readable because it tries to handle
> detection of multiple chips. You'd have to keep an array of found chips
> which would be local, whereas the currently probed chip would be global.
> After probing, the global variable would have to be overwritten by a
> local variable... Not exactly my idea of fun.
>

This support is exactly 3 added and 1 modified line - see my patch here.

> I doubt the binary is going to be smaller, but this suggestion does
> warrant an investigation. I'll try to cook up a patch which adds struct
> flashchip everywhere and measure the size differences.
>

What i was saying that the support in my patch will be smaller than
the way of adding
parameters to each call (which nobody atm uses) - i think (testing appreciated).

-- the patch (also attached) --
Add a global current_flashchip pointer.
Signed-off-by: Urja Rannikko <urjaman at gmail.com>

---
Index: flash.h
===================================================================
--- flash.h	(revision 580)
+++ flash.h	(working copy)
@@ -713,6 +713,7 @@
 void map_flash_registers(struct flashchip *flash);
 int read_memmapped(struct flashchip *flash, uint8_t *buf);
 extern char *pcidev_bdf;
+struct flashchip *current_flashchip;

 /* layout.c */
 int show_id(uint8_t *bios, int size, int force);
Index: flashrom.c
===================================================================
--- flashrom.c	(revision 580)
+++ flashrom.c	(working copy)
@@ -33,6 +33,7 @@
 int exclude_start_page, exclude_end_page;
 int verbose = 0;
 int programmer = PROGRAMMER_INTERNAL;
+struct flashchip* current_flashchip;

 const struct programmer_entry programmer_table[] = {
 	{
@@ -247,6 +248,7 @@
 	char *tmp;

 	for (flash = first_flash; flash && flash->name; flash++) {
+		current_flashchip = flash;
 		if (chip_to_probe && strcmp(flash->name, chip_to_probe) != 0)
 			continue;
 		printf_debug("Probing for %s %s, %d KB: ",
@@ -707,7 +709,7 @@
 		exit(1);
 	}

-	flash = flashes[0];
+	current_flashchip = flash = flashes[0];

 	if (TEST_OK_MASK != (flash->tested & TEST_OK_MASK)) {
 		printf("===\n");


-- 
urjaman
-------------- next part --------------
A non-text attachment was scrubbed...
Name: current_flashchip.diff
Type: application/octet-stream
Size: 1194 bytes
Desc: not available
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20090609/f3faecd0/attachment.obj>