Hi Bin, 2016-01-16 21:23 GMT+08:00 Bin Meng <bmeng...@gmail.com>: > Hi Miao, > > On Fri, Jan 15, 2016 at 11:12 AM, Miao Yan <yanmiaob...@gmail.com> wrote: >> Enable ACPI IO space for piix4 (for pc board) and ich9 (for q35 board) >> >> Signed-off-by: Miao Yan <yanmiaob...@gmail.com> >> --- >> arch/x86/cpu/qemu/qemu.c | 39 >> +++++++++++++++++++++++++++++++++ >> arch/x86/include/asm/arch-qemu/device.h | 8 +++++++ >> 2 files changed, 47 insertions(+) >> >> diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c >> index 46111c9..e7d8a6c 100644 >> --- a/arch/x86/cpu/qemu/qemu.c >> +++ b/arch/x86/cpu/qemu/qemu.c >> @@ -15,6 +15,41 @@ >> >> static bool i440fx; >> >> +static void enable_pm_piix(void) >> +{ >> + u8 en; >> + u16 device, cmd; >> + >> + device = x86_pci_read_config16(PIIX_PM, PCI_DEVICE_ID); >> + if (device != PCI_DEVICE_ID_INTEL_82371AB_3) >> + return; > > Guess the check is already covered in qemu_chipset_init().
Do you mean this check ? device = x86_pci_read_config16(PCI_BDF(0, 0, 0), PCI_DEVICE_ID); i440fx = (device == PCI_DEVICE_ID_INTEL_82441); So is it guaranteed that PIIX_PM is always on that BDF ? IMO, we are operating on another chipset, and we better make sure it's the one we expect, besides, an extra check won't do any harm. > >> + >> + /* Set the PM I/O base. */ > > nits: please remove the ending period. Please fix this globally in this file. > >> + x86_pci_write_config32(PIIX_PM, PMBA, DEFAULT_PMBASE | 1); >> + >> + /* Enable access to the PM I/O space. */ >> + cmd = x86_pci_read_config16(PIIX_PM, PCI_COMMAND); >> + cmd |= PCI_COMMAND_IO; >> + x86_pci_write_config16(PIIX_PM, PCI_COMMAND, cmd); >> + >> + /* PM I/O Space Enable (PMIOSE). */ >> + en = x86_pci_read_config8(PIIX_PM, PMREGMISC); >> + en |= PMIOSE; >> + x86_pci_write_config8(PIIX_PM, PMREGMISC, en); >> +} >> + >> +static void enable_pm_ich9(void) >> +{ >> + u16 device; >> + >> + device = x86_pci_read_config16(ICH9_PM, PCI_DEVICE_ID); >> + if (device != PCI_DEVICE_ID_INTEL_ICH9_8) >> + return; > > Guess the check is already covered in qemu_chipset_init(). > >> + >> + /* Set the PM I/O base. */ >> + x86_pci_write_config32(ICH9_PM, PMBA, DEFAULT_PMBASE | 1); >> +} >> + >> static void qemu_chipset_init(void) >> { >> u16 device, xbcs; >> @@ -53,10 +88,14 @@ static void qemu_chipset_init(void) >> xbcs = x86_pci_read_config16(PIIX_ISA, XBCS); >> xbcs |= APIC_EN; >> x86_pci_write_config16(PIIX_ISA, XBCS, xbcs); >> + >> + enable_pm_piix(); >> } else { >> /* Configure PCIe ECAM base address */ >> x86_pci_write_config32(PCI_BDF(0, 0, 0), PCIEX_BAR, >> CONFIG_PCIE_ECAM_BASE | BAR_EN); >> + >> + enable_pm_ich9(); >> } >> >> qemu_fwcfg_init(); >> diff --git a/arch/x86/include/asm/arch-qemu/device.h >> b/arch/x86/include/asm/arch-qemu/device.h >> index 75a435e..2e11100 100644 >> --- a/arch/x86/include/asm/arch-qemu/device.h >> +++ b/arch/x86/include/asm/arch-qemu/device.h >> @@ -13,9 +13,17 @@ >> #define PIIX_ISA PCI_BDF(0, 1, 0) >> #define PIIX_IDE PCI_BDF(0, 1, 1) >> #define PIIX_USB PCI_BDF(0, 1, 2) >> +#define PIIX_PM PCI_BDF(0, 1, 3) >> +#define ICH9_PM PCI_BDF(0, 0x1f, 0) >> #define I440FX_VGA PCI_BDF(0, 2, 0) >> >> #define QEMU_Q35 PCI_BDF(0, 0, 0) >> #define Q35_VGA PCI_BDF(0, 1, 0) >> >> +#define PMBA 0x40 >> +#define DEFAULT_PMBASE 0xe400 > > See arch/x86/cpu/quark/Kconfig we have ACPI_PM1_BASE already. Maybe we > need consolidate this to introduce a similar one for QEMU. OK, will fix this. > >> +#define PM_IO_BASE DEFAULT_PMBASE > > PM_IO_BASE is not referenced anywhere. > >> +#define PMREGMISC 0x80 >> +#define PMIOSE (1 << 0) >> + > > Please move these register defines to include/asm/arch-qemu/qemu.h OK, will fix this. > >> #endif /* _QEMU_DEVICE_H_ */ >> -- > > Regards, > Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot