[Skiboot] [PATCH v2 5/6] external/pflash: perform flashing on flash sides other than side 0

[Cyril Bur]

On Fri, 2015-05-22 at 12:55 +1000, Alistair Popple wrote:
> Hi,
> 
> Minor comment below, but it should work fine as it is.
> 
> On Thu, 14 May 2015 12:02:38 Cyril Bur wrote:
> > Currently pflash only looks at the first ffs TOC it finds in order to locate
> > an ffs partition. Problems arise when there are identical partitions in
> > both sides of the flash as it is current impossible to specify the second
> > side. This adds a parameter to specify the second side of the flash when
> > pflash does the lookup for the partition.
> > 
> > Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
> > ---
> >  external/pflash/pflash.c | 56
> > +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 51
> > insertions(+), 5 deletions(-)
> > 
> > diff --git a/external/pflash/pflash.c b/external/pflash/pflash.c
> > index 3fedac0..efcf9ff 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 int flash_side = 0;
> 
> In the next patch you add another global (ffs_toc) to track where the toc 
> should be read from. You could simplify the code by just using flash_side to 
> set ffs_toc as described below.
> 
Since I sent out v3 for your comments to 6/6, I added this too.

Thanks.

> >  #ifdef __powerpc__
> >  static bool using_sfc;
> >  #endif
> > @@ -129,7 +130,7 @@ static void print_flash_info(void)
> >  	print_ffs_info(0);
> >  }
> > 
> > -static void lookup_partition(const char *name)
> > +static int open_partition(const char *name)
> >  {
> >  	uint32_t index;
> >  	int rc;
> > @@ -139,7 +140,7 @@ static void lookup_partition(const char *name)
> >  		rc = ffs_open_flash(fl_chip, 0, 0, &ffsh);
> >  		if (rc) {
> >  			fprintf(stderr, "Error %d opening ffs !\n", rc);
> > -			exit(1);
> > +			return rc;
> >  		}
> >  	}
> > 
> > @@ -147,14 +148,47 @@ static void lookup_partition(const char *name)
> >  	rc = ffs_lookup_part(ffsh, name, &index);
> >  	if (rc == FFS_ERR_PART_NOT_FOUND) {
> >  		fprintf(stderr, "Partition '%s' not found !\n", name);
> > -		exit(1);
> > +		return rc;
> >  	}
> >  	if (rc) {
> >  		fprintf(stderr, "Error %d looking for partition '%s' !\n",
> >  			rc, name);
> > -		exit(1);
> > +		return rc;
> >  	}
> > +
> >  	ffs_index = index;
> > +	return 0;
> > +}
> > +
> > +static void lookup_partition(const char *name)
> > +{
> > +	int rc;
> > +
> > +	if (flash_side == 1) {
> > +		uint32_t other_side_offset;
> > +
> > +		rc = open_partition("OTHER_SIDE");
> > +		if (rc == FFS_ERR_PART_NOT_FOUND)
> > +			fprintf(stderr, "side 1 was specified but there 
> doesn't appear"
> > +					" to be a second side to this 
> flash\n");
> > +		if (rc)
> > +			exit(1);
> > +
> > +		/* Just need to know where it starts */
> > +		rc = ffs_part_info(ffsh, ffs_index, NULL, &other_side_offset, 
> NULL, NULL,
> > NULL); +		if (rc)
> > +			exit(1);
> > +
> > +		ffs_close(ffsh);
> 
> Here you would set ffs_toc = other_side_offset
> 
> > +		rc = ffs_open_flash(fl_chip, other_side_offset, 0, &ffsh);
> > +		if (rc)
> > +			exit(1);
> > +	}
> 
> Then you could remove the above and just call it from one place (in 
> open_partition) with ffs_toc.
> 
> > +	rc = open_partition(name);
> > +	if (rc)
> > +		exit(1);
> >  }
> > 
> >  static void erase_chip(void)
> > @@ -510,6 +544,8 @@ static void print_help(const char *pname)
> >  	printf("\t-t, --tune\n");
> >  	printf("\t\tJust tune the flash controller & access size\n");
> >  	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-i, --info\n");
> >  	printf("\t\tDisplay some information about the flash.\n\n");
> >  	printf("\t-h, --help\n");
> > @@ -550,10 +586,11 @@ int main(int argc, char *argv[])
> >  			{"help",	no_argument,		NULL,	'h'},
> >  			{"version",	no_argument,		NULL,	'v'},
> >  			{"debug",	no_argument,		NULL,	'g'},
> > +			{"side",	required_argument,	NULL,	'S'},
> >  		};
> >  		int c, oidx = 0;
> > 
> > -		c = getopt_long(argc, argv, "a:s:P:r:43Eep:fdihlvbtg",
> > +		c = getopt_long(argc, argv, "a:s:P:r:43Eep:fdihlvbtgS:",
> >  				long_opts, &oidx);
> >  		if (c == EOF)
> >  			break;
> > @@ -615,6 +652,9 @@ int main(int argc, char *argv[])
> >  		case 'g':
> >  			libflash_debug = true;
> >  			break;
> > +		case 'S':
> > +			flash_side = atoi(optarg);
> > +			break;
> >  		default:
> >  			exit(1);
> >  		}
> > @@ -682,6 +722,12 @@ int main(int argc, char *argv[])
> >  		exit(1);
> >  	}
> > 
> > +	/* Explicitly only support two sides */
> > +	if (flash_side != 0 && flash_side != 1) {
> > +		fprintf(stderr, "Unexpected value for --side '%d'\n", 
> flash_side);
> > +		exit(1);
> > +	}
> > +
> >  	/* If file specified but not size, get size from file
> >  	 */
> >  	if (write_file && !write_size) {
>