Chip locking has three actions you can do with it: - Print/read the current locking status - Lock the chip - Unlock the chip.
Currently, the code usually does lock printing inside the probe function, and unlocking somewhere in the erase or write function. Only very few chips reactivate the lock after write/erase. Since many chips have identical probe/write/erase functions, but totally different locking, many such functions have been duplicated needlessly. With this patch, it is possible to call the chip-specific locking functions from probe/write/erase functions and unify lots of code.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-lock_refactor/flash.h =================================================================== --- flashrom-lock_refactor/flash.h (Revision 750) +++ flashrom-lock_refactor/flash.h (Arbeitskopie) @@ -175,6 +175,12 @@ CHIP_BUSTYPE_UNKNOWN = CHIP_BUSTYPE_PARALLEL | CHIP_BUSTYPE_LPC | CHIP_BUSTYPE_FWH | CHIP_BUSTYPE_SPI, };
+enum lockaction { + lock_print, + lock_disable, + lock_enable, +}; + /* * How many different contiguous runs of erase blocks with one size each do * we have for a given erase function? @@ -229,6 +235,7 @@
int (*write) (struct flashchip *flash, uint8_t *buf); int (*read) (struct flashchip *flash, uint8_t *buf, int start, int len); + int (*lock) (struct flashchip *flash, enum lockaction action);
/* Some flash devices have an additional register space. */ chipaddr virtual_memory; @@ -551,6 +558,7 @@ int spi_send_multicommand(struct spi_command *cmds); int spi_write_enable(void); int spi_write_disable(void); +int spi_lock(struct flashchip *flash, enum lockaction action); int spi_chip_erase_60(struct flashchip *flash); int spi_chip_erase_c7(struct flashchip *flash); int spi_chip_erase_60_c7(struct flashchip *flash); Index: flashrom-lock_refactor/flashchips.c =================================================================== --- flashrom-lock_refactor/flashchips.c (Revision 750) +++ flashrom-lock_refactor/flashchips.c (Arbeitskopie) @@ -211,6 +211,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -227,6 +228,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -243,6 +245,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -259,6 +262,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -275,6 +279,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -291,6 +296,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -307,6 +313,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -323,6 +330,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -339,6 +347,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -355,6 +364,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -371,6 +381,7 @@ .erase = NULL, .write = NULL /* Incompatible Page write */, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -387,6 +398,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -403,6 +415,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -419,6 +432,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
/*The AT26DF321 has the same ID as the AT25DF321. */ @@ -436,6 +450,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },*/
{ @@ -452,6 +467,7 @@ .erase = spi_chip_erase_60_c7, .write = NULL /* Incompatible Page write */, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -468,7 +484,6 @@ .erase = erase_chip_jedec, .write = write_jedec, .read = read_memmapped, - },
{ @@ -533,6 +548,7 @@ .erase = NULL, .write = NULL /* Incompatible Page write */, .read = NULL /* Incompatible read */, + .lock = spi_lock, },
{ @@ -549,6 +565,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = spi_lock, },
{ @@ -565,6 +582,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = spi_lock, },
{ @@ -581,6 +599,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = spi_lock, },
{ @@ -597,6 +616,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = spi_lock, },
{ @@ -613,6 +633,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = spi_lock, },
{ @@ -629,6 +650,7 @@ .erase = NULL, .write = NULL, .read = NULL /* Incompatible read */, + .lock = spi_lock, },
{ @@ -645,6 +667,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = spi_lock, },
{ @@ -661,6 +684,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = spi_lock, },
{ @@ -725,6 +749,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -821,6 +846,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -837,6 +863,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -853,6 +880,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -869,6 +897,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -885,6 +914,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -901,6 +931,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -917,6 +948,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -933,6 +965,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -949,6 +982,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -965,6 +999,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -981,6 +1016,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -997,6 +1033,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1013,6 +1050,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1029,6 +1067,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1045,6 +1084,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1061,6 +1101,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1256,6 +1297,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1288,6 +1330,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1323,6 +1366,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1358,6 +1402,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1393,6 +1438,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1428,6 +1474,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1444,6 +1491,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1460,6 +1508,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1476,6 +1525,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1492,6 +1542,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1508,6 +1559,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1604,6 +1656,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1620,6 +1673,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1636,6 +1690,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1652,6 +1707,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1668,6 +1724,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1684,6 +1741,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1700,6 +1758,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1716,6 +1775,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1732,6 +1792,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1748,6 +1809,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1764,6 +1826,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1876,6 +1939,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1892,6 +1956,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1908,6 +1973,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1924,6 +1990,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1940,6 +2007,7 @@ .erase = spi_chip_erase_60, .write = spi_chip_write_1, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1956,6 +2024,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -1972,6 +2041,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_1, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -2420,6 +2490,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
/* The ST M25P05 is a bit of a problem. It has the same ID as the @@ -2441,6 +2512,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, /* 128 */ .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -2457,6 +2529,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
/* The ST M25P10 has the same problem as the M25P05. */ @@ -2474,6 +2547,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, /* 128 */ .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -2490,6 +2564,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -2506,6 +2581,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -2522,6 +2598,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -2538,6 +2615,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -2554,6 +2632,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -2570,6 +2649,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -2586,6 +2666,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -2602,6 +2683,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -2954,6 +3036,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -2970,6 +3053,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -2986,6 +3070,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -3002,6 +3087,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -3018,6 +3104,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_lock, },
{ @@ -3258,6 +3345,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = NULL, },
{ @@ -3274,6 +3362,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = NULL, },
{ @@ -3290,6 +3379,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = NULL, },
{ @@ -3306,6 +3396,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = NULL, },
{ @@ -3322,6 +3413,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = NULL, },
{ @@ -3338,6 +3430,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = NULL, },
{ NULL } Index: flashrom-lock_refactor/spi.c =================================================================== --- flashrom-lock_refactor/spi.c (Revision 750) +++ flashrom-lock_refactor/spi.c (Arbeitskopie) @@ -30,8 +30,6 @@ enum spi_controller spi_controller = SPI_CONTROLLER_NONE; void *spibar = NULL;
-void spi_prettyprint_status_register(struct flashchip *flash); - const struct spi_programmer spi_programmer[] = { { /* SPI_CONTROLLER_NONE */ .command = NULL, @@ -268,11 +266,6 @@ printf_debug("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1, id2);
if (id1 == flash->manufacture_id && id2 == flash->model_id) { - /* Print the status register to tell the - * user about possible write protection. - */ - spi_prettyprint_status_register(flash); - return 1; }
@@ -327,11 +320,6 @@ printf_debug("%s: id1 0x%x, id2 0x%x\n", __func__, id1, id2);
if (id1 == flash->manufacture_id && id2 == flash->model_id) { - /* Print the status register to tell the - * user about possible write protection. - */ - spi_prettyprint_status_register(flash); - return 1; }
@@ -363,10 +351,6 @@ if (id2 != flash->model_id) return 0;
- /* Print the status register to tell the - * user about possible write protection. - */ - spi_prettyprint_status_register(flash); return 1; }
@@ -490,6 +474,25 @@ } }
+int spi_lock(struct flashchip *flash, enum lockaction action) +{ + switch (action) { + case lock_print: + spi_prettyprint_status_register(flash); + return 0; + break; + case lock_disable: + return spi_disable_blockprotect(); + break; + case lock_enable: + fprintf(stderr, "lock enable not supported.\n"); + return 1; + break; + default: /* Work around gcc. It can't see we enumerated all values. */ + return 1; + } +} + int spi_chip_erase_60(struct flashchip *flash) { int result; Index: flashrom-lock_refactor/flashrom.c =================================================================== --- flashrom-lock_refactor/flashrom.c (Revision 750) +++ flashrom-lock_refactor/flashrom.c (Arbeitskopie) @@ -448,6 +448,9 @@ printf("Found chip "%s %s" (%d KB, %s) at physical address 0x%lx.\n", flash->vendor, flash->name, flash->total_size, flashbuses_to_text(flash->bustype), base); + /* Print the locking status. */ + if (flash->lock) + flash->lock(flash, lock_print);
return flash; }
New version.
Chip locking has three actions you can do with it: - Print/read the current locking status - Lock the chip - Unlock the chip.
Currently, the code usually does lock printing inside the probe function, and unlocking somewhere in the erase or write function. Only very few chips reactivate the lock after write/erase. Since many chips have identical probe/write/erase functions, but totally different locking, many such functions have been duplicated needlessly. With this patch, it is possible to call the chip-specific locking functions from probe/write/erase functions and unify lots of code.
I converted spi.c and pm49fl00x.c to use the internal lock abstractions, but all other files need the same treatment.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-lock_refactor/flash.h =================================================================== --- flashrom-lock_refactor/flash.h (Revision 751) +++ flashrom-lock_refactor/flash.h (Arbeitskopie) @@ -175,6 +175,12 @@ CHIP_BUSTYPE_UNKNOWN = CHIP_BUSTYPE_PARALLEL | CHIP_BUSTYPE_LPC | CHIP_BUSTYPE_FWH | CHIP_BUSTYPE_SPI, };
+enum lockaction { + lock_print, + lock_disable, + lock_enable, +}; + /* * How many different contiguous runs of erase blocks with one size each do * we have for a given erase function? @@ -229,6 +235,7 @@
int (*write) (struct flashchip *flash, uint8_t *buf); int (*read) (struct flashchip *flash, uint8_t *buf, int start, int len); + int (*lock) (struct flashchip *flash, enum lockaction action);
/* Some flash devices have an additional register space. */ chipaddr virtual_memory; @@ -551,6 +558,7 @@ int spi_send_multicommand(struct spi_command *cmds); int spi_write_enable(void); int spi_write_disable(void); +int spi_chip_lock(struct flashchip *flash, enum lockaction action); int spi_chip_erase_60(struct flashchip *flash); int spi_chip_erase_c7(struct flashchip *flash); int spi_chip_erase_60_c7(struct flashchip *flash); @@ -661,6 +669,7 @@ int probe_49fl00x(struct flashchip *flash); int erase_49fl00x(struct flashchip *flash); int write_49fl00x(struct flashchip *flash, uint8_t *buf); +int lock_49fl00x(struct flashchip *flash, enum lockaction action);
/* sharplhf00l04.c */ int probe_lhf00l04(struct flashchip *flash); Index: flashrom-lock_refactor/pm49fl00x.c =================================================================== --- flashrom-lock_refactor/pm49fl00x.c (Revision 751) +++ flashrom-lock_refactor/pm49fl00x.c (Arbeitskopie) @@ -4,6 +4,7 @@ * Copyright (C) 2004 Tyan Corporation * Copyright (C) 2007 Nikolay Petukhov nikolay.petukhov@gmail.com * Copyright (C) 2007 Reinder E.N. de Haan lb_reha@mveas.com + * Copyright (C) 2009 Carl-Daniel Hailfinger * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -36,6 +37,30 @@ } }
+/* + * Return 0 if successful, 1 if failed, 2 if unsupported. + */ +int lock_49fl00x(struct flashchip *flash, enum lockaction action) +{ + switch (action) { + case lock_print: + fprintf(stderr, "lock printing not supported.\n"); + return 2; + case lock_disable: + write_lockbits_49fl00x(flash->virtual_registers, + flash->total_size * 1024, 0, + flash->page_size); + return 0; + case lock_enable: + write_lockbits_49fl00x(flash->virtual_registers, + flash->total_size * 1024, 1, + flash->page_size); + return 0; + default: /* Work around gcc. It can't see we enumerated all values. */ + return 2; + } +} + int probe_49fl00x(struct flashchip *flash) { int ret = probe_jedec(flash); @@ -53,8 +78,7 @@ int page_size = flash->page_size;
/* unprotected */ - write_lockbits_49fl00x(flash->virtual_registers, - total_size, 0, page_size); + lock_49fl00x(flash, lock_disable);
/* * erase_chip_jedec() will not work... Datasheet says @@ -74,8 +98,7 @@ printf("\n");
/* protected */ - write_lockbits_49fl00x(flash->virtual_registers, - total_size, 1, page_size); + lock_49fl00x(flash, lock_enable);
return 0; } @@ -88,8 +111,7 @@ chipaddr bios = flash->virtual_memory;
/* unprotected */ - write_lockbits_49fl00x(flash->virtual_registers, total_size, 0, - page_size); + lock_49fl00x(flash, lock_disable);
printf("Programming page: "); for (i = 0; i < total_size / page_size; i++) { @@ -109,8 +131,7 @@ printf("\n");
/* protected */ - write_lockbits_49fl00x(flash->virtual_registers, total_size, 1, - page_size); + lock_49fl00x(flash, lock_enable);
return 0; } Index: flashrom-lock_refactor/flashchips.c =================================================================== --- flashrom-lock_refactor/flashchips.c (Revision 751) +++ flashrom-lock_refactor/flashchips.c (Arbeitskopie) @@ -51,6 +51,7 @@ * } * .write = Chip write function * .read = Chip read function + * .lock = Chip locking print/enable/disable function */
{ @@ -211,6 +212,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -227,6 +229,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -243,6 +246,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -259,6 +263,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -275,6 +280,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -291,6 +297,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -307,6 +314,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -323,6 +331,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -339,6 +348,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -355,6 +365,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -387,6 +398,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -403,6 +415,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -419,6 +432,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
/*The AT26DF321 has the same ID as the AT25DF321. */ @@ -436,6 +450,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },*/
{ @@ -468,7 +483,6 @@ .erase = erase_chip_jedec, .write = write_jedec, .read = read_memmapped, - },
{ @@ -725,6 +739,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -789,6 +804,7 @@ .erase = erase_49fl00x, .write = write_49fl00x, .read = read_memmapped, + .lock = lock_49fl00x, },
{ @@ -821,6 +837,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -837,6 +854,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -853,6 +871,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -869,6 +888,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -885,6 +905,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -901,6 +922,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -917,6 +939,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -933,6 +956,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -949,6 +973,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -965,6 +990,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -981,6 +1007,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -997,6 +1024,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1013,6 +1041,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1029,6 +1058,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1045,6 +1075,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1061,6 +1092,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1256,6 +1288,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1288,6 +1321,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1323,6 +1357,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1358,6 +1393,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1393,6 +1429,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1428,6 +1465,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1444,6 +1482,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1460,6 +1499,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1476,6 +1516,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1492,6 +1533,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1508,6 +1550,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1604,6 +1647,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1620,6 +1664,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1636,6 +1681,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1652,6 +1698,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1668,6 +1715,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1684,6 +1732,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1700,6 +1749,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1716,6 +1766,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1732,6 +1783,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1748,6 +1800,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1764,6 +1817,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1828,6 +1882,7 @@ .erase = erase_49fl00x, .write = write_49fl00x, .read = read_memmapped, + .lock = lock_49fl00x, },
{ @@ -1844,6 +1899,7 @@ .erase = erase_49fl00x, .write = write_49fl00x, .read = read_memmapped, + .lock = lock_49fl00x, },
{ @@ -1876,6 +1932,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1892,6 +1949,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1908,6 +1966,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1924,6 +1983,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1940,6 +2000,7 @@ .erase = spi_chip_erase_60, .write = spi_chip_write_1, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1956,6 +2017,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1972,6 +2034,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_1, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2420,6 +2483,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
/* The ST M25P05 is a bit of a problem. It has the same ID as the @@ -2441,6 +2505,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, /* 128 */ .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2457,6 +2522,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
/* The ST M25P10 has the same problem as the M25P05. */ @@ -2474,6 +2540,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, /* 128 */ .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2490,6 +2557,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2506,6 +2574,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2522,6 +2591,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2538,6 +2608,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2554,6 +2625,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2570,6 +2642,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2586,6 +2659,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2602,6 +2676,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2954,6 +3029,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2970,6 +3046,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2986,6 +3063,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -3002,6 +3080,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -3018,6 +3097,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -3258,6 +3338,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = NULL, },
{ @@ -3274,6 +3355,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = NULL, },
{ @@ -3290,6 +3372,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = NULL, },
{ @@ -3306,6 +3389,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = NULL, },
{ @@ -3322,6 +3406,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = NULL, },
{ @@ -3338,6 +3423,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = NULL, },
{ NULL } Index: flashrom-lock_refactor/spi.c =================================================================== --- flashrom-lock_refactor/spi.c (Revision 751) +++ flashrom-lock_refactor/spi.c (Arbeitskopie) @@ -30,8 +30,6 @@ enum spi_controller spi_controller = SPI_CONTROLLER_NONE; void *spibar = NULL;
-void spi_prettyprint_status_register(struct flashchip *flash); - const struct spi_programmer spi_programmer[] = { { /* SPI_CONTROLLER_NONE */ .command = NULL, @@ -268,11 +266,6 @@ printf_debug("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1, id2);
if (id1 == flash->manufacture_id && id2 == flash->model_id) { - /* Print the status register to tell the - * user about possible write protection. - */ - spi_prettyprint_status_register(flash); - return 1; }
@@ -327,11 +320,6 @@ printf_debug("%s: id1 0x%x, id2 0x%x\n", __func__, id1, id2);
if (id1 == flash->manufacture_id && id2 == flash->model_id) { - /* Print the status register to tell the - * user about possible write protection. - */ - spi_prettyprint_status_register(flash); - return 1; }
@@ -363,10 +351,6 @@ if (id2 != flash->model_id) return 0;
- /* Print the status register to tell the - * user about possible write protection. - */ - spi_prettyprint_status_register(flash); return 1; }
@@ -490,6 +474,25 @@ } }
+/* + * Return 0 if successful, 1 if failed, 2 if unsupported. + */ +int spi_chip_lock(struct flashchip *flash, enum lockaction action) +{ + switch (action) { + case lock_print: + spi_prettyprint_status_register(flash); + return 0; + case lock_disable: + return spi_disable_blockprotect(); + case lock_enable: + fprintf(stderr, "lock enable not supported.\n"); + return 2; + default: /* Work around gcc. It can't see we enumerated all values. */ + return 2; + } +} + int spi_chip_erase_60(struct flashchip *flash) { int result; @@ -511,9 +514,9 @@ .readarr = NULL, }}; - result = spi_disable_blockprotect(); + result = spi_chip_lock(flash, lock_disable); if (result) { - fprintf(stderr, "spi_disable_blockprotect failed\n"); + fprintf(stderr, "unprotect failed\n"); return result; } @@ -557,9 +560,9 @@ .readarr = NULL, }};
- result = spi_disable_blockprotect(); + result = spi_chip_lock(flash, lock_disable); if (result) { - fprintf(stderr, "spi_disable_blockprotect failed\n"); + fprintf(stderr, "unprotect failed\n"); return result; }
@@ -676,17 +679,21 @@
int spi_chip_erase_d8(struct flashchip *flash) { - int i, rc = 0; + int i, result = 0; int total_size = flash->total_size * 1024; int erase_size = 64 * 1024;
- spi_disable_blockprotect(); + result = spi_chip_lock(flash, lock_disable); + if (result) { + fprintf(stderr, "unprotect failed\n"); + return result; + }
printf("Erasing chip: \n");
for (i = 0; i < total_size / erase_size; i++) { - rc = spi_block_erase_d8(flash, i * erase_size, erase_size); - if (rc) { + result = spi_block_erase_d8(flash, i * erase_size, erase_size); + if (result) { fprintf(stderr, "Error erasing block at 0x%x\n", i); break; } @@ -694,7 +701,7 @@
printf("\n");
- return rc; + return result; }
/* Sector size is usually 4k, though Macronix eliteflash has 64k */ @@ -972,7 +979,11 @@ int total_size = 1024 * flash->total_size; int i, result = 0;
- spi_disable_blockprotect(); + result = spi_chip_lock(flash, lock_disable); + if (result) { + fprintf(stderr, "unprotect failed\n"); + return result; + } /* Erase first */ printf("Erasing flash before programming... "); if (erase_flash(flash)) { Index: flashrom-lock_refactor/flashrom.c =================================================================== --- flashrom-lock_refactor/flashrom.c (Revision 751) +++ flashrom-lock_refactor/flashrom.c (Arbeitskopie) @@ -448,6 +448,9 @@ printf("Found chip "%s %s" (%d KB, %s) at physical address 0x%lx.\n", flash->vendor, flash->name, flash->total_size, flashbuses_to_text(flash->bustype), base); + /* Print the locking status. */ + if (flash->lock) + flash->lock(flash, lock_print);
return flash; }
G'Day,
Just a small/quick review, You have a extra "," at the end of your enum list.
Cheers, Edward.
2009/10/20 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net:
New version.
Chip locking has three actions you can do with it:
- Print/read the current locking status
- Lock the chip
- Unlock the chip.
Currently, the code usually does lock printing inside the probe function, and unlocking somewhere in the erase or write function. Only very few chips reactivate the lock after write/erase. Since many chips have identical probe/write/erase functions, but totally different locking, many such functions have been duplicated needlessly. With this patch, it is possible to call the chip-specific locking functions from probe/write/erase functions and unify lots of code.
I converted spi.c and pm49fl00x.c to use the internal lock abstractions, but all other files need the same treatment.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-lock_refactor/flash.h
--- flashrom-lock_refactor/flash.h (Revision 751) +++ flashrom-lock_refactor/flash.h (Arbeitskopie) @@ -175,6 +175,12 @@ CHIP_BUSTYPE_UNKNOWN = CHIP_BUSTYPE_PARALLEL | CHIP_BUSTYPE_LPC | CHIP_BUSTYPE_FWH | CHIP_BUSTYPE_SPI, };
+enum lockaction {
- lock_print,
- lock_disable,
- lock_enable,
+};
/* * How many different contiguous runs of erase blocks with one size each do * we have for a given erase function? @@ -229,6 +235,7 @@
int (*write) (struct flashchip *flash, uint8_t *buf); int (*read) (struct flashchip *flash, uint8_t *buf, int start, int len);
- int (*lock) (struct flashchip *flash, enum lockaction action);
/* Some flash devices have an additional register space. */ chipaddr virtual_memory; @@ -551,6 +558,7 @@ int spi_send_multicommand(struct spi_command *cmds); int spi_write_enable(void); int spi_write_disable(void); +int spi_chip_lock(struct flashchip *flash, enum lockaction action); int spi_chip_erase_60(struct flashchip *flash); int spi_chip_erase_c7(struct flashchip *flash); int spi_chip_erase_60_c7(struct flashchip *flash); @@ -661,6 +669,7 @@ int probe_49fl00x(struct flashchip *flash); int erase_49fl00x(struct flashchip *flash); int write_49fl00x(struct flashchip *flash, uint8_t *buf); +int lock_49fl00x(struct flashchip *flash, enum lockaction action);
/* sharplhf00l04.c */ int probe_lhf00l04(struct flashchip *flash); Index: flashrom-lock_refactor/pm49fl00x.c =================================================================== --- flashrom-lock_refactor/pm49fl00x.c (Revision 751) +++ flashrom-lock_refactor/pm49fl00x.c (Arbeitskopie) @@ -4,6 +4,7 @@ * Copyright (C) 2004 Tyan Corporation * Copyright (C) 2007 Nikolay Petukhov nikolay.petukhov@gmail.com * Copyright (C) 2007 Reinder E.N. de Haan lb_reha@mveas.com
- Copyright (C) 2009 Carl-Daniel Hailfinger
* * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -36,6 +37,30 @@ } }
+/*
- Return 0 if successful, 1 if failed, 2 if unsupported.
- */
+int lock_49fl00x(struct flashchip *flash, enum lockaction action) +{
- switch (action) {
- case lock_print:
- fprintf(stderr, "lock printing not supported.\n");
- return 2;
- case lock_disable:
- write_lockbits_49fl00x(flash->virtual_registers,
- flash->total_size * 1024, 0,
- flash->page_size);
- return 0;
- case lock_enable:
- write_lockbits_49fl00x(flash->virtual_registers,
- flash->total_size * 1024, 1,
- flash->page_size);
- return 0;
- default: /* Work around gcc. It can't see we enumerated all values. */
- return 2;
- }
+}
int probe_49fl00x(struct flashchip *flash) { int ret = probe_jedec(flash); @@ -53,8 +78,7 @@ int page_size = flash->page_size;
/* unprotected */
- write_lockbits_49fl00x(flash->virtual_registers,
- total_size, 0, page_size);
- lock_49fl00x(flash, lock_disable);
/* * erase_chip_jedec() will not work... Datasheet says @@ -74,8 +98,7 @@ printf("\n");
/* protected */
- write_lockbits_49fl00x(flash->virtual_registers,
- total_size, 1, page_size);
- lock_49fl00x(flash, lock_enable);
return 0; } @@ -88,8 +111,7 @@ chipaddr bios = flash->virtual_memory;
/* unprotected */
- write_lockbits_49fl00x(flash->virtual_registers, total_size, 0,
- page_size);
- lock_49fl00x(flash, lock_disable);
printf("Programming page: "); for (i = 0; i < total_size / page_size; i++) { @@ -109,8 +131,7 @@ printf("\n");
/* protected */
- write_lockbits_49fl00x(flash->virtual_registers, total_size, 1,
- page_size);
- lock_49fl00x(flash, lock_enable);
return 0; } Index: flashrom-lock_refactor/flashchips.c =================================================================== --- flashrom-lock_refactor/flashchips.c (Revision 751) +++ flashrom-lock_refactor/flashchips.c (Arbeitskopie) @@ -51,6 +51,7 @@ * } * .write = Chip write function * .read = Chip read function
- * .lock = Chip locking print/enable/disable function
*/
{ @@ -211,6 +212,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -227,6 +229,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -243,6 +246,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -259,6 +263,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -275,6 +280,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -291,6 +297,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -307,6 +314,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -323,6 +331,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -339,6 +348,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -355,6 +365,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -387,6 +398,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -403,6 +415,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -419,6 +432,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
/*The AT26DF321 has the same ID as the AT25DF321. */ @@ -436,6 +450,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},*/
{ @@ -468,7 +483,6 @@ .erase = erase_chip_jedec, .write = write_jedec, .read = read_memmapped,
},
{ @@ -725,6 +739,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -789,6 +804,7 @@ .erase = erase_49fl00x, .write = write_49fl00x, .read = read_memmapped,
- .lock = lock_49fl00x,
},
{ @@ -821,6 +837,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -837,6 +854,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -853,6 +871,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -869,6 +888,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -885,6 +905,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -901,6 +922,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -917,6 +939,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -933,6 +956,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -949,6 +973,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -965,6 +990,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -981,6 +1007,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -997,6 +1024,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1013,6 +1041,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1029,6 +1058,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1045,6 +1075,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1061,6 +1092,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1256,6 +1288,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1288,6 +1321,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1323,6 +1357,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1358,6 +1393,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1393,6 +1429,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1428,6 +1465,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1444,6 +1482,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1460,6 +1499,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1476,6 +1516,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1492,6 +1533,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1508,6 +1550,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1604,6 +1647,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1620,6 +1664,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1636,6 +1681,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1652,6 +1698,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1668,6 +1715,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1684,6 +1732,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1700,6 +1749,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1716,6 +1766,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1732,6 +1783,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1748,6 +1800,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1764,6 +1817,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1828,6 +1882,7 @@ .erase = erase_49fl00x, .write = write_49fl00x, .read = read_memmapped,
- .lock = lock_49fl00x,
},
{ @@ -1844,6 +1899,7 @@ .erase = erase_49fl00x, .write = write_49fl00x, .read = read_memmapped,
- .lock = lock_49fl00x,
},
{ @@ -1876,6 +1932,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1892,6 +1949,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1908,6 +1966,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1924,6 +1983,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1940,6 +2000,7 @@ .erase = spi_chip_erase_60, .write = spi_chip_write_1, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1956,6 +2017,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -1972,6 +2034,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_1, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -2420,6 +2483,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
/* The ST M25P05 is a bit of a problem. It has the same ID as the @@ -2441,6 +2505,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, /* 128 */ .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -2457,6 +2522,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
/* The ST M25P10 has the same problem as the M25P05. */ @@ -2474,6 +2540,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, /* 128 */ .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -2490,6 +2557,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -2506,6 +2574,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -2522,6 +2591,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -2538,6 +2608,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -2554,6 +2625,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -2570,6 +2642,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -2586,6 +2659,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -2602,6 +2676,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -2954,6 +3029,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -2970,6 +3046,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -2986,6 +3063,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -3002,6 +3080,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -3018,6 +3097,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read,
- .lock = spi_chip_lock,
},
{ @@ -3258,6 +3338,7 @@ .erase = NULL, .write = NULL, .read = NULL,
- .lock = NULL,
},
{ @@ -3274,6 +3355,7 @@ .erase = NULL, .write = NULL, .read = NULL,
- .lock = NULL,
},
{ @@ -3290,6 +3372,7 @@ .erase = NULL, .write = NULL, .read = NULL,
- .lock = NULL,
},
{ @@ -3306,6 +3389,7 @@ .erase = NULL, .write = NULL, .read = NULL,
- .lock = NULL,
},
{ @@ -3322,6 +3406,7 @@ .erase = NULL, .write = NULL, .read = NULL,
- .lock = NULL,
},
{ @@ -3338,6 +3423,7 @@ .erase = NULL, .write = NULL, .read = NULL,
- .lock = NULL,
},
{ NULL } Index: flashrom-lock_refactor/spi.c =================================================================== --- flashrom-lock_refactor/spi.c (Revision 751) +++ flashrom-lock_refactor/spi.c (Arbeitskopie) @@ -30,8 +30,6 @@ enum spi_controller spi_controller = SPI_CONTROLLER_NONE; void *spibar = NULL;
-void spi_prettyprint_status_register(struct flashchip *flash);
const struct spi_programmer spi_programmer[] = { { /* SPI_CONTROLLER_NONE */ .command = NULL, @@ -268,11 +266,6 @@ printf_debug("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1, id2);
if (id1 == flash->manufacture_id && id2 == flash->model_id) {
- /* Print the status register to tell the
- * user about possible write protection.
- */
- spi_prettyprint_status_register(flash);
return 1; }
@@ -327,11 +320,6 @@ printf_debug("%s: id1 0x%x, id2 0x%x\n", __func__, id1, id2);
if (id1 == flash->manufacture_id && id2 == flash->model_id) {
- /* Print the status register to tell the
- * user about possible write protection.
- */
- spi_prettyprint_status_register(flash);
return 1; }
@@ -363,10 +351,6 @@ if (id2 != flash->model_id) return 0;
- /* Print the status register to tell the
- * user about possible write protection.
- */
- spi_prettyprint_status_register(flash);
return 1; }
@@ -490,6 +474,25 @@ } }
+/*
- Return 0 if successful, 1 if failed, 2 if unsupported.
- */
+int spi_chip_lock(struct flashchip *flash, enum lockaction action) +{
- switch (action) {
- case lock_print:
- spi_prettyprint_status_register(flash);
- return 0;
- case lock_disable:
- return spi_disable_blockprotect();
- case lock_enable:
- fprintf(stderr, "lock enable not supported.\n");
- return 2;
- default: /* Work around gcc. It can't see we enumerated all values. */
- return 2;
- }
+}
int spi_chip_erase_60(struct flashchip *flash) { int result; @@ -511,9 +514,9 @@ .readarr = NULL, }};
- result = spi_disable_blockprotect();
- result = spi_chip_lock(flash, lock_disable);
if (result) {
- fprintf(stderr, "spi_disable_blockprotect failed\n");
- fprintf(stderr, "unprotect failed\n");
return result; }
@@ -557,9 +560,9 @@ .readarr = NULL, }};
- result = spi_disable_blockprotect();
- result = spi_chip_lock(flash, lock_disable);
if (result) {
- fprintf(stderr, "spi_disable_blockprotect failed\n");
- fprintf(stderr, "unprotect failed\n");
return result; }
@@ -676,17 +679,21 @@
int spi_chip_erase_d8(struct flashchip *flash) {
- int i, rc = 0;
- int i, result = 0;
int total_size = flash->total_size * 1024; int erase_size = 64 * 1024;
- spi_disable_blockprotect();
- result = spi_chip_lock(flash, lock_disable);
- if (result) {
- fprintf(stderr, "unprotect failed\n");
- return result;
- }
printf("Erasing chip: \n");
for (i = 0; i < total_size / erase_size; i++) {
- rc = spi_block_erase_d8(flash, i * erase_size, erase_size);
- if (rc) {
- result = spi_block_erase_d8(flash, i * erase_size, erase_size);
- if (result) {
fprintf(stderr, "Error erasing block at 0x%x\n", i); break; } @@ -694,7 +701,7 @@
printf("\n");
- return rc;
- return result;
}
/* Sector size is usually 4k, though Macronix eliteflash has 64k */ @@ -972,7 +979,11 @@ int total_size = 1024 * flash->total_size; int i, result = 0;
- spi_disable_blockprotect();
- result = spi_chip_lock(flash, lock_disable);
- if (result) {
- fprintf(stderr, "unprotect failed\n");
- return result;
- }
/* Erase first */ printf("Erasing flash before programming... "); if (erase_flash(flash)) { Index: flashrom-lock_refactor/flashrom.c =================================================================== --- flashrom-lock_refactor/flashrom.c (Revision 751) +++ flashrom-lock_refactor/flashrom.c (Arbeitskopie) @@ -448,6 +448,9 @@ printf("Found chip "%s %s" (%d KB, %s) at physical address 0x%lx.\n", flash->vendor, flash->name, flash->total_size, flashbuses_to_text(flash->bustype), base);
- /* Print the locking status. */
- if (flash->lock)
- flash->lock(flash, lock_print);
return flash; }
-- Developer quote of the week: "We are juggling too many chainsaws and flaming arrows and tigers."
flashrom mailing list flashrom@flashrom.org http://www.flashrom.org/mailman/listinfo/flashrom
Hi Edward,
On 21.10.2009 11:49, Edward O'Callaghan wrote:
Just a small/quick review, You have a extra "," at the end of your enum list.
Thanks for the review. The usual flashrom coding style has one extra comma at the end of each array initializer to make extending the initializer easier (no need to patch in the comma at the old last entry), but in this case we only want three states, so your point is valid. I'll probably change it, but I'd appreciate more input. Is a simpler coding style (comma everywhere) or a more complicated coding style with improved correctness (comma only where it makes sense) better?
2009/10/20 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net:
--- flashrom-lock_refactor/flash.h (Revision 751) +++ flashrom-lock_refactor/flash.h (Arbeitskopie) @@ -175,6 +175,12 @@ CHIP_BUSTYPE_UNKNOWN = CHIP_BUSTYPE_PARALLEL | CHIP_BUSTYPE_LPC | CHIP_BUSTYPE_FWH | CHIP_BUSTYPE_SPI, };
+enum lockaction {
lock_print,
lock_disable,
lock_enable,
+};
/*
- How many different contiguous runs of erase blocks with one size each do
- we have for a given erase function?
Regards, Carl-Daniel
On 20.10.2009 23:38, Carl-Daniel Hailfinger wrote:
New version.
Chip locking has three actions you can do with it:
- Print/read the current locking status
- Lock the chip
- Unlock the chip.
Currently, the code usually does lock printing inside the probe function, and unlocking somewhere in the erase or write function. Only very few chips reactivate the lock after write/erase. Since many chips have identical probe/write/erase functions, but totally different locking, many such functions have been duplicated needlessly. With this patch, it is possible to call the chip-specific locking functions from probe/write/erase functions and unify lots of code.
I converted spi.c and pm49fl00x.c to use the internal lock abstractions, but all other files need the same treatment.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Ping? This is one month old.
Regards, Carl-Daniel
On czw, 19 lis 2009, Carl-Daniel Hailfinger wrote:
On 20.10.2009 23:38, Carl-Daniel Hailfinger wrote:
New version.
Chip locking has three actions you can do with it:
- Print/read the current locking status
- Lock the chip
- Unlock the chip.
Currently, the code usually does lock printing inside the probe function, and unlocking somewhere in the erase or write function. Only very few chips reactivate the lock after write/erase. Since many chips have identical probe/write/erase functions, but totally different locking, many such functions have been duplicated needlessly. With this patch, it is possible to call the chip-specific locking functions from probe/write/erase functions and unify lots of code.
I converted spi.c and pm49fl00x.c to use the internal lock abstractions, but all other files need the same treatment.
How about adding dummy operation to enum and something like
case dummy_op: default:
in switch clause, first to use 0 value to dummy op, also to make more explicit what default really is.
Additionally i didn't saw any routine that lets check if current chip has any lock abilities like spi_lock(somechip, lock_query) which would return if any locks are available or something that code may use it to check if locking is available (regardless of presence routine in chip data) for this chip.
Maybe this isn't really needed but we have routine that sets and resets lock, prints info to user but no routine that could poll status of lock or verify if chip has locking abilities at all.
Best Regards Maciej
On 20.11.2009 00:05, Maciej Pijanka wrote:
How about adding dummy operation to enum and something like
case dummy_op: default:
in switch clause, first to use 0 value to dummy op, also to make more explicit what default really is.
Adding a dummy enum which does nothing doesn't look like it would help. Apologies if I'm missing your point.
Additionally i didn't saw any routine that lets check if current chip has any lock abilities like spi_lock(somechip, lock_query) which would return if any locks are available or something that code may use it to check if locking is available (regardless of presence routine in chip data) for this chip.
Maybe this isn't really needed but we have routine that sets and resets lock, prints info to user but no routine that could poll status of lock or verify if chip has locking abilities at all.
Good point. Updated patch below.
Chip locking has four actions you can do with it: - Print the current locking status - Read the current locking status - Lock the chip - Unlock the chip.
Currently, the code usually does lock printing inside the probe function, and unlocking somewhere in the erase or write function. Only very few chips reactivate the lock after write/erase. Since many chips have identical probe/write/erase functions, but totally different locking, many such functions have been duplicated needlessly. With this patch, it is possible to call the chip-specific locking functions from probe/write/erase functions and unify lots of code.
Thanks to Maciej Pijanka for pointing out that a lock read function was missing in the previous version.
Thanks to Edward O'Callaghan for checking the coding style and pointing out a comma at the end of an enum list which is only allowed in C99 or later (did I understand that correctly?). Given that flashrom uses lots of other C99 constructs, this doesn't seem to be a problem, but we may want to specify C99 mode to the compiler (or GNU99) to get proper treatment.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-lock_refactor/flash.h =================================================================== --- flashrom-lock_refactor/flash.h (Revision 769) +++ flashrom-lock_refactor/flash.h (Arbeitskopie) @@ -175,6 +175,13 @@ CHIP_BUSTYPE_UNKNOWN = CHIP_BUSTYPE_PARALLEL | CHIP_BUSTYPE_LPC | CHIP_BUSTYPE_FWH | CHIP_BUSTYPE_SPI, };
+enum lockaction { + lock_check_unlocked, /* Check if the chip is completely unlocked or doesn't support locks. */ + lock_print, /* Print verbose current locking status. */ + lock_disable, /* Disable all locks. */ + lock_enable, /* Enable all locks (no permanent lockdown). */ +}; + /* * How many different contiguous runs of erase blocks with one size each do * we have for a given erase function? @@ -229,6 +236,7 @@
int (*write) (struct flashchip *flash, uint8_t *buf); int (*read) (struct flashchip *flash, uint8_t *buf, int start, int len); + int (*lock) (struct flashchip *flash, enum lockaction action);
/* Some flash devices have an additional register space. */ chipaddr virtual_memory; @@ -558,6 +566,7 @@ int spi_send_multicommand(struct spi_command *cmds); int spi_write_enable(void); int spi_write_disable(void); +int spi_chip_lock(struct flashchip *flash, enum lockaction action); int spi_chip_erase_60(struct flashchip *flash); int spi_chip_erase_c7(struct flashchip *flash); int spi_chip_erase_60_c7(struct flashchip *flash); @@ -668,6 +677,7 @@ int probe_49fl00x(struct flashchip *flash); int erase_49fl00x(struct flashchip *flash); int write_49fl00x(struct flashchip *flash, uint8_t *buf); +int lock_49fl00x(struct flashchip *flash, enum lockaction action);
/* sharplhf00l04.c */ int probe_lhf00l04(struct flashchip *flash); Index: flashrom-lock_refactor/pm49fl00x.c =================================================================== --- flashrom-lock_refactor/pm49fl00x.c (Revision 769) +++ flashrom-lock_refactor/pm49fl00x.c (Arbeitskopie) @@ -4,6 +4,7 @@ * Copyright (C) 2004 Tyan Corporation * Copyright (C) 2007 Nikolay Petukhov nikolay.petukhov@gmail.com * Copyright (C) 2007 Reinder E.N. de Haan lb_reha@mveas.com + * Copyright (C) 2009 Carl-Daniel Hailfinger * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -36,6 +37,33 @@ } }
+/* + * Return 0 if successful, 1 if failed, 2 if unsupported. + */ +int lock_49fl00x(struct flashchip *flash, enum lockaction action) +{ + switch (action) { + case lock_check_unlocked: + fprintf(stderr, "Checking lock status not supported.\n"); + return 2; + case lock_print: + fprintf(stderr, "Printing lock status not supported.\n"); + return 2; + case lock_disable: + write_lockbits_49fl00x(flash->virtual_registers, + flash->total_size * 1024, 0, + flash->page_size); + return 0; + case lock_enable: + write_lockbits_49fl00x(flash->virtual_registers, + flash->total_size * 1024, 1, + flash->page_size); + return 0; + default: + return 2; + } +} + int probe_49fl00x(struct flashchip *flash) { int ret = probe_jedec(flash); @@ -53,8 +81,7 @@ int page_size = flash->page_size;
/* unprotected */ - write_lockbits_49fl00x(flash->virtual_registers, - total_size, 0, page_size); + lock_49fl00x(flash, lock_disable);
/* * erase_chip_jedec() will not work... Datasheet says @@ -74,8 +101,7 @@ printf("\n");
/* protected */ - write_lockbits_49fl00x(flash->virtual_registers, - total_size, 1, page_size); + lock_49fl00x(flash, lock_enable);
return 0; } @@ -88,8 +114,7 @@ chipaddr bios = flash->virtual_memory;
/* unprotected */ - write_lockbits_49fl00x(flash->virtual_registers, total_size, 0, - page_size); + lock_49fl00x(flash, lock_disable);
printf("Programming page: "); for (i = 0; i < total_size / page_size; i++) { @@ -109,8 +134,7 @@ printf("\n");
/* protected */ - write_lockbits_49fl00x(flash->virtual_registers, total_size, 1, - page_size); + lock_49fl00x(flash, lock_enable);
return 0; } Index: flashrom-lock_refactor/flashchips.c =================================================================== --- flashrom-lock_refactor/flashchips.c (Revision 769) +++ flashrom-lock_refactor/flashchips.c (Arbeitskopie) @@ -51,6 +51,7 @@ * } * .write = Chip write function * .read = Chip read function + * .lock = Chip locking print/enable/disable function */
{ @@ -211,6 +212,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -227,6 +229,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -243,6 +246,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -259,6 +263,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -275,6 +280,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -291,6 +297,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -307,6 +314,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -323,6 +331,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -339,6 +348,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -355,6 +365,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -387,6 +398,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -403,6 +415,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -419,6 +432,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
/*The AT26DF321 has the same ID as the AT25DF321. */ @@ -436,6 +450,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },*/
{ @@ -468,7 +483,6 @@ .erase = erase_chip_jedec, .write = write_jedec, .read = read_memmapped, - },
{ @@ -725,6 +739,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -789,6 +804,7 @@ .erase = erase_49fl00x, .write = write_49fl00x, .read = read_memmapped, + .lock = lock_49fl00x, },
{ @@ -821,6 +837,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -837,6 +854,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -853,6 +871,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -869,6 +888,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -885,6 +905,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -901,6 +922,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -917,6 +939,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -933,6 +956,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -949,6 +973,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -965,6 +990,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -981,6 +1007,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -997,6 +1024,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1013,6 +1041,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1029,6 +1058,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1045,6 +1075,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1061,6 +1092,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1256,6 +1288,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1288,6 +1321,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1323,6 +1357,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1358,6 +1393,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1393,6 +1429,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1428,6 +1465,7 @@ }, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1444,6 +1482,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1460,6 +1499,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1476,6 +1516,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1492,6 +1533,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1508,6 +1550,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1604,6 +1647,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1620,6 +1664,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1636,6 +1681,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1652,6 +1698,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1668,6 +1715,7 @@ .erase = spi_chip_erase_d8, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1684,6 +1732,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1700,6 +1749,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1716,6 +1766,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1732,6 +1783,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1748,6 +1800,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1764,6 +1817,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1828,6 +1882,7 @@ .erase = erase_49fl00x, .write = write_49fl00x, .read = read_memmapped, + .lock = lock_49fl00x, },
{ @@ -1844,6 +1899,7 @@ .erase = erase_49fl00x, .write = write_49fl00x, .read = read_memmapped, + .lock = lock_49fl00x, },
{ @@ -1876,6 +1932,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1892,6 +1949,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1908,6 +1966,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1924,6 +1983,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1940,6 +2000,7 @@ .erase = spi_chip_erase_60, .write = spi_chip_write_1, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1956,6 +2017,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -1972,6 +2034,7 @@ .erase = spi_chip_erase_60_c7, .write = spi_chip_write_1, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2420,6 +2483,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
/* The ST M25P05 is a bit of a problem. It has the same ID as the @@ -2441,6 +2505,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, /* 128 */ .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2457,6 +2522,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
/* The ST M25P10 has the same problem as the M25P05. */ @@ -2474,6 +2540,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_1, /* 128 */ .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2490,6 +2557,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2506,6 +2574,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2522,6 +2591,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2538,6 +2608,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2554,6 +2625,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2570,6 +2642,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2586,6 +2659,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2602,6 +2676,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2954,6 +3029,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2970,6 +3046,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -2986,6 +3063,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -3002,6 +3080,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -3018,6 +3097,7 @@ .erase = spi_chip_erase_c7, .write = spi_chip_write_256, .read = spi_chip_read, + .lock = spi_chip_lock, },
{ @@ -3258,6 +3338,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = NULL, },
{ @@ -3274,6 +3355,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = NULL, },
{ @@ -3290,6 +3372,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = NULL, },
{ @@ -3306,6 +3389,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = NULL, },
{ @@ -3322,6 +3406,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = NULL, },
{ @@ -3338,6 +3423,7 @@ .erase = NULL, .write = NULL, .read = NULL, + .lock = NULL, },
{ Index: flashrom-lock_refactor/spi.c =================================================================== --- flashrom-lock_refactor/spi.c (Revision 769) +++ flashrom-lock_refactor/spi.c (Arbeitskopie) @@ -30,8 +30,6 @@ enum spi_controller spi_controller = SPI_CONTROLLER_NONE; void *spibar = NULL;
-void spi_prettyprint_status_register(struct flashchip *flash); - const struct spi_programmer spi_programmer[] = { { /* SPI_CONTROLLER_NONE */ .command = NULL, @@ -268,11 +266,6 @@ printf_debug("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1, id2);
if (id1 == flash->manufacture_id && id2 == flash->model_id) { - /* Print the status register to tell the - * user about possible write protection. - */ - spi_prettyprint_status_register(flash); - return 1; }
@@ -332,11 +325,6 @@ printf_debug("%s: id1 0x%x, id2 0x%x\n", __func__, id1, id2);
if (id1 == flash->manufacture_id && id2 == flash->model_id) { - /* Print the status register to tell the - * user about possible write protection. - */ - spi_prettyprint_status_register(flash); - return 1; }
@@ -373,10 +361,6 @@ if (id2 != flash->model_id) return 0;
- /* Print the status register to tell the - * user about possible write protection. - */ - spi_prettyprint_status_register(flash); return 1; }
@@ -467,39 +451,76 @@ bpt[(status & 0x1c) >> 2]); }
-void spi_prettyprint_status_register(struct flashchip *flash) +/* + * Return 0 if successful, 1 if failed, 2 if unsupported. + */ +int spi_prettyprint_status_register(struct flashchip *flash) { uint8_t status; + int result = 2;
status = spi_read_status_register(); printf_debug("Chip status register is %02x\n", status); switch (flash->manufacture_id) { case ST_ID: if (((flash->model_id & 0xff00) == 0x2000) || - ((flash->model_id & 0xff00) == 0x2500)) + ((flash->model_id & 0xff00) == 0x2500)) { spi_prettyprint_status_register_st_m25p(status); + result = 0; + } break; case MX_ID: - if ((flash->model_id & 0xff00) == 0x2000) + if ((flash->model_id & 0xff00) == 0x2000) { spi_prettyprint_status_register_st_m25p(status); + result = 0; + } break; case SST_ID: switch (flash->model_id) { case 0x2541: spi_prettyprint_status_register_sst25vf016(status); + result = 0; break; case 0x8d: case 0x258d: spi_prettyprint_status_register_sst25vf040b(status); + result = 0; break; default: spi_prettyprint_status_register_sst25(status); + result = 0; break; } break; } + + if (result == 2) + fprintf(stderr, "Printing lock status not supported.\n"); + return result; }
+/* + * Return 0 if successful, 1 if failed, 2 if unsupported. + */ +int spi_chip_lock(struct flashchip *flash, enum lockaction action) +{ + switch (action) { + case lock_check_unlocked: + fprintf(stderr, "Checking lock status not supported.\n"); + return 2; + case lock_print: + spi_prettyprint_status_register(flash); + return 0; + case lock_disable: + return spi_disable_blockprotect(); + case lock_enable: + fprintf(stderr, "Lock enable not supported.\n"); + return 2; + default: + return 2; + } +} + int spi_chip_erase_60(struct flashchip *flash) { int result; @@ -521,9 +542,9 @@ .readarr = NULL, }}; - result = spi_disable_blockprotect(); + result = spi_chip_lock(flash, lock_disable); if (result) { - fprintf(stderr, "spi_disable_blockprotect failed\n"); + fprintf(stderr, "unprotect failed\n"); return result; } @@ -567,9 +588,9 @@ .readarr = NULL, }};
- result = spi_disable_blockprotect(); + result = spi_chip_lock(flash, lock_disable); if (result) { - fprintf(stderr, "spi_disable_blockprotect failed\n"); + fprintf(stderr, "unprotect failed\n"); return result; }
@@ -687,17 +708,21 @@
int spi_chip_erase_d8(struct flashchip *flash) { - int i, rc = 0; + int i, result = 0; int total_size = flash->total_size * 1024; int erase_size = 64 * 1024;
- spi_disable_blockprotect(); + result = spi_chip_lock(flash, lock_disable); + if (result) { + fprintf(stderr, "unprotect failed\n"); + return result; + }
printf("Erasing chip: \n");
for (i = 0; i < total_size / erase_size; i++) { - rc = spi_block_erase_d8(flash, i * erase_size, erase_size); - if (rc) { + result = spi_block_erase_d8(flash, i * erase_size, erase_size); + if (result) { fprintf(stderr, "Error erasing block at 0x%x\n", i); break; } @@ -705,7 +730,7 @@
printf("\n");
- return rc; + return result; }
/* Sector size is usually 4k, though Macronix eliteflash has 64k */ @@ -983,7 +1008,11 @@ int total_size = 1024 * flash->total_size; int i, result = 0;
- spi_disable_blockprotect(); + result = spi_chip_lock(flash, lock_disable); + if (result) { + fprintf(stderr, "unprotect failed\n"); + return result; + } /* Erase first */ printf("Erasing flash before programming... "); if (erase_flash(flash)) { Index: flashrom-lock_refactor/flashrom.c =================================================================== --- flashrom-lock_refactor/flashrom.c (Revision 769) +++ flashrom-lock_refactor/flashrom.c (Arbeitskopie) @@ -509,6 +509,26 @@ printf("Found chip "%s %s" (%d KB, %s) at physical address 0x%lx.\n", flash->vendor, flash->name, flash->total_size, flashbuses_to_text(flash->bustype), base); + /* Print the locking status. */ + if (flash->lock) { + flash->lock(flash, lock_print); + switch (flash->lock(flash, lock_check_unlocked)) { + case 0: + printf_debug("Chip is completely unlocked or does not " + "support locking."); + break; + case 1: + printf_debug("Chip is partially/completely locked.\n"); + break; + case 2: + printf_debug("Chip locking status could not be " + "determined.\n"); + break; + } + } else { + printf_debug("This chip has not been converted to the new chip " + "locking infrastructure yet.\n"); + }
return flash; }
On pon, 23 lis 2009, Carl-Daniel Hailfinger wrote:
On 20.11.2009 00:05, Maciej Pijanka wrote:
How about adding dummy operation to enum and something like
case dummy_op: default:
in switch clause, first to use 0 value to dummy op, also to make more explicit what default really is.
Adding a dummy enum which does nothing doesn't look like it would help. Apologies if I'm missing your point.
Its maybe a habit that didnt have much reasons to include dummy op when using integer op enumeration. This was only my point, or i already forgot other reason.
Additionally i didn't saw any routine that lets check if current chip has any lock abilities like spi_lock(somechip, lock_query) which would return if any locks are available or something that code may use it to check if locking is available (regardless of presence routine in chip data) for this chip.
Maybe this isn't really needed but we have routine that sets and resets lock, prints info to user but no routine that could poll status of lock or verify if chip has locking abilities at all.
Good point. Updated patch below.
Chip locking has four actions you can do with it:
- Print the current locking status
- Read the current locking status
- Lock the chip
- Unlock the chip.
Currently, the code usually does lock printing inside the probe function, and unlocking somewhere in the erase or write function. Only very few chips reactivate the lock after write/erase. Since many chips have identical probe/write/erase functions, but totally different locking, many such functions have been duplicated needlessly. With this patch, it is possible to call the chip-specific locking functions from probe/write/erase functions and unify lots of code.
While i like different return for fail/success/not supported i think separate return code for unable-to-determine-if-we-failed might be good because that way we let upper layer decide if fail in this case is disaster or not. Also maybe refining in this way could lead that no lock/unlock code need to use fprintf, only use some shared buffer to leave message if any and return code leaving decision how to print/log output to caller code, thus making future libflashrom more flexible (ie, imagine that somebody not redirect stderr stream while link Xapp with libflashrom, then all printed on stderr that he forget to catch otherwise is lost)
Otherwise code looks fine for me, i left two parts of code and comments to them below.
Index: flashrom-lock_refactor/spi.c
--- flashrom-lock_refactor/spi.c (Revision 769) +++ flashrom-lock_refactor/spi.c (Arbeitskopie) @@ -467,39 +451,76 @@ bpt[(status & 0x1c) >> 2]); }
-void spi_prettyprint_status_register(struct flashchip *flash) +/*
- Return 0 if successful, 1 if failed, 2 if unsupported.
- */
+int spi_prettyprint_status_register(struct flashchip *flash) { uint8_t status;
int result = 2;
status = spi_read_status_register(); printf_debug("Chip status register is %02x\n", status); switch (flash->manufacture_id) { case ST_ID: if (((flash->model_id & 0xff00) == 0x2000) ||
((flash->model_id & 0xff00) == 0x2500))
((flash->model_id & 0xff00) == 0x2500)) { spi_prettyprint_status_register_st_m25p(status);
result = 0;
break; case MX_ID:}
if ((flash->model_id & 0xff00) == 0x2000)
if ((flash->model_id & 0xff00) == 0x2000) { spi_prettyprint_status_register_st_m25p(status);
result = 0;
break; case SST_ID: switch (flash->model_id) { case 0x2541: spi_prettyprint_status_register_sst25vf016(status);}
case 0x8d: case 0x258d: spi_prettyprint_status_register_sst25vf040b(status);result = 0; break;
default: spi_prettyprint_status_register_sst25(status);result = 0; break;
} break; }result = 0; break;
- if (result == 2)
fprintf(stderr, "Printing lock status not supported.\n");
- return result;
}
What about default case in code above to detect situation when somebody has not supported yet spi chip and maybe give more informative warning message ?
Index: flashrom-lock_refactor/flashrom.c
--- flashrom-lock_refactor/flashrom.c (Revision 769) +++ flashrom-lock_refactor/flashrom.c (Arbeitskopie) @@ -509,6 +509,26 @@ printf("Found chip "%s %s" (%d KB, %s) at physical address 0x%lx.\n", flash->vendor, flash->name, flash->total_size, flashbuses_to_text(flash->bustype), base);
/* Print the locking status. */
if (flash->lock) {
flash->lock(flash, lock_print);
switch (flash->lock(flash, lock_check_unlocked)) {
case 0:
printf_debug("Chip is completely unlocked or does not "
"support locking.");
break;
case 1:
printf_debug("Chip is partially/completely locked.\n");
break;
case 2:
printf_debug("Chip locking status could not be "
"determined.\n");
break;
}
} else {
printf_debug("This chip has not been converted to the new chip "
"locking infrastructure yet.\n");
}
return flash;
}
In code above i would like to get exact answer is chip unlocked or not support locking or can't query if its locked (and this is feature of hardware not an error etc).
Best regards Maciej