[PATCH v3 07/10] cmd: ubi: export more APIs to public

[Weijie Gao]

On Tue, 2026-04-28 at 08:11 -0600, Simon Glass wrote:
> Hi Weijie,
> 
> On 2026-04-27T07:09:11, Weijie Gao <weijie.gao at mediatek.com> wrote:
> > cmd: ubi: export more APIs to public
> > 
> > Export the following functions to public:
> > 
> > - ubi_detach(): this is paired with ubi_part(). One may call this
> > function
> >   to completely clean up the ubi subsystem after using ubi_part().
> > 
> > - ubi_{create,find,remove}_vol: this is a set of functions for
> > volume
> >   management.
> > 
> > Signed-off-by: Weijie Gao <weijie.gao at mediatek.com>
> > 
> > cmd/ubi.c           | 10 +++++-----
> >  include/ubi_uboot.h |  5 +++++
> >  2 files changed, 10 insertions(+), 5 deletions(-)
> > diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
> > @@ -47,10 +47,15 @@
> >  int ubi_mtd_param_parse(const char *val, struct kernel_param *kp);
> >  int ubi_init(void);
> >  void ubi_exit(void);
> > +int ubi_detach(void);
> >  int ubi_part(const char *part_name, const char
> > *vid_header_offset);
> >  int ubi_volume_write(const char *volume, const void *buf, loff_t
> > offset,
> >                    size_t size);
> >  int ubi_volume_read(const char *volume, void *buf, loff_t offset,
> > size_t size);
> > +int ubi_create_vol(const char *volume, int64_t size, bool dynamic,
> > int vol_id,
> > +                bool skipcheck);
> > +struct ubi_volume *ubi_find_volume(const char *volume);
> > +int ubi_remove_vol(const char *volume);
> 
> Please squash patch 9 (the kernel-doc comments) into this one.
> Exporting an API and documenting it belong in the same change -
> otherwise reviewers cannot judge the function contract from this
> patch
> alone.
> 
> > diff --git a/cmd/ubi.c b/cmd/ubi.c
> > @@ -270,7 +270,7 @@ static struct ubi_volume
> > *ubi_require_volume(const char *volume)
> >       return vol;
> >  }
> > 
> > -static int ubi_remove_vol(const char *volume)
> > +int ubi_remove_vol(const char *volume)
> >  {
> >       int err, reserved_pebs, i;
> >       struct ubi_volume *vol;
> 
> Hmm, now that this is public, the mixed return convention is a bit
> awkward: ubi_remove_vol() returns positive ENODEV/EROFS in some paths
> and negative values from ubi_change_vtbl_record()/ubi_eba_unmap_leb()
> in others. ubi_create_vol() has the same issue. Worth a follow-up so
> external callers get a consistent convention - ideally negative errno

Using uniformed negative return value is apparently better.
Patch 9 will also benefit from this: we can safely say returning -ve on
error.

The ubi layer does return negative value in all paths already. I plan
to change all return value in ubi.c to negative in all functions and
return 1 (CMD_RET_FAILURE) for the command handler. 

> 
> Reviewed-by: Simon Glass <sjg at chromium.org>