Re: [PATCH v7 02/22] arm/acpi: Add a helper function to get the acpi table offset

[Julien Grall <julien.grall@arm.com>]

Hi Shannon,

On 25/03/16 13:48, Shannon Zhao wrote:
> These tables are aligned with 64bit.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
> v7: add commnets to explain what thsi function does
> ---
>   xen/arch/arm/acpi/lib.c    | 20 ++++++++++++++++++++
>   xen/include/asm-arm/acpi.h |  2 ++
>   2 files changed, 22 insertions(+)
>
> diff --git a/xen/arch/arm/acpi/lib.c b/xen/arch/arm/acpi/lib.c
> index db5c4d8..cee2454 100644
> --- a/xen/arch/arm/acpi/lib.c
> +++ b/xen/arch/arm/acpi/lib.c
> @@ -60,3 +60,23 @@ bool_t __init acpi_psci_hvc_present(void)
>   {
>       return acpi_gbl_FADT.arm_boot_flags & ACPI_FADT_PSCI_USE_HVC;
>   }
> +
> +/*
> + * This function is used to get the offset of some new created ACPI or EFI table
> + * in the allocated memory region. Currently the tables should be created in the
> + * order of enum EFI_MEM_RES.
> + */

The function could be called after the table is created and still return 
the correct value. I would clearly write in the description when this 
function can be called or not. Something along those lines:

"This function returns the offset of a given ACPI/EFI table in the 
allocated memory region.

Currently, the tables should be created in the same order as their 
associated 'index' in the enum EFI_MEM_RES. This means the function 
won't return the correct offset until all the tables before a given 
'index' are created.".

Also, the description of an external function is usually done in the header.

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel