On Tue, 2026-04-28 at 08:11 -0600, Simon Glass wrote: > Hi Weijie, > > On 2026-04-27T07:09:11, Weijie Gao <[email protected]> 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 <[email protected]> > > > > 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 <[email protected]>
