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

[Bin Meng]

Hi Miao,

On Wed, Jan 20, 2016 at 9:58 AM, Miao Yan <yanmiaobest at gmail.com> wrote:
> Hi Bin,
>
> 2016-01-19 17:25 GMT+08:00 Bin Meng <bmeng.cn at gmail.com>:
>> Hi Miao,
>>
>> On Tue, Jan 19, 2016 at 10:46 AM, Miao Yan <yanmiaobest at gmail.com> wrote:
>>> 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 ?
>>
>> I believe so. If you look at the codes in qemu.c, the variable "static
>> bool i440fx" is used to distinguish QEMU machine pc and q35. It does
>> not check whether the chipset is i440fx, or PIIX which is the chipset
>> connected to i440fx.
>>
>>>
>>> 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.
>>>
>>
>> Yes, that makes sense. So if we go with your way, maybe we need expand
>> "static bool i440fx" to multiple variables and use correct variable to
>> check? But this looks a bit complex than a single variable.
>>
>
> Yes, that's a little bit complex and not necessary if their PCI
> addresses are fixed . And I don't think we should do it in this
> patchset.
>
> So how do you suggest we do this ? Either I remove the two checks to
> make it aligned with the rest or create a separate patch to do the
> checks ?
>

I suggest we do it in existing way (single variable).

[snip]

Regards,
Bin