Am 15.03.2011 16:29 schrieb Stefan Tauner:
Signed-off-by: Stefan Taunerstefan.tauner@student.tuwien.ac.at
Thanks for your patch!
--- a/flash.h +++ b/flash.h @@ -107,7 +107,9 @@ struct flashchip { uint32_t manufacture_id; uint32_t model_id;
- /* Total chip size in kilobytes */
You can't know that, but we have a pending patch which changes this to bytes. OTOH, until that patch is applied, your patch makes sense.
int total_size;
- /* Page chip size in bytes */
page_size will be renamed to max_write_size or something like that. Until then, a comment like yours makes sense. Please change it to "Chip page size...".
int page_size; int feature_bits;
@@ -125,7 +127,10 @@ struct flashchip { /* * Erase blocks and associated erase function. Any chip erase function * is stored as chip-sized virtual block together with said function.
*/
* The first one that fits will be chosen, there is currently no way to
* influence that behaviour. For testing just comment out the others elements or
s/others/other/ 80 column limit please.
* set the function pointer to NULL.
- */
Missing blank before *.
struct block_eraser { struct eraseblock{ unsigned int size; /* Eraseblock size */ --- a/spi25.c +++ b/spi25.c @@ -314,13 +314,14 @@ uint8_t spi_read_status_register(void) /* Prettyprint the status register. Common definitions. */ static void spi_prettyprint_status_register_welwip(uint8_t status) {
- msg_cdbg("Chip status register: Write Enable Latch (WEL) is "
- msg_cdbg("Chip status register: Write Enable Latch (WEL/WEN) is " "%sset\n", (status& (1<< 1)) ? "" : "not ");
- msg_cdbg("Chip status register: Write In Progress (WIP/BUSY) is "
- msg_cdbg("Chip status register: Write In Progress (WIP/BUSY/nRDY) is " "%sset\n", (status& (1<< 0)) ? "" : "not "); }
-/* Prettyprint the status register. Common definitions. */ +/* Prettyprint the status register. Common definitions.
- TODO: parameterize number of protected blocks. */
Some chips do not use _common at all, and it may be a good idea to create a separate function for parameter printing. Hmmm... can you drop this change for now?
That said, I should repost my detailed locking patch which allows printing of locking areas as well.
static void spi_prettyprint_status_register_common(uint8_t status) { msg_cdbg("Chip status register: Bit 5 / Block Protect 3 (BP3) is "
Rest looks OK and will be committed immediately once I have a fixed up version.
Regards, Carl-Daniel
On Thu, 31 Mar 2011 08:27:20 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Am 15.03.2011 16:29 schrieb Stefan Tauner:
--- a/flash.h +++ b/flash.h @@ -107,7 +107,9 @@ struct flashchip { uint32_t manufacture_id; uint32_t model_id;
- /* Total chip size in kilobytes */
You can't know that, but we have a pending patch which changes this to bytes. OTOH, until that patch is applied, your patch makes sense.
i hope it is annotated in that patch! helps newbies when they port their first chip ;)
<formatting stuff>
done. done. and done.
struct block_eraser { struct eraseblock{ unsigned int size; /* Eraseblock size */ --- a/spi25.c +++ b/spi25.c @@ -314,13 +314,14 @@ uint8_t spi_read_status_register(void) /* Prettyprint the status register. Common definitions. */ static void spi_prettyprint_status_register_welwip(uint8_t status) {
- msg_cdbg("Chip status register: Write Enable Latch (WEL)
is "
- msg_cdbg("Chip status register: Write Enable Latch
(WEL/WEN) is " "%sset\n", (status& (1<< 1)) ? "" : "not ");
- msg_cdbg("Chip status register: Write In Progress
(WIP/BUSY) is "
- msg_cdbg("Chip status register: Write In Progress
(WIP/BUSY/nRDY) is " "%sset\n", (status& (1<< 0)) ? "" : "not "); }
-/* Prettyprint the status register. Common definitions. */ +/* Prettyprint the status register. Common definitions.
- TODO: parameterize number of protected blocks. */
Some chips do not use _common at all, and it may be a good idea to create a separate function for parameter printing. Hmmm... can you drop this change for now?
That said, I should repost my detailed locking patch which allows printing of locking areas as well.
when i wrote that i was not aware of your patch and what i have done then was the least intrusive patch i could come up with: adding another pretty print function (in patch 4/8). refactoring the whole thing somehow sprang instantly to my mind though and that comment remained there to remind me :)
i have reverted all spi25.c changes in 1/8 now.
static void spi_prettyprint_status_register_common(uint8_t status) { msg_cdbg("Chip status register: Bit 5 / Block Protect 3 (BP3) is "
Rest looks OK and will be committed immediately once I have a fixed up version.
next mail.
Signed-off-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at --- chipset_enable.c | 4 ++-- drkaiser.c | 2 +- flash.h | 5 +++++ flashchips.h | 4 ++-- flashrom.c | 4 ++-- print.c | 2 +- print_wiki.c | 2 +- wbsio_spi.c | 2 +- 8 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/chipset_enable.c b/chipset_enable.c index 0896bce..838d321 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -558,8 +558,8 @@ static int enable_flash_cs5530(struct pci_dev *dev, const char *name) #define CS5530_ENABLE_SA20 (1 << 6)
buses_supported = CHIP_BUSTYPE_PARALLEL; - /* Decode 0x000E0000-0x000FFFFF (128 KB), not just 64 KB, and - * decode 0xFF000000-0xFFFFFFFF (16 MB), not just 256 KB. + /* Decode 0x000E0000-0x000FFFFF (128 kB), not just 64 kB, and + * decode 0xFF000000-0xFFFFFFFF (16 MB), not just 256 kB. * FIXME: Should we really touch the low mapping below 1 MB? Flashrom * ignores that region completely. * Make the configured ROM areas writable. diff --git a/drkaiser.c b/drkaiser.c index 207c738..c2938dd 100644 --- a/drkaiser.c +++ b/drkaiser.c @@ -49,7 +49,7 @@ int drkaiser_init(void) rpci_write_word(pcidev_dev, PCI_MAGIC_DRKAISER_ADDR, PCI_MAGIC_DRKAISER_VALUE);
- /* Map 128KB flash memory window. */ + /* Map 128kB flash memory window. */ drkaiser_bar = physmap("Dr. Kaiser PC-Waechter flash memory", addr, 128 * 1024);
diff --git a/flash.h b/flash.h index 91c4cf3..0974db6 100644 --- a/flash.h +++ b/flash.h @@ -107,7 +107,9 @@ struct flashchip { uint32_t manufacture_id; uint32_t model_id;
+ /* Total chip size in kilobytes */ int total_size; + /* Chip page size in bytes */ int page_size; int feature_bits;
@@ -125,6 +127,9 @@ struct flashchip { /* * Erase blocks and associated erase function. Any chip erase function * is stored as chip-sized virtual block together with said function. + * The first one that fits will be chosen, there is currently no way to + * influence that behaviour. For testing just comment out the other + * elements or set the function pointer to NULL. */ struct block_eraser { struct eraseblock{ diff --git a/flashchips.h b/flashchips.h index 1c5e841..3b2b94f 100644 --- a/flashchips.h +++ b/flashchips.h @@ -145,9 +145,9 @@ #define ATMEL_AT25FS040 0x6604 #define ATMEL_AT26DF041 0x4400 #define ATMEL_AT26DF081 0x4500 /* guessed, no datasheet available */ -#define ATMEL_AT26DF081A 0x4501 +#define ATMEL_AT26DF081A 0x4501 #define ATMEL_AT26DF161 0x4600 -#define ATMEL_AT26DF161A 0x4601 +#define ATMEL_AT26DF161A 0x4601 #define ATMEL_AT26DF321 0x4700 /* Same as 25DF321 */ #define ATMEL_AT26F004 0x0400 #define ATMEL_AT29C040A 0xA4 diff --git a/flashrom.c b/flashrom.c index 34248ed..13ee129 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1142,7 +1142,7 @@ struct flashchip *probe_flash(struct flashchip *first_flash, int force) for (flash = first_flash; flash && flash->name; flash++) { if (chip_to_probe && strcmp(flash->name, chip_to_probe) != 0) continue; - msg_gdbg("Probing for %s %s, %d KB: ", + 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 " @@ -1193,7 +1193,7 @@ notfound: #endif snprintf(location, sizeof(location), "on %s", programmer_table[programmer].name);
- msg_cinfo("%s chip "%s %s" (%d KB, %s) %s.\n", + msg_cinfo("%s chip "%s %s" (%d kB, %s) %s.\n", force ? "Assuming" : "Found", flash->vendor, flash->name, flash->total_size, flashbuses_to_text(flash->bustype), location); diff --git a/print.c b/print.c index b939f67..a6591f9 100644 --- a/print.c +++ b/print.c @@ -100,7 +100,7 @@ static void print_supported_chips(void) for (i = strlen("Device"); i < maxchiplen; i++) printf(" ");
- printf("Tested Known Size/KB: Type:\n"); + printf("Tested Known Size/kB: Type:\n"); for (i = 0; i < okcol; i++) printf(" "); printf("OK Broken\n\n"); diff --git a/print_wiki.c b/print_wiki.c index 8b13f70..22d4a48 100644 --- a/print_wiki.c +++ b/print_wiki.c @@ -59,7 +59,7 @@ mainboards on the [[Mailinglist|mailing list]].\n";
static const char chip_th[] = "{| border="0" style="font-size: smaller" \ valign="top"\n|- bgcolor="#6699dd"\n! align="left" | Vendor\n\ -! align="left" | Device\n! align="left" | Size / KB\n\ +! align="left" | Device\n! align="left" | Size / kB\n\ ! align="left" | Type\n! align="left" colspan="4" | Status\n\n\ |- bgcolor="#6699ff"\n| colspan="4" | \n\ | Probe\n| Read\n| Erase\n| Write\n\n"; diff --git a/wbsio_spi.c b/wbsio_spi.c index acf9cb2..5e08416 100644 --- a/wbsio_spi.c +++ b/wbsio_spi.c @@ -71,7 +71,7 @@ int wbsio_check_for_spi(void) buses_supported |= CHIP_BUSTYPE_SPI; spi_controller = SPI_CONTROLLER_WBSIO; msg_pdbg("%s: Winbond saved on 4 register bits so max chip size is " - "1024 KB!\n", __func__); + "1024 kB!\n", __func__); max_rom_decode.spi = 1024 * 1024;
return 0;