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

Bin Meng
2016-05-07T15:46:50Z

Hi Stefan,

On Sat, May 7, 2016 at 11:42 PM, Stefan Roese <sr@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@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@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?

Regards,
Bin