Hi Saket, On Thu, Aug 13, 2015 at 11:01 AM, Saket Sinha <saket.sinh...@gmail.com> wrote:
Please see my comments in your [1/4] patch regarding to patch title and commit message. > Signed-off-by: Saket Sinha <saket.sinh...@gmail.com> > --- > > arch/x86/include/asm/acpi_table.h | 387 +++++++++++++++++++++++++++++++++++ > arch/x86/lib/Makefile | 1 + > arch/x86/lib/acpi_table.c | 413 > ++++++++++++++++++++++++++++++++++++++ > arch/x86/lib/tables.c | 5 + > scripts/Makefile.lib | 11 + > 5 files changed, 817 insertions(+) > create mode 100644 arch/x86/include/asm/acpi_table.h > create mode 100644 arch/x86/lib/acpi_table.c > > diff --git a/arch/x86/include/asm/acpi_table.h > b/arch/x86/include/asm/acpi_table.h > new file mode 100644 > index 0000000..7b52e7d > --- /dev/null > +++ b/arch/x86/include/asm/acpi_table.h > @@ -0,0 +1,387 @@ Please add a comment header to give others some hints on where this file is ported from. > +#include <linux/string.h> > +#include <malloc.h> > +#include <common.h> > +#include <asm/post.h> We should avoid including header files in header file. > + > +#define RSDP_SIG "RSD PTR " /* RSDT pointer signature */ > +#define ACPI_TABLE_CREATOR "UBOOT " /* Must be exactly 8 bytes long! > */ > +#define OEM_ID "UBOOT " /* Must be exactly 6 bytes long! > */ > +#define ASLC "UBOO" /* Must be exactly 4 bytes long! > */ As I mentioned in my review comments to the v2 series, please change ASLC to "INTL" to indicate that we are using Intel ASL compiler to compile ASL files. > + > +#define APM_CNT 0xb2 > +#define APM_CNT_CST_CONTROL 0x85 > +#define APM_CNT_PST_CONTROL 0x80 > +#define APM_CNT_ACPI_DISABLE 0x1e > +#define APM_CNT_ACPI_ENABLE 0xe1 > +#define APM_CNT_MBI_UPDATE 0xeb > +#define APM_CNT_GNVS_UPDATE 0xea > +#define APM_CNT_FINALIZE 0xcb > +#define APM_CNT_LEGACY 0xcc > +#define APM_STS 0xb3 > + > + We only allow one blank line between code blocks. > +#define MP_IRQ_POLARITY_DEFAULT 0x0 > +#define MP_IRQ_POLARITY_HIGH 0x1 > +#define MP_IRQ_POLARITY_LOW 0x3 > +#define MP_IRQ_POLARITY_MASK 0x3 > +#define MP_IRQ_TRIGGER_DEFAULT 0x0 > +#define MP_IRQ_TRIGGER_EDGE 0x4 > +#define MP_IRQ_TRIGGER_LEVEL 0xc > +#define MP_IRQ_TRIGGER_MASK 0xc > + > +#define ACTL 0x00 > +# define SCIS_MASK 0x07 > +# define SCIS_IRQ9 0x00 > +# define SCIS_IRQ10 0x01 > +# define SCIS_IRQ11 0x02 > +# define SCIS_IRQ20 0x04 > +# define SCIS_IRQ21 0x05 > +# define SCIS_IRQ22 0x06 > +# define SCIS_IRQ23 0x07 Please remove the space between # and define. > + > +#define ILB_BASE_ADDRESS 0xfed08000 > +#define ILB_BASE_SIZE 0x400 > + These two macros look chip-specific. Please remove them. > +enum bus_type { bus_type is too generic, maybe acpi_bus_type? > + PIC = 0, > + APIC = 2, > + ETHIGH = 5 > +}; > + > + > +typedef struct acpi_gen_regaddr { > + u8 space_id; /* Address space ID */ > + u8 bit_width; /* Register size in bits */ > + u8 bit_offset; /* Register bit offset */ > + union { > + u8 resv; /* Reserved in ACPI > 2.0 - 2.0b */ > + u8 access_size; /* Access size in ACPI > 2.0c/3.0/4.0/5.0 */ > + }; The indention is not aligned to others. > + u32 addrl; /* Register address, low 32 bits */ > + u32 addrh; /* Register address, high 32 bits */ > +} __attribute__ ((packed)) acpi_addr_t; I believe this structure is naturally aligned. Please double check. > + > + > +/* RSDP (Root System Description Pointer) */ > +struct __packed acpi_rsdp { > + char signature[8]; /* RSDP signature */ > + u8 checksum; /* Checksum of the first 20 bytes */ > + char oem_id[6]; /* OEM ID */ > + u8 revision; /* 0 for ACPI 1.0, 2 for ACPI 2.0/3.0/4.0 */ > + u32 rsdt_address; /* Physical address of RSDT (32 bits) */ > + u32 length; /* Total RSDP length (incl. extended part) */ > + u64 xsdt_address; /* Physical address of XSDT (64 bits) */ > + u8 ext_checksum; /* Checksum of the whole table */ > + u8 reserved[3]; > +}; I believe this structure is naturally aligned. Please double check. > +/* Note: ACPI 1.0 didn't have length, xsdt_address, and ext_checksum. */ This comment should be moved on top of this structure. > + > +enum acpi_address_space_type { > + ACPI_ADDRESS_SPACE_MEMORY = 0, /* System memory */ > + ACPI_ADDRESS_SPACE_IO = 1, /* System I/O */ > + ACPI_ADDRESS_SPACE_PCI = 2, /* PCI config space */ > + ACPI_ADDRESS_SPACE_EC = 3, /* Embedded controller */ > + ACPI_ADDRESS_SPACE_SMBUS = 4, /* SMBus */ > + ACPI_ADDRESS_SPACE_PCC = 0x0A, /* Platform Comm. Channel */ Please use lower case 0xa. > + ACPI_ADDRESS_SPACE_FIXED = 0x7f /* Functional fixed hardware */ > +}; > + > +#define ACPI_FFIXEDHW_VENDOR_INTEL 1 /* Intel */ > +#define ACPI_FFIXEDHW_CLASS_HLT 0 /* C1 Halt */ > +#define ACPI_FFIXEDHW_CLASS_IO_HLT 1 /* C1 I/O then Halt */ > +#define ACPI_FFIXEDHW_CLASS_MWAIT 2 /* MWAIT Native C-state */ > +#define ACPI_FFIXEDHW_FLAG_HW_COORD 1 /* Hardware Coordination bit > */ > +#define ACPI_FFIXEDHW_FLAG_BM_STS 2 /* BM_STS avoidance bit */ Please do not use two spaces after define. > +/* 0x80-0xbf: Reserved */ > +/* 0xc0-0xff: OEM defined */ > + What are these two for? > +/* Access size definitions for Generic address structure */ > +enum acpi_address_space_size { > + ACPI_ACCESS_SIZE_UNDEFINED = 0, /* Undefined (legacy reasons) */ > + ACPI_ACCESS_SIZE_BYTE_ACCESS = 1, > + ACPI_ACCESS_SIZE_WORD_ACCESS = 2, > + ACPI_ACCESS_SIZE_DWORD_ACCESS = 3, > + ACPI_ACCESS_SIZE_QWORD_ACCESS = 4 > +}; > + > +/* Generic ACPI header, provided by (almost) all tables */ > +typedef struct acpi_table_header { > + char signature[4]; /* ACPI signature (4 ASCII characters) */ > + u32 length; /* Table length in bytes (incl. header) > */ > + u8 revision; /* Table version (not ACPI version!) */ > + u8 checksum; /* To make sum of entire table == 0 */ > + char oem_id[6]; /* OEM identification */ > + char oem_table_id[8]; /* OEM table identification */ > + u32 oem_revision; /* OEM revision number */ > + char asl_compiler_id[4]; /* ASL compiler vendor ID */ > + u32 asl_compiler_revision; /* ASL compiler revision number */ > +} __attribute__ ((packed)) acpi_header_t; I believe this structure is naturally aligned. Please double check. > + > +/* A maximum number of 32 ACPI tables ought to be enough for now. */ > +#define MAX_ACPI_TABLES 32 > + > +/* RSDT (Root System Description Table) */ > +struct __packed acpi_rsdt { > + struct acpi_table_header header; > + u32 entry[MAX_ACPI_TABLES]; > +}; > + > +/* XSDT (Extended System Description Table) */ > +struct __packed acpi_xsdt { > + struct acpi_table_header header; > + u64 entry[MAX_ACPI_TABLES]; > +}; > + > +/* MCFG (PCI Express MMIO config space BAR description table) */ > +struct __packed acpi_mcfg { > + struct acpi_table_header header; > + u8 reserved[8]; > +}; > + > +struct __packed acpi_mcfg_mmconfig { > + u32 base_address; > + u32 base_reserved; > + u16 pci_segment_group_number; > + u8 start_bus_number; > + u8 end_bus_number; > + u8 reserved[4]; > +}; > + > +/* MADT (Multiple APIC Description Table) */ > +struct __packed acpi_madt { > + struct acpi_table_header header; > + u32 lapic_addr; /* Local APIC address */ > + u32 flags; /* Multiple APIC flags */ > +} __attribute__ ((packed)) acpi_madt_t; > + Please check all the 5 tables above. I believe all of them are naturally aligned. > +enum dev_scope_type { > + SCOPE_PCI_ENDPOINT = 1, > + SCOPE_PCI_SUB = 2, > + SCOPE_IOAPIC = 3, > + SCOPE_MSI_HPET = 4 > +}; > + > +typedef struct dev_scope { > + u8 type; > + u8 length; > + u8 reserved[2]; > + u8 enumeration; > + u8 start_bus; > + struct { > + u8 dev; > + u8 fn; > + } __attribute__((packed)) path[0]; > +} __attribute__ ((packed)) dev_scope_t; Please check __packed is really necessary. > + > +/* MADT: APIC Structure Types */ > +/* TODO: Convert to ALLCAPS. */ Please use multi-line comment format. > +enum acpi_apic_types { > + LocalApic = 0, /* Processor local APIC */ > + IOApic = 1, /* I/O APIC */ > + IRQSourceOverride = 2, /* Interrupt source override */ > + NMIType = 3, /* NMI source */ > + LocalApicNMI = 4, /* Local APIC NMI */ > + LApicAddressOverride = 5, /* Local APIC address override */ > + IOSApic = 6, /* I/O SAPIC */ > + LocalSApic = 7, /* Local SAPIC */ > + PlatformIRQSources = 8, /* Platform interrupt sources */ > + Localx2Apic = 9, /* Processor local x2APIC */ > + Localx2ApicNMI = 10, /* Local x2APIC NMI */ Please do not use CamelCase. > + /* 0x0b-0x7f: Reserved */ > + /* 0x80-0xff: Reserved for OEM use */ > +}; > + > +/* MADT: Processor Local APIC Structure */ > +struct __packed acpi_madt_lapic { > + u8 type; /* Type (0) */ > + u8 length; /* Length in bytes (8) */ > + u8 processor_id; /* ACPI processor ID */ > + u8 apic_id; /* Local APIC ID */ > + u32 flags; /* Local APIC flags */ > +}; No need for __packed. > + > +/* MADT: Local APIC NMI Structure */ > +struct __packed acpi_madt_lapic_nmi { > + u8 type; /* Type (4) */ > + u8 length; /* Length in bytes (6) */ > + u8 processor_id; /* ACPI processor ID */ > + u16 flags; /* MPS INTI flags */ > + u8 lint; /* Local APIC LINT# */ > +}; > + > +/* MADT: I/O APIC Structure */ > +struct __packed acpi_madt_ioapic { > + u8 type; /* Type (1) */ > + u8 length; /* Length in bytes (12) */ > + u8 ioapic_id; /* I/O APIC ID */ > + u8 reserved; > + u32 ioapic_addr; /* I/O APIC address */ > + u32 gsi_base; /* Global system interrupt base */ > +}; No need for __packed. > + > +/* MADT: Interrupt Source Override Structure */ > +struct __packed acpi_madt_irqoverride { > + u8 type; /* Type (2) */ > + u8 length; /* Length in bytes (10) */ > + u8 bus; /* ISA (0) */ > + u8 source; /* Bus-relative int. source (IRQ) */ > + u32 gsirq; /* Global system interrupt */ > + u16 flags; /* MPS INTI flags */ > +}; > + > +/* FADT (Fixed ACPI Description Table) */ > +struct __packed acpi_fadt { > + struct acpi_table_header header; > + u32 firmware_ctrl; > + u32 dsdt; > + u8 model; > + u8 preferred_pm_profile; > + u16 sci_int; > + u32 smi_cmd; > + u8 acpi_enable; > + u8 acpi_disable; > + u8 s4bios_req; > + u8 pstate_cnt; > + u32 pm1a_evt_blk; > + u32 pm1b_evt_blk; > + u32 pm1a_cnt_blk; > + u32 pm1b_cnt_blk; > + u32 pm2_cnt_blk; > + u32 pm_tmr_blk; > + u32 gpe0_blk; > + u32 gpe1_blk; > + u8 pm1_evt_len; > + u8 pm1_cnt_len; > + u8 pm2_cnt_len; > + u8 pm_tmr_len; > + u8 gpe0_blk_len; > + u8 gpe1_blk_len; > + u8 gpe1_base; > + u8 cst_cnt; > + u16 p_lvl2_lat; > + u16 p_lvl3_lat; > + u16 flush_size; > + u16 flush_stride; > + u8 duty_offset; > + u8 duty_width; > + u8 day_alrm; > + u8 mon_alrm; > + u8 century; > + u16 iapc_boot_arch; > + u8 res2; > + u32 flags; > + struct acpi_gen_regaddr reset_reg; > + u8 reset_value; > + u8 res3; > + u8 res4; > + u8 res5; > + u32 x_firmware_ctl_l; > + u32 x_firmware_ctl_h; > + u32 x_dsdt_l; > + u32 x_dsdt_h; > + struct acpi_gen_regaddr x_pm1a_evt_blk; > + struct acpi_gen_regaddr x_pm1b_evt_blk; > + struct acpi_gen_regaddr x_pm1a_cnt_blk; > + struct acpi_gen_regaddr x_pm1b_cnt_blk; > + struct acpi_gen_regaddr x_pm2_cnt_blk; > + struct acpi_gen_regaddr x_pm_tmr_blk; > + struct acpi_gen_regaddr x_gpe0_blk; > + struct acpi_gen_regaddr x_gpe1_blk; > +}; > + > +/* FADT TABLE Revision values */ > +#define ACPI_FADT_REV_ACPI_1_0 1 > +#define ACPI_FADT_REV_ACPI_2_0 3 > +#define ACPI_FADT_REV_ACPI_3_0 4 > +#define ACPI_FADT_REV_ACPI_4_0 4 > +#define ACPI_FADT_REV_ACPI_5_0 5 Please use enum. > + > +/* Flags for p_lvl2_lat and p_lvl3_lat */ > +#define ACPI_FADT_C2_NOT_SUPPORTED 101 > +#define ACPI_FADT_C3_NOT_SUPPORTED 1001 > + > +/* FADT Feature Flags */ > +#define ACPI_FADT_WBINVD (1 << 0) > +#define ACPI_FADT_WBINVD_FLUSH (1 << 1) > +#define ACPI_FADT_C1_SUPPORTED (1 << 2) > +#define ACPI_FADT_C2_MP_SUPPORTED (1 << 3) > +#define ACPI_FADT_POWER_BUTTON (1 << 4) > +#define ACPI_FADT_SLEEP_BUTTON (1 << 5) > +#define ACPI_FADT_FIXED_RTC (1 << 6) > +#define ACPI_FADT_S4_RTC_WAKE (1 << 7) > +#define ACPI_FADT_32BIT_TIMER (1 << 8) > +#define ACPI_FADT_DOCKING_SUPPORTED (1 << 9) > +#define ACPI_FADT_RESET_REGISTER (1 << 10) > +#define ACPI_FADT_SEALED_CASE (1 << 11) > +#define ACPI_FADT_HEADLESS (1 << 12) > +#define ACPI_FADT_SLEEP_TYPE (1 << 13) > +#define ACPI_FADT_PCI_EXPRESS_WAKE (1 << 14) > +#define ACPI_FADT_PLATFORM_CLOCK (1 << 15) > +#define ACPI_FADT_S4_RTC_VALID (1 << 16) > +#define ACPI_FADT_REMOTE_POWER_ON (1 << 17) > +#define ACPI_FADT_APIC_CLUSTER (1 << 18) > +#define ACPI_FADT_APIC_PHYSICAL (1 << 19) > +/* Bits 20-31: reserved ACPI 3.0 & 4.0 */ > +#define ACPI_FADT_HW_REDUCED_ACPI (1 << 20) > +#define ACPI_FADT_LOW_PWR_IDLE_S0 (1 << 21) > +/* bits 22-31: reserved ACPI 5.0 */ > + > +/* FADT Boot Architecture Flags */ > +enum acpi_fadt_boot_flags { > + ACPI_FADT_LEGACY_DEVICES = (1 << 0), > + ACPI_FADT_8042 = (1 << 1), > + ACPI_FADT_VGA_NOT_PRESENT = (1 << 2), > + ACPI_FADT_MSI_NOT_SUPPORTED = (1 << 3), > + ACPI_FADT_NO_PCIE_ASPM_CONTROL = (1 << 4), > + ACPI_FADT_LEGACY_FREE = 0x00 /* No legacy devices (including 8042) > */ > +}; To keep consistency, either move these to plain defines, or convert "FADT Feature Flags" above to enum. Please add one blank line here. > +/* FADT Preferred Power Management Profile */ > +enum acpi_preferred_pm_profiles { > + PM_UNSPECIFIED = 0, > + PM_DESKTOP = 1, > + PM_MOBILE = 2, > + PM_WORKSTATION = 3, > + PM_ENTERPRISE_SERVER = 4, > + PM_SOHO_SERVER = 5, > + PM_APPLIANCE_PC = 6, > + PM_PERFORMANCE_SERVER = 7, > + PM_TABLET = 8, /* ACPI 5.0 */ > +}; > + > +/* FACS (Firmware ACPI Control Structure) */ > +struct __packed acpi_facs { > + char signature[4]; /* "FACS" */ > + u32 length; /* Length in bytes (>= 64) */ > + u32 hardware_signature; /* Hardware signature */ > + u32 firmware_waking_vector; /* Firmware waking vector */ > + u32 global_lock; /* Global lock */ > + u32 flags; /* FACS flags */ > + u32 x_firmware_waking_vector_l; /* X FW waking vector, low */ > + u32 x_firmware_waking_vector_h; /* X FW waking vector, high */ > + u8 version; /* ACPI 4.0: 2 */ > + u8 resv[31]; /* FIXME: 4.0: ospm_flags */ > +}; > + > +/* FACS flags */ > +#define ACPI_FACS_S4BIOS_F (1 << 0) > +#define ACPI_FACS_64BIT_WAKE_F (1 << 1) > +/* Bits 31..2: reserved */ > + > +/* These can be used by the target port. */ > + > +void acpi_add_table(struct acpi_rsdp *rsdp, void *table); > +unsigned long write_acpi_tables(unsigned long start); > +int acpi_create_madt_lapic(struct acpi_madt_lapic *lapic, u8 cpu, u8 apic); > +unsigned long acpi_create_madt_lapics(unsigned long current); > +int acpi_create_madt_ioapic(struct acpi_madt_ioapic *ioapic, u8 id, u32 > addr, u32 gsi_base); > +int acpi_create_madt_irqoverride(struct acpi_madt_irqoverride *irqoverride, > u8 bus, u8 source, u32 gsirq, u16 flags); > +unsigned long acpi_fill_mcfg(unsigned long current); > +unsigned long acpi_fill_madt(unsigned long current); > +void acpi_create_fadt(struct acpi_fadt *fadt, struct acpi_facs *facs, void > *dsdt); > +int acpi_create_mcfg_mmconfig(struct acpi_mcfg_mmconfig *mmconfig, u32 base, > u16 seg_nr, u8 start, u8 end); > +int acpi_create_madt_lapic_nmi(struct acpi_madt_lapic_nmi *lapic_nmi, u8 > cpu, u16 flags, u8 lint); Blank line please. > +static inline uint32_t read32(const void *addr) > +{ > + return *(volatile uint32_t *)addr; > +} What is this? > + > diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile > index dcfe9ee..6ecd6db 100644 > --- a/arch/x86/lib/Makefile > +++ b/arch/x86/lib/Makefile > @@ -30,6 +30,7 @@ obj-y += physmem.o > obj-$(CONFIG_X86_RAMTEST) += ramtest.o > obj-y += sfi.o > obj-y += string.o > +obj-$(CONFIG_GENERATE_ACPI_TABLE) += acpi_table.o > obj-y += tables.o > obj-$(CONFIG_SYS_X86_TSC_TIMER) += tsc_timer.o > obj-$(CONFIG_CMD_ZBOOT) += zimage.o > diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c > new file mode 100644 > index 0000000..2e20317 > --- /dev/null > +++ b/arch/x86/lib/acpi_table.c > @@ -0,0 +1,413 @@ > +/* > + * Copyright (C) 2015, Saket Sinha <saket.sinh...@gmail.com> > + * Please add comments to give others some hints on where this file is ported from. > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include <asm/acpi_table.h> > +#include <asm/cpu.h> > +#include <asm/ioapic.h> > +#include <asm/lapic.h> > +#include <asm/tables.h> > +#include <asm/pci.h> > +#include <cpu.h> > +#include <dm.h> > +#include <dm/uclass-internal.h> > +#include <dm/lists.h> > + > +extern const unsigned char AmlCode[]; Can we avoid using CamelCase here? > + > +/** > +* Add an ACPI table to the RSDT (and XSDT) structure, recalculate length > +* and checksum. > +*/ > +void acpi_add_table(struct acpi_rsdp *rsdp, void *table) > +{ > + int i, entries_num; > + struct acpi_rsdt *rsdt; > + struct acpi_xsdt *xsdt = NULL; > + > + /* The RSDT is mandatory... */ > + rsdt = (struct acpi_rsdt *)rsdp->rsdt_address; > + > + /* ...while the XSDT is not. */ Can we merge this comment to /* The RSDT is mandatory... */? > + if (rsdp->xsdt_address) > + xsdt = (struct acpi_xsdt *)((u32)rsdp->xsdt_address); > + > + /* This should always be MAX_ACPI_TABLES. */ Nits: please remove the ending period. Please fix this globally in all files. > + entries_num = ARRAY_SIZE(rsdt->entry); > + > + for (i = 0; i < entries_num; i++) { > + if (rsdt->entry[i] == 0) > + break; > + } > + > + if (i >= entries_num) { > + debug("ACPI: Error: Could not add ACPI table, " > + "too many tables.\n"); > + return; > + } > + > + /* Add table to the RSDT. */ > + rsdt->entry[i] = (u32)table; > + > + /* Fix RSDT length or the kernel will assume invalid entries. */ > + rsdt->header.length = sizeof(acpi_header_t) + (sizeof(u32) * (i + 1)); > + > + /* Re-calculate checksum. */ > + rsdt->header.checksum = 0; /* Hope this won't get optimized away */ This is suspicious. I guess it might be optimized. Can you double check to create some way that guarantees it won't be optimized? > + rsdt->header.checksum = table_compute_checksum((u8 *)rsdt, > rsdt->header.length); > + > + /* > + * And now the same thing for the XSDT. We use the same index as for > + * now we want the XSDT and RSDT to always be in sync in coreboot. coreboot -> U-Boot > + */ > + if (xsdt) { > + /* Add table to the XSDT. */ > + xsdt->entry[i] = (u64)(u32)table; > + > + /* Fix XSDT length. */ > + xsdt->header.length = sizeof(acpi_header_t) + > + (sizeof(u64) * (i + > 1)); > + > + /* Re-calculate checksum. */ > + xsdt->header.checksum = 0; And this might be optimized too. > + xsdt->header.checksum = table_compute_checksum((u8 *)xsdt, > + > xsdt->header.length); > + } > +} > + > +int acpi_create_madt_lapic(struct acpi_madt_lapic *lapic, u8 cpu, u8 apic) > +{ > + lapic->type = 0; /* Local APIC structure */ Please use enum for the type. I mentioned this in the v2 patch. > + lapic->length = sizeof(struct acpi_madt_lapic); > + lapic->flags = (1 << 0); /* Processor/LAPIC enabled */ Please define this flag as a macro. > + lapic->processor_id = cpu; > + lapic->apic_id = apic; > + > + return lapic->length; > +} > + > +unsigned long acpi_create_madt_lapics(unsigned long current) > +{ > + struct udevice *dev; > + > + for (uclass_find_first_device(UCLASS_CPU, &dev); > + dev; > + uclass_find_next_device(&dev)) { > + struct cpu_platdata *plat = dev_get_parent_platdata(dev); > + > + current += acpi_create_madt_lapic((struct acpi_madt_lapic > *)current, plat->cpu_id, plat->cpu_id); > + } > + return current; > +} > + > +int acpi_create_madt_ioapic(struct acpi_madt_ioapic *ioapic, u8 id, u32 addr, > + u32 gsi_base) The indention is not correct. > +{ > + ioapic->type = 1; /* I/O APIC structure */ Please use enum for the type. > + ioapic->length = sizeof(struct acpi_madt_ioapic); > + ioapic->reserved = 0x00; > + ioapic->gsi_base = gsi_base; > + ioapic->ioapic_id = id; > + ioapic->ioapic_addr = addr; > + > + return ioapic->length; > +} > + > +int acpi_create_madt_irqoverride(struct acpi_madt_irqoverride *irqoverride, > + u8 bus, u8 source, u32 gsirq, u16 flags) > +{ > + irqoverride->type = 2; /* Interrupt source override */ Please use enum for the type. > + irqoverride->length = sizeof(struct acpi_madt_irqoverride); > + irqoverride->bus = bus; > + irqoverride->source = source; > + irqoverride->gsirq = gsirq; > + irqoverride->flags = flags; > + > + return irqoverride->length; > +} > + > +int acpi_create_madt_lapic_nmi(struct acpi_madt_lapic_nmi *lapic_nmi, u8 cpu, > + > u16 flags, u8 lint) The indention is not correct. > +{ > + lapic_nmi->type = 4; /* Local APIC NMI structure */ Please use enum for the type. > + lapic_nmi->length = sizeof(struct acpi_madt_lapic_nmi); > + lapic_nmi->flags = flags; > + lapic_nmi->processor_id = cpu; > + lapic_nmi->lint = lint; > + > + return lapic_nmi->length; > +} > + > +static void acpi_create_madt(struct acpi_madt *madt) > +{ > + acpi_header_t *header = &(madt->header); > + unsigned long current = (unsigned long)madt + sizeof(struct > acpi_madt); > + > + memset((void *)madt, 0, sizeof(struct acpi_madt)); > + > + /* Fill out header fields. */ > + memcpy(header->signature, "APIC", 4); > + memcpy(header->oem_id, OEM_ID, 6); > + memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8); > + memcpy(header->asl_compiler_id, ASLC, 4); Please create a function to fill out the common headers. Looks the only variable here is the table signature. > + > + header->length = sizeof(struct acpi_madt); > + header->revision = 1; /* ACPI 1.0/2.0: 1, ACPI 3.0: 2, ACPI 4.0: 3 */ Please use enum for the revision. > + > + madt->lapic_addr = LAPIC_DEFAULT_BASE; > + madt->flags = 0x1; /* PCAT_COMPAT */ Please define this flag as a macro. > + > + current = acpi_fill_madt(current); > + > + /* (Re)calculate length and checksum. */ > + header->length = current - (unsigned long)madt; > + > + header->checksum = table_compute_checksum((void *)madt, > header->length); > +} > + > +int acpi_create_mcfg_mmconfig(struct acpi_mcfg_mmconfig *mmconfig, u32 base, > + u16 seg_nr, u8 start, > u8 end) > +{ > + memset(mmconfig, 0, sizeof(*mmconfig)); > + mmconfig->base_address = base; > + mmconfig->base_reserved = 0; > + mmconfig->pci_segment_group_number = seg_nr; > + mmconfig->start_bus_number = start; > + mmconfig->end_bus_number = end; > + > + return sizeof(struct acpi_mcfg_mmconfig); > +} > + > +/* MCFG is defined in the PCI Firmware Specification 3.0. */ > +static void acpi_create_mcfg(struct acpi_mcfg *mcfg) > +{ > + acpi_header_t *header = &(mcfg->header); > + unsigned long current = (unsigned long)mcfg + sizeof(struct > acpi_mcfg); > + > + memset((void *)mcfg, 0, sizeof(struct acpi_mcfg)); > + > + /* Fill out header fields. */ > + memcpy(header->signature, "MCFG", 4); > + memcpy(header->oem_id, OEM_ID, 6); > + memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8); > + memcpy(header->asl_compiler_id, ASLC, 4); Please create a function to fill out the common headers. > + > + header->length = sizeof(struct acpi_mcfg); > + header->revision = 1; Please use enum for the revision. > + > + current = acpi_fill_mcfg(current); > + > + /* (Re)calculate length and checksum. */ > + header->length = current - (unsigned long)mcfg; > + header->checksum = table_compute_checksum((void *)mcfg, > header->length); > +} > + > +static void acpi_create_facs(struct acpi_facs *facs) > +{ > + memset((void *)facs, 0, sizeof(struct acpi_facs)); > + > + memcpy(facs->signature, "FACS", 4); > + facs->length = sizeof(struct acpi_facs); > + facs->hardware_signature = 0; > + facs->firmware_waking_vector = 0; > + facs->global_lock = 0; > + facs->flags = 0; > + facs->x_firmware_waking_vector_l = 0; > + facs->x_firmware_waking_vector_h = 0; > + facs->version = 1; /* ACPI 1.0: 0, ACPI 2.0/3.0: 1, ACPI 4.0: 2 */ Please use enum for the revision. > +} > + > +static void acpi_write_rsdt(struct acpi_rsdt *rsdt) > +{ > + acpi_header_t *header = &(rsdt->header); > + > + /* Fill out header fields. */ > + memcpy(header->signature, "RSDT", 4); > + memcpy(header->oem_id, OEM_ID, 6); > + memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8); > + memcpy(header->asl_compiler_id, ASLC, 4); Please create a function to fill out the common headers. > + > + header->length = sizeof(struct acpi_rsdt); > + header->revision = 1; /* ACPI 1.0/2.0/3.0/4.0: 1 */ Please use enum for the revision. > + > + /* Entries are filled in later, we come with an empty set. */ > + > + /* Fix checksum. */ > + header->checksum = table_compute_checksum((void *)rsdt, sizeof(struct > acpi_rsdt)); > +} > + > +static void acpi_write_xsdt(struct acpi_xsdt *xsdt) > +{ > + acpi_header_t *header = &(xsdt->header); > + > + /* Fill out header fields. */ > + memcpy(header->signature, "XSDT", 4); > + memcpy(header->oem_id, OEM_ID, 6); > + memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8); > + memcpy(header->asl_compiler_id, ASLC, 4); Please create a function to fill out the common headers. > + > + header->length = sizeof(struct acpi_xsdt); > + header->revision = 1; /* ACPI 1.0: N/A, 2.0/3.0/4.0: 1 */ Please use enum for the revision. > + > + /* Entries are filled in later, we come with an empty set. */ > + > + /* Fix checksum. */ > + header->checksum = table_compute_checksum((void *)xsdt, sizeof(struct > acpi_xsdt)); > +} > + > +static void acpi_write_rsdp(struct acpi_rsdp *rsdp, struct acpi_rsdt *rsdt, > struct acpi_xsdt *xsdt) > +{ > + memset(rsdp, 0, sizeof(struct acpi_rsdp)); > + > + memcpy(rsdp->signature, RSDP_SIG, 8); > + memcpy(rsdp->oem_id, OEM_ID, 6); > + > + rsdp->length = sizeof(struct acpi_rsdp); > + rsdp->rsdt_address = (u32)rsdt; > + > + /* > + * Revision: ACPI 1.0: 0, ACPI 2.0/3.0/4.0: 2. > + * > + * Some OSes expect an XSDT to be present for RSD PTR revisions >= 2. > + * If we don't have an ACPI XSDT, force ACPI 1.0 (and thus RSD PTR > + * revision 0). > + */ > + if (xsdt == NULL) { > + rsdp->revision = 0; > + } else { > + rsdp->xsdt_address = (u64)(u32)xsdt; > + rsdp->revision = 2; > + } > + > + /* Calculate checksums. */ > + rsdp->checksum = table_compute_checksum((void *)rsdp, 20); > + rsdp->ext_checksum = table_compute_checksum((void *)rsdp, > sizeof(struct acpi_rsdp)); > +} > + > +static void acpi_create_ssdt_generator(acpi_header_t *ssdt, const char > *oem_table_id) > +{ > + unsigned long current = (unsigned long)ssdt + sizeof(acpi_header_t); > + > + memset((void *)ssdt, 0, sizeof(acpi_header_t)); > + > + memcpy(&ssdt->signature, "SSDT", 4); > + ssdt->revision = 2; /* ACPI 1.0/2.0: ?, ACPI 3.0/4.0: 2 */ Please use enum for the revision. > + memcpy(&ssdt->oem_id, OEM_ID, 6); > + memcpy(&ssdt->oem_table_id, oem_table_id, 8); > + ssdt->oem_revision = 42; Please use defines for the revision. > + memcpy(&ssdt->asl_compiler_id, ASLC, 4); > + ssdt->asl_compiler_revision = 42; Please use defines for the revision. > + ssdt->length = sizeof(acpi_header_t); > + > + /* (Re)calculate length and checksum. */ > + ssdt->length = current - (unsigned long)ssdt; > + ssdt->checksum = table_compute_checksum((void *)ssdt, ssdt->length); > +} > + > + > +#define ALIGN_CURRENT current = (ALIGN(current, 16)) > +unsigned long write_acpi_tables(unsigned long start) > +{ > + unsigned long current; > + struct acpi_rsdp *rsdp; > + struct acpi_rsdt *rsdt; > + struct acpi_xsdt *xsdt; > + struct acpi_facs *facs; > + acpi_header_t *dsdt; > + struct acpi_fadt *fadt; > + struct acpi_mcfg *mcfg; > + struct acpi_madt *madt; > + acpi_header_t *ssdt; > + > + current = start; > + > + /* Align ACPI tables to 16byte */ > + ALIGN_CURRENT; > + > + debug("ACPI: Writing ACPI tables at %lx.\n", start); > + > + /* We need at least an RSDP and an RSDT Table */ > + rsdp = (struct acpi_rsdp *) current; > + current += sizeof(struct acpi_rsdp); > + ALIGN_CURRENT; > + rsdt = (struct acpi_rsdt *) current; > + current += sizeof(struct acpi_rsdt); > + ALIGN_CURRENT; > + xsdt = (struct acpi_xsdt *) current; > + current += sizeof(struct acpi_xsdt); > + ALIGN_CURRENT; > + > + /* clear all table memory */ > + memset((void *) start, 0, current - start); > + > + acpi_write_rsdp(rsdp, rsdt, xsdt); > + acpi_write_rsdt(rsdt); > + acpi_write_xsdt(xsdt); > + > + debug("ACPI: * FACS\n"); > + facs = (struct acpi_facs *) current; > + current += sizeof(struct acpi_facs); > + ALIGN_CURRENT; > + > + acpi_create_facs(facs); > + > + debug("ACPI: * DSDT\n"); > + dsdt = (acpi_header_t *) current; > + memcpy(dsdt, &AmlCode, sizeof(acpi_header_t)); > + if (dsdt->length >= sizeof(acpi_header_t)) { > + current += sizeof(acpi_header_t); > + memcpy((char *)current, > + (char *)&AmlCode + sizeof(acpi_header_t), > + dsdt->length - sizeof(acpi_header_t)); > + current += dsdt->length - sizeof(acpi_header_t); > + > + /* (Re)calculate length and checksum. */ > + dsdt->length = current - (unsigned long)dsdt; > + dsdt->checksum = 0; > + dsdt->checksum = table_compute_checksum((void *)dsdt, dsdt->length); > + } > + ALIGN_CURRENT; > + > + debug("ACPI: * FADT\n"); > + fadt = (struct acpi_fadt *) current; > + current += sizeof(struct acpi_fadt); > + ALIGN_CURRENT; > + acpi_create_fadt(fadt, facs, dsdt); > + acpi_add_table(rsdp, fadt); > + > + debug("ACPI: * MCFG\n"); > + mcfg = (struct acpi_mcfg *) current; > + acpi_create_mcfg(mcfg); > + if (mcfg->header.length > sizeof(struct acpi_mcfg)) { > + current += mcfg->header.length; > + ALIGN_CURRENT; > + acpi_add_table(rsdp, mcfg); > + } > + > + debug("ACPI: * MADT\n"); > + madt = (struct acpi_madt *) current; > + acpi_create_madt(madt); > + if (madt->header.length > sizeof(struct acpi_madt)) { > + current += madt->header.length; > + acpi_add_table(rsdp, madt); > + } > + ALIGN_CURRENT; > + > + debug("ACPI: * SSDT\n"); > + ssdt = (acpi_header_t *)current; > + acpi_create_ssdt_generator(ssdt, ACPI_TABLE_CREATOR); > + if (ssdt->length > sizeof(acpi_header_t)) { > + current += ssdt->length; > + acpi_add_table(rsdp, ssdt); > + ALIGN_CURRENT; > + } > + > + Please remove the blank line. > + debug("current = %lx\n", current); > + > + debug("ACPI: done.\n"); > + return current; > +} > + > diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c > index 75ffbc1..f15b2e2 100644 > --- a/arch/x86/lib/tables.c > +++ b/arch/x86/lib/tables.c > @@ -8,6 +8,7 @@ > #include <asm/sfi.h> > #include <asm/mpspec.h> > #include <asm/tables.h> > +#include <asm/acpi_table.h> > > u8 table_compute_checksum(void *v, int len) > { > @@ -51,4 +52,8 @@ void write_tables(void) > rom_table_end = write_mp_table(rom_table_end); > rom_table_end = ALIGN(rom_table_end, 1024); > #endif > +#ifdef CONFIG_GENERATE_ACPI_TABLE > + rom_table_end = write_acpi_tables(rom_table_end); > + rom_table_end = ALIGN(rom_table_end, 1024); > +#endif > } > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 1c949fc..c259683 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -297,6 +297,17 @@ $(obj)/%.dtb: $(src)/%.dts FORCE > > dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp) > > +# ACPI > +# --------------------------------------------------------------------------- > +quiet_cmd_acpi_c_asl= ASL $@ > +cmd_acpi_c_asl= \ > + $(CPP) -x assembler-with-cpp -P -o $<.tmp $<; \ Why do we use $(CPP) here? Can we also suppress the IASL output (see below) to the shell when building U-Boot without 'V=1'? Intel ACPI Component Architecture ASL Optimizing Compiler version 20140828-64 [Sep 18 2014] Copyright (c) 2000 - 2014 Intel Corporation ASL Input: arch/x86/cpu/qemu/dsdt.asl.tmp - 442 lines, 25925 bytes, 342 keywords AML Output: arch/x86/cpu/qemu/dsdt.aml - 6952 bytes, 226 named objects, 116 executable opcodes Hex Dump: arch/x86/cpu/qemu/dsdt.hex - 65515 bytes > + iasl -tc -p $< -tc $<.tmp; \ There are two -tc here. Duplicated? > + mv $(patsubst %.asl,%.hex,$<) $@ > + > +$(obj)/%.c: $(src)/%.asl > + $(call cmd,acpi_c_asl) > + > # Bzip2 > # --------------------------------------------------------------------------- > > -- Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot