Re: [PATCH 04/12] ACPI: HW reduced mode does not allow use of the FADT sci_interrupt field

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Saturday, November 09, 2013 06:36:14 PM al.stone@xxxxxxxxxx wrote:
> From: Al Stone <ahs3@xxxxxxxxxx>

-ENOCHANGELOG

> Signed-off-by: Al Stone <al.stone@xxxxxxxxxx>
> ---
>  drivers/acpi/bus.c      |  3 ++-
>  drivers/acpi/osl.c      | 10 ++++++----
>  drivers/acpi/pci_link.c | 14 ++++++++------
>  3 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index b587ec8..6a54dd5 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -540,7 +540,8 @@ void __init acpi_early_init(void)
>  		goto error0;
>  	}
>  
> -#ifdef CONFIG_X86
> +#if (!CONFIG_ACPI_REDUCED_HARDWARE)

Why don't you use #ifndef here?

> +	/* NB: in HW reduced mode, FADT sci_interrupt has no meaning */

I'm not sure what the "NB" stands for, but it looks like that's what "NOTE:" is
used for elsewhere.

>  	if (!acpi_ioapic) {
>  		/* compatible (0) means level (3) */
>  		if (!(acpi_sci_flags & ACPI_MADT_TRIGGER_MASK)) {
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 54a20ff..017b85c 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -84,6 +84,7 @@ static int (*__acpi_os_prepare_extended_sleep)(u8 sleep_state, u32 val_a,
>  
>  static acpi_osd_handler acpi_irq_handler;
>  static void *acpi_irq_context;
> +static u32 acpi_irq_number;
>  static struct workqueue_struct *kacpid_wq;
>  static struct workqueue_struct *kacpi_notify_wq;
>  static struct workqueue_struct *kacpi_hotplug_wq;
> @@ -797,9 +798,9 @@ acpi_os_install_interrupt_handler(u32 gsi, acpi_osd_handler handler,
>  
>  	/*
>  	 * ACPI interrupts different from the SCI in our copy of the FADT are
> -	 * not supported.
> +	 * not supported, except in HW reduced mode.
>  	 */
> -	if (gsi != acpi_gbl_FADT.sci_interrupt)
> +	if (!acpi_gbl_reduced_hardware && (gsi != acpi_gbl_FADT.sci_interrupt))

The inner parens are not necessary.

Also it seems that we may need to support gsi != acpi_gbl_FADT.sci_interrupt
generically, because there may be GPE device objects with interrupts different
from the SCI.

>  		return AE_BAD_PARAMETER;
>  
>  	if (acpi_irq_handler)
> @@ -818,13 +819,14 @@ acpi_os_install_interrupt_handler(u32 gsi, acpi_osd_handler handler,
>  		acpi_irq_handler = NULL;
>  		return AE_NOT_ACQUIRED;
>  	}
> +	acpi_irq_number = irq;
>  
>  	return AE_OK;
>  }
>  
>  acpi_status acpi_os_remove_interrupt_handler(u32 irq, acpi_osd_handler handler)
>  {
> -	if (irq != acpi_gbl_FADT.sci_interrupt)
> +	if (!acpi_gbl_reduced_hardware && (irq != acpi_gbl_FADT.sci_interrupt))

The inner parens are not necessary.

>  		return AE_BAD_PARAMETER;
>  
>  	free_irq(irq, acpi_irq);
> @@ -1806,7 +1808,7 @@ acpi_status __init acpi_os_initialize1(void)
>  acpi_status acpi_os_terminate(void)
>  {
>  	if (acpi_irq_handler) {
> -		acpi_os_remove_interrupt_handler(acpi_gbl_FADT.sci_interrupt,
> +		acpi_os_remove_interrupt_handler(acpi_irq_number,
>  						 acpi_irq_handler);

It looks like this could be one line now?

>  	}
>  
> diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
> index 2652a61..c0ab28a 100644
> --- a/drivers/acpi/pci_link.c
> +++ b/drivers/acpi/pci_link.c

I'm not sure how the comment changes below belong to this patch.

> @@ -23,7 +23,7 @@
>   *
>   * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   *
> - * TBD: 
> + * TBD:
>   *      1. Support more than one IRQ resource entry per link device (index).
>   *	2. Implement start/stop mechanism and use ACPI Bus Driver facilities
>   *	   for IRQ management (e.g. start()->_SRS).
> @@ -268,8 +268,8 @@ static int acpi_pci_link_get_current(struct acpi_pci_link *link)
>  		}
>  	}
>  
> -	/* 
> -	 * Query and parse _CRS to get the current IRQ assignment. 
> +	/*
> +	 * Query and parse _CRS to get the current IRQ assignment.
>  	 */
>  
>  	status = acpi_walk_resources(link->device->handle, METHOD_NAME__CRS,
> @@ -415,7 +415,7 @@ static int acpi_pci_link_set(struct acpi_pci_link *link, int irq)
>  /*
>   * "acpi_irq_balance" (default in APIC mode) enables ACPI to use PIC Interrupt
>   * Link Devices to move the PIRQs around to minimize sharing.
> - * 
> + *
>   * "acpi_irq_nobalance" (default in PIC mode) tells ACPI not to move any PIC IRQs
>   * that the BIOS has already set to active.  This is necessary because
>   * ACPI has no automatic means of knowing what ISA IRQs are used.  Note that
> @@ -433,7 +433,7 @@ static int acpi_pci_link_set(struct acpi_pci_link *link, int irq)
>   *
>   * Note that PCI IRQ routers have a list of possible IRQs,
>   * which may not include the IRQs this table says are available.
> - * 
> + *
>   * Since this heuristic can't tell the difference between a link
>   * that no device will attach to, vs. a link which may be shared
>   * by multiple active devices -- it is not optimal.
> @@ -505,7 +505,9 @@ int __init acpi_irq_penalty_init(void)
>  		}
>  	}

Why don't you simply put

	if (acpi_gbl_reduced_hardware)
		return 0;

here?

>  	/* Add a penalty for the SCI */
> -	acpi_irq_penalty[acpi_gbl_FADT.sci_interrupt] += PIRQ_PENALTY_PCI_USING;
> +	if (!acpi_gbl_reduced_hardware)
> +		acpi_irq_penalty[acpi_gbl_FADT.sci_interrupt] +=
> +			PIRQ_PENALTY_PCI_USING;
>  	return 0;
>  }
>  
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux