Re: [PATCH v7 9/9] docs/doxygen: doxygen documentation for grant_table.h

From: Jan Beulich <jbeulich@suse.com>
To: Luca Fancellu <luca.fancellu@arm.com>
Cc: Bertrand Marquis <bertrand.marquis@arm.com>,
	wei.chen@arm.com, Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Ian Jackson <iwj@xenproject.org>, Julien Grall <julien@xen.org>,
	Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH v7 9/9] docs/doxygen: doxygen documentation for grant_table.h
Date: Mon, 5 Jul 2021 15:27:43 +0200	[thread overview]
Message-ID: <90673e86-cbf3-8a21-4251-e77f22327485@suse.com> (raw)
In-Reply-To: <54ADD75F-83DC-4891-914D-B85FBA69B440@arm.com>

On 05.07.2021 15:23, Luca Fancellu wrote:
> Hi Jan,
> 
>> On 5 Jul 2021, at 14:03, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 05.07.2021 12:51, Luca Fancellu wrote:
>>> Modification to include/public/grant_table.h:
>>>
>>> 1) Add doxygen tags to:
>>> - Create Grant tables section
>>> - include variables in the generated documentation
>>> - Used @keepindent/@endkeepindent to enclose comment
>>>   section that are indented using spaces, to keep
>>>   the indentation.
>>> 2) Add .rst file for grant table
>>>
>>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
>>> ---
>>> v7 changes:
>>> - commit message changed
>>> - Add comment about grant table queries and uses
>>> to the documentation
>>> v6 changes:
>>> - Fix misaligned comment
>>> - Moved comments to make them display in the docs
>>> - Included more documentation in the docs
>>> v5 changes:
>>> - Move GNTCOPY_* define next to the flags field
>>> v4 changes:
>>> - Used @keepindent/@endkeepindent doxygen commands
>>>  to keep text with spaces indentation.
>>> - drop changes to grant_entry_v1 comment, it will
>>>  be changed and included in the docs in a future patch
>>> - Move docs .rst to "common" folder
>>> v3 changes:
>>> - removed tags to skip anonymous union/struct
>>> - moved back comment pointed out by Jan
>>> - moved down defines related to struct gnttab_copy
>>>  as pointed out by Jan
>>> v2 changes:
>>> - Revert back to anonymous union/struct
>>> - add doxygen tags to skip anonymous union/struct
>>> ---
>>> docs/hypercall-interfaces/arm64.rst           |   1 +
>>> .../common/grant_tables.rst                   |   9 +
>>
>> In patch 8 you now add arm32.rst and x86.rst as well, so it's at
>> least odd that here you alter only one of the three. However, ...
>>
>>> --- a/docs/hypercall-interfaces/arm64.rst
>>> +++ b/docs/hypercall-interfaces/arm64.rst
>>> @@ -8,6 +8,7 @@ Starting points
>>> .. toctree::
>>>    :maxdepth: 2
>>>
>>> +   common/grant_tables
>>
>> ... to me this seems the wrong way round anyway: I'd rather see
>> common stuff not be linked from per-arch locations, but per-arch
>> docs to be down the hierarchy from common ones.
> 
> The things is that common stuff is not really common, if you go and solve every
> define and so on, you end up with a different content for x86, aarch64, arm.
> 
> So you can’t have a common.rst containing the common things because depending on each
> platform the content will change.

Wait - the documentation should be uniform for all architectures.
If there are architecture specific aspects, then these should imo
still be mentioned in the common section of the doc, just pointing
out what those specifics are for which architectures. Architecture
specific doc pieces ought to cover architecture specific hypercalls
or sub-functions of common ones.

Jan