[PATCH v3 09/10] ubi: add comments for exported ubi API functions

[Weijie Gao]

On Tue, 2026-04-28 at 08:08 -0600, Simon Glass wrote:
> Hi Weijie,
> 
> On 2026-04-27T07:09:11, Weijie Gao <weijie.gao at mediatek.com> wrote:
> > ubi: add comments for exported ubi API functions
> > 
> > This patch adds comments for exported ubi command API functions.
> > 
> > Signed-off-by: Weijie Gao <weijie.gao at mediatek.com>
> > 
> > include/ubi_uboot.h | 86
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 86 insertions(+)
> > diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
> > @@ -47,14 +47,100 @@
> > + * ubi_part() - attach UBI to MTD partition
> > + * @part_name: name of the MTD partition to attach
> > + * @vid_header_offset: VID header offset (string)
> > + *
> > + * This function detaches any existing UBI device, then probes for
> > the
> > + * specified MTD partition, and then scans it to initialize UBI.
> > + *
> > + * @vid_header_offset is optional and is usually set to NULL.
> > + *
> > + * Return: 0 on success, 1 if partition not found, or -ve on
> > error.
> > + */
> > int ubi_part(const char *part_name, const char *vid_header_offset);
> 
> Most error paths in this file return positive errno values, so '-ve
> on
> error' is not quite right. Please double-check and document the
> actual
> convention.
> 
> > diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
> > @@ -47,14 +47,100 @@
> > + * ubi_volume_read() - read data from UBI volume
> > + * @volume: name of the volume to read from
> > + * @buf: buffer to hold the read data
> > + * @offset: start offset for reading
> > + * @size: number of bytes to read
> > + *
> > + * This function reads data from the specified UBI volume. If
> > @size is zero,
> > + * the function reads the entire volume content starting from
> > @offset.
> > + *
> > + * Return: 0 on success, or -ve on error.
> > + */
> > int ubi_volume_read(const char *volume, void *buf, loff_t offset,
> > size_t size);
> 
> Same here early-exit paths return positive errno (ENODEV, EBUSY,
> EBADF, ENOMEM) while the read loop negates ubi_eba_read_leb()'s
> error.
> Better to normalise to one convention, and please be consistent
> across
> all of these.

For the above two comments for returning -ve, I'll send another patch
to change all return value to negative.

> 
> > diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
> > @@ -47,14 +47,100 @@
> > + * This function creates a new UBI volume with the specified
> > parameters.
> > + * If @size is negative, all available spaces will be used.
> > + * if @vol_id is negative, volume ID will be selected and assigned
> > by UBI.
> > + *
> > + * Return: 0 on success, or -ve on error.
> > + */
> > int ubi_create_vol(const char *volume, int64_t size, bool dynamic,
> > int vol_id,
> >                  bool skipcheck);
> 
> Capitlal I on 'If'. Also 'all available spaces' reads better as 'all
> available space'. Also worth mentioning that @vol_id should be
> UBI_VOL_NUM_AUTO for auto-assignment.
> 
> > diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
> > @@ -47,14 +47,100 @@
> > + * ubi_detach() - detach UBI from MTD partition
> > + *
> > + * This function performs the cleanup of the UBI subsystem to make
> > sure the
> > + * MTD partition can be safely used by other purpose, or be
> > attached again
> > + * with ubi_part().
> > + *
> > + * Any mounted UBIFS will be unmounted automatically.
> > + *
> > + * Return: 0 on success.
> > + */
> > int ubi_detach(void);
> 
> 'used by other purpose' should be 'used for another purpose'. Since
> this only ever returns 0, you could say 'Return: 0' and drop 'on
> success'.

I'll fix the rest two comments.

> 
> Regards,
> Simon