[Skiboot] [PATCH 10/16] libffs: Add ffs_flash_read()

Benjamin Herrenschmidt benh at kernel.crashing.org
Fri Feb 20 15:06:13 AEDT 2015


On Fri, 2015-02-20 at 14:27 +1030, Joel Stanley wrote:
> On Fri, Feb 20, 2015 at 11:07 AM, Michael Neuling <mikey at neuling.org> wrote:
> > Add ffs_flash_read() which mimics flash_read() but handles ECC checking,
> > correction and uncorrectable errors.
> 
> Shouldn't this be in libflash? It doesn't take a struct ffs_handle,
> and doesn't need anything in libffs.h afaict.
> 
> The implementation looks okay. A comment about block size aligned reads below.

That or in a separate libflash_ecc ...

Cheers,
Ben.

> >
> > Signed-off-by: Michael Neuling <mikey at neuling.org>
> > ---
> >  libflash/libffs.c   | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  libflash/libffs.h   |  4 ++++
> >  libflash/libflash.h |  1 +
> >  3 files changed, 57 insertions(+)
> >
> > diff --git a/libflash/libffs.c b/libflash/libffs.c
> > index cfd5456..c28fd66 100644
> > --- a/libflash/libffs.c
> > +++ b/libflash/libffs.c
> 
> > +int ffs_flash_read(struct flash_chip *c, uint32_t pos, void *buf, uint32_t len,
> > +                  bool ecc)
> > +{
> > +       uint64_t *bufecc;
> > +       uint32_t copylen;
> > +       int rc;
> > +       uint8_t ret;
> > +
> > +       if (!ecc)
> > +               return flash_read(c, pos, buf, len);
> > +
> > +       /* Copy the buffer in chunks */
> > +       bufecc = malloc(ECC_BUFFER_SIZE(COPY_BUFFER_LENGTH));
> > +       if (!bufecc)
> > +               return FLASH_ERR_MALLOC_FAILED;
> > +
> > +       while (len > 0) {
> > +               /* What's left to copy? */
> > +               copylen = MIN(len, COPY_BUFFER_LENGTH);
> > +
> 
> Is there any code to ensure that copylen is going to be flash page
> size aligned? Similar with pos.
> 
> > +               /* Read ECCed data from flash */
> > +               rc = flash_read(c, pos, bufecc, ECC_BUFFER_SIZE(copylen));
> > +               if (rc)
> > +                       goto err;




More information about the Skiboot mailing list