[U-Boot] [PATCH 05/10] x86: mpspec: Allow platform to determine how PIRQ is connected to I/O APIC

[Simon Glass]

Hi Bin,

On 18 July 2015 at 10:30, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon,
>
> On Sat, Jul 18, 2015 at 10:37 PM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Bin,
>>
>> On 15 July 2015 at 02:23, Bin Meng <bmeng.cn at gmail.com> wrote:
>>> Currently during writing MP table I/O interrupt assignment entry, we
>>> assume the PIRQ is directly mapped to I/O APIC INTPIN#16-23, which
>>> however is not always the case on some platforms.
>>>
>>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>> ---
>>>
>>>  arch/x86/include/asm/mpspec.h | 17 +++++++++++++++++
>>>  arch/x86/lib/mpspec.c         | 23 ++++++++++++++++-------
>>>  2 files changed, 33 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/arch/x86/include/asm/mpspec.h b/arch/x86/include/asm/mpspec.h
>>> index efa9231..ad8eba9 100644
>>> --- a/arch/x86/include/asm/mpspec.h
>>> +++ b/arch/x86/include/asm/mpspec.h
>>> @@ -432,6 +432,23 @@ void mp_write_compat_address_space(struct mp_config_table *mc, int busid,
>>>  u32 mptable_finalize(struct mp_config_table *mc);
>>>
>>>  /**
>>> + * mp_determine_pci_dstirq() - Determine PCI device's int pin on the I/O APIC
>>> + *
>>> + * This determines a PCI device's interrupt pin number on the I/O APIC.
>>> + *
>>> + * This can be implemented by platform codes to handle specifal cases, which
>>> + * do not conform to the normal chipset/board design where PIRQ[A-H] are mapped
>>> + * directly to I/O APIC INTPIN#16-23.
>>> + *
>>> + * @bus:       bus number of the pci device
>>> + * @dev:       device number of the pci device
>>> + * @func:      function number of the pci device
>>> + * @pirq:      PIRQ number the PCI device's interrupt pin is routed to
>>> + * @return:    interrupt pin number on the I/O APIC
>>> + */
>>> +int mp_determine_pci_dstirq(int bus, int dev, int func, int pirq);
>>> +
>>> +/**
>>>   * write_mp_table() - Write MP table
>>>   *
>>>   * This writes MP table at a given address.
>>> diff --git a/arch/x86/lib/mpspec.c b/arch/x86/lib/mpspec.c
>>> index f16fbcb..f90567c 100644
>>> --- a/arch/x86/lib/mpspec.c
>>> +++ b/arch/x86/lib/mpspec.c
>>> @@ -269,6 +269,12 @@ static bool check_dup_entry(struct mpc_config_intsrc *intsrc_base,
>>>         return (i == entry_num) ? false : true;
>>>  }
>>>
>>> +__weak int mp_determine_pci_dstirq(int bus, int dev, int func, int pirq)
>>
>> I'd like to avoid using weak functions. Can we use device tree or
>> driver model instead?
>>
>
> As you can see in this patch
> http://patchwork.ozlabs.org/patch/495524/, that the return value of
> this function (dstirq) relies on the runtime value, so we cannot
> describe it in the device tree. As for driver model, I am not sure if
> if it suits. I will need think about it. If we do driver model, we
> might be able to consolidate PIRQ and MP table codes. Can we leave
> this to a possible future work?

OK. Please add a TODO for now.

>
>> In principle we should be able to build all of the code in and boot an
>> image on multiple boards. These sorts of things make it harder to
>> understand and follow the code IMO.
>>
>
> Yep, ideally although I don't know how practical for now :-)
>
> [snip]

Well Rome wasn't built in a day. Let's get this in as is while you
figure it out.

Regards,
Simon