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

[Simon Glass]

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.

> 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'.

Regards,
Simon