[U-Boot] [PATCH 2/4] x86: qemu: setup PM IO base for ACPI in southbridge

[Miao Yan]

Hi Bin,

2016-01-16 21:23 GMT+08:00 Bin Meng <bmeng.cn at gmail.com>:
> Hi Miao,
>
> On Fri, Jan 15, 2016 at 11:12 AM, Miao Yan <yanmiaobest at gmail.com> wrote:
>> Enable ACPI IO space for piix4 (for pc board) and ich9 (for q35 board)
>>
>> Signed-off-by: Miao Yan <yanmiaobest at 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