[Skiboot] [RFC] libflash/libffs: decouple libflash and libffs.
Cyril Bur
cyrilbur at gmail.com
Fri Apr 10 15:33:26 AEST 2015
I've had thoughts and to get the ball rolling...
On Wed, 2015-04-08 at 11:19 +1000, Cyril Bur wrote:
> Currently libffs uses libflash to read data. This does works well if the
> data is directly on flash hardware that libflash is designed to read from.
> Problems arise when the data is not on an actual flash hardware but libffs is
> needed to parse ffs partition data.
>
> This patch begins the work to decouple libffs and libflash as well as starting
> the work to provide an abstraction so that libffs may eventually be able to
> read its data from essentially any source.
>
> ---
> V2: Added Alistairs feedback.
> Modified core/flash.c to use the new 'highlevel' interface
> Modified pflash to use the new 'highlevel' interface
> V3: Changed everything to add the container_of stuff. Looks good!
> V4: Very minor modifications. Made the tests work.
>
> Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
> ---
> core/flash.c | 34 ++++++++++--------
> external/pflash/Makefile | 2 +-
> external/pflash/ast.h | 1 +
> external/pflash/pflash.c | 33 +++++++++---------
> libflash/highlevel.c | 51 +++++++++++++++++++++++++++
> libflash/highlevel.h | 38 ++++++++++++++++++++
> libflash/libffs.c | 51 +++++++++++----------------
> libflash/libffs.h | 7 ++--
> libflash/libflash.c | 83 +++++++++++++++++++++++++++-----------------
> libflash/libflash.h | 25 +++++--------
> libflash/test/Makefile.check | 1 +
> libflash/test/test-flash.c | 20 +++++------
> platforms/astbmc/pnor.c | 12 ++++---
> platforms/rhesus/rhesus.c | 10 +++---
> 14 files changed, 235 insertions(+), 133 deletions(-)
> create mode 100644 libflash/highlevel.c
> create mode 100644 libflash/highlevel.h
>
> diff --git a/core/flash.c b/core/flash.c
> index 7d32c8d..3dc0a0d 100644
> --- a/core/flash.c
> +++ b/core/flash.c
> @@ -21,12 +21,13 @@
> #include <device.h>
> #include <libflash/libflash.h>
> #include <libflash/libffs.h>
> +#include <libflash/highlevel.h>
> #include <ecc.h>
>
> struct flash {
> bool registered;
> bool busy;
> - struct flash_chip *chip;
> + struct highlevel_ops *ops;
> uint32_t size;
> uint32_t block_size;
> };
> @@ -107,7 +108,7 @@ static int flash_nvram_start_read(void *dst, uint32_t src, uint32_t len)
> goto out;
> }
>
> - rc = flash_read(nvram_flash->chip, nvram_offset + src, dst, len);
> + rc = highlevel_read(nvram_flash->ops, nvram_offset + src, len, dst);
>
> out:
> unlock(&flash_lock);
> @@ -136,7 +137,7 @@ static int flash_nvram_write(uint32_t dst, void *src, uint32_t len)
> rc = OPAL_PARAMETER;
> goto out;
> }
> - rc = flash_smart_write(nvram_flash->chip, nvram_offset + dst, src, len);
> + rc = highlevel_write(nvram_flash->ops, nvram_offset + dst, len, src);
>
> out:
> unlock(&flash_lock);
> @@ -249,7 +250,7 @@ static void setup_system_flash(struct flash *flash, struct dt_node *node,
> flash_nvram_probe(flash, ffs);
> }
>
> -int flash_register(struct flash_chip *chip, bool is_system_flash)
> +int flash_register(struct highlevel_ops *hl, bool is_system_flash)
> {
> uint32_t size, block_size;
> struct ffs_handle *ffs;
> @@ -259,7 +260,7 @@ int flash_register(struct flash_chip *chip, bool is_system_flash)
> unsigned int i;
> int rc;
>
> - rc = flash_get_info(chip, &name, &size, &block_size);
> + rc = highlevel_get_info(hl, &name, &size, &block_size);
> if (rc)
> return rc;
>
> @@ -275,7 +276,7 @@ int flash_register(struct flash_chip *chip, bool is_system_flash)
> flash = &flashes[i];
> flash->registered = true;
> flash->busy = false;
> - flash->chip = chip;
> + flash->funcs = hl;
> flash->size = size;
> flash->block_size = block_size;
> break;
> @@ -287,7 +288,7 @@ int flash_register(struct flash_chip *chip, bool is_system_flash)
> return OPAL_RESOURCE;
> }
>
> - rc = ffs_open_flash(chip, 0, flash->size, &ffs);
> + rc = ffs_init(0, flash->size, hl, &ffs);
> if (rc) {
> prlog(PR_WARNING, "FLASH: No ffs info; "
> "using raw device only\n");
> @@ -345,13 +346,17 @@ static int64_t opal_flash_op(enum flash_op op, uint64_t id, uint64_t offset,
>
> switch (op) {
> case FLASH_OP_READ:
> - rc = flash_read(flash->chip, offset, (void *)buf, size);
> + rc = highlevel_read(flash->ops, offset, size, (void *)buf);
> break;
> case FLASH_OP_WRITE:
> - rc = flash_write(flash->chip, offset, (void *)buf, size, false);
> + /*
> + * Note: highlevel_write() uses flash_smart_write(), this call used to
> + * be flash_write()
> + */
> + rc = highlevel_write(flash->ops, offset, size, (void *)buf);
> break;
> case FLASH_OP_ERASE:
> - rc = flash_erase(flash->chip, offset, size);
> + rc = highlevel_erase(flash->ops, offset, size);
> break;
> default:
> assert(0);
> @@ -424,10 +429,11 @@ struct flash_hostboot_header {
> };
>
> /* start and total size include ECC */
> -static int flash_find_subpartition(struct flash_chip *chip, uint32_t subid,
> +static int flash_find_subpartition(struct highlevel_ops *funcs, uint32_t subid,
> uint32_t *start, uint32_t *total_size,
> bool *ecc)
> {
> + struct flash_chip *chip;
> struct flash_hostboot_header *header;
> char eyecatcher[5];
> uint32_t i, partsize;
> @@ -563,7 +569,7 @@ int flash_load_resource(enum resource_id id, uint32_t subid,
> goto out_unlock;
> }
>
> - rc = ffs_open_flash(flash->chip, 0, flash->size, &ffs);
> + rc = ffs_init(0, flash->size, flash->funcs, &ffs);
> if (rc) {
> prerror("FLASH: Can't open ffs handle\n");
> goto out_unlock;
> @@ -590,7 +596,7 @@ int flash_load_resource(enum resource_id id, uint32_t subid,
>
> /* Find the sub partition if required */
> if (subid != RESOURCE_SUBID_NONE) {
> - rc = flash_find_subpartition(flash->chip, subid, &part_start,
> + rc = flash_find_subpartition(flash->funcs, subid, &part_start,
> &part_size, &ecc);
> if (rc)
> goto out_free_ffs;
> @@ -613,7 +619,7 @@ int flash_load_resource(enum resource_id id, uint32_t subid,
> goto out_free_ffs;
> }
>
> - rc = flash_read_corrected(flash->chip, part_start, buf, size, ecc);
> + rc = flash_read_corrected(flash->funcs, part_start, buf, size, ecc);
> if (rc) {
> prerror("FLASH: failed to read %s partition\n", name);
> goto out_free_ffs;
> diff --git a/external/pflash/Makefile b/external/pflash/Makefile
> index c28e359..f806b34 100644
> --- a/external/pflash/Makefile
> +++ b/external/pflash/Makefile
> @@ -14,7 +14,7 @@ endif
> CFLAGS = -O2 -Wall -I.
> LDFLAGS = -lrt
> OBJS = pflash.o progress.o ast-sf-ctrl.o
> -OBJS += libflash/libflash.o libflash/libffs.o libflash/ecc.o
> +OBJS += libflash/libflash.o libflash/libffs.o libflash/ecc.o libflash/highlevel.o
> OBJS += $(ARCH_OBJS)
> EXE = pflash
>
> diff --git a/external/pflash/ast.h b/external/pflash/ast.h
> index 92cafa4..3789b39 100644
> --- a/external/pflash/ast.h
> +++ b/external/pflash/ast.h
> @@ -57,6 +57,7 @@ void ast_io_init(void);
> */
> #define AST_SF_TYPE_PNOR 0
> #define AST_SF_TYPE_BMC 1
> +#define AST_SF_TYPE_MEM 2
>
This hunk should probably be a separate patch... its not really related
to what I'm doing, I think I'll post it separately.
> struct spi_flash_ctrl;
> int ast_sf_open(uint8_t type, struct spi_flash_ctrl **ctrl);
> diff --git a/external/pflash/pflash.c b/external/pflash/pflash.c
> index fb783a2..2eb3164 100644
> --- a/external/pflash/pflash.c
> +++ b/external/pflash/pflash.c
> @@ -16,6 +16,7 @@
>
> #include <libflash/libflash.h>
> #include <libflash/libffs.h>
> +#include <libflash/highlevel.h>
> #include "progress.h"
> #include "io.h"
> #include "ast.h"
> @@ -23,7 +24,7 @@
>
> #define __aligned(x) __attribute__((aligned(x)))
>
> -#define PFLASH_VERSION "0.8.6"
> +#define PFLASH_VERSION "0.8.7"
>
Probably should get some consensus here...
> static bool must_confirm = true;
> static bool dummy_run;
> @@ -36,8 +37,8 @@ static bool using_sfc;
> #define FILE_BUF_SIZE 0x10000
> static uint8_t file_buf[FILE_BUF_SIZE] __aligned(0x1000);
>
> +static struct highlevel_funcs *hl;
> static struct spi_flash_ctrl *fl_ctrl;
> -static struct flash_chip *fl_chip;
> static struct ffs_handle *ffsh;
> static uint32_t fl_total_size, fl_erase_granule;
> static const char *fl_name;
> @@ -84,7 +85,7 @@ static void print_flash_info(void)
> return;
>
> if (!ffsh) {
> - rc = ffs_open_flash(fl_chip, 0, 0, &ffsh);
> + rc = ffs_init(0, fl_total_size, hl, &ffsh);
> if (rc) {
> fprintf(stderr, "Error %d opening ffs !\n", rc);
> ffsh = NULL;
> @@ -122,7 +123,7 @@ static void lookup_partition(const char *name)
>
> /* Open libffs if needed */
> if (!ffsh) {
> - rc = ffs_open_flash(fl_chip, 0, 0, &ffsh);
> + rc = ffs_init(0, fl_total_size, hl, &ffsh);
> if (rc) {
> fprintf(stderr, "Error %d opening ffs !\n", rc);
> exit(1);
> @@ -158,7 +159,7 @@ static void erase_chip(void)
> return;
> }
>
> - rc = flash_erase_chip(fl_chip);
> + rc = flash_erase_chip(hl);
> if (rc) {
> fprintf(stderr, "Error %d erasing chip\n", rc);
> exit(1);
> @@ -185,7 +186,7 @@ static void erase_range(uint32_t start, uint32_t size, bool will_program)
> while(size) {
> /* If aligned to 64k and at least 64k, use 64k erase */
> if ((start & 0xffff) == 0 && size >= 0x10000) {
> - rc = flash_erase(fl_chip, start, 0x10000);
> + rc = highlevel_erase(hl, start, 0x10000);
> if (rc) {
> fprintf(stderr, "Error %d erasing 0x%08x\n",
> rc, start);
> @@ -195,7 +196,7 @@ static void erase_range(uint32_t start, uint32_t size, bool will_program)
> size -= 0x10000;
> done += 0x10000;
> } else {
> - rc = flash_erase(fl_chip, start, 0x1000);
> + rc = highlevel_erase(hl, start, 0x1000);
> if (rc) {
> fprintf(stderr, "Error %d erasing 0x%08x\n",
> rc, start);
> @@ -252,7 +253,7 @@ static void program_file(const char *file, uint32_t start, uint32_t size)
> len = size;
> size -= len;
> actual_size += len;
> - rc = flash_write(fl_chip, start, file_buf, len, true);
> + rc = highlevel_write(hl, start, len, file_buf);
> if (rc) {
> if (rc == FLASH_ERR_VERIFY_FAILURE)
> fprintf(stderr, "Verification failed for"
> @@ -292,7 +293,7 @@ static void do_read_file(const char *file, uint32_t start, uint32_t size)
> progress_init(size >> 8);
> while(size) {
> len = size > FILE_BUF_SIZE ? FILE_BUF_SIZE : size;
> - rc = flash_read(fl_chip, start, file_buf, len);
> + rc = highlevel_read(hl, start, len, file_buf);
> if (rc) {
> fprintf(stderr, "Flash read error %d for"
> " chunk at 0x%08x\n", rc, start);
> @@ -318,7 +319,7 @@ static void enable_4B_addresses(void)
>
> printf("Switching to 4-bytes address mode\n");
>
> - rc = flash_force_4b_mode(fl_chip, true);
> + rc = flash_force_4b_mode(hl, true);
> if (rc) {
> fprintf(stderr, "Error %d enabling 4b mode\n", rc);
> exit(1);
> @@ -331,7 +332,7 @@ static void disable_4B_addresses(void)
>
> printf("Switching to 3-bytes address mode\n");
>
> - rc = flash_force_4b_mode(fl_chip, false);
> + rc = flash_force_4b_mode(hl, false);
> if (rc) {
> fprintf(stderr, "Error %d disabling 4b mode\n", rc);
> exit(1);
> @@ -342,7 +343,7 @@ static void flash_access_cleanup_bmc(void)
> {
> if (ffsh)
> ffs_close(ffsh);
> - flash_exit(fl_chip);
> + flash_exit(hl);
> ast_sf_close(fl_ctrl);
> close_devs();
> }
> @@ -362,7 +363,7 @@ static void flash_access_setup_bmc(bool use_lpc, bool need_write)
> }
>
> /* Open flash chip */
> - rc = flash_init(fl_ctrl, &fl_chip);
> + rc = flash_init(fl_ctrl, &hl);
> if (rc) {
> fprintf(stderr, "Failed to open flash chip\n");
> exit(1);
> @@ -380,7 +381,7 @@ static void flash_access_cleanup_pnor(void)
>
> if (ffsh)
> ffs_close(ffsh);
> - flash_exit(fl_chip);
> + flash_exit(hl);
> #ifdef __powerpc__
> if (using_sfc)
> sfc_close(fl_ctrl);
> @@ -421,7 +422,7 @@ static void flash_access_setup_pnor(bool use_lpc, bool use_sfc, bool need_write)
> #endif
>
> /* Open flash chip */
> - rc = flash_init(fl_ctrl, &fl_chip);
> + rc = flash_init(fl_ctrl, &hl);
> if (rc) {
> fprintf(stderr, "Failed to open flash chip\n");
> exit(1);
> @@ -698,7 +699,7 @@ int main(int argc, char *argv[])
> flash_access_setup_pnor(use_lpc, has_sfc, erase || program);
> }
>
> - rc = flash_get_info(fl_chip, &fl_name,
> + rc = flash_get_info(hl, &fl_name,
> &fl_total_size, &fl_erase_granule);
> if (rc) {
> fprintf(stderr, "Error %d getting flash info\n", rc);
> diff --git a/libflash/highlevel.c b/libflash/highlevel.c
> new file mode 100644
> index 0000000..e29020f
> --- /dev/null
> +++ b/libflash/highlevel.c
> @@ -0,0 +1,51 @@
> +/* Copyright 2013-2014 IBM Corp.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> + * implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include "highlevel.h"
> +
> +int highlevel_read(struct highlevel_funcs *funcs, uint32_t pos, uint32_t len, void *buf)
> +{
> + if (!funcs || !funcs->read || !buf)
> + return -1;
> +
> + return funcs->read(funcs, pos, len, buf);
> +}
> +
> +int highlevel_write(struct highlevel_funcs *funcs, uint32_t pos, uint32_t len, const void *buf)
> +{
> + if (!funcs || !funcs->write || !buf)
> + return -1;
> +
> + return funcs->write(funcs, pos, len, buf);
> +}
> +
> +int highlevel_erase(struct highlevel_funcs *funcs, uint32_t pos, uint32_t len)
> +{
> + if (!funcs || !funcs->erase)
> + return -1;
> +
> + return funcs->erase(funcs, pos, len);
> +}
> +
> +int highlevel_get_info(struct highlevel_funcs *funcs, const char **name, uint32_t *total_size, uint32_t *erase_granule)
> +{
> + if (!funcs || !funcs->get_info)
> + return -1;
> +
> + return funcs->get_info(funcs, name, total_size, erase_granule);
> +}
> diff --git a/libflash/highlevel.h b/libflash/highlevel.h
> new file mode 100644
> index 0000000..177ef4e
> --- /dev/null
> +++ b/libflash/highlevel.h
> @@ -0,0 +1,38 @@
> +/* Copyright 2013-2014 IBM Corp.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> + * implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +#ifndef __LIBFLASH_HIGHLEVEL_H
> +#define __LIBFLASH_HIGHLEVEL_H
> +
> +#include <stdint.h>
> +
> +/*
> + * libffs may be used with different backends, all should provide these for
> + * libflash to get the information it needs
> + */
> +struct highlevel_ops {
> + void *priv;
> + int (*read)(struct highlevel_ops *hl, uint32_t pos, uint32_t len, void *buf);
> + int (*write)(struct highlevel_ops *hl, uint32_t pos, uint32_t len, const void *buf);
> + int (*erase)(struct highlevel_ops *hl, uint32_t pos, uint32_t len);
> + int (*get_info)(struct highlevel_ops *hl, const char **name, uint32_t *total_size, uint32_t *erase_granule);
> +};
> +
> +int highlevel_read(struct highlevel_ops *funcs, uint32_t pos, uint32_t len, void *buf);
> +int highlevel_write(struct highlevel_ops *funcs, uint32_t pos, uint32_t len, const void *buf);
> +int highlevel_erase(struct highlevel_ops *funcs, uint32_t pos, uint32_t len);
> +int highlevel_get_info(struct highlevel_ops *funcs, const char **name, uint32_t *total_size, uint32_t *erase_granule);
> +
> +#endif /* __LIBFLASH_HIGHLEVEL_H */
> diff --git a/libflash/libffs.c b/libflash/libffs.c
> index 2d05cc9..4d8cbf9 100644
> --- a/libflash/libffs.c
> +++ b/libflash/libffs.c
> @@ -41,6 +41,7 @@ struct ffs_handle {
> uint32_t max_size;
> void *cache;
> uint32_t cached_size;
> + struct highlevel_ops *ops;
> };
>
> static uint32_t ffs_checksum(void* data, size_t size)
> @@ -71,31 +72,22 @@ static int ffs_check_convert_header(struct ffs_hdr *dst, struct ffs_hdr *src)
> return 0;
> }
>
> -int ffs_open_flash(struct flash_chip *chip, uint32_t offset,
> - uint32_t max_size, struct ffs_handle **ffs)
> +int ffs_init(uint32_t offset, uint32_t max_size, struct highlevel_ops *hl,
> + struct ffs_handle **ffs)
> {
> struct ffs_hdr hdr;
> struct ffs_handle *f;
> - uint32_t fl_size, erase_size;
> int rc;
>
> - if (!ffs)
> + if (!ffs || !funcs || !funcs->read || !funcs->write || !funcs->erase)
> return FLASH_ERR_PARM_ERROR;
> *ffs = NULL;
>
> - /* Grab some info about our flash chip */
> - rc = flash_get_info(chip, NULL, &fl_size, &erase_size);
> - if (rc) {
> - FL_ERR("FFS: Error %d retrieving flash info\n", rc);
> - return rc;
> - }
> - if ((offset + max_size) < offset)
> - return FLASH_ERR_PARM_ERROR;
> - if ((offset + max_size) > fl_size)
> + if ((offset + max_size) < offset || (offset + max_size) > funcs->total_size)
> return FLASH_ERR_PARM_ERROR;
>
> /* Read flash header */
> - rc = flash_read(chip, offset, &hdr, sizeof(hdr));
> + rc = highlevel_read(hl, offset, sizeof(hdr), &hdr);
> if (rc) {
> FL_ERR("FFS: Error %d reading flash header\n", rc);
> return rc;
> @@ -106,17 +98,15 @@ int ffs_open_flash(struct flash_chip *chip, uint32_t offset,
> if (!f)
> return FLASH_ERR_MALLOC_FAILED;
> memset(f, 0, sizeof(*f));
> - f->type = ffs_type_flash;
> f->flash_offset = offset;
> - f->max_size = max_size ? max_size : (fl_size - offset);
> - f->chip = chip;
> + f->max_size = max_size;
> + f->funcs = funcs;
>
> /* Convert and check flash header */
> rc = ffs_check_convert_header(&f->hdr, &hdr);
> if (rc) {
> FL_ERR("FFS: Error %d checking flash header\n", rc);
> - free(f);
> - return rc;
> + goto out;
> }
>
> /*
> @@ -126,26 +116,24 @@ int ffs_open_flash(struct flash_chip *chip, uint32_t offset,
> f->cached_size = f->hdr.block_size * f->hdr.size;
> FL_DBG("FFS: Partition map size: 0x%x\n", f->cached_size);
>
> - /* Align to erase size */
> - f->cached_size |= (erase_size - 1);
> - f->cached_size &= ~(erase_size - 1);
> - FL_DBG("FFS: Aligned to: 0x%x\n", f->cached_size);
> -
> /* Allocate cache */
> f->cache = malloc(f->cached_size);
> if (!f->cache) {
> - free(f);
> - return FLASH_ERR_MALLOC_FAILED;
> + rc = FLASH_ERR_MALLOC_FAILED;
> + goto out;
> }
>
> /* Read the cached map */
> - rc = flash_read(chip, offset, f->cache, f->cached_size);
> - if (rc) {
> + rc = highlevel_read(hl, offset, f->cached_size, f->cache);
> + if (rc)
> FL_ERR("FFS: Error %d reading flash partition map\n", rc);
> - free(f);
> - }
> +
> +out:
> if (rc == 0)
> *ffs = f;
> + else
> + free(f);
> +
> return rc;
> }
>
> @@ -361,6 +349,7 @@ int ffs_update_act_size(struct ffs_handle *ffs, uint32_t part_idx,
> ent->checksum = ffs_checksum(ent, FFS_ENTRY_SIZE_CSUM);
> if (!ffs->chip)
> return 0;
> - return flash_smart_write(ffs->chip, offset, ent, FFS_ENTRY_SIZE);
> +
> + return highlevel_write(ffs->ops, offset, FFS_ENTRY_SIZE, ent);
> }
>
> diff --git a/libflash/libffs.h b/libflash/libffs.h
> index dd58d28..0fabe7f 100644
> --- a/libflash/libffs.h
> +++ b/libflash/libffs.h
> @@ -18,6 +18,7 @@
>
> #include <libflash/libflash.h>
> #include <libflash/ffs.h>
> +#include <libflash/highlevel.h>
>
> /* FFS handle, opaque */
> struct ffs_handle;
> @@ -34,8 +35,10 @@ struct ffs_handle;
> #define FFS_ERR_PART_NOT_FOUND 103
> #define FFS_ERR_BAD_ECC 104
>
> -int ffs_open_flash(struct flash_chip *chip, uint32_t offset,
> - uint32_t max_size, struct ffs_handle **ffs);
> +/* Init */
> +
> +int ffs_init(uint32_t offset, uint32_t max_size,
> + struct highlevel_funcs *ops, struct ffs_handle **ffs);
>
> /* ffs_open_image is Linux only as it uses lseek, which skiboot does not
> * implement */
> diff --git a/libflash/libflash.c b/libflash/libflash.c
> index a81ff4d..3303bb4 100644
> --- a/libflash/libflash.c
> +++ b/libflash/libflash.c
> @@ -20,6 +20,7 @@
> #include "libflash.h"
> #include "libflash-priv.h"
> #include "ecc.h"
> +#include "highlevel.h"
>
> #ifndef MIN
> #define MIN(a, b) ((a) < (b) ? (a) : (b))
> @@ -45,6 +46,7 @@ struct flash_chip {
> bool mode_4b; /* Flash currently in 4b mode */
> struct flash_req *cur_req; /* Current request */
> void *smart_buf; /* Buffer for smart writes */
> + struct highlevel_ops ops;
> };
>
> #ifndef __SKIBOOT__
> @@ -116,8 +118,9 @@ int fl_wren(struct spi_flash_ctrl *ct)
> return FLASH_ERR_WREN_TIMEOUT;
> }
>
> -int flash_read(struct flash_chip *c, uint32_t pos, void *buf, uint32_t len)
> +static int flash_read(struct highlevel_ops *hl, uint32_t pos, uint32_t len, void *buf)
> {
> + struct flash_chip *c = container_of(hl, struct flash_chip, ops);
> struct spi_flash_ctrl *ct = c->ctrl;
>
> /* XXX Add sanity/bound checking */
> @@ -142,7 +145,7 @@ int flash_read(struct flash_chip *c, uint32_t pos, void *buf, uint32_t len)
> * This provides a wrapper around flash_read on ECCed data
> * len is length of data without ECC attached
> */
> -int flash_read_corrected(struct flash_chip *c, uint32_t pos, void *buf,
> +int flash_read_corrected(struct highlevel_ops *hl, uint32_t pos, void *buf,
> uint32_t len, bool ecc)
> {
> struct ecc64 *bufecc;
> @@ -151,7 +154,7 @@ int flash_read_corrected(struct flash_chip *c, uint32_t pos, void *buf,
> uint8_t ret;
>
> if (!ecc)
> - return flash_read(c, pos, buf, len);
> + return flash_read(hl, pos, len, buf);
>
> /* Copy the buffer in chunks */
> bufecc = malloc(ECC_BUFFER_SIZE(COPY_BUFFER_LENGTH));
> @@ -163,7 +166,7 @@ int flash_read_corrected(struct flash_chip *c, uint32_t pos, void *buf,
> copylen = MIN(len, COPY_BUFFER_LENGTH);
>
> /* Read ECCed data from flash */
> - rc = flash_read(c, pos, bufecc, ECC_BUFFER_SIZE(copylen));
> + rc = flash_read(hl, pos, ECC_BUFFER_SIZE(copylen), bufecc);
> if (rc)
> goto err;
>
> @@ -219,8 +222,9 @@ static void fl_get_best_erase(struct flash_chip *c, uint32_t dst, uint32_t size,
> *cmd = CMD_BE;
> }
>
> -int flash_erase(struct flash_chip *c, uint32_t dst, uint32_t size)
> +static int flash_erase(struct highlevel_ops *hl, uint32_t dst, uint32_t size)
> {
> + struct flash_chip *c = container_of(hl, struct flash_chip, ops);
> struct spi_flash_ctrl *ct = c->ctrl;
> uint32_t chunk;
> uint8_t cmd;
> @@ -266,8 +270,9 @@ int flash_erase(struct flash_chip *c, uint32_t dst, uint32_t size)
> return 0;
> }
>
> -int flash_erase_chip(struct flash_chip *c)
> +int flash_erase_chip(struct highlevel_ops *hl)
> {
> + struct flash_chip *c = container_of(hl, struct flash_chip, ops);
> struct spi_flash_ctrl *ct = c->ctrl;
> int rc;
>
> @@ -315,9 +320,10 @@ static int fl_wpage(struct flash_chip *c, uint32_t dst, const void *src,
> return fl_sync_wait_idle(ct);
> }
>
> -int flash_write(struct flash_chip *c, uint32_t dst, const void *src,
> +static int flash_write(struct highlevel_ops *hl, uint32_t dst, const void *src,
> uint32_t size, bool verify)
> {
> + struct flash_chip *c = container_of(hl, struct flash_chip, ops);
> struct spi_flash_ctrl *ct = c->ctrl;
> uint32_t todo = size;
> uint32_t d = dst;
> @@ -378,7 +384,7 @@ int flash_write(struct flash_chip *c, uint32_t dst, const void *src,
> chunk = sizeof(vbuf);
> if (chunk > size)
> chunk = size;
> - rc = flash_read(c, dst, vbuf, chunk);
> + rc = flash_read(hl, dst, chunk, vbuf);
> if (rc) return rc;
> if (memcmp(vbuf, src, chunk)) {
> FL_ERR("LIBFLASH: Miscompare at 0x%08x\n", dst);
> @@ -391,7 +397,7 @@ int flash_write(struct flash_chip *c, uint32_t dst, const void *src,
> return 0;
> }
>
> -int flash_write_corrected(struct flash_chip *c, uint32_t pos, const void *buf,
> +int flash_write_corrected(struct highlevel_ops *hl, uint32_t pos, const void *buf,
> uint32_t len, bool verify, bool ecc)
> {
> struct ecc64 *bufecc;
> @@ -400,7 +406,7 @@ int flash_write_corrected(struct flash_chip *c, uint32_t pos, const void *buf,
> uint8_t ret;
>
> if (!ecc)
> - return flash_write(c, pos, buf, len, verify);
> + return flash_write(hl, pos, buf, len, verify);
>
> /* Copy the buffer in chunks */
> bufecc = malloc(ECC_BUFFER_SIZE(COPY_BUFFER_LENGTH));
> @@ -419,7 +425,7 @@ int flash_write_corrected(struct flash_chip *c, uint32_t pos, const void *buf,
> }
>
> /* Write ECCed data to the flash */
> - rc = flash_write(c, pos, bufecc, copylen, verify);
> + rc = flash_write(hl, pos, bufecc, copylen, verify);
> if (rc)
> goto err;
>
> @@ -467,12 +473,12 @@ static enum sm_comp_res flash_smart_comp(struct flash_chip *c,
> return is_same ? sm_no_change : sm_need_write;
> }
>
> -int flash_smart_write(struct flash_chip *c, uint32_t dst, const void *src,
> - uint32_t size)
> +static int flash_smart_write(struct highlevel_ops *hl, uint32_t dst, uint32_t size, const void *src)
> {
> + struct flash_chip *c = container_of(hl, struct flash_chip, ops);
> uint32_t er_size = c->min_erase_mask + 1;
> uint32_t end = dst + size;
> - int rc;
> + int rc;
>
> /* Some sanity checking */
> if (end <= dst || !size || end > c->tsize) {
> @@ -493,7 +499,7 @@ int flash_smart_write(struct flash_chip *c, uint32_t dst, const void *src,
> off = dst & c->min_erase_mask;
> FL_DBG("LIBFLASH: reading page 0x%08x..0x%08x...",
> page, page + er_size);
> - rc = flash_read(c, page, c->smart_buf, er_size);
> + rc = flash_read(hl, page, er_size, c->smart_buf);
> if (rc) {
> FL_DBG(" error %d!\n", rc);
> return rc;
> @@ -514,7 +520,7 @@ int flash_smart_write(struct flash_chip *c, uint32_t dst, const void *src,
> case sm_need_write:
> /* Just needs writing over */
> FL_DBG(" need write !\n");
> - rc = flash_write(c, dst, src, chunk, true);
> + rc = flash_write(hl, dst, src, chunk, true);
> if (rc) {
> FL_DBG("LIBFLASH: Write error %d !\n", rc);
> return rc;
> @@ -522,14 +528,14 @@ int flash_smart_write(struct flash_chip *c, uint32_t dst, const void *src,
> break;
> case sm_need_erase:
> FL_DBG(" need erase !\n");
> - rc = flash_erase(c, page, er_size);
> + rc = flash_erase(hl, page, er_size);
> if (rc) {
> FL_DBG("LIBFLASH: erase error %d !\n", rc);
> return rc;
> }
> /* Then update the portion of the buffer and write the block */
> memcpy(c->smart_buf + off, src, chunk);
> - rc = flash_write(c, page, c->smart_buf, er_size, true);
> + rc = flash_write(hl, page, c->smart_buf, er_size, true);
> if (rc) {
> FL_DBG("LIBFLASH: write error %d !\n", rc);
> return rc;
> @@ -543,14 +549,14 @@ int flash_smart_write(struct flash_chip *c, uint32_t dst, const void *src,
> return 0;
> }
>
> -int flash_smart_write_corrected(struct flash_chip *c, uint32_t dst, const void *src,
> +int flash_smart_write_corrected(struct highlevel_ops *hl, uint32_t dst, const void *src,
> uint32_t size, bool ecc)
> {
> struct ecc64 *buf;
> int rc;
>
> if (!ecc)
> - return flash_smart_write(c, dst, src, size);
> + return flash_smart_write(hl, dst, size, src);
>
> buf = malloc(ECC_BUFFER_SIZE(size));
> if (!buf)
> @@ -562,7 +568,7 @@ int flash_smart_write_corrected(struct flash_chip *c, uint32_t dst, const void *
> goto out;
> }
>
> - rc = flash_smart_write(c, dst, buf, ECC_BUFFER_SIZE(size));
> + rc = flash_smart_write(hl, dst, ECC_BUFFER_SIZE(size), buf);
>
> out:
> free(buf);
> @@ -690,8 +696,9 @@ static int flash_set_4b(struct flash_chip *c, bool enable)
> return ct->cmd_wr(ct, enable ? CMD_EN4B : CMD_EX4B, false, 0, NULL, 0);
> }
>
> -int flash_force_4b_mode(struct flash_chip *c, bool enable_4b)
> +int flash_force_4b_mode(struct highlevel_ops *hl, bool enable_4b)
> {
> + struct flash_chip *c = container_of(hl, struct flash_chip, ops);
> struct spi_flash_ctrl *ct = c->ctrl;
> int rc;
>
> @@ -782,24 +789,29 @@ static int flash_configure(struct flash_chip *c)
> return 0;
> }
>
> -int flash_get_info(struct flash_chip *chip, const char **name,
> +static int flash_get_info(struct highlevel_ops *hl, const char **name,
> uint32_t *total_size, uint32_t *erase_granule)
> {
> + struct flash_chip *c = container_of(hl, struct flash_chip, ops);
> if (name)
> - *name = chip->info.name;
> + *name = c->info.name;
> if (total_size)
> - *total_size = chip->tsize;
> + *total_size = c->tsize;
> if (erase_granule)
> - *erase_granule = chip->min_erase_mask + 1;
> + *erase_granule = c->min_erase_mask + 1;
> return 0;
> }
>
> -int flash_init(struct spi_flash_ctrl *ctrl, struct flash_chip **flash)
> +int flash_init(struct spi_flash_ctrl *ctrl, struct highlevel_ops **hl)
> {
> struct flash_chip *c;
> int rc;
>
> - *flash = NULL;
> + if (!hl)
> + return FLASH_ERR_PARM_ERROR;
> +
> + *hl = NULL;
> +
> c = malloc(sizeof(struct flash_chip));
> if (!c)
> return FLASH_ERR_MALLOC_FAILED;
> @@ -820,18 +832,25 @@ int flash_init(struct spi_flash_ctrl *ctrl, struct flash_chip **flash)
> rc = flash_configure(c);
> if (rc)
> FL_ERR("LIBFLASH: Flash configuration failed\n");
> - bail:
> +bail:
> if (rc) {
> free(c);
> return rc;
> }
> - *flash = c;
> +
> + c->ops.read = &flash_read;
> + c->ops.write = &flash_smart_write;
> + c->ops.erase = &flash_erase;
> + c->ops.get_info = &flash_get_info;
> +
> + *hl = &(c->ops);
> return 0;
> }
>
> -void flash_exit(struct flash_chip *chip)
> +void flash_exit(struct highlevel_ops *hl)
> {
> /* XXX Make sure we are idle etc... */
> - free(chip);
> + if (hl)
> + free(container_of(hl, struct flash_chip, ops));
> }
>
> diff --git a/libflash/libflash.h b/libflash/libflash.h
> index 7ed65d0..3d81813 100644
> --- a/libflash/libflash.h
> +++ b/libflash/libflash.h
> @@ -18,6 +18,7 @@
>
> #include <stdint.h>
> #include <stdbool.h>
> +#include <libflash/highlevel.h>
>
> #ifdef __SKIBOOT__
> #include <skiboot.h>
> @@ -59,18 +60,13 @@ extern bool libflash_debug;
> struct flash_chip;
> struct spi_flash_ctrl;
>
> -int flash_init(struct spi_flash_ctrl *ctrl, struct flash_chip **flash);
> -void flash_exit(struct flash_chip *chip);
> -
> -int flash_get_info(struct flash_chip *chip, const char **name,
> - uint32_t *total_size, uint32_t *erase_granule);
> +int flash_init(struct spi_flash_ctrl *ctrl, struct highlevel_ops **hl);
> +void flash_exit(struct highlevel_ops *hl);
>
> /* libflash sets the 4b mode automatically based on the flash
> * size and controller capabilities but it can be overriden
> */
> -int flash_force_4b_mode(struct flash_chip *chip, bool enable_4b);
> -
> -int flash_read(struct flash_chip *c, uint32_t pos, void *buf, uint32_t len);
> +int flash_force_4b_mode(struct highlevel_ops *hl, bool enable_4b);
>
> /*
> * This provides a wapper around flash_read() on ECCed data. All params are
> @@ -80,11 +76,8 @@ int flash_read(struct flash_chip *c, uint32_t pos, void *buf, uint32_t len);
> * len is length of data without ecc attached therefore this will read beyond
> * pos + len.
> */
> -int flash_read_corrected(struct flash_chip *c, uint32_t pos, void *buf,
> +int flash_read_corrected(struct highlevel_ops *hl, 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_write(struct flash_chip *c, uint32_t dst, const void *src,
> - uint32_t size, bool verify);
>
> /*
> * This provides a wrapper around flash_write() on ECCed data. All params are
> @@ -94,10 +87,8 @@ int flash_write(struct flash_chip *c, uint32_t dst, const void *src,
> * size is length of data without ECC attached therefore this will write beyond
> * dst + size.
> */
> -int flash_write_corrected(struct flash_chip *c, uint32_t dst, const void *src,
> +int flash_write_corrected(struct highlevel_ops *hl, uint32_t dst, const void *src,
> uint32_t size, bool verify, bool ecc);
> -int flash_smart_write(struct flash_chip *c, uint32_t dst, const void *src,
> - uint32_t size);
>
> /*
> * This provides a wrapper around flash_smart_write() on ECCed data. All
> @@ -107,12 +98,12 @@ int flash_smart_write(struct flash_chip *c, uint32_t dst, const void *src,
> * size is length of data without ECC attached therefore this will write beyond
> * dst + size.
> */
> -int flash_smart_write_corrected(struct flash_chip *c, uint32_t dst, const void *src,
> +int flash_smart_write_corrected(struct highlevel_ops *hl, uint32_t dst, const void *src,
> uint32_t size, bool ecc);
>
> /* chip erase may not be supported by all chips/controllers, get ready
> * for FLASH_ERR_CHIP_ER_NOT_SUPPORTED
> */
> -int flash_erase_chip(struct flash_chip *c);
> +int flash_erase_chip(struct highlevel_ops *hl);
>
> #endif /* __LIBFLASH_H */
> diff --git a/libflash/test/Makefile.check b/libflash/test/Makefile.check
> index 276de0c..7e765a8 100644
> --- a/libflash/test/Makefile.check
> +++ b/libflash/test/Makefile.check
> @@ -30,4 +30,5 @@ clean: libflash-test-clean
>
> libflash-test-clean:
> $(RM) libflash/test/*.o $(LIBFLASH_TEST)
> + $(RM) libflash/test/*.d
> $(RM) libflash/test/*-gcov
Also a hunk that isn't all that related to what I'm doing and could
really be applied at any time. I'll send a standalone
> diff --git a/libflash/test/test-flash.c b/libflash/test/test-flash.c
> index 4479b5a..98205de 100644
> --- a/libflash/test/test-flash.c
> +++ b/libflash/test/test-flash.c
> @@ -366,7 +366,7 @@ struct spi_flash_ctrl sim_ctrl = {
>
> int main(void)
> {
> - struct flash_chip *fl;
> + struct highlevel_ops *hl;
> uint32_t total_size, erase_granule;
> const char *name;
> uint16_t *test;
> @@ -378,12 +378,12 @@ int main(void)
> memset(sim_image, 0xff, sim_image_sz);
> test = malloc(0x10000 * 2);
>
> - rc = flash_init(&sim_ctrl, &fl);
> + rc = flash_init(&sim_ctrl, &hl);
> if (rc) {
> ERR("flash_init failed with err %d\n", rc);
> exit(1);
> }
> - rc = flash_get_info(fl, &name, &total_size, &erase_granule);
> + rc = flash_get_info(hl, &name, &total_size, &erase_granule);
> if (rc) {
> ERR("flash_get_info failed with err %d\n", rc);
> exit(1);
> @@ -395,13 +395,13 @@ int main(void)
>
> /* Write 64k of stuff at 0 and at 128k */
> printf("Writing test patterns...\n");
> - flash_smart_write(fl, 0, test, 0x10000);
> - flash_smart_write(fl, 0x20000, test, 0x10000);
> + flash_smart_write(hl, 0, 0x10000, test);
> + flash_smart_write(hl, 0x20000, 0x10000, test);
>
> /* Write "Hello world" straddling the 64k boundary */
> #define HW "Hello World"
> printf("Writing test string...\n");
> - flash_smart_write(fl, 0xfffc, HW, sizeof(HW));
> + flash_smart_write(hl, 0xfffc, sizeof(HW), HW);
>
> /* Check result */
> if (memcmp(sim_image + 0xfffc, HW, sizeof(HW))) {
> @@ -416,7 +416,7 @@ int main(void)
> printf("Test pattern pass\n");
>
> printf("Test ECC interfaces\n");
> - flash_smart_write_corrected(fl, 0, test, 0x10000, 1);
> + flash_smart_write_corrected(hl, 0, test, 0x10000, 1);
> ecc_test = (struct ecc64 *)sim_image;
> test64 = (uint64_t *)test;
> for (i = 0; i < 0x10000 / sizeof(*ecc_test); i++) {
> @@ -433,7 +433,7 @@ int main(void)
> printf("Test ECC interface pass\n");
>
> printf("Test ECC erase\n");
> - if (flash_erase(fl, 0, 0x10000) != 0) {
> + if (flash_erase(hl, 0, 0x10000) != 0) {
> ERR("flash_erase didn't return 0\n");
> exit(1);
> }
> @@ -444,7 +444,7 @@ int main(void)
> ERR("Data not properly cleared at %d\n", i);
> exit(1);
> }
> - rc = flash_write(fl, i * sizeof(*ecc_test) + 8, &zero, 1, 0);
> + rc = flash_write(hl, i * sizeof(*ecc_test) + 8, &zero, 1, 0);
> if (rc || ecc_test[i].ecc != 0) {
> ERR("Cleared data not correctly ECCed: 0x%02x (0x%016lx) expecting 0 at %d\n", ecc_test[i].ecc, ecc_test[i].data, i);
> exit(1);
> @@ -452,7 +452,7 @@ int main(void)
> }
> printf("Test ECC erase pass\n");
>
> - flash_exit(fl);
> + flash_exit(hl);
>
> return 0;
> }
> diff --git a/platforms/astbmc/pnor.c b/platforms/astbmc/pnor.c
> index 8e88edd..c30322e 100644
> --- a/platforms/astbmc/pnor.c
> +++ b/platforms/astbmc/pnor.c
> @@ -20,6 +20,7 @@
> #include <opal.h>
> #include <libflash/libflash.h>
> #include <libflash/libffs.h>
> +#include <libflash/highlevel.h>
> #include <ast.h>
>
> #include "astbmc.h"
> @@ -27,7 +28,7 @@
> int pnor_init(void)
> {
> struct spi_flash_ctrl *pnor_ctrl;
> - struct flash_chip *pnor_chip;
> + struct highlevel_funcs *funcs;
> int rc;
>
> /* Open controller and flash. If the LPC->AHB doesn't point to
> @@ -44,19 +45,20 @@ int pnor_init(void)
> prerror("PLAT: Failed to open PNOR flash controller\n");
> goto fail;
> }
> - rc = flash_init(pnor_ctrl, &pnor_chip);
> +
> + rc = flash_init(pnor_ctrl, &funcs);
> if (rc) {
> prerror("PLAT: Failed to open init PNOR driver\n");
> goto fail;
> }
>
> - rc = flash_register(pnor_chip, true);
> + rc = flash_register(funcs, true);
> if (!rc)
> return 0;
>
> fail:
> - if (pnor_chip)
> - flash_exit(pnor_chip);
> + if (funcs)
> + flash_exit(funcs);
> if (pnor_ctrl)
> ast_sf_close(pnor_ctrl);
>
> diff --git a/platforms/rhesus/rhesus.c b/platforms/rhesus/rhesus.c
> index 50769cf..6182315 100644
> --- a/platforms/rhesus/rhesus.c
> +++ b/platforms/rhesus/rhesus.c
> @@ -118,7 +118,7 @@ static int64_t rhesus_power_down(uint64_t request __unused)
> static int rhesus_pnor_init(void)
> {
> struct spi_flash_ctrl *pnor_ctrl;
> - struct flash_chip *pnor_chip;
> + struct highlevel_funcs *funcs;
> int rc;
>
> /* Open controller, flash and ffs */
> @@ -127,19 +127,19 @@ static int rhesus_pnor_init(void)
> prerror("PLAT: Failed to open PNOR flash controller\n");
> goto fail;
> }
> - rc = flash_init(pnor_ctrl, &pnor_chip);
> + rc = flash_init(pnor_ctrl, &funcs);
> if (rc) {
> prerror("PLAT: Failed to open init PNOR driver\n");
> goto fail;
> }
>
> - rc = flash_register(pnor_chip, true);
> + rc = flash_register(funcs, true);
> if (!rc)
> return 0;
>
> fail:
> - if (pnor_chip)
> - flash_exit(pnor_chip);
> + if (funcs)
> + flash_exit(funcs);
> if (pnor_ctrl)
> sfc_close(pnor_ctrl);
>
More information about the Skiboot
mailing list