2022-12-20 08:25:06

by Ricardo Cañuelo

[permalink] [raw]
Subject: [PATCH] virtio: fix virtio_config_ops kerneldocs

Fixes two warning messages when building htmldocs:

warning: duplicate section name 'Note'
warning: expecting prototype for virtio_config_ops().
Prototype was for vq_callback_t() instead

Signed-off-by: Ricardo Cañuelo <[email protected]>
---
include/linux/virtio_config.h | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 4b517649cfe8..f9a33062c089 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -16,8 +16,10 @@ struct virtio_shm_region {
u64 len;
};

+typedef void vq_callback_t(struct virtqueue *);
+
/**
- * virtio_config_ops - operations for configuring a virtio device
+ * struct virtio_config_ops - operations for configuring a virtio device
* Note: Do not assume that a transport implements all of the operations
* getting/setting a value as a simple read/write! Generally speaking,
* any of @get/@set, @get_status/@set_status, or @get_features/
@@ -68,8 +70,8 @@ struct virtio_shm_region {
* @finalize_features: confirm what device features we'll be using.
* vdev: the virtio_device
* This sends the driver feature bits to the device: it can change
- * the dev->feature bits if it wants.
- * Note: despite the name this can be called any number of times.
+ * the dev->feature bits if it wants. Note: despite the name this
+ * can be called any number of times.
* Returns 0 on success or error status
* @bus_name: return the bus name associated with the device (optional)
* vdev: the virtio_device
@@ -91,7 +93,6 @@ struct virtio_shm_region {
* If disable_vq_and_reset is set, then enable_vq_after_reset must also be
* set.
*/
-typedef void vq_callback_t(struct virtqueue *);
struct virtio_config_ops {
void (*get)(struct virtio_device *vdev, unsigned offset,
void *buf, unsigned len);
--
2.25.1


2022-12-20 09:33:16

by Bagas Sanjaya

[permalink] [raw]
Subject: Re: [PATCH] virtio: fix virtio_config_ops kerneldocs

On Tue, Dec 20, 2022 at 08:37:09AM +0100, Ricardo Cañuelo wrote:
> Fixes two warning messages when building htmldocs:
>
> warning: duplicate section name 'Note'
> warning: expecting prototype for virtio_config_ops().
> Prototype was for vq_callback_t() instead
>

Describe the steps needed to fix both warnings above. I see in the diff that:

* move vq_callback_t() declaration above;
* match entity type of virtio_config_ops; and
* reformat @finalize_features description.

> Signed-off-by: Ricardo Cañuelo <[email protected]>

You need to add appropriate tags:

Link: https://lore.kernel.org/linux-next/[email protected]/
Fixes: 333723e8bc393d ("docs: driver-api: virtio: virtio on Linux")
Reported-by: Stephen Rothwell <[email protected]>

> ---
> include/linux/virtio_config.h | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> index 4b517649cfe8..f9a33062c089 100644
> --- a/include/linux/virtio_config.h
> +++ b/include/linux/virtio_config.h
> @@ -16,8 +16,10 @@ struct virtio_shm_region {
> u64 len;
> };
>
> +typedef void vq_callback_t(struct virtqueue *);
> +
> /**
> - * virtio_config_ops - operations for configuring a virtio device
> + * struct virtio_config_ops - operations for configuring a virtio device
> * Note: Do not assume that a transport implements all of the operations
> * getting/setting a value as a simple read/write! Generally speaking,
> * any of @get/@set, @get_status/@set_status, or @get_features/
> @@ -68,8 +70,8 @@ struct virtio_shm_region {
> * @finalize_features: confirm what device features we'll be using.
> * vdev: the virtio_device
> * This sends the driver feature bits to the device: it can change
> - * the dev->feature bits if it wants.
> - * Note: despite the name this can be called any number of times.
> + * the dev->feature bits if it wants. Note: despite the name this
> + * can be called any number of times.
> * Returns 0 on success or error status
> * @bus_name: return the bus name associated with the device (optional)
> * vdev: the virtio_device
> @@ -91,7 +93,6 @@ struct virtio_shm_region {
> * If disable_vq_and_reset is set, then enable_vq_after_reset must also be
> * set.
> */
> -typedef void vq_callback_t(struct virtqueue *);
> struct virtio_config_ops {
> void (*get)(struct virtio_device *vdev, unsigned offset,
> void *buf, unsigned len);

Anyway, the warnings went away, thanks for the fixup!

Reviewed-by: Bagas Sanjaya <[email protected]>

--
An old man doll... just what I always wanted! - Clara


Attachments:
(No filename) (2.70 kB)
signature.asc (235.00 B)
Download all attachments
Subject: Re: [PATCH] virtio: fix virtio_config_ops kerneldocs

Il 20/12/22 08:37, Ricardo Cañuelo ha scritto:
> Fixes two warning messages when building htmldocs:
>
> warning: duplicate section name 'Note'
> warning: expecting prototype for virtio_config_ops().
> Prototype was for vq_callback_t() instead
>
> Signed-off-by: Ricardo Cañuelo <[email protected]>
> ---
> include/linux/virtio_config.h | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> index 4b517649cfe8..f9a33062c089 100644
> --- a/include/linux/virtio_config.h
> +++ b/include/linux/virtio_config.h
> @@ -16,8 +16,10 @@ struct virtio_shm_region {
> u64 len;
> };
>
> +typedef void vq_callback_t(struct virtqueue *);
> +
> /**
> - * virtio_config_ops - operations for configuring a virtio device
> + * struct virtio_config_ops - operations for configuring a virtio device
> * Note: Do not assume that a transport implements all of the operations
> * getting/setting a value as a simple read/write! Generally speaking,
> * any of @get/@set, @get_status/@set_status, or @get_features/
> @@ -68,8 +70,8 @@ struct virtio_shm_region {
> * @finalize_features: confirm what device features we'll be using.
> * vdev: the virtio_device
> * This sends the driver feature bits to the device: it can change
> - * the dev->feature bits if it wants.
> - * Note: despite the name this can be called any number of times.
> + * the dev->feature bits if it wants. Note: despite the name this
> + * can be called any number of times.

To avoid getting the same warning in the future (developer mistake and/or other
reasons), what about dropping this instance of "Note:" entirely?

I think that something like...

the dev->feature bits if it wants. Note that despite the name....

Cheers,
Angelo

2022-12-20 10:16:07

by Ricardo Cañuelo

[permalink] [raw]
Subject: Re: [PATCH] virtio: fix virtio_config_ops kerneldocs



On 20/12/22 10:48, AngeloGioacchino Del Regno wrote:
> To avoid getting the same warning in the future (developer mistake and/or other
> reasons), what about dropping this instance of "Note:" entirely?
>
> I think that something like...
>
> the dev->feature bits if it wants. Note that despite the name....

Thanks for the suggestion, Angelo. Applied in v2.

Cheers,
Ricardo

2022-12-20 10:53:43

by Ricardo Cañuelo

[permalink] [raw]
Subject: Re: [PATCH] virtio: fix virtio_config_ops kerneldocs

Hi Bagas,

Thanks for the review, some comments below:

On 20/12/22 10:12, Bagas Sanjaya wrote:> On Tue, Dec 20, 2022 at 08:37:09AM +0100, Ricardo Cañuelo wrote:
> Describe the steps needed to fix both warnings above. I see in the diff that:
>
> * move vq_callback_t() declaration above;
> * match entity type of virtio_config_ops; and
> * reformat @finalize_features description.

I wouldn't like to add redundant info in the commit message for
such a trivial patch. The commit message describes _what_ the
patch does. The _how_ is just as clear in the patch itself as in
this description, IMO.

>
>> Signed-off-by: Ricardo Cañuelo <[email protected]>
>
> You need to add appropriate tags:
>
> Link: https://lore.kernel.org/linux-next/[email protected]/
> Fixes: 333723e8bc393d ("docs: driver-api: virtio: virtio on Linux")
> Reported-by: Stephen Rothwell <[email protected]>

Thanks for the tip although, actually, it's not that commit that
needs to be fixed but the kerneldoc itself. The warnings were
made visible after that commit but not introduced by it. I'll add
the Reported-by tag in v2.

Cheers,
Ricardo

2022-12-20 11:26:09

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [PATCH] virtio: fix virtio_config_ops kerneldocs

On Tue, Dec 20, 2022 at 10:54:17AM +0100, Ricardo Ca?uelo wrote:
> Hi Bagas,
>
> Thanks for the review, some comments below:
>
> On 20/12/22 10:12, Bagas Sanjaya wrote:> On Tue, Dec 20, 2022 at 08:37:09AM +0100, Ricardo Ca?uelo wrote:
> > Describe the steps needed to fix both warnings above. I see in the diff that:
> >
> > * move vq_callback_t() declaration above;
> > * match entity type of virtio_config_ops; and
> > * reformat @finalize_features description.
>
> I wouldn't like to add redundant info in the commit message for
> such a trivial patch. The commit message describes _what_ the
> patch does. The _how_ is just as clear in the patch itself as in
> this description, IMO.

Yea it's overkill for this patch.

> >
> > > Signed-off-by: Ricardo Ca?uelo <[email protected]>
> >
> > You need to add appropriate tags:
> >
> > Link: https://lore.kernel.org/linux-next/[email protected]/
> > Fixes: 333723e8bc393d ("docs: driver-api: virtio: virtio on Linux")
> > Reported-by: Stephen Rothwell <[email protected]>
>
> Thanks for the tip although, actually, it's not that commit that
> needs to be fixed but the kerneldoc itself.

This doesn't matter I think, what Fixes tag does is tell tools
if you have commit A you want this one on top.

> The warnings were
> made visible after that commit but not introduced by it. I'll add
> the Reported-by tag in v2.
>
> Cheers,
> Ricardo

2022-12-20 12:10:22

by Ricardo Cañuelo

[permalink] [raw]
Subject: Re: [PATCH] virtio: fix virtio_config_ops kerneldocs

On 20/12/22 11:25, Michael S. Tsirkin wrote:
> This doesn't matter I think, what Fixes tag does is tell tools
> if you have commit A you want this one on top.

Ok, thanks for clarifying. v3 submitted.

Cheers,
Ricardo