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

[Joel Stanley]

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.

>
> 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;