[0/5] remove ACPI motherboard driver, use PNP system driver instead (take 2)

[0/5] remove ACPI motherboard driver, use PNP system driver instead (take 2)

[Bjorn Helgaas]

The ACPI motherboard driver is mostly redundant with the PNP system
driver.  This series moves the little bit of ACPI-specific stuff out
of the motherboard driver into acpi/osl.c, adds a little bit to the
PNP system driver so it covers all the remaining functionality of
the ACPI driver, and removes the ACPI driver.

Thanks for the comments on the previous version.  This set of patches:

  - Makes acpi_reserve_resources() a device_initcall(), so the FADT
    resources will be reserved after the pnp/system driver claims its
    resources.  This lets us reserve a motherboard region larger than
    the FADT describes without conflicts.

  - Adds CONFIG_PNPACPI=y to the i386/defconfig patch.  This is needed
    for the pnp/system driver to find its devices.

Len asked about the CONFIG_PNP=n, CONFIG_ACPI=y case.  In this case,
we won't have a pnp/system driver to claim motherboard resources.  But
we already have this situation with other devices.  My IR device
responds to ioports 0x100-0x10f and my ECP parallel port responds to
ioports 0x778-0x77a.  If those drivers aren't loaded (or if they
aren't smart enough to claim all the resources), the resources just
aren't claimed.

So I think a better solution for this problem would be to make the
PNP core reserve all the regions for every active device, similar to
what PCI does.

Bjorn

[patch 1/5] ACPI: move FADT resource reservations from motherboard driver to osl

[Bjorn Helgaas]

Resources described by the FADT aren't really a good fit for the
ACPI motherboard driver.

The motherboard driver cares about PNP0C01 and PNP0C02 devices and
their resources.

The FADT describes some resources used by the ACPI core.  Often, they
are also described by by the _CRS of a motherboard device, but I think
it's better to reserve them specifically in the ACPI osl.c because
(a) the motherboard driver is optional and ACPI uses the resources even
if the driver is absent, and (b) I want to remove the ACPI motherboard
driver because it's mostly redundant with the PNP system.c driver.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: mm-work3/drivers/acpi/motherboard.c
===================================================================
--- mm-work3.orig/drivers/acpi/motherboard.c	2007-01-14 14:52:30.000000000 -0700
+++ mm-work3/drivers/acpi/motherboard.c	2007-01-18 10:21:13.000000000 -0700
@@ -118,58 +118,9 @@
 		},
 };
 
-static void __init acpi_request_region (struct acpi_generic_address *addr,
-	unsigned int length, char *desc)
-{
-	if (!addr->address || !length)
-		return;
-
-	if (addr->address_space_id == ACPI_ADR_SPACE_SYSTEM_IO)
-		request_region(addr->address, length, desc);
-	else if (addr->address_space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)
-		request_mem_region(addr->address, length, desc);
-}
-
-static void __init acpi_reserve_resources(void)
-{
-	acpi_request_region(&acpi_gbl_FADT->xpm1a_evt_blk,
-			       acpi_gbl_FADT->pm1_evt_len, "ACPI PM1a_EVT_BLK");
-
-	acpi_request_region(&acpi_gbl_FADT->xpm1b_evt_blk,
-			       acpi_gbl_FADT->pm1_evt_len, "ACPI PM1b_EVT_BLK");
-
-	acpi_request_region(&acpi_gbl_FADT->xpm1a_cnt_blk,
-			       acpi_gbl_FADT->pm1_cnt_len, "ACPI PM1a_CNT_BLK");
-
-	acpi_request_region(&acpi_gbl_FADT->xpm1b_cnt_blk,
-			       acpi_gbl_FADT->pm1_cnt_len, "ACPI PM1b_CNT_BLK");
-
-	if (acpi_gbl_FADT->pm_tm_len == 4)
-		acpi_request_region(&acpi_gbl_FADT->xpm_tmr_blk, 4, "ACPI PM_TMR");
-
-	acpi_request_region(&acpi_gbl_FADT->xpm2_cnt_blk,
-			       acpi_gbl_FADT->pm2_cnt_len, "ACPI PM2_CNT_BLK");
-
-	/* Length of GPE blocks must be a non-negative multiple of 2 */
-
-	if (!(acpi_gbl_FADT->gpe0_blk_len & 0x1))
-		acpi_request_region(&acpi_gbl_FADT->xgpe0_blk,
-			       acpi_gbl_FADT->gpe0_blk_len, "ACPI GPE0_BLK");
-
-	if (!(acpi_gbl_FADT->gpe1_blk_len & 0x1))
-		acpi_request_region(&acpi_gbl_FADT->xgpe1_blk,
-			       acpi_gbl_FADT->gpe1_blk_len, "ACPI GPE1_BLK");
-}
-
 static int __init acpi_motherboard_init(void)
 {
 	acpi_bus_register_driver(&acpi_motherboard_driver);
-	/*
-	 * Guarantee motherboard IO reservation first
-	 * This module must run after scan.c
-	 */
-	if (!acpi_disabled)
-		acpi_reserve_resources();
 	return 0;
 }
 
Index: mm-work3/drivers/acpi/osl.c
===================================================================
--- mm-work3.orig/drivers/acpi/osl.c	2007-01-14 14:52:30.000000000 -0700
+++ mm-work3/drivers/acpi/osl.c	2007-01-18 14:39:47.000000000 -0700
@@ -75,6 +75,54 @@
 static void *acpi_irq_context;
 static struct workqueue_struct *kacpid_wq;
 
+static void __init acpi_request_region (struct acpi_generic_address *addr,
+	unsigned int length, char *desc)
+{
+	struct resource *res;
+
+	if (!addr->address || !length)
+		return;
+
+	if (addr->address_space_id == ACPI_ADR_SPACE_SYSTEM_IO)
+		res = request_region(addr->address, length, desc);
+	else if (addr->address_space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)
+		res = request_mem_region(addr->address, length, desc);
+}
+
+static int __init acpi_reserve_resources(void)
+{
+	acpi_request_region(&acpi_fadt.xpm1a_evt_blk, acpi_fadt.pm1_evt_len,
+		"ACPI PM1a_EVT_BLK");
+
+	acpi_request_region(&acpi_fadt.xpm1b_evt_blk, acpi_fadt.pm1_evt_len,
+		"ACPI PM1b_EVT_BLK");
+
+	acpi_request_region(&acpi_fadt.xpm1a_cnt_blk, acpi_fadt.pm1_cnt_len,
+		"ACPI PM1a_CNT_BLK");
+
+	acpi_request_region(&acpi_fadt.xpm1b_cnt_blk, acpi_fadt.pm1_cnt_len,
+		"ACPI PM1b_CNT_BLK");
+
+	if (acpi_fadt.pm_tm_len == 4)
+		acpi_request_region(&acpi_fadt.xpm_tmr_blk, 4, "ACPI PM_TMR");
+
+	acpi_request_region(&acpi_fadt.xpm2_cnt_blk, acpi_fadt.pm2_cnt_len,
+		"ACPI PM2_CNT_BLK");
+
+	/* Length of GPE blocks must be a non-negative multiple of 2 */
+
+	if (!(acpi_fadt.gpe0_blk_len & 0x1))
+		acpi_request_region(&acpi_fadt.xgpe0_blk,
+			       acpi_fadt.gpe0_blk_len, "ACPI GPE0_BLK");
+
+	if (!(acpi_fadt.gpe1_blk_len & 0x1))
+		acpi_request_region(&acpi_fadt.xgpe1_blk,
+			       acpi_fadt.gpe1_blk_len, "ACPI GPE1_BLK");
+
+	return 0;
+}
+device_initcall(acpi_reserve_resources);
+
 acpi_status acpi_os_initialize(void)
 {
 	return AE_OK;

[patch 2/5] PNP: reserve system board iomem resources as well as ioport resources

[Bjorn Helgaas]

Most x86 boxes have no iomem system board resources, but some ia64
boxes do.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: mm-work3/drivers/pnp/system.c
===================================================================
--- mm-work3.orig/drivers/pnp/system.c	2007-01-11 15:56:57.000000000 -0700
+++ mm-work3/drivers/pnp/system.c	2007-01-12 12:14:53.000000000 -0700
@@ -3,7 +3,8 @@
  *
  * Some code is based on pnpbios_core.c
  * Copyright 2002 Adam Belay <ambx1@neo.rr.com>
- *
+ * (c) Copyright 2007 Hewlett-Packard Development Company, L.P.
+ *	Bjorn Helgaas <bjorn.helgaas@hp.com>
  */
 
 #include <linux/pnp.h>
@@ -21,7 +22,7 @@
 	{	"",			0	}
 };
 
