All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jan Beulich" <jbeulich@novell.com>
To: "Andi Kleen" <andi@firstfloor.org>, "Len Brown" <lenb@kernel.org>
Cc: <robert.moore@intel.com>, "Andrew Paprocki" <andrew@ishiboo.com>,
	"Takashi Iwai" <tiwai@suse.de>,
	"LKML" <linux-kernel@vger.kernel.org>
Subject: Re: Endless ACPI errors on Linus tree (5b664cb235)
Date: Wed, 23 Jul 2008 12:27:57 +0100	[thread overview]
Message-ID: <488731DD.76E4.0078.0@novell.com> (raw)
In-Reply-To: <20080721084354.GB20258@basil.nowhere.org>

>>> Andi Kleen <andi@firstfloor.org> 21.07.08 10:43 >>>
>> That would basically mean there's no way to use the v2 fields here - all
>> the code could do is report the inconsitency, but it would have to use
>> the v1 field in any case. That's really a shame, and as before I'm
>> somewhat hesitant to do so. The only way I could easily find myself
>> doing this would be to introduce a command line option controlling the
>> behavior here, but that would need to be done outside of the ACPICA
>> code I'd assume.
>> 
>> Andi, Bob, Len?
>
>I think we should only use v1 FADT fields for now and ignore v2.

Here we go:

Subject: ACPI: fix FADT parsing (v3)

The (1.0 inherited) separate length fields in the FADT are byte
granular. Further, PM1a/b may live in distinct address spaces, and they
also could have different widths (if using the respective v2 fields was
okay, which in practice it apparently isn't). acpi_tb_convert_fadt()
should account for all of these conditions.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

---
 drivers/acpi/tables/tbfadt.c |   32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

--- linux-2.6.26/drivers/acpi/tables/tbfadt.c	2008-07-13 23:51:29.000000000 +0200
+++ 2.6.26-acpi-fadt-parse/drivers/acpi/tables/tbfadt.c	2008-07-23 13:21:22.000000000 +0200
@@ -50,7 +50,7 @@ ACPI_MODULE_NAME("tbfadt")
 /* Local prototypes */
 static void inline
 acpi_tb_init_generic_address(struct acpi_generic_address *generic_address,
-			     u8 bit_width, u64 address);
+			     u8 byte_width, u64 address);
 
 static void acpi_tb_convert_fadt(void);
 
@@ -111,7 +111,7 @@ static struct acpi_fadt_info fadt_info_t
  * FUNCTION:    acpi_tb_init_generic_address
  *
  * PARAMETERS:  generic_address     - GAS struct to be initialized
- *              bit_width           - Width of this register
+ *              byte_width          - Width of this register
  *              Address             - Address of the register
  *
  * RETURN:      None
@@ -124,7 +124,7 @@ static struct acpi_fadt_info fadt_info_t
 
 static void inline
 acpi_tb_init_generic_address(struct acpi_generic_address *generic_address,
-			     u8 bit_width, u64 address)
+			     u8 byte_width, u64 address)
 {
 
 	/*
@@ -136,7 +136,7 @@ acpi_tb_init_generic_address(struct acpi
 	/* All other fields are byte-wide */
 
 	generic_address->space_id = ACPI_ADR_SPACE_SYSTEM_IO;
-	generic_address->bit_width = bit_width;
+	generic_address->bit_width = byte_width << 3;
 	generic_address->bit_offset = 0;
 	generic_address->access_width = 0;
 }
@@ -342,9 +342,20 @@ static void acpi_tb_convert_fadt(void)
 	 * useful to calculate them once, here.
 	 *
 	 * The PM event blocks are split into two register blocks, first is the
-	 * PM Status Register block, followed immediately by the PM Enable Register
-	 * block. Each is of length (pm1_event_length/2)
+	 * PM Status Register block, followed immediately by the PM Enable
+	 * Register block. Each is of length (xpm1x_event_block.bit_width/2).
+	 *
+	 * On various systems the v2 fields (and particularly the bit widths)
+	 * cannot be relied upon, though. Hence resort to using the v1 length
+	 * here (and warn about the inconsistency).
 	 */
+	if (acpi_gbl_FADT.xpm1a_event_block.bit_width
+	    != acpi_gbl_FADT.pm1_event_length * 8)
+		printk(KERN_WARNING "FADT: "
+		       "X_PM1a_EVT_BLK.bit_width (%u) does not match"
+		       " PM1_EVT_LEN (%u)\n",
+		       acpi_gbl_FADT.xpm1a_event_block.bit_width,
+		       acpi_gbl_FADT.pm1_event_length);
 	pm1_register_length = (u8) ACPI_DIV_2(acpi_gbl_FADT.pm1_event_length);
 
 	/* The PM1A register block is required */
@@ -360,13 +371,20 @@ static void acpi_tb_convert_fadt(void)
 	/* The PM1B register block is optional, ignore if not present */
 
 	if (acpi_gbl_FADT.xpm1b_event_block.address) {
+		if (acpi_gbl_FADT.xpm1b_event_block.bit_width
+		    != acpi_gbl_FADT.pm1_event_length * 8)
+			printk(KERN_WARNING "FADT: "
+			       "X_PM1b_EVT_BLK.bit_width (%u) does not match"
+			       " PM1_EVT_LEN (%u)\n",
+			       acpi_gbl_FADT.xpm1b_event_block.bit_width,
+			       acpi_gbl_FADT.pm1_event_length);
 		acpi_tb_init_generic_address(&acpi_gbl_xpm1b_enable,
 					     pm1_register_length,
 					     (acpi_gbl_FADT.xpm1b_event_block.
 					      address + pm1_register_length));
 		/* Don't forget to copy space_id of the GAS */
 		acpi_gbl_xpm1b_enable.space_id =
-		    acpi_gbl_FADT.xpm1a_event_block.space_id;
+		    acpi_gbl_FADT.xpm1b_event_block.space_id;
 
 	}
 }



  reply	other threads:[~2008-07-23 11:27 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-18 11:24 Endless ACPI errors on Linus tree (5b664cb235) Takashi Iwai
2008-07-18 11:35 ` Andi Kleen
2008-07-18 13:15   ` Takashi Iwai
2008-07-18 13:34     ` Takashi Iwai
2008-07-18 15:48       ` Takashi Iwai
2008-07-18 15:52         ` Andi Kleen
2008-07-18 15:56           ` Takashi Iwai
2008-07-18 16:16         ` Jan Beulich
2008-07-18 16:38           ` Takashi Iwai
2008-07-18 16:58             ` Andi Kleen
2008-07-19  9:23               ` Takashi Iwai
2008-07-21  8:11                 ` Jan Beulich
2008-07-21  8:43                   ` Andi Kleen
2008-07-23 11:27                     ` Jan Beulich [this message]
2008-07-23 15:43                       ` Moore, Robert
2008-07-23 16:14                         ` Jan Beulich
2008-09-17  4:56                           ` Andrew Paprocki
2008-09-17  6:43                             ` Jan Beulich
2008-09-17 15:04                               ` Andi Kleen
2008-07-18 15:52       ` Andi Kleen

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=488731DD.76E4.0078.0@novell.com \
    --to=jbeulich@novell.com \
    --cc=andi@firstfloor.org \
    --cc=andrew@ishiboo.com \
    --cc=lenb@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robert.moore@intel.com \
    --cc=tiwai@suse.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.