> From: linux-acpi-owner@xxxxxxxxxxxxxxx
> Sent: Friday, August 02, 2013 7:32 AM
>
> On Thu, 2013-08-01 at 15:06 +0800, Tang Chen wrote:
> > In ACPI, SRAT(System Resource Affinity Table) contains NUMA info.
> > The memory affinities in SRAT record every memory range in the
> > system, and also, flags specifying if the memory range is
> > hotpluggable.
> > (Please refer to ACPI spec 5.0 5.2.16)
> >
> > memblock starts to work at very early time, and SRAT has not been
> > parsed. So we don't know which memory is hotpluggable. In order
> > to use memblock to reserve hotpluggable memory, we need to obtain
> > SRAT memory affinity info earlier.
> >
> > In the current acpi_boot_table_init(), it does the following:
> > 1. Parse RSDT, so that we can find all the tables.
> > 2. Initialize acpi_gbl_root_table_list, an array of acpi table
> > descriptorsused to store each table's address, length, signature,
A missing space here.
> > and so on.
> > 3. Check if there is any table in initrd intending to override
> > tables from firmware. If so, override the firmware tables.
> > 4. Initialize all the data in acpi_gbl_root_table_list.
> >
> > In order to parse SRAT at early time, we need to do similar job as
> > step 1 and 2 above earlier to obtain SRAT. It will be very convenient
> > if we have acpi_gbl_root_table_list initialized. We can use address
> > and signature to find SRAT.
> >
> > Since step 1 and 2 allocates no memory, it is OK to do these two
> > steps earlier.
> >
> > But step 3 will check acpi initrd table override, not just SRAT,
> > but also all the other tables. So it is better to keep it untouched.
> >
> > This patch splits acpi_boot_table_init() into two steps:
> > 1. Parse RSDT, which cannot be overrided, and initialize
> > acpi_gbl_root_table_list. (step 1 + 2 above)
> > 2. Install all ACPI tables into acpi_gbl_root_table_list.
> > (step 3 + 4 above)
> >
> > In later patches, we will do step 1 + 2 earlier.
> >
> > Signed-off-by: Tang Chen <tangchen@xxxxxxxxxxxxxx>
> > Reviewed-by: Zhang Yanfei <zhangyanfei@xxxxxxxxxxxxxx>
> > ---
> > drivers/acpi/acpica/tbutils.c | 25 ++++++++++++++++++++++---
> > drivers/acpi/tables.c | 2 ++
> > include/acpi/acpixf.h | 2 ++
> > 3 files changed, 26 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/acpi/acpica/tbutils.c b/drivers/acpi/acpica/tbutils.c
>
> This patch needs to be reviewed by ACPICA folks. I'd suggest to change
> the patch title to "x86, ACPICA:". I added Bob to the list.
>
> Thanks,
> -Toshi
>
>
>
> > index ce3d5db..9d68ffc 100644
> > --- a/drivers/acpi/acpica/tbutils.c
> > +++ b/drivers/acpi/acpica/tbutils.c
> > @@ -766,9 +766,30 @@
> acpi_tb_parse_root_table(acpi_physical_address rsdp_address)
> > */
> > acpi_os_unmap_memory(table, length);
> >
> > + return_ACPI_STATUS(AE_OK);
> > +}
> > +
> >
I don't think you can split the function here.
ACPICA still need to continue to parse the table using the logic implemented in the acpi_tb_install_table() and acpi_tb_parse_fadt(). (for example, endianess of the signature).
You'd better to keep them as is and split some codes from 'acpi_tb_install_table' to form another function: acpi_tb_override_table().
> +/*********************************************************
> **********************
> > + *
> > + * FUNCTION: acpi_tb_install_root_table
I think this function should be acpi_tb_override_tables, and call acpi_tb_override_table() inside this function for each table.
> > + *
> > + * DESCRIPTION: This function installs all the ACPI tables in RSDT into
> > + * acpi_gbl_root_table_list.
> > + *
> > +
> **********************************************************
> ********************/
> > +
> > +void __init
Basically, ACPICA will use acpi_status as return value.
> > +acpi_tb_install_root_table()
(void)?
> > +{
> > + int i;
Please use u32 instead of int.
> > +
> > /*
> > * Complete the initialization of the root table array by examining
> > - * the header of each table
> > + * the header of each table.
> > + *
> > + * First two entries in the table array are reserved for the DSDT
> > + * and FACS, which are not actually present in the RSDT/XSDT - they
> > + * come from the FADT.
> > */
> > for (i = 2; i < acpi_gbl_root_table_list.current_table_count; i++) {
> > acpi_tb_install_table(acpi_gbl_root_table_list.tables[i].
> > @@ -782,6 +803,4 @@
> acpi_tb_parse_root_table(acpi_physical_address rsdp_address)
> > acpi_tb_parse_fadt(i);
> > }
> > }
> > -
> > - return_ACPI_STATUS(AE_OK);
> > }
> > diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
> > index d67a1fe..8860e79 100644
> > --- a/drivers/acpi/tables.c
> > +++ b/drivers/acpi/tables.c
> > @@ -353,6 +353,8 @@ int __init acpi_table_init(void)
> > if (ACPI_FAILURE(status))
> > return 1;
> >
> > + acpi_tb_install_root_table();
> > +
I think you shouldn't call a function named as acpi_tb_xxx directly in a file belongs to drivers/acpi not drivers/acpi/acpica.
This kind of a function name can only be used inside ACPICA.
You need an interface wrapper in drivers/acpi/acpica/tbxface.c to call such internal functions.
> > check_multiple_madt();
> > return 0;
> > }
> > diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
> > index 454881e..f5549b5 100644
> > --- a/include/acpi/acpixf.h
> > +++ b/include/acpi/acpixf.h
> > @@ -116,6 +116,8 @@ acpi_status
> > acpi_initialize_tables(struct acpi_table_desc *initial_storage,
> > u32 initial_table_count, u8 allow_resize);
> >
> > +void acpi_tb_install_root_table(void);
> > +
The reason is same as the above.
It doesn't make sense that a function with such name appears in the acpixf.h.
Thanks and best regards
-Lv
> > acpi_status __init acpi_initialize_subsystem(void);
> >
> > acpi_status acpi_enable_subsystem(u32 flags);
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
��.n������g��� �a��� �&ޖ)���)��h���&������梷���� �Ǟ�m������)������^�����������v���O��zf������