-static void reserve_ioport_range(char *pnpid, int start, int end)
+static void reserve_range(char *pnpid, int start, int end, int port)
 {
 	struct resource *res;
 	char *regionid;
@@ -30,7 +31,10 @@
 	if ( regionid == NULL )
 		return;
 	snprintf(regionid, 16, "pnp %s", pnpid);
-	res = request_region(start,end-start+1,regionid);
+	if (port)
+		res = request_region(start,end-start+1,regionid);
+	else
+		res = request_mem_region(start,end-start+1,regionid);
 	if ( res == NULL )
 		kfree( regionid );
 	else
@@ -41,8 +45,8 @@
 	 * have double reservations.
 	 */
 	printk(KERN_INFO
-		"pnp: %s: ioport range 0x%x-0x%x %s reserved\n",
-		pnpid, start, end,
+		"pnp: %s: %s range 0x%x-0x%x %s reserved\n",
+		pnpid, port ? "ioport" : "iomem", start, end,
 		NULL != res ? "has been" : "could not be"
 	);
 
@@ -75,13 +79,21 @@
 			/* invalid endpoint */
 			/* Do nothing */
 			continue;
-		reserve_ioport_range(
+		reserve_range(
 			dev->dev.bus_id,
 			pnp_port_start(dev, i),
-			pnp_port_end(dev, i)
+			pnp_port_end(dev, i), 1
 		);
 	}
 
+	for (i = 0; i < PNP_MAX_MEM; i++) {
+		if (!pnp_mem_valid(dev, i))
+			continue;
+
+		reserve_range(dev->dev.bus_id, pnp_mem_start(dev, i),
+			pnp_mem_end(dev, i), 0);
+	}
+
 	return;
 }
 

[patch 3/5] PNP: system.c whitespace cleanup

[Bjorn Helgaas]

No functional change.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: mm-work3/drivers/pnp/system.c
===================================================================
--- mm-work3.orig/drivers/pnp/system.c	2007-01-12 12:14:53.000000000 -0700
+++ mm-work3/drivers/pnp/system.c	2007-01-12 12:16:59.000000000 -0700
@@ -28,15 +28,15 @@
 	char *regionid;
 
 	regionid = kmalloc(16, GFP_KERNEL);
-	if ( regionid == NULL )
+	if (regionid == NULL)
 		return;
 	snprintf(regionid, 16, "pnp %s", pnpid);
 	if (port)
 		res = request_region(start,end-start+1,regionid);
 	else
 		res = request_mem_region(start,end-start+1,regionid);
-	if ( res == NULL )
-		kfree( regionid );
+	if (res == NULL)
+		kfree(regionid);
 	else
 		res->flags &= ~IORESOURCE_BUSY;
 	/*
@@ -47,24 +47,18 @@
 	printk(KERN_INFO
 		"pnp: %s: %s range 0x%x-0x%x %s reserved\n",
 		pnpid, port ? "ioport" : "iomem", start, end,
-		NULL != res ? "has been" : "could not be"
-	);
-
-	return;
+		NULL != res ? "has been" : "could not be");
 }
 
-static void reserve_resources_of_dev( struct pnp_dev *dev )
+static void reserve_resources_of_dev(struct pnp_dev *dev)
 {
 	int i;
 
-	for (i=0;i<PNP_MAX_PORT;i++) {
+	for (i = 0; i < PNP_MAX_PORT; i++) {
 		if (!pnp_port_valid(dev, i))
-			/* end of resources */
 			continue;
 		if (pnp_port_start(dev, i) == 0)
-			/* disabled */
-			/* Do nothing */
-			continue;
+			continue;	/* disabled */
 		if (pnp_port_start(dev, i) < 0x100)
 			/*
 			 * Below 0x100 is only standard PC hardware
@@ -76,14 +70,10 @@
 			 */
 			continue;
 		if (pnp_port_end(dev, i) < pnp_port_start(dev, i))
