On Thu, Jun 13, 2019 at 04:34:31PM +0200, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>
> ---
> include/hw/i386/pc.h | 37 +++++++++++++++++++++++++++++++++++--
> 1 file changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 7c07185dd5..fc66b61ff8 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -293,9 +293,42 @@ typedef enum {
> E820_UNUSABLE = 5
> } E820Type;
>
> -ssize_t e820_add_entry(uint64_t, uint64_t, uint32_t);
> +/**
> + * e820_add_entry: Add an #e820_entry to the @e820_table.
> + *
> + * Returns the number of entries of the e820_table on success,
> + * or a negative errno otherwise.
> + *
> + * @address: The base address of the structure which the BIOS is to fill in.
> + * @length: The length in bytes of the structure passed to the BIOS.
> + * @type: The #E820Type of the address range.
> + */
> +ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type);
> +
> +/**
> + * e820_get_num_entries: The number of entries of the @e820_table.
> + *
> + * Returns the number of entries of the e820_table.
> + */
> size_t e820_get_num_entries(void);
> -bool e820_get_entry(unsigned int, uint32_t, uint64_t *, uint64_t *);
> +
> +/**
> + * e820_get_entry: Get the address/length of an #e820_entry.
> + *
> + * If the #e820_entry stored at @index is of #E820Type @type, fills @address
> + * and @length with the #e820_entry values and return @true.
> + * Return @false otherwise.
> + *
> + * @index: The index of the #e820_entry to get values.
> + * @type: The @E820Type of the address range expected.
> + * @address: Pointer to the base address of the #e820_entry structure to
> + * be filled.
> + * @length: Pointer to the length (in bytes) of the #e820_entry structure
> + * to be filled.
> + * @return: true if the entry was found, false otherwise.
I don't actually care whether it's @E820Type, #E820Type or just type,
we should be consistent. I also find this style of documentation
underwhelming. what is to be filled? length or the structure?
upper case after : also looks somewhat wrong.
Same applies to other comments too.
> + */
> +bool e820_get_entry(unsigned int index, E820Type type,
> + uint64_t *address, uint64_t *length);
>
> extern GlobalProperty pc_compat_4_0_1[];
> extern const size_t pc_compat_4_0_1_len;
> --
> 2.20.1
