[PATCH] virtio: fix virtio_config_ops kerneldocs

[PATCH] virtio: fix virtio_config_ops kerneldocs

[Ricardo Cañuelo]

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 <ricardo.canuelo@collabora.com>
---
 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

Re: [PATCH] virtio: fix virtio_config_ops kerneldocs

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 2761 bytes --]

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 <ricardo.canuelo@collabora.com>

You need to add appropriate tags:

Link: https://lore.kernel.org/linux-next/20221220105956.4786852d@canb.auug.org.au/
Fixes: 333723e8bc393d ("docs: driver-api: virtio: virtio on Linux")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>

> ---
>  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 <bagasdotme@gmail.com> 

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] virtio: fix virtio_config_ops kerneldocs

[AngeloGioacchino Del Regno]

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 <ricardo.canuelo@collabora.com>
> ---
>   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

Re: [PATCH] virtio: fix virtio_config_ops kerneldocs

[Ricardo Cañuelo]

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 <ricardo.canuelo@collabora.com>
> 
> You need to add appropriate tags:
> 
> Link: https://lore.kernel.org/linux-next/20221220105956.4786852d@canb.auug.org.au/
> Fixes: 333723e8bc393d ("docs: driver-api: virtio: virtio on Linux")
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>

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

Re: [PATCH] virtio: fix virtio_config_ops kerneldocs

[Ricardo Cañuelo]

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

Re: [PATCH] virtio: fix virtio_config_ops kerneldocs

[Michael S. Tsirkin]

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 <ricardo.canuelo@collabora.com>
> > 
> > You need to add appropriate tags:
> > 
> > Link: https://lore.kernel.org/linux-next/20221220105956.4786852d@canb.auug.org.au/
> > Fixes: 333723e8bc393d ("docs: driver-api: virtio: virtio on Linux")
> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> 
> 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

Re: [PATCH] virtio: fix virtio_config_ops kerneldocs

[Ricardo Cañuelo]

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