[Skiboot] [PATCH 2/3] flash: Use blocklevel to do ECC reads

[Alistair Popple]

On Tue, 5 Jul 2016 14:31:03 Cyril Bur wrote:
> On Mon,  4 Jul 2016 17:46:41 +1000
> Michael Neuling <mikey at neuling.org> wrote:
> 
> > From: Alistair Popple <alistair at popple.id.au>
> > 
> > flash_read_corrected() assumes the passed blocklevel device is an
> > actual flash device. However the blocklevel flash abstraction supports
> > automatically reading ECC protected data so use that instead.
> > 
> > Signed-off-by: Alistair Popple <alistair at popple.id.au>
> > Signed-off-by: Michael Neuling <mikey at neuling.org>
> 
> My only concern would be that the blocklevel abstraction introduces a 
slowdown.
> It wasn't written to be quick, simply to provide that abstraction. Mikey 
says
> it performs fine.

It actually just replaces the call to flash_read_corrected() which does 
exactly the same thing as blocklevel_read() except that it also calls 
ecc_protected().

Therefore any slowdown would only be due to the loop in ecc_protected() which 
appears to be bounded by the number of non-contiguous ECC protected flash 
regions. On a Garrison system that amounts to 5 so I'd be surprised if there 
was any significant slowdown there.

However in the ECC case blocklevel_read() mallocs an entire buffer for the 
ECC'd data rather than copying ECC data to the non-ECC buffer blocks at a time 
like flash_read_corrected(), so it may increase memory usage. In practice it 
won't change much though as we don't actually have any large ECC protected 
partitions at present.

- Alistair

> Reviewed-by: Cyril Bur <cyrilbur at gmail.com>
> 
> > ---
> >  core/flash.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/core/flash.c b/core/flash.c
> > index 5036707..81bb59b 100644
> > --- a/core/flash.c
> > +++ b/core/flash.c
> > @@ -573,7 +573,7 @@ static int flash_load_resource(enum resource_id id, 
uint32_t subid,
> >  		goto out_unlock;
> >  	}
> >  
> > -	rc = ffs_init(0, flash->size, flash->bl, &ffs, 0);
> > +	rc = ffs_init(0, flash->size, flash->bl, &ffs, 1);
> >  	if (rc) {
> >  		prerror("FLASH: Can't open ffs handle\n");
> >  		goto out_unlock;
> > @@ -623,7 +623,7 @@ static int flash_load_resource(enum resource_id id, 
uint32_t subid,
> >  		goto out_free_ffs;
> >  	}
> >  
> > -	rc = flash_read_corrected(flash->bl, part_start, buf, size, ecc);
> > +	rc = blocklevel_read(flash->bl, part_start, buf, size);
> >  	if (rc) {
> >  		prerror("FLASH: failed to read %s partition\n", name);
> >  		goto out_free_ffs;
>