-			/* invalid endpoint */
-			/* Do nothing */
-			continue;
-		reserve_range(
-			dev->dev.bus_id,
-			pnp_port_start(dev, i),
-			pnp_port_end(dev, i), 1
-		);
+			continue;	/* invalid */
+
+		reserve_range(dev->dev.bus_id, pnp_port_start(dev, i),
+			pnp_port_end(dev, i), 1);
 	}
 
 	for (i = 0; i < PNP_MAX_MEM; i++) {

[patch 4/5] i386: turn on CONFIG_PNP in defconfig

[Bjorn Helgaas]

I'm trying to remove drivers/acpi/motherboard.c, which is mostly
redundant with drivers/pnp/system.c.  So make sure that we include the
PNP driver in the default config.  Most distros enable this already.

Turning on CONFIG_PNP also causes the following options to be enabled:

    CONFIG_PNPACPI
    CONFIG_SERIAL_8250_PNP

CONFIG_SERIAL_8250_PNP causes legacy serial ports to be discovered
twice, which is ugly but harmless:

    serial8250: ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A
    00:07: ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: mm-work3/arch/i386/defconfig
===================================================================
--- mm-work3.orig/arch/i386/defconfig	2007-01-18 15:27:53.000000000 -0700
+++ mm-work3/arch/i386/defconfig	2007-01-18 15:36:49.000000000 -0700
@@ -466,7 +466,8 @@
 #
 # Plug and Play support
 #
-# CONFIG_PNP is not set
+CONFIG_PNP=y
+CONFIG_PNPACPI=y
 
 #
 # Block devices

[patch 5/5] ACPI: remove motherboard driver (redundant with PNP system driver)

[Bjorn Helgaas]

The PNP system board driver (drivers/pnp/system.c) contains all the
same functionality, so we don't need the ACPI version.

Previously, a motherboard device would be claimed by *both* the ACPI and
PNP drivers, resulting in stuff like this in /proc/ioports:

    1200-121f : motherboard	<-- from drivers/acpi/motherboard.c
      1200-121f : pnp 00:0d	<-- from drivers/pnp/system.c

Make sure to enable CONFIG_PNP (and CONFIG_PNPACPI) to include the
PNP system board driver.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: mm-work3/drivers/acpi/Makefile
===================================================================
--- mm-work3.orig/drivers/acpi/Makefile	2007-01-18 16:15:00.000000000 -0700
+++ mm-work3/drivers/acpi/Makefile	2007-01-18 16:15:29.000000000 -0700
@@ -37,7 +37,7 @@
 
 obj-y				+= sleep/
 obj-y				+= bus.o glue.o
-obj-y				+= scan.o motherboard.o
+obj-y				+= scan.o
 obj-$(CONFIG_ACPI_AC) 		+= ac.o
 obj-$(CONFIG_ACPI_BATTERY)	+= battery.o
 obj-$(CONFIG_ACPI_BUTTON)	+= button.o
Index: mm-work3/drivers/acpi/motherboard.c
===================================================================
--- mm-work3.orig/drivers/acpi/motherboard.c	2007-01-18 16:15:00.000000000 -0700
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,131 +0,0 @@
-/* 
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or (at
- *  your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful, but
- *  WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- *  General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License along
- *  with this program; if not, write to the Free Software Foundation, Inc.,
- *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
- *
- * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- */
-
-/* Purpose: Prevent PCMCIA cards from using motherboard resources. */
-
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/types.h>
-#include <linux/pci.h>
-#include <linux/ioport.h>
-#include <asm/io.h>
-
-#include <acpi/acpi_bus.h>
-#include <acpi/acpi_drivers.h>
-
-#define _COMPONENT		ACPI_SYSTEM_COMPONENT
-ACPI_MODULE_NAME("acpi_motherboard")
-
-/* Dell use PNP0C01 instead of PNP0C02 */
-#define ACPI_MB_HID			"PNP0C01,PNP0C02"
-/**
- * Doesn't care about legacy IO ports, only IO ports beyond 0x1000 are reserved
- * Doesn't care about the failure of 'request_region', since other may reserve
- * the io ports as well
- */
-#define IS_RESERVED_ADDR(base, len) \
-	(((len) > 0) && ((base) > 0) && ((base) + (len) < IO_SPACE_LIMIT) \
-	&& ((base) + (len) > PCIBIOS_MIN_IO))
-/*
- * Clearing the flag (IORESOURCE_BUSY) allows drivers to use
- * the io ports if they really know they can use it, while
- * still preventing hotplug PCI devices from using it.
- */
-
-/*
- * When CONFIG_PNP is enabled, pnp/system.c binds to PNP0C01
- * and PNP0C02, redundant with acpi_reserve_io_ranges().
- * But acpi_reserve_io_ranges() is necessary for !CONFIG_PNP.
- */
-static acpi_status acpi_reserve_io_ranges(struct acpi_resource *res, void *data)
-{
-	struct resource *requested_res = NULL;
-
-
-	if (res->type == ACPI_RESOURCE_TYPE_IO) {
-		struct acpi_resource_io *io_res = &res->data.io;
-
-		if (io_res->minimum != io_res->maximum)
-			return AE_OK;
-		if (IS_RESERVED_ADDR
-		    (io_res->minimum, io_res->address_length)) {
-			ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-					  "Motherboard resources 0x%08x - 0x%08x\n",
-					  io_res->minimum,
-					  io_res->minimum +
-					  io_res->address_length));
-			requested_res =
-			    request_region(io_res->minimum,
-					   io_res->address_length, "motherboard");
-		}
-	} else if (res->type == ACPI_RESOURCE_TYPE_FIXED_IO) {
-		struct acpi_resource_fixed_io *fixed_io_res =
-		    &res->data.fixed_io;
-
-		if (IS_RESERVED_ADDR
-		    (fixed_io_res->address, fixed_io_res->address_length)) {
-			ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-					  "Motherboard resources 0x%08x - 0x%08x\n",
-					  fixed_io_res->address,
-					  fixed_io_res->address +
-					  fixed_io_res->address_length));
-			requested_res =
-			    request_region(fixed_io_res->address,
-					   fixed_io_res->address_length,
-					   "motherboard");
-		}
-	} else {
-		/* Memory mapped IO? */
-	}
-
-	if (requested_res)
-		requested_res->flags &= ~IORESOURCE_BUSY;
-	return AE_OK;
-}
-
-static int acpi_motherboard_add(struct acpi_device *device)
-{
-	if (!device)
-		return -EINVAL;
-	acpi_walk_resources(device->handle, METHOD_NAME__CRS,
-			    acpi_reserve_io_ranges, NULL);
-
-	return 0;
-}
-
-static struct acpi_driver acpi_motherboard_driver = {
-	.name = "motherboard",
-	.class = "",
-	.ids = ACPI_MB_HID,
-	.ops = {
-		.add = acpi_motherboard_add,
-		},
-};
-
-static int __init acpi_motherboard_init(void)
-{
-	acpi_bus_register_driver(&acpi_motherboard_driver);
-	return 0;
-}
-
-/**
- * Reserve motherboard resources after PCI claim BARs,
- * but before PCI assign resources for uninitialized PCI devices
- */
-fs_initcall(acpi_motherboard_init);

Re: [patch 1/5] ACPI: move FADT resource reservations from motherboard driver to osl

[Shaohua Li]

On Thu, 2007-01-18 at 16:42 -0700, Bjorn Helgaas wrote:
> Resources described by the FADT aren't really a good fit for the
> ACPI motherboard driver.
> 
> The motherboard driver cares about PNP0C01 and PNP0C02 devices and
> their resources.
> 
> The FADT describes some resources used by the ACPI core.  Often, they
> are also described by by the _CRS of a motherboard device, but I think
> it's better to reserve them specifically in the ACPI osl.c because
> (a) the motherboard driver is optional and ACPI uses the resources even
> if the driver is absent, and (b) I want to remove the ACPI motherboard
> driver because it's mostly redundant with the PNP system.c driver.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> 
> Index: mm-work3/drivers/acpi/motherboard.c
> ===================================================================
> --- mm-work3.orig/drivers/acpi/motherboard.c	2007-01-14 14:52:30.000000000 -0700
> +++ mm-work3/drivers/acpi/motherboard.c	2007-01-18 10:21:13.000000000 -0700
> @@ -118,58 +118,9 @@
>  		},
>  };
>  
> -static void __init acpi_request_region (struct acpi_generic_address *addr,
> -	unsigned int length, char *desc)
> -{
> -	if (!addr->address || !length)
> -		return;
> -
> -	if (addr->address_space_id == ACPI_ADR_SPACE_SYSTEM_IO)
> -		request_region(addr->address, length, desc);
> -	else if (addr->address_space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)
> -		request_mem_region(addr->address, length, desc);
> -}
> -
> -static void __init acpi_reserve_resources(void)
> -{
> -	acpi_request_region(&acpi_gbl_FADT->xpm1a_evt_blk,
> -			       acpi_gbl_FADT->pm1_evt_len, "ACPI PM1a_EVT_BLK");
> -
> -	acpi_request_region(&acpi_gbl_FADT->xpm1b_evt_blk,
> -			       acpi_gbl_FADT->pm1_evt_len, "ACPI PM1b_EVT_BLK");
> -
> -	acpi_request_region(&acpi_gbl_FADT->xpm1a_cnt_blk,
> -			       acpi_gbl_FADT->pm1_cnt_len, "ACPI PM1a_CNT_BLK");
> -
> -	acpi_request_region(&acpi_gbl_FADT->xpm1b_cnt_blk,
> -			       acpi_gbl_FADT->pm1_cnt_len, "ACPI PM1b_CNT_BLK");
> -
> -	if (acpi_gbl_FADT->pm_tm_len == 4)
> -		acpi_request_region(&acpi_gbl_FADT->xpm_tmr_blk, 4, "ACPI PM_TMR");
> -
> -	acpi_request_region(&acpi_gbl_FADT->xpm2_cnt_blk,
> -			       acpi_gbl_FADT->pm2_cnt_len, "ACPI PM2_CNT_BLK");
> -
> -	/* Length of GPE blocks must be a non-negative multiple of 2 */
> -
> -	if (!(acpi_gbl_FADT->gpe0_blk_len & 0x1))
> -		acpi_request_region(&acpi_gbl_FADT->xgpe0_blk,
> -			       acpi_gbl_FADT->gpe0_blk_len, "ACPI GPE0_BLK");
> -
> -	if (!(acpi_gbl_FADT->gpe1_blk_len & 0x1))
> -		acpi_request_region(&acpi_gbl_FADT->xgpe1_blk,
> -			       acpi_gbl_FADT->gpe1_blk_len, "ACPI GPE1_BLK");
> -}
> -
>  static int __init acpi_motherboard_init(void)
>  {
>  	acpi_bus_register_driver(&acpi_motherboard_driver);
> -	/*
> -	 * Guarantee motherboard IO reservation first
> -	 * This module must run after scan.c
> -	 */
> -	if (!acpi_disabled)
> -		acpi_reserve_resources();
>  	return 0;
>  }
>  
> Index: mm-work3/drivers/acpi/osl.c
> ===================================================================
> --- mm-work3.orig/drivers/acpi/osl.c	2007-01-14 14:52:30.000000000 -0700
> +++ mm-work3/drivers/acpi/osl.c	2007-01-18 14:39:47.000000000 -0700
> @@ -75,6 +75,54 @@
>  static void *acpi_irq_context;
>  static struct workqueue_struct *kacpid_wq;
>  
> +static void __init acpi_request_region (struct acpi_generic_address *addr,
> +	unsigned int length, char *desc)
> +{
> +	struct resource *res;
> +
> +	if (!addr->address || !length)
> +		return;
> +
> +	if (addr->address_space_id == ACPI_ADR_SPACE_SYSTEM_IO)
> +		res = request_region(addr->address, length, desc);
> +	else if (addr->address_space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)
> +		res = request_mem_region(addr->address, length, desc);
> +}
> +
> +static int __init acpi_reserve_resources(void)
> +{
> +	acpi_request_region(&acpi_fadt.xpm1a_evt_blk, acpi_fadt.pm1_evt_len,
> +		"ACPI PM1a_EVT_BLK");
> +
> +	acpi_request_region(&acpi_fadt.xpm1b_evt_blk, acpi_fadt.pm1_evt_len,
> +		"ACPI PM1b_EVT_BLK");
> +
> +	acpi_request_region(&acpi_fadt.xpm1a_cnt_blk, acpi_fadt.pm1_cnt_len,
> +		"ACPI PM1a_CNT_BLK");
> +
> +	acpi_request_region(&acpi_fadt.xpm1b_cnt_blk, acpi_fadt.pm1_cnt_len,
> +		"ACPI PM1b_CNT_BLK");
> +
> +	if (acpi_fadt.pm_tm_len == 4)
> +		acpi_request_region(&acpi_fadt.xpm_tmr_blk, 4, "ACPI PM_TMR");
> +
> +	acpi_request_region(&acpi_fadt.xpm2_cnt_blk, acpi_fadt.pm2_cnt_len,
> +		"ACPI PM2_CNT_BLK");
> +
> +	/* Length of GPE blocks must be a non-negative multiple of 2 */
> +
> +	if (!(acpi_fadt.gpe0_blk_len & 0x1))
> +		acpi_request_region(&acpi_fadt.xgpe0_blk,
> +			       acpi_fadt.gpe0_blk_len, "ACPI GPE0_BLK");
> +
> +	if (!(acpi_fadt.gpe1_blk_len & 0x1))
> +		acpi_request_region(&acpi_fadt.xgpe1_blk,
> +			       acpi_fadt.gpe1_blk_len, "ACPI GPE1_BLK");
> +
> +	return 0;
> +}
> +device_initcall(acpi_reserve_resources);
Will this be called before PCI assigned resources to PCI devices? We use
fs_initcall in motherboard to avoid PCI devices use motherboard's
resources before.

