From: Lin Ming <ming.m.lin@intel.com>
To: "Lan, Tianyu" <tianyu.lan@intel.com>
Cc: "lenb@kernel.org" <lenb@kernel.org>,
"linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
"Moore, Robert" <robert.moore@intel.com>,
"Zhang, Rui" <rui.zhang@intel.com>
Subject: Re: [PATCH] Replace acpi_gbl_FADT.sci_interrupt with two functions in some places.
Date: Wed, 04 May 2011 09:48:38 +0800 [thread overview]
Message-ID: <1304473718.3924.56.camel@minggr.sh.intel.com> (raw)
In-Reply-To: <1304403041.3179.29.camel@lantianyu-ws>
On Tue, 2011-05-03 at 14:10 +0800, Lan, Tianyu wrote:
> Some places use the acpi_gbl_FADT.sci_interrupt directly to get the ACPI SCI.ACPI SCI will not only be stored in the FADT.
> So this patch replaces acpi_gbl_FADT.sci_interrupt with two functions for getting and setting.
Some trivial patch style comments,
We usually add "acpi:" prefix to the subject, like
[PATCH] acpi: xxxxx
And if acpica files are also touched, we add "acpi/acpica:" prefix, like
[PATCH] acpi/acpica: xxxx
Another thing, the change logs seems too long for each line, you'd
better split them short for each line. So that makes logs easy to read.
Thanks,
Lin Ming
>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
> arch/x86/kernel/acpi/boot.c | 8 ++---
> drivers/acpi/acpica/evgpeinit.c | 5 +--
> drivers/acpi/acpica/evgpeutil.c | 4 +-
> drivers/acpi/acpica/evsci.c | 4 +-
> drivers/acpi/acpica/evxface.c | 38 +++++++++++++++++++++++++++
> drivers/acpi/bus.c | 4 +-
> drivers/acpi/osl.c | 6 ++--
> drivers/acpi/pci_link.c | 2 -
> drivers/staging/olpc_dcon/olpc_dcon_xo_1_5.c | 2 -
> include/acpi/acpixf.h | 4 ++
> 10 files changed, 59 insertions(+), 18 deletions(-)
>
> Index: linux-2.6/arch/x86/kernel/acpi/boot.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/acpi/boot.c 2011-05-03 10:19:00.163029998 +0800
> +++ linux-2.6/arch/x86/kernel/acpi/boot.c 2011-05-03 10:20:50.833030004 +0800
> @@ -408,7 +408,7 @@
>
> acpi_table_print_madt_entry(header);
>
> - if (intsrc->source_irq == acpi_gbl_FADT.sci_interrupt) {
> + if (intsrc->source_irq == acpi_get_sci_interrupt()) {
> acpi_sci_ioapic_setup(intsrc->source_irq,
> intsrc->inti_flags & ACPI_MADT_POLARITY_MASK,
> (intsrc->inti_flags & ACPI_MADT_TRIGGER_MASK) >> 2,
> @@ -1100,7 +1100,7 @@
> return gsi;
>
> /* Don't set up the ACPI SCI because it's already set up */
> - if (acpi_gbl_FADT.sci_interrupt == gsi)
> + if (acpi_get_sci_interrupt() == gsi)
> return gsi;
>
> ioapic = mp_find_ioapic(gsi);
> @@ -1184,8 +1184,8 @@
> * pretend we got one so we can set the SCI flags.
> */
> if (!acpi_sci_override_gsi)
> - acpi_sci_ioapic_setup(acpi_gbl_FADT.sci_interrupt, 0, 0,
> - acpi_gbl_FADT.sci_interrupt);
> + acpi_sci_ioapic_setup(acpi_get_sci_interrupt(), 0, 0,
> + acpi_get_sci_interrupt());
>
> /* Fill in identity legacy mappings where no override */
> mp_config_acpi_legacy_irqs();
> Index: linux-2.6/drivers/acpi/acpica/evgpeinit.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/acpica/evgpeinit.c 2011-05-03 10:19:00.243029997 +0800
> +++ linux-2.6/drivers/acpi/acpica/evgpeinit.c 2011-05-03 10:20:50.833030004 +0800
> @@ -131,7 +131,7 @@
> status = acpi_ev_create_gpe_block(acpi_gbl_fadt_gpe_device,
> &acpi_gbl_FADT.xgpe0_block,
> register_count0, 0,
> - acpi_gbl_FADT.sci_interrupt,
> + acpi_get_sci_interrupt(),
> &acpi_gbl_gpe_fadt_blocks[0]);
>
> if (ACPI_FAILURE(status)) {
> @@ -170,8 +170,7 @@
> &acpi_gbl_FADT.xgpe1_block,
> register_count1,
> acpi_gbl_FADT.gpe1_base,
> - acpi_gbl_FADT.
> - sci_interrupt,
> + acpi_get_sci_interrupt(),
> &acpi_gbl_gpe_fadt_blocks
> [1]);
>
> Index: linux-2.6/drivers/acpi/acpica/evgpeutil.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/acpica/evgpeutil.c 2011-05-03 10:19:00.213030003 +0800
> +++ linux-2.6/drivers/acpi/acpica/evgpeutil.c 2011-05-03 10:20:50.833030004 +0800
> @@ -253,7 +253,7 @@
>
> /* Install new interrupt handler if not SCI_INT */
>
> - if (interrupt_number != acpi_gbl_FADT.sci_interrupt) {
> + if (interrupt_number != acpi_get_sci_interrupt()) {
> status = acpi_os_install_interrupt_handler(interrupt_number,
> acpi_ev_gpe_xrupt_handler,
> gpe_xrupt);
> @@ -290,7 +290,7 @@
>
> /* We never want to remove the SCI interrupt handler */
>
> - if (gpe_xrupt->interrupt_number == acpi_gbl_FADT.sci_interrupt) {
> + if (gpe_xrupt->interrupt_number == acpi_get_sci_interrupt()) {
> gpe_xrupt->gpe_block_list_head = NULL;
> return_ACPI_STATUS(AE_OK);
> }
> Index: linux-2.6/drivers/acpi/acpica/evsci.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/acpica/evsci.c 2011-05-03 10:19:00.223030004 +0800
> +++ linux-2.6/drivers/acpi/acpica/evsci.c 2011-05-03 10:20:50.833030004 +0800
> @@ -142,7 +142,7 @@
> ACPI_FUNCTION_TRACE(ev_install_sci_handler);
>
> status =
> - acpi_os_install_interrupt_handler((u32) acpi_gbl_FADT.sci_interrupt,
> + acpi_os_install_interrupt_handler((u32) acpi_get_sci_interrupt(),
> acpi_ev_sci_xrupt_handler,
> acpi_gbl_gpe_xrupt_list_head);
> return_ACPI_STATUS(status);
> @@ -176,7 +176,7 @@
> /* Just let the OS remove the handler and disable the level */
>
> status =
> - acpi_os_remove_interrupt_handler((u32) acpi_gbl_FADT.sci_interrupt,
> + acpi_os_remove_interrupt_handler((u32) acpi_get_sci_interrupt(),
> acpi_ev_sci_xrupt_handler);
>
> return_ACPI_STATUS(status);
> Index: linux-2.6/drivers/acpi/acpica/evxface.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/acpica/evxface.c 2011-05-03 10:19:00.213030003 +0800
> +++ linux-2.6/drivers/acpi/acpica/evxface.c 2011-05-03 10:20:50.833030004 +0800
> @@ -983,3 +983,41 @@
> }
>
> ACPI_EXPORT_SYMBOL(acpi_release_global_lock)
> +
> +/*******************************************************************************
> + *
> + * FUNCTION: acpi_get_sci_interrupt
> + *
> + * PARAMETERS: none
> + *
> + * RETURN: SCI Interrupt
> + *
> + * DESCRIPTION: Return the ACPI SCI interrupt
> + *
> + ******************************************************************************/
> +u16 acpi_get_sci_interrupt(void)
> +{
> + return acpi_gbl_FADT.sci_interrupt;
> +}
> +
> +ACPI_EXPORT_SYMBOL(acpi_get_sci_interrupt)
> +
> +/*******************************************************************************
> + *
> + * FUNCTION: acpi_set_sci_interrupt
> + *
> + * PARAMETERS: override sci irq
> + *
> + * RETURN: status
> + *
> + * DESCRIPTION: Set the ACPI SCI interrupt
> + *
> + ******************************************************************************/
> +acpi_status acpi_set_sci_interrupt(u16 irq)
> +{
> + acpi_gbl_FADT.sci_interrupt = irq;
> + return AE_OK;
> +}
> +
> +ACPI_EXPORT_SYMBOL(acpi_set_sci_interrupt)
> +
> Index: linux-2.6/drivers/acpi/bus.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/bus.c 2011-05-03 10:19:00.203030002 +0800
> +++ linux-2.6/drivers/acpi/bus.c 2011-05-03 10:20:50.833030004 +0800
> @@ -890,14 +890,14 @@
> acpi_sci_flags |= ACPI_MADT_TRIGGER_LEVEL;
> }
> /* Set PIC-mode SCI trigger type */
> - acpi_pic_sci_set_trigger(acpi_gbl_FADT.sci_interrupt,
> + acpi_pic_sci_set_trigger(acpi_get_sci_interrupt(),
> (acpi_sci_flags & ACPI_MADT_TRIGGER_MASK) >> 2);
> } else {
> /*
> * now that acpi_gbl_FADT is initialized,
> * update it with result from INT_SRC_OVR parsing
> */
> - acpi_gbl_FADT.sci_interrupt = acpi_sci_override_gsi;
> + acpi_set_sci_interrupt(acpi_sci_override_gsi);
> }
> #endif
>
> Index: linux-2.6/drivers/acpi/osl.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/osl.c 2011-05-03 10:19:48.993030009 +0800
> +++ linux-2.6/drivers/acpi/osl.c 2011-05-03 10:20:50.833030004 +0800
> @@ -534,7 +534,7 @@
> * ACPI interrupts different from the SCI in our copy of the FADT are
> * not supported.
> */
> - if (gsi != acpi_gbl_FADT.sci_interrupt)
> + if (gsi != acpi_get_sci_interrupt())
> return AE_BAD_PARAMETER;
>
> if (acpi_irq_handler)
> @@ -559,7 +559,7 @@
>
> acpi_status acpi_os_remove_interrupt_handler(u32 irq, acpi_osd_handler handler)
> {
> - if (irq != acpi_gbl_FADT.sci_interrupt)
> + if (irq != acpi_get_sci_interrupt())
> return AE_BAD_PARAMETER;
>
> free_irq(irq, acpi_irq);
> @@ -1622,7 +1622,7 @@
> 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_get_sci_interrupt(),
> acpi_irq_handler);
> }
>
> Index: linux-2.6/drivers/acpi/pci_link.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/pci_link.c 2011-05-03 10:19:00.203030002 +0800
> +++ linux-2.6/drivers/acpi/pci_link.c 2011-05-03 10:20:50.833030004 +0800
> @@ -508,7 +508,7 @@
> }
> }
> /* Add a penalty for the SCI */
> - acpi_irq_penalty[acpi_gbl_FADT.sci_interrupt] += PIRQ_PENALTY_PCI_USING;
> + acpi_irq_penalty[acpi_get_sci_interrupt()] += PIRQ_PENALTY_PCI_USING;
> return 0;
> }
>
> Index: linux-2.6/drivers/staging/olpc_dcon/olpc_dcon_xo_1_5.c
> ===================================================================
> --- linux-2.6.orig/drivers/staging/olpc_dcon/olpc_dcon_xo_1_5.c 2011-05-03 10:19:00.193030001 +0800
> +++ linux-2.6/drivers/staging/olpc_dcon/olpc_dcon_xo_1_5.c 2011-05-03 10:20:50.833030004 +0800
> @@ -104,7 +104,7 @@
> pci_dev_put(pdev);
>
> /* we're sharing the IRQ with ACPI */
> - irq = acpi_gbl_FADT.sci_interrupt;
> + irq = acpi_get_sci_interrupt();
> if (request_irq(irq, &dcon_interrupt, IRQF_SHARED, "DCON", dcon)) {
> printk(KERN_ERR PREFIX "DCON (IRQ%d) allocation failed\n", irq);
> return 1;
> Index: linux-2.6/include/acpi/acpixf.h
> ===================================================================
> --- linux-2.6.orig/include/acpi/acpixf.h 2011-05-03 10:19:00.253029998 +0800
> +++ linux-2.6/include/acpi/acpixf.h 2011-05-03 10:20:50.833030004 +0800
> @@ -411,6 +411,10 @@
> acpi_info(const char *module_name,
> u32 line_number, const char *format, ...) ACPI_PRINTF_LIKE(3);
>
> +u16 acpi_get_sci_interrupt(void);
> +
> +acpi_status acpi_set_sci_interrupt(u16 irq);
> +
> /*
> * Debug output
> */
>
> ---
>
next prev parent reply other threads:[~2011-05-04 1:48 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-03 6:10 [PATCH] Replace acpi_gbl_FADT.sci_interrupt with two functions in some places lan,Tianyu
2011-05-04 1:48 ` Lin Ming [this message]
2011-05-05 0:24 ` lan,Tianyu
2011-05-05 17:30 ` Bjorn Helgaas
2011-05-05 17:48 ` Moore, Robert
2011-05-06 2:07 ` lan,Tianyu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1304473718.3924.56.camel@minggr.sh.intel.com \
--to=ming.m.lin@intel.com \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=robert.moore@intel.com \
--cc=rui.zhang@intel.com \
--cc=tianyu.lan@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox