[U-Boot] [PATCH 4/4] x86: queensbay: Implement PIRQ routing

[Bin Meng]

Hi Simon,

On Fri, Apr 24, 2015 at 11:39 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Bin,
>
> On 23 April 2015 at 21:36, Bin Meng <bmeng.cn at gmail.com> wrote:
>> Hi Simon,
>>
>> On Fri, Apr 24, 2015 at 11:02 AM, Simon Glass <sjg at chromium.org> wrote:
>>> Hi Bin,
>>>
>>> On 21 April 2015 at 19:56, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>> Implement Intel Queensbay platform-specific PIRQ routing support.
>>>> The chipset PIRQ routing setup is called in the arch_mist_init().
>>>
>>> arch_misc_init()?
>>
>> Will fix.
>>
>>> Also how can this be used on other boards? Should part of this code be
>>> moved to a generic place?
>>
>> Generally Intel interrupt router should be compatible, but the offsets
>> of these interrupt routing registers might be different from platform
>> to platform. The platform-specific part is the internal PCI devices'
>> interrupt pin to PIRQ map.
>>
>>>>
>>>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>>>
>>>> ---
>>>>
>>>>  arch/x86/cpu/queensbay/Makefile              |   2 +-
>>>>  arch/x86/cpu/queensbay/irq.c                 | 236 +++++++++++++++++++++++++++
>>>>  arch/x86/cpu/queensbay/tnc.c                 |  10 +-
>>>>  arch/x86/include/asm/arch-queensbay/device.h |  94 +++++++++++
>>>>  arch/x86/include/asm/arch-queensbay/irq.h    |  55 +++++++
>>>>  arch/x86/include/asm/arch-queensbay/tnc.h    |  26 ++-
>>>>  arch/x86/include/asm/u-boot-x86.h            |   2 +
>>>>  configs/crownbay_defconfig                   |   1 +
>>>>  include/configs/crownbay.h                   |   1 +
>>>>  9 files changed, 423 insertions(+), 4 deletions(-)
>>>>  create mode 100644 arch/x86/cpu/queensbay/irq.c
>>>>  create mode 100644 arch/x86/include/asm/arch-queensbay/device.h
>>>>  create mode 100644 arch/x86/include/asm/arch-queensbay/irq.h
>>>>
> [snip]
>
>>>> +       writew(PIRQE, rcba + D02IR);
>>>> +       writew(PIRQF, rcba + D03IR);
>>>> +       writew(PIRQG, rcba + D27IR);
>>>> +       writew(PIRQH, rcba + D31IR);
>>>> +       writew(PIRQE, rcba + D23IR);
>>>> +       writew(PIRQF, rcba + D24IR);
>>>> +       writew(PIRQG, rcba + D25IR);
>>>> +       writew(PIRQH, rcba + D26IR);
>>>> +
>>>> +       if (!create_pirq_routing_table()) {
>>>
>>> Can you add a comment here as to why you do this next bit if
>>> create_pirq_routing_table() fails?
>>
>> I am not sure if you misread the codes? If create_pirq_routing_table()
>> fails, it returns non-zero value and the following pirq_route_irqs()
>> will not be called.
>
> Yes I misread it. So should this function return the error code it
> gets? Or maybe add debug()?

Yep, adding one debug() is nice. Will do that in v2.

>>
>>>> +               /* Route PIRQ */
>>>> +               pirq_route_irqs(pirq_routing_table->slots,
>>>> +                               get_irq_slot_count(pirq_routing_table));
>>>> +       }
>>>> +}
>>>> +

Regards,
Bin