Thanks,
Shaohua

Re: [0/5] remove ACPI motherboard driver, use PNP system driver instead (take 2)

[emisca]

I have posted some days ago, some problems on resume from suspend to
disk on my motherboard (asus p5ld2 se) related to acpi pnp. The serial
didn't worked after resume. Using pnpacpi=off it worked.
Now using this patch what will be the behaviour?

Bye

2007/1/19, Bjorn Helgaas <bjorn.helgaas@hp.com>:
> The ACPI motherboard driver is mostly redundant with the PNP system
> driver.  This series moves the little bit of ACPI-specific stuff out
> of the motherboard driver into acpi/osl.c, adds a little bit to the
> PNP system driver so it covers all the remaining functionality of
> the ACPI driver, and removes the ACPI driver.
>
> Thanks for the comments on the previous version.  This set of patches:
>
>   - Makes acpi_reserve_resources() a device_initcall(), so the FADT
>     resources will be reserved after the pnp/system driver claims its
>     resources.  This lets us reserve a motherboard region larger than
>     the FADT describes without conflicts.
>
>   - Adds CONFIG_PNPACPI=y to the i386/defconfig patch.  This is needed
>     for the pnp/system driver to find its devices.
>
> Len asked about the CONFIG_PNP=n, CONFIG_ACPI=y case.  In this case,
> we won't have a pnp/system driver to claim motherboard resources.  But
> we already have this situation with other devices.  My IR device
> responds to ioports 0x100-0x10f and my ECP parallel port responds to
> ioports 0x778-0x77a.  If those drivers aren't loaded (or if they
> aren't smart enough to claim all the resources), the resources just
> aren't claimed.
>
> So I think a better solution for this problem would be to make the
> PNP core reserve all the regions for every active device, similar to
> what PCI does.
>
> Bjorn
> -
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [patch 1/5] ACPI: move FADT resource reservations from motherboard driver to osl

[Bjorn Helgaas]

On Thursday 18 January 2007 18:12, Shaohua Li wrote:
> > +device_initcall(acpi_reserve_resources);
> Will this be called before PCI assigned resources to PCI devices? We use
> fs_initcall in motherboard to avoid PCI devices use motherboard's
> resources before.

I think we're OK.  Here's the current order:

  pnp_system_init            fs_initcall       drivers/pnp/system.c
  pcibios_assign_resources   fs_initcall       arch/i386/pci/i386.c
  acpi_reserve_resources     device_initcall   drivers/acpi/osl.c

So pnp_system_init() will reserve all the motherboard resources before
the PCI resources are assigned.  What do you think?

Bjorn

Re: [0/5] remove ACPI motherboard driver, use PNP system driver instead (take 2)

[Bjorn Helgaas]

On Friday 19 January 2007 08:37, emisca wrote:
> I have posted some days ago, some problems on resume from suspend to
> disk on my motherboard (asus p5ld2 se) related to acpi pnp. The serial
> didn't worked after resume. Using pnpacpi=off it worked.
> Now using this patch what will be the behaviour?

My patches shouldn't change the behavior you're seeing.

Since it seems to be related to PNPACPI, can you post the dmesgs with
and without pnpacpi=off to linux-acpi?  I see the ones on LKML, but
it sounds like there was some user error in collecting them.

Bjorn

Re: [0/5] remove ACPI motherboard driver, use PNP system driver instead (take 2)

[emisca]

Could you tell me which informations I must collect and in which
conditions I must collect them so I could post them?
They replied me on the other post that I must manually debug the
pnpacpi code.......

2007/1/19, Bjorn Helgaas <bjorn.helgaas@hp.com>:
> On Friday 19 January 2007 08:37, emisca wrote:
> > I have posted some days ago, some problems on resume from suspend to
> > disk on my motherboard (asus p5ld2 se) related to acpi pnp. The serial
> > didn't worked after resume. Using pnpacpi=off it worked.
> > Now using this patch what will be the behaviour?
>
> My patches shouldn't change the behavior you're seeing.
>
> Since it seems to be related to PNPACPI, can you post the dmesgs with
> and without pnpacpi=off to linux-acpi?  I see the ones on LKML, but
> it sounds like there was some user error in collecting them.
>
> Bjorn
>
>

Re: [patch 1/5] ACPI: move FADT resource reservations from motherboard driver to osl

[Shaohua Li]

