Re: [PATCH v7 3/4] admin: Add group member legacy register access commands

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Jun 29, 2023 at 09:32:21PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Thursday, June 29, 2023 3:50 PM
> 
> > > +When command completes successfully, \field{command_specific_result}
> > > +uses following structure:
> > > +
> > > +\begin{lstlisting}
> > > +struct virtio_admin_cmd_legacy_notify_query_entry {
> > > +        u8 region[8];
> > > +};
> > 
> > This confuses more than it clarifies.  Do this:
> > 
> I rename region to region_data and link for the transport.
> Mostly implementer will directly jump after learning this theory of operation so, it should be ok to list in pci.
> 
> > struct virtio_admin_cmd_legacy_notify_query_entry {
> > 	union {
> > 		virtio_pci_notify_region region;
> > 	};
> > };
> > 
> Yes, I thought about it, but it was pci transport listing so kept it generic.
> More below.
> 
> > 
> > > +
> > > +struct virtio_admin_cmd_legacy_notify_query_result {
> > > +	struct virtio_virtio_admin_cmd_legacy_notify_query_entry entries[];
> > > +}; \end{lstlisting}
> > > +
> > > +The driver should pick the suitable entry when multiple entries are
> > > +supplied by the device.
> > > +
> > > +Refer to the specific transport section for the definition of the
> > > +\field{region}.
> > 
> > Where? How does user know where to look?  Add a link.
> >
> Will add the link.
>  
> > 
> > Or preferably I would just include that tex right here to avoid the need to jump
> > back and forth.
> > 
> We have vq notify config data as generic and transport specific listing,

But note how that is included directly in multiple places -
not a link. In the resulting PDF it appears inline.

> So will improve this part of text with link.

anyway, that's not part of ABI.