Operate on the Status register

[Niklas Söderlund]

Hi,

I'm interested in reading and writing to the status register in much the 
same way as the ChomiumOS flashrom do with it's --wp-* commands. But in 
a more generic way say:

--sr-read
--sr-write 0xAA

I'm thinking of this implementation:
- Porting ChomiumOS writeprotect.c to flashrom
- Implement a pure read/write of status register
- Change the --wp-* commands to use the status register read/write functions

Is this anything the flashrom community is interested in? I'd love to 
get feedback on this. My knowledge of flashroms structure is not as 
comprehensive yet so suggestions on implementation is appreciated.

The porting of the ChomiumOS flashrom --wp-* is not necessary for my 
goal but I though it might be the right path to go for flashrom, please 
correct me if I'm wrong.

[David Hendricks]

On Mon, Apr 2, 2012 at 1:01 AM, Niklas Söderlund <
[e-mail]> wrote:
> Hi,
>
> I'm interested in reading and writing to the status register in much the
> same way as the ChomiumOS flashrom do with it's --wp-* commands. But in a
> more generic way say:
>
> --sr-read
> --sr-write 0xAA
>
> I'm thinking of this implementation:
> Porting ChomiumOS writeprotect.c to flashrom
> Implement a pure read/write of status register
> Change the --wp-* commands to use the status register read/write
>
> functions
>
> Is this anything the flashrom community is interested in? I'd love to get
> feedback on this. My knowledge of flashroms structure is not as
> comprehensive yet so suggestions on implementation is appreciated.
>
> The porting of the ChomiumOS flashrom --wp-* is not necessary for my goal
> but I though it might be the right path to go for flashrom, please correct
> me if I'm wrong.

I can see how this would be handy for people who really know what they're
doing. OTOH, please be aware that status register layout, especially with
regard to block protection, is rather inconsistent. If you have scripts
that rely on a specific register layout it will likely break when you
switch chips. It would help to know exactly what you're trying to
accomplish so that we can tell if it's really necessary (or even a good
idea) to operate on the raw status register content.

Be advised that some newer SPI chips also have a second status register.
LPC chips have a status register (or at least some do... I'm looking at the
SST49LF016 datasheet), but the information provided is quite a bit
different from what's to be expected on SPI chips.

For the Chrome/Chromium OS branch, we were really only trying to make a
script-able interface for write-protecting regions of 25-series SPI flash.
So for example, a script could write-protect the bottom half the chip by
doing something like: flashrom --wp-range 0 $(($(flashrom --chip-size)/2))
--wp-enable

More food for thought on the subject of write-protection:
http://www.flashrom.org/pipermail/flashrom/2011-February/005795.html . One
issue we found is that 25-series and 26-series SPI chips tend to have
drastically different block protection capabilities. 25-series chips
usually have uniform block protection, where by you block protect 1/Nth the
size of the chip, while 26-series is much more flexible. Supporting all the
different variations presenting them to the user in a sane manner takes
some thought.

RFC: Operate on the Status register

[Niklas Söderlund]

After a discussion on IRC it was brought up that vanilla flashrom in the
past disagreed with the ChromiumOS implementation of the --wp-*
operations. In light of this I have dropped this from my implementation
and only done the parts I'm really interested in, raw access to the
status register.

Please let me know where I can improve it and if there is any interest
at all from flashrom to include such a feature.

[PATCH] Add cli cmds to read/write raw status register values

[Niklas Söderlund]

When working with a lot of different flash chips it's sometimes useful
to be able to print and update the status register directly as a separate
operation.

Add --print-status and --write-status command line options to that maps
to spi_read_status_register and spi_write_status_register.

The print operation reads the status register and prints it content to
the user. While the write operation takes a parameter as a hex value
(0x00 to 0xff) or a decimal value (0 to 255) and tries to write it to
the status register.

This operation is limited to flash chips that have a SPI bustype.

Signed-off-by: Niklas Söderlund 
---
 cli_classic.c |   37 ++++++++++++++++++++++++++++++++-----
 flash.h       |    2 +-
 flashrom.c    |   49 ++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 81 insertions(+), 7 deletions(-)

diff --git a/cli_classic.c b/cli_classic.c
index 972043b..87af1f3 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -144,11 +144,17 @@ static void cli_classic_usage(const char *name)
             "device\n");

list_programmers_linebreak(37, 80, 1);
+
+	printf("\nAdvance options (use at your own risk)\n"
+	       "   -P | --print-status               print status register\n"
+	       "   -W | --write-status <value>       write <value> to status "
+	       "register\n");
+
    printf("\nYou can specify one of -h, -R, -L, "
 #if CONFIG_PRINT_WIKI == 1
             "-z, "
 #endif
-	         "-E, -r, -w, -v or no operation.\n"
+	         "-E, -r, -w, -v -P, -W or no operation.\n"
           "If no operation is specified, flashrom will only probe for "
             "flash chips.\n\n");
 }
@@ -173,15 +179,18 @@ int main(int argc, char *argv[])
    int list_supported_wiki = 0;
 #endif
    int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0;
+	int statusreg_op = 0;
    int dont_verify_it = 0, list_supported = 0, operation_specified = 0;
    enum programmer prog = PROGRAMMER_INVALID;
    int ret = 0;

