On Sat, Jul 05, 2025 at 10:58:03AM +0800, jiang.pe...@zte.com.cn wrote:
> From: Peng Jiang <jiang.pe...@zte.com.cn>
>
> Fix kernel-doc descriptions in virtio_dma_buf.c to fix W=1 warnings:
>
> drivers/virtio/virtio_dma_buf.c:41 function parameter 'dma_buf' not described
> in 'virtio_dma_buf_attach'
> drivers/virtio/virtio_dma_buf.c:41 function parameter 'attach' not described
> in 'virtio_dma_buf_attach'
>
> Signed-off-by: Peng Jiang <jiang.pe...@zte.com.cn>
The extra documentation unfortunately just mechanically repeats what the
code does. Code comments should explain the reasoning behind the code,
instead.
> ---
> drivers/virtio/virtio_dma_buf.c | 30 +++++++++++++++++++++++++-----
> 1 file changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/virtio/virtio_dma_buf.c b/drivers/virtio/virtio_dma_buf.c
> index 3fe1d03b0645..0b39b1b209ee 100644
> --- a/drivers/virtio/virtio_dma_buf.c
> +++ b/drivers/virtio/virtio_dma_buf.c
> @@ -9,13 +9,20 @@
> #include <linux/virtio_dma_buf.h>
>
> /**
> - * virtio_dma_buf_export - Creates a new dma-buf for a virtio exported object
> + * virtio_dma_buf_export() - Creates a new dma-buf for a virtio exported
> object
virtio core uses the form without () consistently, everywhere.
> * @exp_info: [in] see dma_buf_export(). ops MUST refer to a dma_buf_ops
> * struct embedded in a virtio_dma_buf_ops.
> *
> * This wraps dma_buf_export() to allow virtio drivers to create a dma-buf
> * for an virtio exported object that can be queried by other virtio drivers
> * for the object's UUID.
> + *
> + * Returns:
> + * - Valid struct dma_buf pointer on success
> + * - ERR_PTR(-EINVAL) if:
> + * - exp_info->ops is NULL
> + * - attach callback isn't virtio_dma_buf_attach()
> + * - virtio_ops->get_uuid() is not implemented
Too verbose, no one will rememeber to update this when changing code.
Just
Returns the dma_buf or ERR_PTR
is enough.
> */
> struct dma_buf *virtio_dma_buf_export
> (const struct dma_buf_export_info *exp_info)
> @@ -35,7 +42,16 @@ struct dma_buf *virtio_dma_buf_export
> EXPORT_SYMBOL(virtio_dma_buf_export);
>
> /**
> - * virtio_dma_buf_attach - mandatory attach callback for virtio dma-bufs
> + * virtio_dma_buf_attach() - Mandatory attach callback for virtio dma-bufs
> + * @dma_buf: Pointer to the shared dma-buf structure
> + * @attach: Pointer to the newly created attachment metadata
> + *
> + * Implements the standard dma-buf attach operation for virtio devices.
> + * Retrieves virtio-specific operations through container_of macro,
> + * then invokes device-specific attach callback if present.
> + * This enables virtio devices to participate in dma-buf sharing.
> + *
Same thing pls do not repeat all of code.
> + * Return: 0 on success, error code on failure
we say "Returns zero or a negative error" elsewhere. seems clearer.
> */
> int virtio_dma_buf_attach(struct dma_buf *dma_buf,
> struct dma_buf_attachment *attach)
> @@ -55,8 +71,12 @@ int virtio_dma_buf_attach(struct dma_buf *dma_buf,
> EXPORT_SYMBOL(virtio_dma_buf_attach);
>
> /**
> - * is_virtio_dma_buf - returns true if the given dma-buf is a virtio dma-buf
> - * @dma_buf: buffer to query
> + * is_virtio_dma_buf() - Check if a dma-buf was created by virtio DMA
> framework
> + * @dma_buf: [in] buffer to query
> + *
> + * Returns:
> + * - true if the dma-buf uses virtio_dma_buf_attach() as its attach
> callback
> + * - false otherwise
one is return one is returns ... no consistency.
> */
> bool is_virtio_dma_buf(struct dma_buf *dma_buf)
> {
> @@ -65,7 +85,7 @@ bool is_virtio_dma_buf(struct dma_buf *dma_buf)
> EXPORT_SYMBOL(is_virtio_dma_buf);
>
> /**
> - * virtio_dma_buf_get_uuid - gets a virtio dma-buf's exported object's uuid
> + * virtio_dma_buf_get_uuid() - gets a virtio dma-buf's exported object's uuid
> * @dma_buf: [in] buffer to query
> * @uuid: [out] the uuid
> *
> --
> 2.25.1