On Fri, 2007-01-19 at 12:46 -0700, Bjorn Helgaas wrote:
> On Thursday 18 January 2007 18:12, Shaohua Li wrote:
> > > +device_initcall(acpi_reserve_resources);
> > Will this be called before PCI assigned resources to PCI devices? We use
> > fs_initcall in motherboard to avoid PCI devices use motherboard's
> > resources before.
> 
> I think we're OK.  Here's the current order:
> 
>   pnp_system_init            fs_initcall       drivers/pnp/system.c
>   pcibios_assign_resources   fs_initcall       arch/i386/pci/i386.c
>   acpi_reserve_resources     device_initcall   drivers/acpi/osl.c
> 
> So pnp_system_init() will reserve all the motherboard resources before
> the PCI resources are assigned.  What do you think?
In this way, we could just remove acpi_reserve_resources as pci already
assigns resources to pci devices. The concern is motherboard might not
list all acpi resources in its _CRS, but I don't know if there are such
BIOS. Microsoft seems to reserve motherboard resources in the same way.
Maybe we could just remove acpi_reserve_resources and give it a spin in
-mm to see if anybody complains.

Thanks,
Shaohua

Re: [patch 1/5] ACPI: move FADT resource reservations from motherboard driver to osl

[Bjorn Helgaas]

On Sunday 21 January 2007 18:27, Shaohua Li wrote:
> On Fri, 2007-01-19 at 12:46 -0700, Bjorn Helgaas wrote:
> > On Thursday 18 January 2007 18:12, Shaohua Li wrote:
> > > > +device_initcall(acpi_reserve_resources);
> > > Will this be called before PCI assigned resources to PCI devices? We use
> > > fs_initcall in motherboard to avoid PCI devices use motherboard's
> > > resources before.
> > 
> > I think we're OK.  Here's the current order:
> > 
> >   pnp_system_init            fs_initcall       drivers/pnp/system.c
> >   pcibios_assign_resources   fs_initcall       arch/i386/pci/i386.c
> >   acpi_reserve_resources     device_initcall   drivers/acpi/osl.c
> > 
> > So pnp_system_init() will reserve all the motherboard resources before
> > the PCI resources are assigned.  What do you think?
>
> In this way, we could just remove acpi_reserve_resources as pci already
> assigns resources to pci devices. The concern is motherboard might not
> list all acpi resources in its _CRS, but I don't know if there are such
> BIOS. Microsoft seems to reserve motherboard resources in the same way.
> Maybe we could just remove acpi_reserve_resources and give it a spin in
> -mm to see if anybody complains.

I agree that in theory we should be able to remove acpi_reserve_resources()
and just depend on pnp_system_init() reserving all the motherboard
resources.  But your concern is very valid: it looks like both HP DL360
(x86) and rx2600 (ia64) firmware list things in the FADT that aren't
included in _CRS for motherboard devices.  I think these are BIOS bugs,
but for now, it seems safest to keep acpi_reserve_resources() to work
around the bugs.

Bjorn

Re: [0/5] remove ACPI motherboard driver, use PNP system driver instead (take 2)

[Bjorn Helgaas]

On Saturday 20 January 2007 11:00, emisca wrote:
> Could you tell me which informations I must collect and in which
> conditions I must collect them so I could post them?
> They replied me on the other post that I must manually debug the
> pnpacpi code.......

Sure.  I'm here to help with the manual debug of pnpacpi :-)

If I understand correctly, you're seeing this:

  1a) Original boot (serial port works fine)
  1b) Suspend to disk
  1c) Resume (serial port doesn't work)

  2a) Original boot with pnpacpi=off (serial port works fine)
  2b) Suspend to disk
  2c) Resume (serial port works fine)

Can you please apply the patch below and collect this info:

  - Your .config file
  - dmesg logs (use "dmesg -s64000") from 1a, 1c, 2a, and 2c
  - Contents of /proc/interrupts from 1c and 2c

I'm pretty ignorant of suspend/resume, so I don't know what the
dmesg log of a resume looks like.  My dim understanding is that
resume starts with a normal boot, and then loads a suspended memory
image from disk.  I suppose the suspended memory image contains
a dmesg buffer from the original boot.  So I'm not sure what sort
of log you'll get from the 2c case.

Bjorn



Index: mm-work3/drivers/serial/8250.c
===================================================================
--- mm-work3.orig/drivers/serial/8250.c	2007-01-22 09:10:16.000000000 -0700
+++ mm-work3/drivers/serial/8250.c	2007-01-22 09:23:32.000000000 -0700
@@ -2511,6 +2511,7 @@
  */
 void serial8250_suspend_port(int line)
 {
+	printk("%s: line %d\n", __FUNCTION__, line);
 	uart_suspend_port(&serial8250_reg, &serial8250_ports[line].port);
 }
 
@@ -2522,6 +2523,7 @@
  */
 void serial8250_resume_port(int line)
 {
+	printk("%s: line %d\n", __FUNCTION__, line);
 	uart_resume_port(&serial8250_reg, &serial8250_ports[line].port);
 }
 
@@ -2584,8 +2586,10 @@
 	for (i = 0; i < UART_NR; i++) {
 		struct uart_8250_port *up = &serial8250_ports[i];
 
-		if (up->port.type != PORT_UNKNOWN && up->port.dev == &dev->dev)
+		if (up->port.type != PORT_UNKNOWN && up->port.dev == &dev->dev) {
+			printk("%s: line %d\n", __FUNCTION__, i);
 			uart_suspend_port(&serial8250_reg, &up->port);
+		}
 	}
 
 	return 0;
@@ -2598,8 +2602,10 @@
 	for (i = 0; i < UART_NR; i++) {
 		struct uart_8250_port *up = &serial8250_ports[i];
 
-		if (up->port.type != PORT_UNKNOWN && up->port.dev == &dev->dev)
+		if (up->port.type != PORT_UNKNOWN && up->port.dev == &dev->dev) {
+			printk("%s: line %d\n", __FUNCTION__, i);
 			uart_resume_port(&serial8250_reg, &up->port);
+		}
 	}
 
 	return 0;
