Re: [Skiboot] [PATCH v4] pflash option to retrieve PNOR partition flags

[Stewart Smith <stewart@linux.vnet.ibm.com>]

Michael Tritz <mtritz@us.ibm.com> writes:
> This commit extends pflash with an option to retrieve and print
> information for a particular partition, including the content from
> "pflash -i" and a verbose list of set miscellaneous flags. -i option
> is also updated to print a short list of flags in addition to the
> ECC flag, with one character per flag. A test of the new option is
> included in libflash/test.
>
> Change-Id: Iebb8a6d34c537cecd2eb44ddf41271c8fbe25258
> Signed-off-by: Michael Tritz <mtritz@us.ibm.com>
> ---
>  external/pflash/pflash.c          | 134 ++++++++++++++++++++++++++++++++++----
>  libflash/libffs.c                 |  19 +++---
>  libflash/libffs.h                 |   8 ++-
>  libflash/test/test-miscprint.pnor | 103 +++++++++++++++++++++++++++++
>  libflash/test/test-miscprint.sh   |  25 +++++++
>  5 files changed, 269 insertions(+), 20 deletions(-)
>  create mode 100644 libflash/test/test-miscprint.pnor
>  create mode 100644 libflash/test/test-miscprint.sh

I think this looks mostly good (a few comments below), one thing is taht
the test-miscprint test isn't run by the 'make libflash-check' target,
so it'll have to be added to libflash/test/Makefile.check so that we can run it
automatically.

Although... there's libflash versus pflash tests too... I know Cyril has
been working on some pflash tests, but the libflash ones just need to
call the API to do the tests.

> @@ -103,25 +106,40 @@ static void print_ffs_info(uint32_t toc_offset)
>  		return;
>  	}
>  
> -	for (i = 0;; i++) {
> +	for (i = 0; rc == 0; i++) {
>  		uint32_t start, size, act, end;
> -		bool ecc;
> -		char *name;
> +		char *name = NULL, *flags;
> +		int l;
>  
> -		rc = ffs_part_info(ffs_handle, i, &name, &start, &size, &act, &ecc);
> +		rc = ffs_part_info(ffs_handle, i, &name, &start, &size, &act, NULL);
>  		if (rc == FFS_ERR_PART_NOT_FOUND)
>  			break;
> -		if (rc) {
> -			fprintf(stderr, "Error %d scanning partitions\n", rc);
> -			break;
> +
> +        ent = ffs_entry_get(ffs_handle, i);

Your indenting here is off.


-- 
Stewart Smith
OPAL Architect, IBM.