[Skiboot] [PATCH] blocklevel_info: Refactor the passing of output data
William Kennington
wak at google.com
Tue Aug 29 16:34:57 AEST 2017
Cyril,
Sounds good. I actually wrote this up a couple of months back, but I'd like
to upstream as many of our changes as possible. It just occurred to me that
we may not actually need this change at all to support one of the flash
changes we made. It would be nice to clean up these interfaces though.
On Mon, Aug 28, 2017 at 10:38 PM Cyril Bur <cyrilbur at gmail.com> wrote:
> On Mon, 2017-08-28 at 21:19 -0700, William A. Kennington III wrote:
> > This changes refactors all of the output variables from flash_get_info
> > into a new output struct. Callers now supply the info struct, and always
> > get all of the info with each call. There are never any allocations done
> > by this function so it shouldn't be much slower to always populate all
> > of the information.
> >
>
> Hi William,
>
> As nice as this refactor is (thanks for the effort!), unfortunately
> we're a bit stuck with the interface as is, both openbmc and petitboot
> projects link against a dynamic libflash.
>
> Ironically I'm about to send a series which which does rework the
> libffs api a bit - although I'm quite sure only external/ffspart
> actually uses what I've changed. blocklevel_get_info() though is pretty
> core and changing that won't work.
>
> Perhaps you've prompted a discussion that now might be a good time to
> do a significant rewrite of libflash/libffs which are now being used
> for a completely different purpose for which they were first written,
> hopefully Stewart will weigh in.
>
>
> Cyril
>
> > Signed-off-by: William A. Kennington III <wak at google.com>
> > ---
> > core/flash.c | 15 +++++------
> > external/common/arch_flash_arm.c | 6 ++---
> > external/common/arch_flash_common.c | 6 ++---
> > external/gard/gard.c | 10 +++----
> > external/opal-prd/pnor.c | 12 ++++-----
> > external/opal-prd/pnor.h | 3 +--
> > external/opal-prd/test/test_pnor_ops.c | 7 +++--
> > external/pflash/pflash.c | 46
> ++++++++++++++++----------------
> > libflash/blocklevel.c | 35 ++++++++++++-------------
> > libflash/blocklevel.h | 16 +++++++++---
> > libflash/file.c | 48
> ++++++++++++++--------------------
> > libflash/libffs.c | 8 +++---
> > libflash/libflash.c | 15 +++++------
> > libflash/mbox-flash.c | 19 ++++----------
> > libflash/test/test-flash.c | 6 ++---
> > platforms/mambo/mambo.c | 15 ++++-------
> > 16 files changed, 125 insertions(+), 142 deletions(-)
> >
> > diff --git a/core/flash.c b/core/flash.c
> > index 53e6eba08..35a0c4c32 100644
> > --- a/core/flash.c
> > +++ b/core/flash.c
> > @@ -254,21 +254,20 @@ static int num_flashes(void)
> >
> > int flash_register(struct blocklevel_device *bl)
> > {
> > - uint64_t size;
> > - uint32_t block_size;
> > + struct blocklevel_info bl_info;
> > struct ffs_handle *ffs;
> > struct dt_node *node;
> > struct flash *flash;
> > - const char *name;
> > int rc;
> >
> > - rc = blocklevel_get_info(bl, &name, &size, &block_size);
> > + rc = blocklevel_get_info(bl, &bl_info);
> > if (rc)
> > return rc;
> >
> > prlog(PR_INFO, "FLASH: registering flash device %s "
> > "(size 0x%llx, blocksize 0x%x)\n",
> > - name ?: "(unnamed)", size, block_size);
> > + bl_info.name ? bl_info.name : "(unnamed)",
> bl_info.size,
> > + bl_info.erase_granule);
> >
> > lock(&flash_lock);
> >
> > @@ -281,8 +280,8 @@ int flash_register(struct blocklevel_device *bl)
> >
> > flash->busy = false;
> > flash->bl = bl;
> > - flash->size = size;
> > - flash->block_size = block_size;
> > + flash->size = bl_info.size;
> > + flash->block_size = bl_info.erase_granule;
> > flash->id = num_flashes();
> >
> > list_add(&flashes, &flash->list);
> > @@ -302,7 +301,7 @@ int flash_register(struct blocklevel_device *bl)
> >
> > node = flash_add_dt_node(flash, flash->id);
> >
> > - setup_system_flash(flash, node, name, ffs);
> > + setup_system_flash(flash, node, bl_info.name, ffs);
> >
> > if (ffs)
> > ffs_close(ffs);
> > diff --git a/external/common/arch_flash_arm.c
> b/external/common/arch_flash_arm.c
> > index 3cdd41ded..ac43de0c4 100644
> > --- a/external/common/arch_flash_arm.c
> > +++ b/external/common/arch_flash_arm.c
> > @@ -303,13 +303,13 @@ int arch_flash_erase_chip(struct blocklevel_device
> *bl)
> > if (!arch_data.flash_chip) {
> > /* Just assume its a regular erase */
> > int rc;
> > - uint64_t total_size;
> > + struct blocklevel_info bl_info;
> >
> > - rc = blocklevel_get_info(bl, NULL, &total_size, NULL);
> > + rc = blocklevel_get_info(bl, &bl_info);
> > if (rc)
> > return rc;
> >
> > - return blocklevel_erase(bl, 0, total_size);
> > + return blocklevel_erase(bl, 0, bl_info.size);
> > }
> >
> > return flash_erase_chip(arch_data.flash_chip);
> > diff --git a/external/common/arch_flash_common.c
> b/external/common/arch_flash_common.c
> > index 6bce7e1ba..41239662a 100644
> > --- a/external/common/arch_flash_common.c
> > +++ b/external/common/arch_flash_common.c
> > @@ -31,13 +31,13 @@
> > int __attribute__((weak)) arch_flash_erase_chip(struct
> blocklevel_device *bl)
> > {
> > int rc;
> > - uint64_t total_size;
> > + struct blocklevel_info bl_info;
> >
> > - rc = blocklevel_get_info(bl, NULL, &total_size, NULL);
> > + rc = blocklevel_get_info(bl, &bl_info);
> > if (rc)
> > return rc;
> >
> > - return blocklevel_erase(bl, 0, total_size);
> > + return blocklevel_erase(bl, 0, bl_info.size);
> > }
> >
> > int __attribute__((weak,const)) arch_flash_4b_mode(struct
> blocklevel_device *bl, int set_4b)
> > diff --git a/external/gard/gard.c b/external/gard/gard.c
> > index c5cb93b88..3946cad6f 100644
> > --- a/external/gard/gard.c
> > +++ b/external/gard/gard.c
> > @@ -596,7 +596,7 @@ int main(int argc, char **argv)
> > const char *action, *progname;
> > char *filename = NULL;
> > struct gard_ctx _ctx, *ctx;
> > - uint64_t bl_size;
> > + struct blocklevel_info bl_info;
> > int rc, i = 0;
> > bool part = 0;
> > bool ecc = 0;
> > @@ -665,17 +665,17 @@ int main(int argc, char **argv)
> > goto out_free;
> > }
> >
> > - rc = blocklevel_get_info(ctx->bl, NULL, &bl_size, NULL);
> > + rc = blocklevel_get_info(ctx->bl, &bl_info);
> > if (rc)
> > goto out;
> >
> > - if (bl_size > UINT_MAX) {
> > + if (bl_info.size > UINT_MAX) {
> > fprintf(stderr, "MTD device bigger than %i: size: %"
> PRIu64 "\n",
> > - UINT_MAX, bl_size);
> > + UINT_MAX, bl_info.size);
> > rc = EXIT_FAILURE;
> > goto out;
> > }
> > - ctx->f_size = bl_size;
> > + ctx->f_size = bl_info.size;
> >
> > if (!part) {
> > rc = ffs_init(0, ctx->f_size, ctx->bl, &ctx->ffs, 1);
> > diff --git a/external/opal-prd/pnor.c b/external/opal-prd/pnor.c
> > index c032421d1..afb6ccd1f 100644
> > --- a/external/opal-prd/pnor.c
> > +++ b/external/opal-prd/pnor.c
> > @@ -64,13 +64,13 @@ int pnor_init(struct pnor *pnor)
> > return -1;
> > }
> >
> > - rc = blocklevel_get_info(pnor->bl, NULL, &(pnor->size),
> &(pnor->erasesize));
> > + rc = blocklevel_get_info(pnor->bl, &pnor->bl_info);
> > if (rc) {
> > pr_log(LOG_ERR, "PNOR: blocklevel_get_info() failed. Can't
> use PNOR");
> > goto out;
> > }
> >
> > - rc = ffs_init(0, pnor->size, pnor->bl, &pnor->ffsh, 0);
> > + rc = ffs_init(0, pnor->bl_info.size, pnor->bl, &pnor->ffsh, 0);
> > if (rc) {
> > pr_log(LOG_ERR, "PNOR: Failed to open pnor partition
> table");
> > goto out;
> > @@ -121,8 +121,8 @@ static int mtd_write(struct pnor *pnor, void *data,
> uint64_t offset,
> > {
> > int rc;
> >
> > - if (len > pnor->size || offset > pnor->size ||
> > - len + offset > pnor->size)
> > + if (len > pnor->bl_info.size || offset > pnor->bl_info.size ||
> > + len + offset > pnor->bl_info.size)
> > return -ERANGE;
> >
> > rc = blocklevel_smart_write(pnor->bl, offset, data, len);
> > @@ -137,8 +137,8 @@ static int mtd_read(struct pnor *pnor, void *data,
> uint64_t offset,
> > {
> > int rc;
> >
> > - if (len > pnor->size || offset > pnor->size ||
> > - len + offset > pnor->size)
> > + if (len > pnor->bl_info.size || offset > pnor->bl_info.size ||
> > + len + offset > pnor->bl_info.size)
> > return -ERANGE;
> >
> > rc = blocklevel_read(pnor->bl, offset, data, len);
> > diff --git a/external/opal-prd/pnor.h b/external/opal-prd/pnor.h
> > index 28571af64..1ee033659 100644
> > --- a/external/opal-prd/pnor.h
> > +++ b/external/opal-prd/pnor.h
> > @@ -7,8 +7,7 @@
> > struct pnor {
> > char *path;
> > struct ffs_handle *ffsh;
> > - uint64_t size;
> > - uint32_t erasesize;
> > + struct blocklevel_info bl_info;
> > struct blocklevel_device *bl;
> > };
> >
> > diff --git a/external/opal-prd/test/test_pnor_ops.c
> b/external/opal-prd/test/test_pnor_ops.c
> > index fd5e2c22a..3743362e1 100644
> > --- a/external/opal-prd/test/test_pnor_ops.c
> > +++ b/external/opal-prd/test/test_pnor_ops.c
> > @@ -130,11 +130,14 @@ int main(int argc, char **argv)
> > for (i = 0; i < 2; i++)
> > write(fd, data, 16);
> >
> > + /* Unused */
> > + pnor.bl_info.name = NULL;
> > +
> > /* Adjust this if making the file smaller */
> > - pnor.size = 32;
> > + pnor.bl_info.size = 32;
> >
> > /* This is fake. Make it smaller than the size */
> > - pnor.erasesize = 4;
> > + pnor.bl_info.erase_granule = 4;
> >
> > printf("Write: ");
> > memset(data, 'A', sizeof(data));
> > diff --git a/external/pflash/pflash.c b/external/pflash/pflash.c
> > index bfc975fe5..3f501e583 100644
> > --- a/external/pflash/pflash.c
> > +++ b/external/pflash/pflash.c
> > @@ -43,11 +43,9 @@
> >
> > struct flash_details {
> > struct blocklevel_device *bl;
> > + struct blocklevel_info bl_info;
> > int need_relock;
> > - const char *name;
> > uint64_t toc;
> > - uint64_t total_size;
> > - uint32_t erase_granule;
> > };
> >
> > /* Full pflash version number (possibly includes gitid). */
> > @@ -144,7 +142,7 @@ static struct ffs_handle *open_ffs(struct
> flash_details *flash)
> > struct ffs_handle *ffsh;
> > int rc;
> >
> > - rc = ffs_init(flash->toc, flash->total_size,
> > + rc = ffs_init(flash->toc, flash->bl_info.size,
> > flash->bl, &ffsh, 0);
> > if (rc) {
> > fprintf(stderr, "Error %d opening ffs !\n", rc);
> > @@ -166,11 +164,11 @@ static void print_flash_info(struct flash_details
> *flash)
> >
> > printf("Flash info:\n");
> > printf("-----------\n");
> > - printf("Name = %s\n", flash->name);
> > + printf("Name = %s\n", flash->bl_info.name);
> > printf("Total size = %" PRIu64 "MB\t Flags E:ECC, P:PRESERVED,
> R:READONLY\n",
> > - flash->total_size >> 20);
> > + flash->bl_info.size >> 20);
> > printf("Erase granule = %2d%-13sB:BACKUP, F:REPROVISION\n",
> > - flash->erase_granule >> 10, "KB");
> > + flash->bl_info.erase_granule >> 10, "KB");
> >
> > if (bmc_flash)
> > return;
> > @@ -282,9 +280,11 @@ static int erase_chip(struct flash_details *flash)
> > * likes the progress bars.
> > * Lets do an erase block at a time erase then...
> > */
> > - progress_init(flash->total_size);
> > - for (pos = 0; pos < flash->total_size; pos +=
> flash->erase_granule) {
> > - rc = blocklevel_erase(flash->bl, pos,
> flash->erase_granule);
> > + progress_init(flash->bl_info.size);
> > + for (pos = 0; pos < flash->bl_info.size;
> > + pos += flash->bl_info.erase_granule) {
> > + rc = blocklevel_erase(flash->bl, pos,
> > + flash->bl_info.erase_granule);
> > if (rc)
> > break;
> > progress_tick(pos);
> > @@ -303,7 +303,7 @@ static int erase_range(struct flash_details *flash,
> > uint32_t start, uint32_t size, bool will_program,
> > struct ffs_handle *ffsh, int ffs_index)
> > {
> > - uint32_t done = 0, erase_mask = flash->erase_granule - 1;
> > + uint32_t done = 0, erase_mask = flash->bl_info.erase_granule - 1;
> > bool confirm;
> > int rc;
> >
> > @@ -326,25 +326,26 @@ static int erase_range(struct flash_details *flash,
> > if (start & erase_mask) {
> > /* Align to next erase block */
> > rc = blocklevel_smart_erase(flash->bl, start,
> > - flash->erase_granule - (start &
> erase_mask));
> > + flash->bl_info.erase_granule - (start &
> erase_mask));
> > if (rc) {
> > fprintf(stderr, "Failed to
> blocklevel_smart_erase(): %d\n", rc);
> > return 1;
> > }
> > - start += flash->erase_granule - (start & erase_mask);
> > - size -= flash->erase_granule - (start & erase_mask);
> > - done = flash->erase_granule - (start & erase_mask);
> > + start += flash->bl_info.erase_granule - (start &
> erase_mask);
> > + size -= flash->bl_info.erase_granule - (start &
> erase_mask);
> > + done = flash->bl_info.erase_granule - (start & erase_mask);
> > }
> > progress_tick(done);
> > while (size & ~(erase_mask)) {
> > - rc = blocklevel_smart_erase(flash->bl, start,
> flash->erase_granule);
> > + rc = blocklevel_smart_erase(flash->bl, start,
> > + flash->bl_info.erase_granule);
> > if (rc) {
> > fprintf(stderr, "Failed to
> blocklevel_smart_erase(): %d\n", rc);
> > return 1;
> > }
> > - start += flash->erase_granule;
> > - size -= flash->erase_granule;
> > - done += flash->erase_granule;
> > + start += flash->bl_info.erase_granule;
> > + size -= flash->bl_info.erase_granule;
> > + done += flash->bl_info.erase_granule;
> > progress_tick(done);
> > }
> > if (size) {
> > @@ -1003,8 +1004,7 @@ int main(int argc, char *argv[])
> > goto out;
> > }
> >
> > - rc = blocklevel_get_info(flash.bl, &flash.name,
> > - &flash.total_size, &flash.erase_granule);
> > + rc = blocklevel_get_info(flash.bl, &flash.bl_info);
> > if (rc) {
> > fprintf(stderr, "Error %d getting flash info\n", rc);
> > rc = 1;
> > @@ -1025,7 +1025,7 @@ int main(int argc, char *argv[])
> >
> > /* If read specified and no read_size, use flash size */
> > if (do_read && !read_size && !part_name)
> > - read_size = flash.total_size;
> > + read_size = flash.bl_info.size;
> >
> > /* We have a partition, adjust read/write size if needed */
> > if (part_name || print_detail) {
> > @@ -1087,7 +1087,7 @@ int main(int argc, char *argv[])
> > /* Set address */
> > address = pstart;
> > } else if (erase) {
> > - if ((address | write_size) & (flash.erase_granule - 1)) {
> > + if ((address | write_size) & (flash.bl_info.erase_granule
> - 1)) {
> > if (must_confirm) {
> > printf("ERROR: Erase at 0x%08x for 0x%08x
> isn't erase block aligned\n",
> > address, write_size);
> > diff --git a/libflash/blocklevel.c b/libflash/blocklevel.c
> > index 33d5c5d2f..3955be823 100644
> > --- a/libflash/blocklevel.c
> > +++ b/libflash/blocklevel.c
> > @@ -229,12 +229,11 @@ int blocklevel_erase(struct blocklevel_device *bl,
> uint64_t pos, uint64_t len)
> > return rc;
> > }
> >
> > -int blocklevel_get_info(struct blocklevel_device *bl, const char
> **name, uint64_t *total_size,
> > - uint32_t *erase_granule)
> > +int blocklevel_get_info(struct blocklevel_device *bl, struct
> blocklevel_info *bl_info)
> > {
> > int rc;
> >
> > - if (!bl || !bl->get_info) {
> > + if (!bl || !bl->get_info || !bl_info) {
> > errno = EINVAL;
> > return FLASH_ERR_PARM_ERROR;
> > }
> > @@ -243,12 +242,12 @@ int blocklevel_get_info(struct blocklevel_device
> *bl, const char **name, uint64_
> > if (rc)
> > return rc;
> >
> > - rc = bl->get_info(bl, name, total_size, erase_granule);
> > + rc = bl->get_info(bl, bl_info);
> >
> > /* Check the validity of what we are being told */
> > - if (erase_granule && *erase_granule != bl->erase_mask + 1)
> > + if (bl_info->erase_granule != bl->erase_mask + 1)
> > FL_ERR("blocklevel_get_info: WARNING: erase_granule
> (0x%08x) and erase_mask"
> > - " (0x%08x) don't match\n", *erase_granule,
> bl->erase_mask + 1);
> > + " (0x%08x) don't match\n",
> bl_info->erase_granule, bl->erase_mask + 1);
> >
> > release(bl);
> >
> > @@ -417,7 +416,7 @@ out:
> >
> > int blocklevel_smart_write(struct blocklevel_device *bl, uint64_t pos,
> const void *buf, uint64_t len)
> > {
> > - uint32_t erase_size;
> > + struct blocklevel_info bl_info;
> > const void *write_buf = buf;
> > void *write_buf_start = NULL;
> > void *erase_buf;
> > @@ -435,7 +434,7 @@ int blocklevel_smart_write(struct blocklevel_device
> *bl, uint64_t pos, const voi
> > return blocklevel_write(bl, pos, buf, len);
> > }
> >
> > - rc = blocklevel_get_info(bl, NULL, NULL, &erase_size);
> > + rc = blocklevel_get_info(bl, &bl_info);
> > if (rc)
> > return rc;
> >
> > @@ -458,7 +457,7 @@ int blocklevel_smart_write(struct blocklevel_device
> *bl, uint64_t pos, const voi
> > write_buf = write_buf_start;
> > }
> >
> > - erase_buf = malloc(erase_size);
> > + erase_buf = malloc(bl_info.erase_granule);
> > if (!erase_buf) {
> > errno = ENOMEM;
> > rc = FLASH_ERR_MALLOC_FAILED;
> > @@ -470,32 +469,32 @@ int blocklevel_smart_write(struct
> blocklevel_device *bl, uint64_t pos, const voi
> > goto out_free;
> >
> > while (len > 0) {
> > - uint32_t erase_block = pos & ~(erase_size - 1);
> > - uint32_t block_offset = pos & (erase_size - 1);
> > - uint32_t size = erase_size > len ? len : erase_size;
> > + uint32_t erase_block = pos & ~(bl_info.erase_granule - 1);
> > + uint32_t block_offset = pos & (bl_info.erase_granule - 1);
> > + uint32_t size = bl_info.erase_granule > len ? len :
> bl_info.erase_granule;
> > int cmp;
> >
> > /* Write crosses an erase boundary, shrink the write to
> the boundary */
> > - if (erase_size < block_offset + size) {
> > - size = erase_size - block_offset;
> > + if (bl_info.erase_granule < block_offset + size) {
> > + size = bl_info.erase_granule - block_offset;
> > }
> >
> > - rc = bl->read(bl, erase_block, erase_buf, erase_size);
> > + rc = bl->read(bl, erase_block, erase_buf,
> bl_info.erase_granule);
> > if (rc)
> > goto out;
> >
> > cmp = blocklevel_flashcmp(erase_buf + block_offset,
> write_buf, size);
> > FL_DBG("%s: region 0x%08x..0x%08x ", __func__,
> > - erase_block, erase_size);
> > + erase_block, bl_info.erase_granule);
> > if (cmp != 0) {
> > FL_DBG("needs ");
> > if (cmp == -1) {
> > FL_DBG("erase and ");
> > - bl->erase(bl, erase_block, erase_size);
> > + bl->erase(bl, erase_block,
> bl_info.erase_granule);
> > }
> > FL_DBG("write\n");
> > memcpy(erase_buf + block_offset, write_buf, size);
> > - rc = bl->write(bl, erase_block, erase_buf,
> erase_size);
> > + rc = bl->write(bl, erase_block, erase_buf,
> bl_info.erase_granule);
> > if (rc)
> > goto out;
> > }
> > diff --git a/libflash/blocklevel.h b/libflash/blocklevel.h
> > index ba42c83d0..d2d269054 100644
> > --- a/libflash/blocklevel.h
> > +++ b/libflash/blocklevel.h
> > @@ -34,6 +34,15 @@ enum blocklevel_flags {
> > WRITE_NEED_ERASE = 1,
> > };
> >
> > +struct blocklevel_info {
> > + /* name is not owned by this struct and must persist for the
> > + * liftime of blocklevel_info
> > + */
> > + const char *name;
> > + uint64_t size;
> > + uint32_t erase_granule;
> > +};
> > +
> > /*
> > * libffs may be used with different backends, all should provide these
> for
> > * libflash to get the information it needs
> > @@ -45,8 +54,7 @@ struct blocklevel_device {
> > int (*read)(struct blocklevel_device *bl, uint64_t pos, void *buf,
> uint64_t len);
> > int (*write)(struct blocklevel_device *bl, uint64_t pos, const
> void *buf, uint64_t len);
> > int (*erase)(struct blocklevel_device *bl, uint64_t pos, uint64_t
> len);
> > - int (*get_info)(struct blocklevel_device *bl, const char **name,
> uint64_t *total_size,
> > - uint32_t *erase_granule);
> > + int (*get_info)(struct blocklevel_device *bl, struct
> blocklevel_info *bl_info);
> >
> > /*
> > * Keep the erase mask so that blocklevel_erase() can do sanity
> checking
> > @@ -62,8 +70,8 @@ int blocklevel_read(struct blocklevel_device *bl,
> uint64_t pos, void *buf, uint6
> > int blocklevel_raw_write(struct blocklevel_device *bl, uint64_t pos,
> const void *buf, uint64_t len);
> > int blocklevel_write(struct blocklevel_device *bl, uint64_t pos, const
> void *buf, uint64_t len);
> > int blocklevel_erase(struct blocklevel_device *bl, uint64_t pos,
> uint64_t len);
> > -int blocklevel_get_info(struct blocklevel_device *bl, const char
> **name, uint64_t *total_size,
> > - uint32_t *erase_granule);
> > +
> > +int blocklevel_get_info(struct blocklevel_device *bl, struct
> blocklevel_info *bl_info);
> >
> > /*
> > * blocklevel_smart_write() performs reads on the data to see if it
> > diff --git a/libflash/file.c b/libflash/file.c
> > index 7065eb4fa..b87679cd1 100644
> > --- a/libflash/file.c
> > +++ b/libflash/file.c
> > @@ -212,6 +212,7 @@ static int get_info_name(struct file_data
> *file_data, char **name)
> > }
> > lpath[len] = '\0';
> >
> > + free(*name);
> > *name = lpath;
> >
> > free(path);
> > @@ -219,8 +220,8 @@ static int get_info_name(struct file_data
> *file_data, char **name)
> > }
> >
> >
> > -static int mtd_get_info(struct blocklevel_device *bl, const char **name,
> > - uint64_t *total_size, uint32_t *erase_granule)
> > +static int mtd_get_info(struct blocklevel_device *bl,
> > + struct blocklevel_info *bl_info)
> > {
> > struct file_data *file_data = container_of(bl, struct file_data,
> bl);
> > struct mtd_info_user mtd_info;
> > @@ -230,24 +231,19 @@ static int mtd_get_info(struct blocklevel_device
> *bl, const char **name,
> > if (rc == -1)
> > return FLASH_ERR_BAD_READ;
> >
> > - if (total_size)
> > - *total_size = mtd_info.size;
> > -
> > - if (erase_granule)
> > - *erase_granule = mtd_info.erasesize;
> > + rc = get_info_name(file_data, &(file_data->name));
> > + if (rc)
> > + return rc;
> >
> > - if (name) {
> > - rc = get_info_name(file_data, &(file_data->name));
> > - if (rc)
> > - return rc;
> > - *name = file_data->name;
> > - }
> > + bl_info->name = file_data->name;
> > + bl_info->size = mtd_info.size;
> > + bl_info->erase_granule = mtd_info.erasesize;
> >
> > return 0;
> > }
> >
> > -static int file_get_info(struct blocklevel_device *bl, const char
> **name,
> > - uint64_t *total_size, uint32_t *erase_granule)
> > +static int file_get_info(struct blocklevel_device *bl,
> > + struct blocklevel_info *bl_info)
> > {
> > struct file_data *file_data = container_of(bl, struct file_data,
> bl);
> > struct stat st;
> > @@ -256,18 +252,13 @@ static int file_get_info(struct blocklevel_device
> *bl, const char **name,
> > if (fstat(file_data->fd, &st))
> > return FLASH_ERR_PARM_ERROR;
> >
> > - if (total_size)
> > - *total_size = st.st_size;
> > -
> > - if (erase_granule)
> > - *erase_granule = 1;
> > + rc = get_info_name(file_data, &(file_data->name));
> > + if (rc)
> > + return rc;
> >
> > - if (name) {
> > - rc = get_info_name(file_data, &(file_data->name));
> > - if (rc)
> > - return rc;
> > - *name = file_data->name;
> > - }
> > + bl_info->name = file_data->name;
> > + bl_info->size = st.st_size;
> > + bl_info->erase_granule = 1;
> >
> > return 0;
> > }
> > @@ -315,8 +306,9 @@ int file_init(int fd, struct blocklevel_device **bl)
> > file_data->bl.erase = &mtd_erase;
> > file_data->bl.get_info = &mtd_get_info;
> > file_data->bl.flags = WRITE_NEED_ERASE;
> > - mtd_get_info(&file_data->bl, NULL, NULL,
> &(file_data->bl.erase_mask));
> > - file_data->bl.erase_mask--;
> > + struct blocklevel_info bl_info;
> > + mtd_get_info(&file_data->bl, &bl_info);
> > + file_data->bl.erase_mask = bl_info.erase_granule - 1;
> > } else if (!S_ISREG(sbuf.st_mode)) {
> > /* If not a char device or a regular file something went
> wrong */
> > goto out;
> > diff --git a/libflash/libffs.c b/libflash/libffs.c
> > index 038f59425..6fc23eee9 100644
> > --- a/libflash/libffs.c
> > +++ b/libflash/libffs.c
> > @@ -216,25 +216,25 @@ int ffs_init(uint32_t offset, uint32_t max_size,
> struct blocklevel_device *bl,
> > {
> > struct __ffs_hdr blank_hdr;
> > struct __ffs_hdr raw_hdr;
> > + struct blocklevel_info bl_info;
> > struct ffs_handle *f;
> > - uint64_t total_size;
> > int rc, i;
> >
> > if (!ffs || !bl)
> > return FLASH_ERR_PARM_ERROR;
> > *ffs = NULL;
> >
> > - rc = blocklevel_get_info(bl, NULL, &total_size, NULL);
> > + rc = blocklevel_get_info(bl, &bl_info);
> > if (rc) {
> > FL_ERR("FFS: Error %d retrieving flash info\n", rc);
> > return rc;
> > }
> > - if (total_size > UINT_MAX)
> > + if (bl_info.size > UINT_MAX)
> > return FLASH_ERR_VERIFY_FAILURE;
> > if ((offset + max_size) < offset)
> > return FLASH_ERR_PARM_ERROR;
> >
> > - if ((max_size > total_size))
> > + if ((max_size > bl_info.size))
> > return FLASH_ERR_PARM_ERROR;
> >
> > /* Read flash header */
> > diff --git a/libflash/libflash.c b/libflash/libflash.c
> > index 38f87b86e..82a2baecc 100644
> > --- a/libflash/libflash.c
> > +++ b/libflash/libflash.c
> > @@ -792,16 +792,15 @@ static int flash_configure(struct flash_chip *c)
> > return 0;
> > }
> >
> > -static int flash_get_info(struct blocklevel_device *bl, const char
> **name,
> > - uint64_t *total_size, uint32_t *erase_granule)
> > +static int flash_get_info(struct blocklevel_device *bl,
> > + struct blocklevel_info *bl_info)
> > {
> > struct flash_chip *c = container_of(bl, struct flash_chip, bl);
> > - if (name)
> > - *name = c->info.name;
> > - if (total_size)
> > - *total_size = c->tsize;
> > - if (erase_granule)
> > - *erase_granule = c->min_erase_mask + 1;
> > +
> > + bl_info->name = c->info.name;
> > + bl_info->size = c->tsize;
> > + bl_info->erase_granule = c->min_erase_mask + 1;
> > +
> > return 0;
> > }
> >
> > diff --git a/libflash/mbox-flash.c b/libflash/mbox-flash.c
> > index 950e24f05..d4200ce53 100644
> > --- a/libflash/mbox-flash.c
> > +++ b/libflash/mbox-flash.c
> > @@ -745,8 +745,8 @@ static int mbox_flash_read(struct blocklevel_device
> *bl, uint64_t pos,
> > return rc;
> > }
> >
> > -static int mbox_flash_get_info(struct blocklevel_device *bl, const char
> **name,
> > - uint64_t *total_size, uint32_t *erase_granule)
> > +static int mbox_flash_get_info(struct blocklevel_device *bl,
> > + struct blocklevel_info *bl_info)
> > {
> > struct mbox_flash_data *mbox_flash;
> > struct bmc_mbox_msg *msg;
> > @@ -761,14 +761,7 @@ static int mbox_flash_get_info(struct
> blocklevel_device *bl, const char **name,
> > if (!msg)
> > return FLASH_ERR_MALLOC_FAILED;
> >
> > - /*
> > - * We want to avoid runtime mallocs in skiboot. The expected
> > - * behavour to uses of libflash is that one can free() the memory
> > - * returned.
> > - * NULL will do for now.
> > - */
> > - if (name)
> > - *name = NULL;
> > + bl_info->name = "pnor_mbox";
> >
> > mbox_flash->busy = true;
> > rc = msg_send(mbox_flash, msg);
> > @@ -784,10 +777,8 @@ static int mbox_flash_get_info(struct
> blocklevel_device *bl, const char **name,
> >
> > mbox_flash->bl.erase_mask = mbox_flash->erase_granule - 1;
> >
> > - if (total_size)
> > - *total_size = mbox_flash->total_size;
> > - if (erase_granule)
> > - *erase_granule = mbox_flash->erase_granule;
> > + bl_info->size = mbox_flash->total_size;
> > + bl_info->erase_granule = mbox_flash->erase_granule;
> >
> > out:
> > msg_free_memory(msg);
> > diff --git a/libflash/test/test-flash.c b/libflash/test/test-flash.c
> > index 3f77d6f3e..1ca467bb0 100644
> > --- a/libflash/test/test-flash.c
> > +++ b/libflash/test/test-flash.c
> > @@ -367,9 +367,7 @@ struct spi_flash_ctrl sim_ctrl = {
> > int main(void)
> > {
> > struct blocklevel_device *bl;
> > - uint64_t total_size;
> > - uint32_t erase_granule;
> > - const char *name;
> > + struct blocklevel_info bl_info;
> > uint16_t *test;
> > struct ecc64 *ecc_test;
> > uint64_t *test64;
> > @@ -384,7 +382,7 @@ int main(void)
> > ERR("flash_init failed with err %d\n", rc);
> > exit(1);
> > }
> > - rc = flash_get_info(bl, &name, &total_size, &erase_granule);
> > + rc = flash_get_info(bl, &bl_info);
> > if (rc) {
> > ERR("flash_get_info failed with err %d\n", rc);
> > exit(1);
> > diff --git a/platforms/mambo/mambo.c b/platforms/mambo/mambo.c
> > index cb6e103cc..dd7872a8e 100644
> > --- a/platforms/mambo/mambo.c
> > +++ b/platforms/mambo/mambo.c
> > @@ -125,19 +125,14 @@ static int bogus_disk_erase(struct
> blocklevel_device *bl __unused,
> > return 0; /* NOP */
> > }
> >
> > -static int bogus_disk_get_info(struct blocklevel_device *bl, const char
> **name,
> > - uint64_t *total_size, uint32_t
> *erase_granule)
> > +static int bogus_disk_get_info(struct blocklevel_device *bl,
> > + struct blocklevel_info *bl_info)
> > {
> > struct bogus_disk_info *bdi = bl->priv;
> >
> > - if (total_size)
> > - *total_size = bdi->size;
> > -
> > - if (erase_granule)
> > - *erase_granule = BD_SECT_SZ;
> > -
> > - if (name)
> > - *name = "mambobogus";
> > + bl_info->name = "mambobogus";
> > + bl_info->size = bdi->size;
> > + bl_info->erase_granule = BD_SECT_SZ;
> >
> > return 0;
> > }
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/skiboot/attachments/20170829/e95a2d91/attachment-0001.html>
More information about the Skiboot
mailing list