Re: [virtio-comment] [PATCH v3] virtio-net: fix Driver Notification description related to VIRTIO_F_NOTIF_CONFIG_DATA

[Halil Pasic <pasic@linux.ibm.com>]

On Thu, 7 Jan 2021 17:39:48 +0000
Vitaly Mireyno <vmireyno@marvell.com> wrote:

> Incorporated comments for the "[PATCH v9] virtio-net: Add support for the flexible driver notification structure".
> Made Driver Notifications description more consistent throughout the document wrt VIRTIO_F_NOTIF_CONFIG_DATA.
> 
> Changes from v2:
>  * Reworked 'vqn' name and definition
> 

First sorry for being late.

> 
> Signed-off-by: Vitaly Mireyno <vmireyno@marvell.com>
> ---
>  content.tex        | 30 ++++++++++++++----------------
>  notifications-be.c |  2 +-
>  notifications-le.c |  2 +-
>  3 files changed, 16 insertions(+), 18 deletions(-)
> 
> diff --git a/content.tex b/content.tex
> index 00bc050..19f09d9 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -337,8 +337,12 @@ \section{Driver Notifications} \label{sec:Virtqueues / Driver notifications}
>  notification to the device.
>  
>  When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
> -this notification involves sending the
> -virtqueue number to the device (method depending on the transport).
> +this notification involves sending to the device the virtqueue token
> +(method depending on the transport).

I don't think changing the word order is a good idea. I would prefer
'this notification involves sending the virtqueue token to the device'

> +if VIRTIO_F_NOTIF_CONFIG_DATA has not been negotiated, the virtqueue token is

This is a new sentence. You should start it with a capital letter.

> +the virtqueue number to be notified

To be notified is off here. I believe it can be omitted, or we need
something like 'the virtqueue number of the virtqueue to be notified'

> +if VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated, the virtqueue token is

This is probably supposed to be a new sentence as well, but you lack
both the period and the capital initial letter.

> +the queue notification data of the virtqueue to be notified.
>  

[..]

I will have a look at the rest tomorrow.

Regards,
Halil