Hi Heinrich,
On Mon, 4 Nov 2024 at 15:15, Heinrich Schuchardt <xypron.g...@gmx.de> wrote:
>
> On 11/4/24 18:50, Simon Glass wrote:
> > This relates to more than just the bootdev, since there is a global list
> > of bootflows. Move the function to the bootstd file and rename it.
> >
> > Signed-off-by: Simon Glass <s...@chromium.org>
> > ---
> >
> > (no changes since v1)
> >
> > boot/bootdev-uclass.c | 25 -------------------------
> > boot/bootstd-uclass.c | 25 +++++++++++++++++++++++++
> > cmd/bootflow.c | 2 +-
> > include/bootdev.h | 15 ---------------
> > include/bootstd.h | 17 +++++++++++++++++
> > 5 files changed, 43 insertions(+), 41 deletions(-)
> >
> > diff --git a/boot/bootdev-uclass.c b/boot/bootdev-uclass.c
> > index 64ec4fde493..eddbf60600c 100644
> > --- a/boot/bootdev-uclass.c
> > +++ b/boot/bootdev-uclass.c
> > @@ -32,31 +32,6 @@ enum {
> > BOOT_TARGETS_MAX_LEN = 100,
> > };
> >
> > -int bootdev_add_bootflow(struct bootflow *bflow)
> > -{
> > - struct bootstd_priv *std;
> > - struct bootflow *new;
> > - int ret;
> > -
> > - ret = bootstd_get_priv(&std);
> > - if (ret)
> > - return ret;
> > -
> > - new = malloc(sizeof(*bflow));
> > - if (!new)
> > - return log_msg_ret("bflow", -ENOMEM);
> > - memcpy(new, bflow, sizeof(*bflow));
> > -
> > - list_add_tail(&new->glob_node, &std->glob_head);
> > - if (bflow->dev) {
> > - struct bootdev_uc_plat *ucp = dev_get_uclass_plat(bflow->dev);
> > -
> > - list_add_tail(&new->bm_node, &ucp->bootflow_head);
> > - }
> > -
> > - return 0;
> > -}
> > -
> > int bootdev_first_bootflow(struct udevice *dev, struct bootflow **bflowp)
> > {
> > struct bootdev_uc_plat *ucp = dev_get_uclass_plat(dev);
> > diff --git a/boot/bootstd-uclass.c b/boot/bootstd-uclass.c
> > index fdb8d69e320..bf6e49ad97a 100644
> > --- a/boot/bootstd-uclass.c
> > +++ b/boot/bootstd-uclass.c
> > @@ -61,6 +61,31 @@ void bootstd_clear_glob(void)
> > bootstd_clear_glob_(std);
> > }
> >
> > +int bootstd_add_bootflow(struct bootflow *bflow)
> > +{
> > + struct bootstd_priv *std;
> > + struct bootflow *new;
> > + int ret;
> > +
> > + ret = bootstd_get_priv(&std);
> > + if (ret)
> > + return ret;
> > +
> > + new = malloc(sizeof(*bflow));
> > + if (!new)
> > + return log_msg_ret("bflow", -ENOMEM);
> > + memcpy(new, bflow, sizeof(*bflow));
> > +
> > + list_add_tail(&new->glob_node, &std->glob_head);
> > + if (bflow->dev) {
> > + struct bootdev_uc_plat *ucp = dev_get_uclass_plat(bflow->dev);
> > +
> > + list_add_tail(&new->bm_node, &ucp->bootflow_head);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int bootstd_remove(struct udevice *dev)
> > {
> > struct bootstd_priv *priv = dev_get_priv(dev);
> > diff --git a/cmd/bootflow.c b/cmd/bootflow.c
> > index f67948d7368..8962464bbf8 100644
> > --- a/cmd/bootflow.c
> > +++ b/cmd/bootflow.c
> > @@ -207,7 +207,7 @@ static int do_bootflow_scan(struct cmd_tbl *cmdtp, int
> > flag, int argc,
> > bflow.err = ret;
> > if (!ret)
> > num_valid++;
> > - ret = bootdev_add_bootflow(&bflow);
> > + ret = bootstd_add_bootflow(&bflow);
> > if (ret) {
> > printf("Out of memory\n");
> > return CMD_RET_FAILURE;
> > diff --git a/include/bootdev.h b/include/bootdev.h
> > index ad4af0d1310..8db198dd56b 100644
> > --- a/include/bootdev.h
> > +++ b/include/bootdev.h
> > @@ -195,21 +195,6 @@ void bootdev_list(bool probe);
> > */
> > void bootdev_clear_bootflows(struct udevice *dev);
> >
> > -/**
> > - * bootdev_add_bootflow() - Add a bootflow to the bootdev's list
> > - *
> > - * All fields in @bflow must be set up. Note that @bflow->dev is used to
> > add the
> > - * bootflow to that device.
> > - *
> > - * @dev: Bootdev device to add to
> > - * @bflow: Bootflow to add. Note that fields within bflow must be allocated
> > - * since this function takes over ownership of these. This functions
> > makes
> > - * a copy of @bflow itself (without allocating its fields again), so the
> > - * caller must dispose of the memory used by the @bflow pointer itself
> > - * Return: 0 if OK, -ENOMEM if out of memory
> > - */
> > -int bootdev_add_bootflow(struct bootflow *bflow);
> > -
> > /**
> > * bootdev_first_bootflow() - Get the first bootflow from a bootdev
> > *
> > diff --git a/include/bootstd.h b/include/bootstd.h
> > index ac756e98d84..3fc93a4ec2e 100644
> > --- a/include/bootstd.h
> > +++ b/include/bootstd.h
> > @@ -105,4 +105,21 @@ void bootstd_clear_glob(void);
> > */
> > int bootstd_prog_boot(void);
> >
> > +/**
> > + * bootstd_add_bootflow() - Add a bootflow to the bootdev's and global list
> > + *
> > + * All fields in @bflow must be set up. Note that @bflow->dev is used to
> > add the
> > + * bootflow to that device.
> > + *
> > + * The bootflow is also added to the global list of all bootflows
> > + *
> > + * @dev: Bootdev device to add to
>
> This parameter does not exist.
>
> > + * @bflow: Bootflow to add. Note that fields within bflow must be allocated
> > + * since this function takes over ownership of these. This functions
> > makes
> > + * a copy of @bflow itself (without allocating its fields again), so the
> > + * caller must dispose of the memory used by the @bflow pointer itself
>
> Please, add the headers to the API documentation and fix the numerous
> documentation issues like:
>
> ./include/bootdev.h:57: warning: Enum value 'BOOTDEVP_0_NONE' not
> described in enum 'bootdev_prio_t'
> ./include/bootdev.h:57: warning: Enum value 'BOOTDEVP_1_PRE_SCAN' not
> described in enum 'bootdev_prio_t'
> ./include/bootdev.h:57: warning: Enum value 'BOOTDEVP_COUNT' not
> described in enum 'bootdev_prio_t'
> ./include/bootdev.h:57: warning: Excess enum value 'BOOTDEVP_6_PRE_SCAN'
> description in 'bootdev_prio_t'
> ./include/bootdev.h:118: warning: Function parameter or member
> 'bootflow_head' not described in 'bootdev_uc_plat'
> ./include/bootdev.h:118: warning: Function parameter or member 'prio'
> not described in 'bootdev_uc_plat'
> ./include/bootdev.h:412: warning: Function parameter or member 'blk' not
> described in 'bootdev_setup_for_sibling_blk'
> ./include/bootdev.h:412: warning: Excess function parameter 'parent'
> description in 'bootdev_setup_for_sibling_blk'
I don't mind doing that in a follow-up once this series is in, but it
is out-of-scope here.
Regards,
Simon
