[coreboot] [RFC] Adding struct flashchip * to all chip functions
Urja Rannikko
urjaman at gmail.com
Tue Jun 9 18:32:43 CEST 2009
>
> 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>
More information about the coreboot
mailing list