[Skiboot] [RFC PATCH 3/3] libflash: add flash_erase_corrected() to set ECC bytes in erased areas

[Cyril Bur]

On Fri, 2015-03-06 at 15:48 +1100, Michael Neuling wrote:
> On Thu, 2015-03-05 at 20:39 +1100, Cyril Bur wrote:
> > From: Cyril Bur <cyril.bur at au1.ibm.com>
> > 
> > Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
> > ---
> >  libflash/libflash.c | 32 ++++++++++++++++++++++++++++++++
> >  libflash/libflash.h |  1 +
> >  2 files changed, 33 insertions(+)
> > 
> > diff --git a/libflash/libflash.c b/libflash/libflash.c
> > index cae62ec..84a78f6 100644
> > --- a/libflash/libflash.c
> > +++ b/libflash/libflash.c
> > @@ -309,6 +309,38 @@ int flash_erase(struct flash_chip *c, uint32_t dst, uint32_t size)
> >  	return 0;
> >  }
> >  
> > +int flash_erase_corrected(struct flash_chip *c, uint32_t dst, uint32_t size, bool ecc)
> > +{
> > +
> > +	int rc = flash_erase(c, dst, size);
> > +
> > +	if (rc == 0 && ecc) {
> > +		uint32_t i;
> > +		uint8_t zero = 0;
> > +
> > +		/*
> > +		 * We have just erased this region which means it will be all FF.
> > +		 * The ECC byte for that is 00 so the easiest way to deal is just to
> > +		 * write 00 every 9th byte.
> > +		 * Really should optimise this.
> > +		 */
> > +
> > +		/* Align dst, probably already is but best to be sure */
> > +		if (dst & 0x7) {
> > +			dst &= ~0x7UL;
> > +			dst += 8;
> > +		}
> 
> This alignment doesn't seem like the right thing to do.  If I passed in
> a dst pointer, I wouldn't expect this function to align it for me.  
> 
Hmmmm yes but we still want the ECC bytes 'aligned' (I use the term
loosely). After offline discussion with Mikey, there's no real way to
know where that alignment is so we'll have to trust the caller to get it
right.

> Why not reuse your packed struct ecc64 here again and avoid the magic "+
> 9" maths.

Doh, yeah of course, I'll fix that.
> 
Thanks

Cyril

> Thanks for fixing this.
> 
> Mikey
> 
> > +
> > +		i = 9;
> > +		while (rc == 0 && i < size) {
> > +			rc = flash_write(c, dst + i, &zero, 1, 0);
> > +			i += 9;
> > +		}
> > +	}
> > +
> > +	return rc;
> > +}
> > +
> >  int flash_erase_chip(struct flash_chip *c)
> >  {
> >  	struct spi_flash_ctrl *ct = c->ctrl;
> > diff --git a/libflash/libflash.h b/libflash/libflash.h
> > index 51584da..f7ca205 100644
> > --- a/libflash/libflash.h
> > +++ b/libflash/libflash.h
> > @@ -74,6 +74,7 @@ int flash_read(struct flash_chip *c, uint32_t pos, void *buf, uint32_t len);
> >  int flash_read_corrected(struct flash_chip *c, uint32_t pos, void *buf,
> >  		uint32_t len, bool ecc);
> >  int flash_erase(struct flash_chip *c, uint32_t dst, uint32_t size);
> > +int flash_erase_corrected(struct flash_chip *c, uint32_t dst, uint32_t size, bool ecc);
> >  int flash_write(struct flash_chip *c, uint32_t dst, const void *src,
> >  		uint32_t size, bool verify);
> >  int flash_write_corrected(struct flash_chip *c, uint32_t dst, const void *src,
>