| Message ID | 20201021000011.15351-1-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,01/14] kernel-doc: public/arch-arm.h | expand |
On 21/10/2020 00:59, Stefano Stabellini wrote: > diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h > index c365b1b39e..409697dede 100644 > --- a/xen/include/public/arch-arm.h > +++ b/xen/include/public/arch-arm.h > @@ -201,7 +208,9 @@ typedef uint64_t xen_pfn_t; > #define PRI_xen_pfn PRIx64 > #define PRIu_xen_pfn PRIu64 > > -/* > +/** > + * DOC: XEN_LEGACY_MAX_VCPUS > + * > * Maximum number of virtual CPUs in legacy multi-processor guests. > * Only one. All other VCPUS must use VCPUOP_register_vcpu_info. > */ I suppose I don't really want to know why this exists in the ARM ABI? It looks to be a misfeature. Shouldn't it be labelled as obsolete ? (Is that even a thing you can do in kernel-doc? It surely must be...) > @@ -299,26 +308,28 @@ struct vcpu_guest_context { > typedef struct vcpu_guest_context vcpu_guest_context_t; > DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t); > > -/* > + > +/** > + * struct xen_arch_domainconfig - arch-specific domain creation params > + * > * struct xen_arch_domainconfig's ABI is covered by > * XEN_DOMCTL_INTERFACE_VERSION. > */ > +struct xen_arch_domainconfig { > + /** @gic_version: IN/OUT parameter */ > #define XEN_DOMCTL_CONFIG_GIC_NATIVE 0 > #define XEN_DOMCTL_CONFIG_GIC_V2 1 > #define XEN_DOMCTL_CONFIG_GIC_V3 2 > - > + uint8_t gic_version; Please can we have a newline in here, and elsewhere separating blocks of logically connected field/constant/comments. It will make a world of difference to the readability of the header files themselves. ~Andrew
On Wed, 21 Oct 2020, Andrew Cooper wrote: > On 21/10/2020 00:59, Stefano Stabellini wrote: > > diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h > > index c365b1b39e..409697dede 100644 > > --- a/xen/include/public/arch-arm.h > > +++ b/xen/include/public/arch-arm.h > > @@ -201,7 +208,9 @@ typedef uint64_t xen_pfn_t; > > #define PRI_xen_pfn PRIx64 > > #define PRIu_xen_pfn PRIu64 > > > > -/* > > +/** > > + * DOC: XEN_LEGACY_MAX_VCPUS > > + * > > * Maximum number of virtual CPUs in legacy multi-processor guests. > > * Only one. All other VCPUS must use VCPUOP_register_vcpu_info. > > */ > > I suppose I don't really want to know why this exists in the ARM ABI? > It looks to be a misfeature. > > Shouldn't it be labelled as obsolete ? (Is that even a thing you can do > in kernel-doc? It surely must be...) I tried not to make any content changes as part of this series, but as we are looking into this, I could append patches to the end of the series to make some additional changes. I.e. I'd prefer to keep the mechanical patches mechanical. In regards to XEN_LEGACY_MAX_VCPUS, it is part of struct shared_info so it must be defined. It makes sense to define it to the smallest number given that the newer interface (VCPUOP_register_vcpu_info) is preferred. In regards to labelling things as obsolete, I couldn't find a way to do it with kernel-doc, but keep in mind that the end goal is to use doxygen. It might become possible then. > > @@ -299,26 +308,28 @@ struct vcpu_guest_context { > > typedef struct vcpu_guest_context vcpu_guest_context_t; > > DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t); > > > > -/* > > + > > +/** > > + * struct xen_arch_domainconfig - arch-specific domain creation params > > + * > > * struct xen_arch_domainconfig's ABI is covered by > > * XEN_DOMCTL_INTERFACE_VERSION. > > */ > > +struct xen_arch_domainconfig { > > + /** @gic_version: IN/OUT parameter */ > > #define XEN_DOMCTL_CONFIG_GIC_NATIVE 0 > > #define XEN_DOMCTL_CONFIG_GIC_V2 1 > > #define XEN_DOMCTL_CONFIG_GIC_V3 2 > > - > > + uint8_t gic_version; > > Please can we have a newline in here, and elsewhere separating blocks of > logically connected field/constant/comments. > > It will make a world of difference to the readability of the header > files themselves. Sure, I can do that
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h index c365b1b39e..409697dede 100644 --- a/xen/include/public/arch-arm.h +++ b/xen/include/public/arch-arm.h @@ -27,8 +27,8 @@ #ifndef __XEN_PUBLIC_ARCH_ARM_H__ #define __XEN_PUBLIC_ARCH_ARM_H__ -/* - * `incontents 50 arm_abi Hypercall Calling Convention +/** + * DOC: Hypercall Calling Convention * * A hypercall is issued using the ARM HVC instruction. * @@ -72,8 +72,8 @@ * Any cache allocation hints are acceptable. */ -/* - * `incontents 55 arm_hcall Supported Hypercalls +/** + * DOC: Supported Hypercalls * * Xen on ARM makes extensive use of hardware facilities and therefore * only a subset of the potential hypercalls are required. @@ -175,10 +175,17 @@ typedef union { type *p; uint64_aligned_t q; } \ __guest_handle_64_ ## name -/* +/** + * DOC: XEN_GUEST_HANDLE - a guest pointer in a struct + * * XEN_GUEST_HANDLE represents a guest pointer, when passed as a field * in a struct in memory. On ARM is always 8 bytes sizes and 8 bytes * aligned. + */ + +/** + * DOC: XEN_GUEST_HANDLE_PARAM - a guest pointer as a hypercall arg + * * XEN_GUEST_HANDLE_PARAM represents a guest pointer, when passed as an * hypercall argument. It is 4 bytes on aarch32 and 8 bytes on aarch64. */ @@ -201,7 +208,9 @@ typedef uint64_t xen_pfn_t; #define PRI_xen_pfn PRIx64 #define PRIu_xen_pfn PRIu64 -/* +/** + * DOC: XEN_LEGACY_MAX_VCPUS + * * Maximum number of virtual CPUs in legacy multi-processor guests. * Only one. All other VCPUS must use VCPUOP_register_vcpu_info. */ @@ -299,26 +308,28 @@ struct vcpu_guest_context { typedef struct vcpu_guest_context vcpu_guest_context_t; DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t); -/* + +/** + * struct xen_arch_domainconfig - arch-specific domain creation params + * * struct xen_arch_domainconfig's ABI is covered by * XEN_DOMCTL_INTERFACE_VERSION. */ +struct xen_arch_domainconfig { + /** @gic_version: IN/OUT parameter */ #define XEN_DOMCTL_CONFIG_GIC_NATIVE 0 #define XEN_DOMCTL_CONFIG_GIC_V2 1 #define XEN_DOMCTL_CONFIG_GIC_V3 2 - + uint8_t gic_version; + /** @tee_type: IN parameter */ #define XEN_DOMCTL_CONFIG_TEE_NONE 0 #define XEN_DOMCTL_CONFIG_TEE_OPTEE 1 - -struct xen_arch_domainconfig { - /* IN/OUT */ - uint8_t gic_version; - /* IN */ uint16_t tee_type; - /* IN */ + /** @nr_spis: IN parameter */ uint32_t nr_spis; - /* - * OUT + /** + * @clock_frequency: OUT parameter + * * Based on the property clock-frequency in the DT timer node. * The property may be present when the bootloader/firmware doesn't * set correctly CNTFRQ which hold the timer frequency.
Convert in-code comments to kernel-doc format wherever possible. Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com> --- xen/include/public/arch-arm.h | 43 ++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 16 deletions(-)