-	static const char optstring[] = "r:Rw:v:nVEfc:l:i:p:Lzh";
+	static const char optstring[] = "r:Rw:v:nVEfc:l:i:p:LzhPW:";
    static const struct option long_options[] = {
    	{"read",		1, NULL, 'r'},
    	{"write",		1, NULL, 'w'},
    	{"erase",		0, NULL, 'E'},
+		{"print-status",	0, NULL, 'P'},
+		{"write-status",	1, NULL, 'W'},
    	{"verify",		1, NULL, 'v'},
    	{"noverify",		0, NULL, 'n'},
    	{"chip",		1, NULL, 'c'},
@@ -201,6 +210,7 @@ int main(int argc, char *argv[])
    char *layoutfile = NULL;
    char *tempstr = NULL;
    char *pparam = NULL;
+	char *statusreg_value = NULL;

print_version();
    print_banner();
@@ -270,6 +280,23 @@ int main(int argc, char *argv[])
    		}
    		erase_it = 1;
    		break;
+		case 'P':
+			if (++operation_specified > 1) {
+				fprintf(stderr, "More than one operation "
+					"specified. Aborting.\n");
+				cli_classic_abort_usage();
+			}
+			statusreg_op = 1;
+			break;
+		case 'W':
+			if (++operation_specified > 1) {
+				fprintf(stderr, "More than one operation "
+					"specified. Aborting.\n");
+				cli_classic_abort_usage();
+			}
+			statusreg_op = 1;
+			statusreg_value = strdup(optarg);
+			break;
    	case 'f':
    		force = 1;
    		break;
@@ -511,12 +538,12 @@ int main(int argc, char *argv[])
    	goto out_shutdown;
    }

-	if (!(read_it | write_it | verify_it | erase_it)) {
+	if (!(read_it | write_it | verify_it | erase_it | statusreg_op)) {
    	printf("No operations were specified.\n");
    	goto out_shutdown;
    }

-	if (!filename && !erase_it) {
+	if (!filename && !erase_it && !statusreg_op) {
    	printf("Error: No filename specified.\n");
    	ret = 1;
    	goto out_shutdown;
@@ -531,7 +558,7 @@ int main(int argc, char *argv[])
     * Give the chip time to settle.
     */
    programmer_delay(100000);
-	return doit(fill_flash, force, filename, read_it, write_it, erase_it, verify_it);
+	return doit(fill_flash, force, filename, read_it, write_it, erase_it, verify_it, statusreg_op, statusreg_value);

out_shutdown:
    programmer_shutdown();
diff --git a/flash.h b/flash.h
index 0dac13d..65ee0af 100644
--- a/flash.h
+++ b/flash.h
@@ -247,7 +247,7 @@ void print_version(void);
 void print_banner(void);
 void list_programmers_linebreak(int startcol, int cols, int paren);
 int selfcheck(void);
-int doit(struct flashctx *flash, int force, const char *filename, int read_it, int write_it, int erase_it, int verify_it);
+int doit(struct flashctx *flash, int force, const char *filename, int read_it, int write_it, int erase_it, int verify_it, int statureg_op, const char *statusreg_op);
 int read_buf_from_file(unsigned char *buf, unsigned long size, const char *filename);
 int write_buf_to_file(unsigned char *buf, unsigned long size, const char *filename);

diff --git a/flashrom.c b/flashrom.c
index cad043b..b3ba376 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -31,12 +31,14 @@
 #include <stdlib.h>
 #include <ctype.h>
 #include <getopt.h>
+#include <errno.h>
 #if HAVE_UTSNAME == 1
 #include <sys/utsname.h>
 #endif
 #include "flash.h"
 #include "flashchips.h"
 #include "programmer.h"
+#include "chipdrivers.h"

const char flashrom_version[] = FLASHROM_VERSION;
 char *chip_to_probe = NULL;
@@ -1396,6 +1398,45 @@ int erase_and_write_flash(struct flashctx *flash, uint8_t *oldcontents,
    return ret;
 }

+int operate_on_statusregister(struct flashctx *flash, const char* value) {
+	long status;
+	char *endptr;
+
+	if (flash->bustype != BUS_SPI) {
+		msg_cerr("Flash is not on SPI bus. Can't operate on status register\n");
+		return 1;
+	}
+
+	if (value) {
+		/* Try to parse as hex then dec */
+		errno = 0;
+		status = strtol(value, &endptr, 0);
+		if (endptr == value || *endptr != '\0' || errno != 0) {
+			msg_cerr("Error parsing value %s for status register\n",
+					value);
+			return 1;
+		}
+
+		if (status < 0 || status > 0xFF) {
+			msg_cerr("Invalid status register value: %s\n", value);
+			return 1;
+		}
+
+		if (spi_write_status_register(flash, status)) {
+			msg_cerr("spi_write_status_register failed\n");
+			return 1;
+		}
+
+		msg_cinfo("Wrote 0x%02lx to status register\n", status);
+	}
+
+	/* Read and print current value */
+	status = spi_read_status_register(flash);
+	msg_cinfo("Status register: 0x%02lx\n", status);
+
+	return 0;
+}
+
 void nonfatal_help_message(void)
 {
    msg_gerr("Writing to the flash chip apparently didn't do anything.\n"
@@ -1705,7 +1746,8 @@ int chip_safety_check(struct flashctx *flash, int force, int read_it,
  * Besides that, the function itself is a textbook example of abysmal code flow.
  */
 int doit(struct flashctx *flash, int force, const char *filename, int read_it,
-	 int write_it, int erase_it, int verify_it)
+	 int write_it, int erase_it, int verify_it,
+	 int statusreg_op, const char *statusreg_value)
 {
    uint8_t *oldcontents;
    uint8_t *newcontents;
@@ -1718,6 +1760,11 @@ int doit(struct flashctx *flash, int force, const char *filename, int read_it,
    	goto out_nofree;
    }

+	if (statusreg_op) {
+		ret = operate_on_statusregister(flash, statusreg_value);
+		goto out_nofree;
+	}
+
    /* Given the existence of read locks, we want to unlock for read,
     * erase and write.
     */