[v2,05/20] hw/i386/pc: Add documentation to the e820_*() functions
diff mbox series

Message ID 20190613143446.23937-6-philmd@redhat.com
State New
Headers show
Series
  • hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine
Related show

Commit Message

Philippe Mathieu-Daudé June 13, 2019, 2:34 p.m. UTC
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/i386/pc.h | 37 +++++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

Comments

Michael S. Tsirkin June 20, 2019, 3:36 p.m. UTC | #1
On Thu, Jun 13, 2019 at 04:34:31PM +0200, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  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

Patch
diff mbox series

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.
+ */
+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;