[Skiboot] [PATCH v2 6/6] external/pflash: allow the libffs TOC to operate on to be specified

[Cyril Bur]

On Fri, 2015-05-22 at 13:03 +1000, Alistair Popple wrote:
> Cyril,
> 
> This doesn't appear to do is work with the --info/-i argument, displaying 
> partition info seems to ignore --toc/-T. It would be useful if --info could 
> display the partition table at the given TOC. For example most flash images 
> also have a backup TOC and it would be nice if --info could be used to display 
> that for debugging.
> 

More or less trivial addition, added to v3 I just sent out.

> However that could be left for a future enhancement. I assume it always uses 
> the specified TOC during read/write? ie. pflash -e -T 0x01ff8000 -P PAYLOAD is 
> going to use the TOC at 0x01ff8000 to find the PAYLOAD partition?
> 
Correct, it works as you describe.

Thanks for the review of the series!

> Reviewed-by: Alistair Popple <alistair at popple.id.au>
> 
> On Thu, 14 May 2015 12:02:39 Cyril Bur wrote:
> > On systems with multiple TOCs it may be useful to be able to specify which
> > one should be operated on.
> > 
> > Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
> > ---
> >  external/pflash/pflash.c | 22 ++++++++++++++++++++--
> >  1 file changed, 20 insertions(+), 2 deletions(-)
> > 
> > diff --git a/external/pflash/pflash.c b/external/pflash/pflash.c
> > index efcf9ff..4319321 100644
> > --- a/external/pflash/pflash.c
> > +++ b/external/pflash/pflash.c
> > @@ -29,6 +29,7 @@ static bool must_confirm = true;
> >  static bool dummy_run;
> >  static bool need_relock;
> >  static bool bmc_flash;
> > +static uint32_t ffs_toc = 0;
> >  static int flash_side = 0;
> >  #ifdef __powerpc__
> >  static bool using_sfc;
> > @@ -137,9 +138,12 @@ static int open_partition(const char *name)
> > 
> >  	/* Open libffs if needed */
> >  	if (!ffsh) {
> > -		rc = ffs_open_flash(fl_chip, 0, 0, &ffsh);
> > +		rc = ffs_open_flash(fl_chip, ffs_toc, 0, &ffsh);
> >  		if (rc) {
> >  			fprintf(stderr, "Error %d opening ffs !\n", rc);
> > +			if (ffs_toc)
> > +				fprintf(stderr, "You specified 0x%08x as the 
> libffs TOC"
> > +						" looks like it doesn't 
> exist\n", ffs_toc);
> >  			return rc;
> >  		}
> >  	}
> > @@ -546,6 +550,9 @@ static void print_help(const char *pname)
> >  	printf("\t\t(Implicit for all other operations)\n\n");
> >  	printf("\t-S, --side\n");
> >  	printf("\t\tSide of the flash on which to operate, 0 (default) or 
> 1\n\n");
> > +	printf("\t-T, --toc\n");
> > +	printf("\t\tlibffs TOC on which to operate, defaults to 0.\n");
> > +	printf("\t\tleading 0x is required for interpretation of a hex
> > value\n\n"); printf("\t-i, --info\n");
> >  	printf("\t\tDisplay some information about the flash.\n\n");
> >  	printf("\t-h, --help\n");
> > @@ -564,6 +571,7 @@ int main(int argc, char *argv[])
> >  	bool has_sfc = false, has_ast = false;
> >  	bool no_action = false, tune = false;
> >  	char *write_file = NULL, *read_file = NULL, *part_name = NULL;
> > +	bool ffs_toc_seen = false;
> >  	int rc;
> > 
> >  	while(1) {
> > @@ -587,10 +595,11 @@ int main(int argc, char *argv[])
> >  			{"version",	no_argument,		NULL,	'v'},
> >  			{"debug",	no_argument,		NULL,	'g'},
> >  			{"side",	required_argument,	NULL,	'S'},
> > +			{"toc",		required_argument,	NULL,	'T'},
> >  		};
> >  		int c, oidx = 0;
> > 
> > -		c = getopt_long(argc, argv, "a:s:P:r:43Eep:fdihlvbtgS:",
> > +		c = getopt_long(argc, argv, "a:s:P:r:43Eep:fdihlvbtgS:T:",
> >  				long_opts, &oidx);
> >  		if (c == EOF)
> >  			break;
> > @@ -655,6 +664,10 @@ int main(int argc, char *argv[])
> >  		case 'S':
> >  			flash_side = atoi(optarg);
> >  			break;
> > +		case 'T':
> > +			ffs_toc_seen = true;
> > +			ffs_toc = strtoul(optarg, NULL, 0);
> > +			break;
> >  		default:
> >  			exit(1);
> >  		}
> > @@ -728,6 +741,11 @@ int main(int argc, char *argv[])
> >  		exit(1);
> >  	}
> > 
> > +	if (ffs_toc_seen && flash_side) {
> > +		fprintf(stderr, "--toc and --side are exclusive");
> > +		exit(1);
> > +	}
> > +
> >  	/* If file specified but not size, get size from file
> >  	 */
> >  	if (write_file && !write_size) {
>