Re: [Linaro-acpi] [PATCH 04/20] ARM64 / ACPI: Introduce arm_core.cand its related head file

[Tomasz Nowicki]

Hi Lorenzo,

W dniu 22.01.2014 12:54, Lorenzo Pieralisi pisze:
> On Fri, Jan 17, 2014 at 12:24:58PM +0000, Hanjun Guo wrote:
>
> [...]
>
> > diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> > index bd9bbd0..2210353 100644
> > --- a/arch/arm64/kernel/setup.c
> > +++ b/arch/arm64/kernel/setup.c
> > @@ -41,6 +41,7 @@
> >   #include <linux/memblock.h>
> >   #include <linux/of_fdt.h>
> >   #include <linux/of_platform.h>
> > +#include <linux/acpi.h>
> >
> >   #include <asm/cputype.h>
> >   #include <asm/elf.h>
> > @@ -225,6 +226,11 @@ void __init setup_arch(char **cmdline_p)
> >
> >   	arm64_memblock_init();
> >
> > +	/* Parse the ACPI tables for possible boot-time configuration */
> > +	acpi_boot_table_init();
> > +	early_acpi_boot_init();
> > +	acpi_boot_init();
> > +
> >   	paging_init();
>
> Can I ask you please why we need to parse ACPI tables before
> paging_init() ?
This is for future usage and because of couple of reasons. Mainly SRAT 
table parsing should be done (before paging_init()) for proper NUMA 
initialization and then paging_init().

Regards,
Tomasz
> [...]
>
> > +/*
> > + * __acpi_map_table() will be called before page_init(), so early_ioremap()
> > + * or early_memremap() should be called here.
>
> Again, why is this needed ? What's needed before paging_init() from ACPI ?
>
> [...]
>
> > +/*
> > + * acpi_boot_table_init() and acpi_boot_init()
> > + *  called from setup_arch(), always.
> > + *	1. checksums all tables
> > + *	2. enumerates lapics
> > + *	3. enumerates io-apics
> > + *
> > + * acpi_table_init() is separated to allow reading SRAT without
> > + * other side effects.
> > + */
> > +void __init acpi_boot_table_init(void)
> > +{
> > +	/*
> > +	 * If acpi_disabled, bail out
> > +	 */
> > +	if (acpi_disabled)
> > +		return;
> > +
> > +	/*
> > +	 * Initialize the ACPI boot-time table parser.
> > +	 */
> > +	if (acpi_table_init()) {
> > +		disable_acpi();
> > +		return;
> > +	}
> > +}
> > +
> > +int __init early_acpi_boot_init(void)
> > +{
> > +	/*
> > +	 * If acpi_disabled, bail out
> > +	 */
> > +	if (acpi_disabled)
> > +		return -ENODEV;
> > +
> > +	/*
> > +	 * Process the Multiple APIC Description Table (MADT), if present
> > +	 */
> > +	early_acpi_process_madt();
> > +
> > +	return 0;
> > +}
> > +
> > +int __init acpi_boot_init(void)
> > +{
> > +	/*
> > +	 * If acpi_disabled, bail out
> > +	 */
> > +	if (acpi_disabled)
> > +		return -ENODEV;
> > +
> > +	acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt);
> > +
> > +	/*
> > +	 * Process the Multiple APIC Description Table (MADT), if present
> > +	 */
> > +	acpi_process_madt();
> > +
> > +	return 0;
> > +}
>
> Well, apart from having three init calls, one returning void and two
> returning proper values, do not understand why, and do not understand
> why we need three calls in the first place...why should we process MADT
> twice in two separate calls ? What is supposed to change in between that
> prevents you from merging the two together ?
>
> Thanks,
> Lorenzo
>
>
> _______________________________________________
> Linaro-acpi mailing list
> Linaro-acpi@xxxxxxxxxxxxxxxx
> http://lists.linaro.org/mailman/listinfo/linaro-acpi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/