[U-Boot] [PATCH 05/26] x86: irq: Enable SCI on IRQ9

[Stefan Roese]

Hi Bin,

On 07.05.2016 17:46, Bin Meng wrote:
> On Sat, May 7, 2016 at 11:42 PM, Stefan Roese <sr at denx.de> wrote:
>> Hi Bin,
>>
>>
>> On 07.05.2016 16:06, Bin Meng wrote:
>>>
>>> On Tue, May 3, 2016 at 8:31 PM, Stefan Roese <sr at denx.de> wrote:
>>>>
>>>> Hi Bin,
>>>>
>>>>
>>>> On 02.05.2016 09:33, Bin Meng wrote:
>>>>>
>>>>>
>>>>> By default SCI is disabled after power on. ACTL is the register to
>>>>> enable SCI and route it to PIC/APIC. To support both ACPI in PIC
>>>>> mode and APIC mode, configure SCI to use IRQ9.
>>>>>
>>>>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>>>> ---
>>>>>
>>>>>     arch/x86/cpu/irq.c                                 | 25
>>>>> ++++++++++++++++++++++
>>>>>     arch/x86/include/asm/irq.h                         |  4 ++++
>>>>>     doc/device-tree-bindings/misc/intel,irq-router.txt |  5 +++++
>>>>>     3 files changed, 34 insertions(+)
>>>>>
>>>>> diff --git a/arch/x86/cpu/irq.c b/arch/x86/cpu/irq.c
>>>>> index ae90b0c..487ac23 100644
>>>>> --- a/arch/x86/cpu/irq.c
>>>>> +++ b/arch/x86/cpu/irq.c
>>>>> @@ -146,6 +146,9 @@ static int create_pirq_routing_table(struct udevice
>>>>> *dev)
>>>>>                   priv->ibase &= ~0xf;
>>>>>           }
>>>>>
>>>>> +       priv->actl_8bit = fdtdec_get_bool(blob, node,
>>>>> "intel,actl-8bit");
>>>>> +       priv->actl_addr = fdtdec_get_int(blob, node, "intel,actl-addr",
>>>>> 0);
>>>>> +
>>>>>           cell = fdt_getprop(blob, node, "intel,pirq-routing", &len);
>>>>>           if (!cell || len % sizeof(struct pirq_routing))
>>>>>                   return -EINVAL;
>>>>> @@ -215,6 +218,24 @@ static int create_pirq_routing_table(struct udevice
>>>>> *dev)
>>>>>           return 0;
>>>>>     }
>>>>>
>>>>> +#ifdef CONFIG_GENERATE_ACPI_TABLE
>>>>> +static void irq_enable_sci(struct udevice *dev)
>>>>> +{
>>>>> +       struct irq_router *priv = dev_get_priv(dev);
>>>>> +
>>>>> +       if (priv->actl_8bit) {
>>>>> +               /* Bit7 must be turned on to enable ACPI */
>>>>> +               dm_pci_write_config8(dev->parent, priv->actl_addr,
>>>>> 0x80);
>>>>> +       } else {
>>>>> +               /* Write 0 to enable SCI on IRQ9 */
>>>>> +               if (priv->config == PIRQ_VIA_PCI)
>>>>> +                       dm_pci_write_config32(dev->parent,
>>>>> priv->actl_addr, 0);
>>>>> +               else
>>>>> +                       writel(0, priv->ibase + priv->actl_addr);
>>>>> +       }
>>>>> +}
>>>>> +#endif
>>>>> +
>>>>>     int irq_router_common_init(struct udevice *dev)
>>>>>     {
>>>>>           int ret;
>>>>> @@ -228,6 +249,10 @@ int irq_router_common_init(struct udevice *dev)
>>>>>           pirq_route_irqs(dev, pirq_routing_table->slots,
>>>>>                           get_irq_slot_count(pirq_routing_table));
>>>>>
>>>>> +#ifdef CONFIG_GENERATE_ACPI_TABLE
>>>>> +       irq_enable_sci(dev);
>>>>> +#endif
>>>>
>>>>
>>>>
>>>> Again, this could also use IS_ENABLED.
>>>>
>>>
>>> If changing to use IS_ENABLED here, buildman reports:
>>>
>>> 05: x86: irq: Enable SCI on IRQ9
>>>          x86:  +   coreboot-x86 cougarcanyon2 minnowmax
>>> chromebox_panther chromebook_samus crownbay conga-qeval20-qa3-e3845
>>> efi-x86 bayleybay galileo chromebook_link
>>> +   irq_enable_sci(dev);
>>> +   ^
>>> w+../arch/x86/cpu/irq.c: In function 'irq_router_common_init':
>>> w+../arch/x86/cpu/irq.c:254:3: warning: implicit declaration of
>>> function 'irq_enable_sci' [-Wimplicit-function-declaration]
>>>
>>> This is due to irq_enable_sci() is wrapped by #ifdef
>>> CONFIG_GENERATE_ACPI_TABLE and I don't think IS_ENABLED can be used
>>> there.
>>
>>
>> Right. This only works in functions. I think the general rule is,
>> that the #ifdef in the code itself are much worse than "just"
>> around a whole function body. Thats why its okay (I think) to
>> do it this way.
>>
>
> So for this commit, leave it as it was?

What does the compiler generate when you remove the #ifdef
around the function body?

Thanks,
Stefan