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

Hi Bin,

On 07.05.2016 17:46, Bin Meng wrote:

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?

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

Thanks, Stefan