Index: mm-work3/drivers/serial/8250_pnp.c
===================================================================
--- mm-work3.orig/drivers/serial/8250_pnp.c	2007-01-22 09:08:19.000000000 -0700
+++ mm-work3/drivers/serial/8250_pnp.c	2007-01-22 09:23:53.000000000 -0700
@@ -469,6 +469,7 @@
 {
 	long line = (long)pnp_get_drvdata(dev);
 
+	printk("%s: dev %s line %ld\n", __FUNCTION__, dev->name, line);
 	if (!line)
 		return -ENODEV;
 	serial8250_suspend_port(line - 1);
@@ -479,6 +480,7 @@
 {
 	long line = (long)pnp_get_drvdata(dev);
 
+	printk("%s: dev %s line %ld\n", __FUNCTION__, dev->name, line);
 	if (!line)
 		return -ENODEV;
 	serial8250_resume_port(line - 1);
Index: mm-work3/drivers/pnp/driver.c
===================================================================
--- mm-work3.orig/drivers/pnp/driver.c	2007-01-22 09:11:02.000000000 -0700
+++ mm-work3/drivers/pnp/driver.c	2007-01-22 09:17:47.000000000 -0700
@@ -155,6 +155,7 @@
 		return 0;
 
 	if (pnp_drv->suspend) {
+		printk("%s: %s\n", __FUNCTION__, pnp_dev->name);
 		error = pnp_drv->suspend(pnp_dev, state);
 		if (error)
 			return error;
@@ -185,8 +186,10 @@
 			return error;
 	}
 
-	if (pnp_drv->resume)
+	if (pnp_drv->resume) {
+		printk("%s: %s\n", __FUNCTION__, pnp_dev->name);
 		return pnp_drv->resume(pnp_dev);
+	}
 
 	return 0;
 }

Re: [0/5] remove ACPI motherboard driver, use PNP system driver instead (take 2)

[emisca]

What kernel version I should use? If I should use git, could you give
me the path of the branch you use?
I tried with vanilla 2.6.19.2 kernel, but obviously 8250_pnp.c hasn't
the suspend and resume functions......

Thanks and sorry for the delay..

2007/1/22, Bjorn Helgaas <bjorn.helgaas@hp.com>:
> On Saturday 20 January 2007 11:00, emisca wrote:
> > Could you tell me which informations I must collect and in which
> > conditions I must collect them so I could post them?
> > They replied me on the other post that I must manually debug the
> > pnpacpi code.......
>
> Sure.  I'm here to help with the manual debug of pnpacpi :-)
>
> If I understand correctly, you're seeing this:
>
>   1a) Original boot (serial port works fine)
>   1b) Suspend to disk
>   1c) Resume (serial port doesn't work)
>
>   2a) Original boot with pnpacpi=off (serial port works fine)
>   2b) Suspend to disk
>   2c) Resume (serial port works fine)
>
> Can you please apply the patch below and collect this info:
>
>   - Your .config file
>   - dmesg logs (use "dmesg -s64000") from 1a, 1c, 2a, and 2c
>   - Contents of /proc/interrupts from 1c and 2c
>
> I'm pretty ignorant of suspend/resume, so I don't know what the
> dmesg log of a resume looks like.  My dim understanding is that
> resume starts with a normal boot, and then loads a suspended memory
> image from disk.  I suppose the suspended memory image contains
> a dmesg buffer from the original boot.  So I'm not sure what sort
> of log you'll get from the 2c case.
>
> Bjorn
>
>
>
> Index: mm-work3/drivers/serial/8250.c
> ===================================================================
> --- mm-work3.orig/drivers/serial/8250.c 2007-01-22 09:10:16.000000000 -0700
> +++ mm-work3/drivers/serial/8250.c      2007-01-22 09:23:32.000000000 -0700
> @@ -2511,6 +2511,7 @@
>   */
>  void serial8250_suspend_port(int line)
>  {
> +       printk("%s: line %d\n", __FUNCTION__, line);
>         uart_suspend_port(&serial8250_reg, &serial8250_ports[line].port);
>  }
>
> @@ -2522,6 +2523,7 @@
>   */
>  void serial8250_resume_port(int line)
>  {
> +       printk("%s: line %d\n", __FUNCTION__, line);
>         uart_resume_port(&serial8250_reg, &serial8250_ports[line].port);
>  }
>
> @@ -2584,8 +2586,10 @@
>         for (i = 0; i < UART_NR; i++) {
>                 struct uart_8250_port *up = &serial8250_ports[i];
>
> -               if (up->port.type != PORT_UNKNOWN && up->port.dev == &dev->dev)
> +               if (up->port.type != PORT_UNKNOWN && up->port.dev == &dev->dev) {
> +                       printk("%s: line %d\n", __FUNCTION__, i);
>                         uart_suspend_port(&serial8250_reg, &up->port);
> +               }
>         }
>
>         return 0;
> @@ -2598,8 +2602,10 @@
>         for (i = 0; i < UART_NR; i++) {
>                 struct uart_8250_port *up = &serial8250_ports[i];
>
> -               if (up->port.type != PORT_UNKNOWN && up->port.dev == &dev->dev)
> +               if (up->port.type != PORT_UNKNOWN && up->port.dev == &dev->dev) {
> +                       printk("%s: line %d\n", __FUNCTION__, i);
>                         uart_resume_port(&serial8250_reg, &up->port);
> +               }
>         }
>
>         return 0;
> Index: mm-work3/drivers/serial/8250_pnp.c
> ===================================================================
> --- mm-work3.orig/drivers/serial/8250_pnp.c     2007-01-22 09:08:19.000000000 -0700
> +++ mm-work3/drivers/serial/8250_pnp.c  2007-01-22 09:23:53.000000000 -0700
> @@ -469,6 +469,7 @@
>  {
>         long line = (long)pnp_get_drvdata(dev);
>
> +       printk("%s: dev %s line %ld\n", __FUNCTION__, dev->name, line);
>         if (!line)
>                 return -ENODEV;
>         serial8250_suspend_port(line - 1);
> @@ -479,6 +480,7 @@
>  {
>         long line = (long)pnp_get_drvdata(dev);
>
> +       printk("%s: dev %s line %ld\n", __FUNCTION__, dev->name, line);
>         if (!line)
>                 return -ENODEV;
>         serial8250_resume_port(line - 1);
> Index: mm-work3/drivers/pnp/driver.c
> ===================================================================
> --- mm-work3.orig/drivers/pnp/driver.c  2007-01-22 09:11:02.000000000 -0700
> +++ mm-work3/drivers/pnp/driver.c       2007-01-22 09:17:47.000000000 -0700
> @@ -155,6 +155,7 @@
>                 return 0;
>
>         if (pnp_drv->suspend) {
> +               printk("%s: %s\n", __FUNCTION__, pnp_dev->name);
>                 error = pnp_drv->suspend(pnp_dev, state);
>                 if (error)
>                         return error;
> @@ -185,8 +186,10 @@
>                         return error;
>         }
>
> -       if (pnp_drv->resume)
> +       if (pnp_drv->resume) {
> +               printk("%s: %s\n", __FUNCTION__, pnp_dev->name);
>                 return pnp_drv->resume(pnp_dev);
> +       }
>
>         return 0;
>  }
>

Re: [0/5] remove ACPI motherboard driver, use PNP system driver instead (take 2)

[Bjorn Helgaas]

On Tuesday 23 January 2007 02:07, emisca wrote:
> What kernel version I should use? If I should use git, could you give
> me the path of the branch you use?
> I tried with vanilla 2.6.19.2 kernel, but obviously 8250_pnp.c hasn't
> the suspend and resume functions......

I made the patch from 2.6.20-rc3-mm1, and it should apply on any recent
mm kernel.  That'd be the easiest thing to debug, I think.

If that's not convenient for you, we can work on 2.6.19.2, and you can
just ignore the 8250_pnp patch (you can just remove that section from
the patch).

Re: [0/5] remove ACPI motherboard driver, use PNP system driver instead (take 2)

[emisca]

I'm going to compile a kernel only for these tests.. now I'm using
debian kernel 2.6.18.
So I'll compile a kernel 2.6.20-rc3-mm1 with your patch.
I'll post you the results....
Bye!

2007/1/23, Bjorn Helgaas <bjorn.helgaas@hp.com>:
> On Tuesday 23 January 2007 02:07, emisca wrote:
> > What kernel version I should use? If I should use git, could you give
> > me the path of the branch you use?
> > I tried with vanilla 2.6.19.2 kernel, but obviously 8250_pnp.c hasn't
> > the suspend and resume functions......
>
> I made the patch from 2.6.20-rc3-mm1, and it should apply on any recent
> mm kernel.  That'd be the easiest thing to debug, I think.
>
> If that's not convenient for you, we can work on 2.6.19.2, and you can
> just ignore the 8250_pnp patch (you can just remove that section from
> the patch).
>

Re: [0/5] remove ACPI motherboard driver, use PNP system driver instead (take 2)

[Bjorn Helgaas]

On Tuesday 23 January 2007 12:24, emisca wrote:
> I'm going to compile a kernel only for these tests.. now I'm using
> debian kernel 2.6.18.
> So I'll compile a kernel 2.6.20-rc3-mm1 with your patch.
> I'll post you the results....
> Bye!

Ping!  Any luck with this?

> 2007/1/23, Bjorn Helgaas <bjorn.helgaas@hp.com>:
> > On Tuesday 23 January 2007 02:07, emisca wrote:
> > > What kernel version I should use? If I should use git, could you give
> > > me the path of the branch you use?
> > > I tried with vanilla 2.6.19.2 kernel, but obviously 8250_pnp.c hasn't
> > > the suspend and resume functions......
> >
> > I made the patch from 2.6.20-rc3-mm1, and it should apply on any recent
> > mm kernel.  That'd be the easiest thing to debug, I think.
> >
> > If that's not convenient for you, we can work on 2.6.19.2, and you can
> > just ignore the 8250_pnp patch (you can just remove that section from
> > the patch).

Re: [0/5] remove ACPI motherboard driver, use PNP system driver instead (take 2)

[Bjorn Helgaas]

Shaohua, Len, others,

Did I resolve all the concerns you had about these patches?  Should
I re-post them?

Thanks,
  Bjorn

Re: [0/5] remove ACPI motherboard driver, use PNP system driver instead (take 2)

[Len Brown]

On Monday 29 January 2007 18:00, Bjorn Helgaas wrote:
> Shaohua, Len, others,
> 
> Did I resolve all the concerns you had about these patches?  Should
> I re-post them?

Bjorn,
I didn't see any problems with them.

On 1/24 I checked in your series posted on 1/18, and they've been in acpi-test since.
Sorry if I neglected to Ack them then -- sometimes I get interrupted and drop an Ack.
So unless you have an update since 1/18, then we're good.

Note that andrew ran into merge conflicts with the latest acpi-test and dropped back to a version
of git-acpi.pach just before this - which is why your patch series isn't in 2.6.20-rc6-mm2 via acpi-test.
Hopefully we'll get past that merge conflict issue soon.

thanks,
-Len

Re: [0/5] remove ACPI motherboard driver, use PNP system driver instead (take 2)

[Bjorn Helgaas]

On Monday 29 January 2007 16:19, Len Brown wrote:
> On Monday 29 January 2007 18:00, Bjorn Helgaas wrote:
> > Shaohua, Len, others,
> > 
> > Did I resolve all the concerns you had about these patches?  Should
> > I re-post them?
> 
> Bjorn,
> I didn't see any problems with them.
> 
> On 1/24 I checked in your series posted on 1/18, and they've been in acpi-test since.
> Sorry if I neglected to Ack them then -- sometimes I get interrupted and drop an Ack.
> So unless you have an update since 1/18, then we're good.

Nope, no updates since then.  Thanks!

Re: [0/5] remove ACPI motherboard driver, use PNP system driver instead (take 2)

[emisca]

Sorry, I had no free time to try them on my motherboard.............. :-(
I hope to try them tomorrow evening.

2007/1/30, Bjorn Helgaas <bjorn.helgaas@hp.com>:
> On Monday 29 January 2007 16:19, Len Brown wrote:
> > On Monday 29 January 2007 18:00, Bjorn Helgaas wrote:
> > > Shaohua, Len, others,
> > >
> > > Did I resolve all the concerns you had about these patches?  Should
> > > I re-post them?
> >
> > Bjorn,
> > I didn't see any problems with them.
> >
> > On 1/24 I checked in your series posted on 1/18, and they've been in acpi-test since.
> > Sorry if I neglected to Ack them then -- sometimes I get interrupted and drop an Ack.
> > So unless you have an update since 1/18, then we're good.
>
> Nope, no updates since then.  Thanks!
> -
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [0/5] remove ACPI motherboard driver, use PNP system driver instead (take 2)

[emisca]

[-- Attachment #1: Type: text/plain, Size: 1468 bytes --]

I've finally found some time to do that test. I compiled kernel
2.6.20-rc6-mm3, and patched those files with your patch.
Here are the
I've finally found some time to do that test. I compiled kernel
2.6.20-rc6-mm3, and patched those files with your patch.
Here are the dmesg, pre and post suspend. My serial continues to not
work after resume, without using pnpacpi=off.

I hope you'll find something useful on that dmesg dumps..

2007/1/30, emisca <emisca.ml@gmail.com>:
> Sorry, I had no free time to try them on my motherboard.............. :-(
> I hope to try them tomorrow evening.
>
> 2007/1/30, Bjorn Helgaas <bjorn.helgaas@hp.com>:
> > On Monday 29 January 2007 16:19, Len Brown wrote:
> > > On Monday 29 January 2007 18:00, Bjorn Helgaas wrote:
> > > > Shaohua, Len, others,
> > > >
> > > > Did I resolve all the concerns you had about these patches?  Should
> > > > I re-post them?
> > >
> > > Bjorn,
> > > I didn't see any problems with them.
> > >
> > > On 1/24 I checked in your series posted on 1/18, and they've been in acpi-test since.
> > > Sorry if I neglected to Ack them then -- sometimes I get interrupted and drop an Ack.
> > > So unless you have an update since 1/18, then we're good.
> >
> > Nope, no updates since then.  Thanks!
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
>

[-- Attachment #2: bug_pnp_acpi.tar.bz2 --]
[-- Type: application/x-bzip2, Size: 10041 bytes --]

Re: [0/5] remove ACPI motherboard driver, use PNP system driver instead (take 2)

[emisca]

I saw this error message using pnpacpi=off, on kernel boot, before resume:
PnPBIOS: Resource structure does not contain an end tag.

What does it means?

2007/2/3, emisca <emisca.ml@gmail.com>:
> I've finally found some time to do that test. I compiled kernel
> 2.6.20-rc6-mm3, and patched those files with your patch.
> Here are the
> I've finally found some time to do that test. I compiled kernel
> 2.6.20-rc6-mm3, and patched those files with your patch.
> Here are the dmesg, pre and post suspend. My serial continues to not
> work after resume, without using pnpacpi=off.
>
> I hope you'll find something useful on that dmesg dumps..
>
> 2007/1/30, emisca <emisca.ml@gmail.com>:
> > Sorry, I had no free time to try them on my motherboard.............. :-(
> > I hope to try them tomorrow evening.
> >
> > 2007/1/30, Bjorn Helgaas <bjorn.helgaas@hp.com>:
> > > On Monday 29 January 2007 16:19, Len Brown wrote:
> > > > On Monday 29 January 2007 18:00, Bjorn Helgaas wrote:
> > > > > Shaohua, Len, others,
> > > > >
> > > > > Did I resolve all the concerns you had about these patches?  Should
> > > > > I re-post them?
> > > >
> > > > Bjorn,
> > > > I didn't see any problems with them.
> > > >
> > > > On 1/24 I checked in your series posted on 1/18, and they've been in acpi-test since.
> > > > Sorry if I neglected to Ack them then -- sometimes I get interrupted and drop an Ack.
> > > > So unless you have an update since 1/18, then we're good.
> > >
> > > Nope, no updates since then.  Thanks!
> > > -
> > > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > >
> >
>
>

Re: [0/5] remove ACPI motherboard driver, use PNP system driver instead (take 2)

[Bjorn Helgaas]

On Friday 09 February 2007 12:15, emisca wrote:
> I saw this error message using pnpacpi=off, on kernel boot, before resume:
> PnPBIOS: Resource structure does not contain an end tag.
> 
> What does it means?

I think this indicates a BIOS defect.  PNPBIOS gives us a list
of resources a device is using.  The list contains things like
the memory and I/O port ranges claimed by a device, the 
interrupt it uses, etc.  There's supposed to be an "end"
tag at the end of the list.  Apparently your BIOS didn't
supply the "end" tag.

This should be harmless, as long as the resources that *are*
in the list are correct.  The message is currently KERN_ERR;
maybe it would make more sense to be KERN_WARN.

Bjorn

P.S.  I promise to look at the logs you posted soon :-)

Re: [patch 1/5] ACPI: move FADT resource reservations from motherboard driver to osl

[Bjorn Helgaas]

On Tuesday 16 January 2007 18:59, Shaohua Li wrote:
> >  acpi_status acpi_os_initialize1(void)
> >  {
> > +	acpi_reserve_resources();
> This sounds a little earlier. motherboard device usually returns a super
> set of the acpi io ports. reserving acpi io ports first might make some
> motherboard io port isn't reserved as there might be conflict.

That's true; this does reserve the FADT resources earlier, and there
are often conflicts.

I'll try to figure out a way to reserve the FADT resources later.

Re: [patch 1/5] ACPI: move FADT resource reservations from motherboard driver to osl

[Shaohua Li]

On Tue, 2007-01-16 at 10:33 -0700, Bjorn Helgaas wrote:
> Resources described by the FADT aren't really a good fit for the
> ACPI motherboard driver.
> 
> The motherboard driver cares about PNP0C01 and PNP0C02 devices and
> their resources.
> 
> The FADT describes some resources used by the ACPI core.  Often, they
> are also described by by the _CRS of a motherboard device, but I think
> it's better to reserve them specifically in the ACPI osl.c because
> (a) the motherboard driver is optional and ACPI uses the resources even
> if the driver is absent, and (b) I want to remove the ACPI motherboard
> driver because it's mostly redundant with the PNP system.c driver.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Thanks for the patch set, this is in the wish list for a long time.

> +
>  acpi_status acpi_os_initialize(void)
>  {
>  	return AE_OK;
> @@ -82,6 +127,8 @@
>  
>  acpi_status acpi_os_initialize1(void)
>  {
> +	acpi_reserve_resources();
This sounds a little earlier. motherboard device usually returns a super
set of the acpi io ports. reserving acpi io ports first might make some
motherboard io port isn't reserved as there might be conflict.

Thanks,
Shaohua

[patch 1/5] ACPI: move FADT resource reservations from motherboard driver to osl

[Bjorn Helgaas]

Resources described by the FADT aren't really a good fit for the
ACPI motherboard driver.

The motherboard driver cares about PNP0C01 and PNP0C02 devices and
their resources.

The FADT describes some resources used by the ACPI core.  Often, they
are also described by by the _CRS of a motherboard device, but I think
it's better to reserve them specifically in the ACPI osl.c because
(a) the motherboard driver is optional and ACPI uses the resources even
if the driver is absent, and (b) I want to remove the ACPI motherboard
driver because it's mostly redundant with the PNP system.c driver.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: mm-work13/drivers/acpi/motherboard.c
===================================================================
--- mm-work13.orig/drivers/acpi/motherboard.c	2007-01-11 12:34:57.000000000 -0700
+++ mm-work13/drivers/acpi/motherboard.c	2007-01-11 12:35:28.000000000 -0700
@@ -118,58 +118,9 @@
 		},
 };
 
-static void __init acpi_request_region (struct acpi_generic_address *addr,
-	unsigned int length, char *desc)
-{
-	if (!addr->address || !length)
-		return;
-
-	if (addr->address_space_id == ACPI_ADR_SPACE_SYSTEM_IO)
-		request_region(addr->address, length, desc);
-	else if (addr->address_space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)
-		request_mem_region(addr->address, length, desc);
-}
-
-static void __init acpi_reserve_resources(void)
-{
-	acpi_request_region(&acpi_gbl_FADT->xpm1a_evt_blk,
-			       acpi_gbl_FADT->pm1_evt_len, "ACPI PM1a_EVT_BLK");
-
-	acpi_request_region(&acpi_gbl_FADT->xpm1b_evt_blk,
-			       acpi_gbl_FADT->pm1_evt_len, "ACPI PM1b_EVT_BLK");
-
-	acpi_request_region(&acpi_gbl_FADT->xpm1a_cnt_blk,
-			       acpi_gbl_FADT->pm1_cnt_len, "ACPI PM1a_CNT_BLK");
-
-	acpi_request_region(&acpi_gbl_FADT->xpm1b_cnt_blk,
-			       acpi_gbl_FADT->pm1_cnt_len, "ACPI PM1b_CNT_BLK");
-
-	if (acpi_gbl_FADT->pm_tm_len == 4)
-		acpi_request_region(&acpi_gbl_FADT->xpm_tmr_blk, 4, "ACPI PM_TMR");
-
-	acpi_request_region(&acpi_gbl_FADT->xpm2_cnt_blk,
-			       acpi_gbl_FADT->pm2_cnt_len, "ACPI PM2_CNT_BLK");
-
-	/* Length of GPE blocks must be a non-negative multiple of 2 */
-
-	if (!(acpi_gbl_FADT->gpe0_blk_len & 0x1))
-		acpi_request_region(&acpi_gbl_FADT->xgpe0_blk,
-			       acpi_gbl_FADT->gpe0_blk_len, "ACPI GPE0_BLK");
-
-	if (!(acpi_gbl_FADT->gpe1_blk_len & 0x1))
-		acpi_request_region(&acpi_gbl_FADT->xgpe1_blk,
-			       acpi_gbl_FADT->gpe1_blk_len, "ACPI GPE1_BLK");
-}
-
 static int __init acpi_motherboard_init(void)
 {
 	acpi_bus_register_driver(&acpi_motherboard_driver);
-	/*
-	 * Guarantee motherboard IO reservation first
-	 * This module must run after scan.c
-	 */
-	if (!acpi_disabled)
-		acpi_reserve_resources();
 	return 0;
 }
 
Index: mm-work13/drivers/acpi/osl.c
===================================================================
--- mm-work13.orig/drivers/acpi/osl.c	2007-01-11 12:34:57.000000000 -0700
+++ mm-work13/drivers/acpi/osl.c	2007-01-11 12:37:10.000000000 -0700
@@ -75,6 +75,51 @@
 static void *acpi_irq_context;
 static struct workqueue_struct *kacpid_wq;
 
+static void __init acpi_request_region (struct acpi_generic_address *addr,
+	unsigned int length, char *desc)
+{
+	struct resource *res;
+
+	if (!addr->address || !length)
+		return;
+
+	if (addr->address_space_id == ACPI_ADR_SPACE_SYSTEM_IO)
+		res = request_region(addr->address, length, desc);
+	else if (addr->address_space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)
+		res = request_mem_region(addr->address, length, desc);
+}
+
+static void __init acpi_reserve_resources(void)
+{
+	acpi_request_region(&acpi_fadt.xpm1a_evt_blk, acpi_fadt.pm1_evt_len,
+		"ACPI PM1a_EVT_BLK");
+
+	acpi_request_region(&acpi_fadt.xpm1b_evt_blk, acpi_fadt.pm1_evt_len,
+		"ACPI PM1b_EVT_BLK");
+
+	acpi_request_region(&acpi_fadt.xpm1a_cnt_blk, acpi_fadt.pm1_cnt_len,
+		"ACPI PM1a_CNT_BLK");
+
+	acpi_request_region(&acpi_fadt.xpm1b_cnt_blk, acpi_fadt.pm1_cnt_len,
+		"ACPI PM1b_CNT_BLK");
+
+	if (acpi_fadt.pm_tm_len == 4)
+		acpi_request_region(&acpi_fadt.xpm_tmr_blk, 4, "ACPI PM_TMR");
+
+	acpi_request_region(&acpi_fadt.xpm2_cnt_blk, acpi_fadt.pm2_cnt_len,
+		"ACPI PM2_CNT_BLK");
+
+	/* Length of GPE blocks must be a non-negative multiple of 2 */
+
+	if (!(acpi_fadt.gpe0_blk_len & 0x1))
+		acpi_request_region(&acpi_fadt.xgpe0_blk,
+			       acpi_fadt.gpe0_blk_len, "ACPI GPE0_BLK");
+
+	if (!(acpi_fadt.gpe1_blk_len & 0x1))
+		acpi_request_region(&acpi_fadt.xgpe1_blk,
+			       acpi_fadt.gpe1_blk_len, "ACPI GPE1_BLK");
+}
+
 acpi_status acpi_os_initialize(void)
 {
 	return AE_OK;
@@ -82,6 +127,8 @@
 
 acpi_status acpi_os_initialize1(void)
 {
+	acpi_reserve_resources();
+
 	/*
 	 * Initialize PCI configuration space access, as we'll need to access
 	 * it while walking the namespace (bus 0 and root bridges w/ _BBNs).