[V4,07/11] ACPI: use platform bus as the default bus for _HID enumeration
diff mbox series

Message ID 1395042580-3344-8-git-send-email-rui.zhang@intel.com
State New, archived
Headers show
Series
  • ACPI: change the way of enumerating PNPACPI/Platform devices
Related show

Commit Message

Zhang, Rui March 17, 2014, 7:49 a.m. UTC
Because of the growing demand for enumerating ACPI devices to platform bus,
this patch changes the code to enumerate ACPI devices with _HID to
platform bus by default, unless the device already has a scan handler attached.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/acpi/acpi_platform.c |   28 ----------------------------
 drivers/acpi/scan.c          |   12 ++++++------
 2 files changed, 6 insertions(+), 34 deletions(-)

Comments

Rafael J. Wysocki March 24, 2014, 2 a.m. UTC | #1
On Monday, March 17, 2014 03:49:36 PM Zhang Rui wrote:
> Because of the growing demand for enumerating ACPI devices to platform bus,
> this patch changes the code to enumerate ACPI devices with _HID to
> platform bus by default, unless the device already has a scan handler attached.

I think we need to be more careful here still.

For example, we shouldn't create platform devices for ACPI device objects that
correspond to I2C devices (or any other "simple peripheral bus" devices for that
matter).

> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/acpi/acpi_platform.c |   28 ----------------------------
>  drivers/acpi/scan.c          |   12 ++++++------
>  2 files changed, 6 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
> index dbfe49e..33376a9 100644
> --- a/drivers/acpi/acpi_platform.c
> +++ b/drivers/acpi/acpi_platform.c
> @@ -22,24 +22,6 @@
>  
>  ACPI_MODULE_NAME("platform");
>  
> -/*
> - * The following ACPI IDs are known to be suitable for representing as
> - * platform devices.
> - */
> -static const struct acpi_device_id acpi_platform_device_ids[] = {
> -
> -	{ "PNP0D40" },
> -	{ "ACPI0003" },
> -	{ "VPC2004" },
> -	{ "BCM4752" },
> -
> -	/* Intel Smart Sound Technology */
> -	{ "INT33C8" },
> -	{ "80860F28" },
> -
> -	{ }
> -};
> -
>  /**
>   * acpi_create_platform_device - Create platform device for ACPI device node
>   * @adev: ACPI device node to create a platform device for.
> @@ -125,13 +107,3 @@ int acpi_create_platform_device(struct acpi_device *adev,
>  	kfree(resources);
>  	return 1;
>  }
> -
> -static struct acpi_scan_handler platform_handler = {
> -	.ids = acpi_platform_device_ids,
> -	.attach = acpi_create_platform_device,
> -};
> -
> -void __init acpi_platform_init(void)
> -{
> -	acpi_scan_add_handler(&platform_handler);
> -}
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index e41b5e6..c0456e7 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2028,14 +2028,15 @@ static int acpi_scan_attach_handler(struct acpi_device *device)
>  		handler = acpi_scan_match_handler(hwid->id, &devid);
>  		if (handler) {
>  			ret = handler->attach(device, devid);
> -			if (ret > 0) {
> +			if (ret > 0)
>  				device->handler = handler;
> -				break;
> -			} else if (ret < 0) {
> -				break;
> -			}
> +			if (ret)
> +				goto end;
>  		}
>  	}
> +end:
> +	if (device->pnp.type.platform_id && !device->handler)
> +		acpi_create_platform_device(device, NULL);
>  	return ret;
>  }
>  
> @@ -2191,7 +2192,6 @@ int __init acpi_scan_init(void)
>  	acpi_pci_root_init();
>  	acpi_pci_link_init();
>  	acpi_processor_init();
> -	acpi_platform_init();
>  	acpi_lpss_init();
>  	acpi_cmos_rtc_init();
>  	acpi_container_init();
>
Zhang, Rui March 24, 2014, 4:06 a.m. UTC | #2
On Mon, 2014-03-24 at 03:00 +0100, Rafael J. Wysocki wrote:
> On Monday, March 17, 2014 03:49:36 PM Zhang Rui wrote:
> > Because of the growing demand for enumerating ACPI devices to platform bus,
> > this patch changes the code to enumerate ACPI devices with _HID to
> > platform bus by default, unless the device already has a scan handler attached.
> 
> I think we need to be more careful here still.
> 
> For example, we shouldn't create platform devices for ACPI device objects that
> correspond to I2C devices (or any other "simple peripheral bus" devices for that
> matter).
> 
agreed. Then how about the patch below?
Note that I've just finished with build test.

thanks,
rui


>From a21cef9a840d979362bfaac3feeae19236a93d22 Mon Sep 17 00:00:00 2001
From: Zhang Rui <rui.zhang@intel.com>
Date: Mon, 24 Mar 2014 11:48:05 +0800
Subject: [PATCH] ACPI: introduce .handle_children flag for acpi scan handler

For some devices with scan handler attached, their children devices
are enumerated by the scan handler, indirectly, as well.

In this case, we do not want to enumerate the children devices in
acpi scan code explicitly.

Thus a new flag .handle_children is introduced in this patch.

For scan handlers with this flag set, we will do default enumeration neither
for the attached devices nor for the children of the attached devices.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/acpi/acpi_lpss.c |    1 +
 drivers/acpi/scan.c      |   28 ++++++++++++++++++++++++++--
 include/acpi/acpi_bus.h  |    4 +++-
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index cfbf2c0..9857343 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -453,6 +453,7 @@ static int acpi_lpss_dummy_attach(struct acpi_device *adev,
 static struct acpi_scan_handler lpss_dummy_handler = {
 	.ids = acpi_lpss_device_ids,
 	.attach = acpi_lpss_dummy_attach,
+	.handle_children = true,
 };
 
 void __init acpi_lpss_init(void)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index c0456e7..cd833f4 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2016,6 +2016,31 @@ static acpi_status acpi_bus_check_add(acpi_handle handle, u32 lvl_not_used,
 	return AE_OK;
 }
 
+static void acpi_do_default_enumeration(struct acpi_device *device)
+{
+	/*
+	 * Do not do enumeration for device object that
+	 * its parent doesn't want to
+	 */
+	if (device->parent && device->parent->flags.no_child_enumeration) {
+		device->flags.no_child_enumeration = 1;
+		return;
+	}
+
+	/* Do not do enumeration for device object with scan handler attached */
+	if (device->handler) {
+		if (device->handler->handle_children)
+			device->flags.no_child_enumeration = 1;
+		return;
+	}
+
+	/* Do not do enumeration for device object w/o platform_id */
+	if (!device->pnp.type.platform_id)
+		return;
+
+	acpi_create_platform_device(device, NULL);
+}
+
 static int acpi_scan_attach_handler(struct acpi_device *device)
 {
 	struct acpi_hardware_id *hwid;
@@ -2035,8 +2060,7 @@ static int acpi_scan_attach_handler(struct acpi_device *device)
 		}
 	}
 end:
-	if (device->pnp.type.platform_id && !device->handler)
-		acpi_create_platform_device(device, NULL);
+	acpi_do_default_enumeration(device);
 	return ret;
 }
 
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index f998746..bc7235f 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -135,6 +135,7 @@ struct acpi_scan_handler {
 	int (*attach)(struct acpi_device *dev, const struct acpi_device_id *id);
 	void (*detach)(struct acpi_device *dev);
 	struct acpi_hotplug_profile hotplug;
+	bool handle_children;
 };
 
 /*
@@ -191,7 +192,8 @@ struct acpi_device_flags {
 	u32 initialized:1;
 	u32 visited:1;
 	u32 no_hotplug:1;
-	u32 reserved:24;
+	u32 no_child_enumeration:1;
+	u32 reserved:23;
 };
 
 /* File System */
Zhang, Rui March 31, 2014, 5:24 a.m. UTC | #3
On Mon, 2014-03-24 at 12:06 +0800, Zhang Rui wrote:
> On Mon, 2014-03-24 at 03:00 +0100, Rafael J. Wysocki wrote:
> > On Monday, March 17, 2014 03:49:36 PM Zhang Rui wrote:
> > > Because of the growing demand for enumerating ACPI devices to platform bus,
> > > this patch changes the code to enumerate ACPI devices with _HID to
> > > platform bus by default, unless the device already has a scan handler attached.
> > 
> > I think we need to be more careful here still.
> > 
> > For example, we shouldn't create platform devices for ACPI device objects that
> > correspond to I2C devices (or any other "simple peripheral bus" devices for that
> > matter).
> > 
> agreed. Then how about the patch below?
> Note that I've just finished with build test.
> 
updated one attached.
I faked a LPSS device and its child device in DSDT, test result shows
that only the LPSS device is enumerated to platform bus via the LPSS
scan handler.

>From 9ac1537b52e5882e13fd60cb435af56ed04456ac Mon Sep 17 00:00:00 2001
From: Zhang Rui <rui.zhang@intel.com>
Date: Mon, 24 Mar 2014 11:48:05 +0800
Subject: [PATCH] ACPI: introduce .handle_children flag for acpi scan handler

For some devices with scan handler attached, their children devices
are enumerated by the scan handler, indirectly, as well.

In this case, we do not want to enumerate the children devices in
acpi scan code explicitly.

Thus a new flag .handle_children is introduced in this patch.

For scan handlers with this flag set, we will do default enumeration neither
for the attached devices nor for the children of the attached devices.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/acpi/acpi_lpss.c |    2 ++
 drivers/acpi/scan.c      |   28 ++++++++++++++++++++++++++--
 include/acpi/acpi_bus.h  |    4 +++-
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index cfbf2c0..6c3e7a8 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -440,6 +440,7 @@ static struct notifier_block acpi_lpss_nb = {
 static struct acpi_scan_handler lpss_handler = {
 	.ids = acpi_lpss_device_ids,
 	.attach = acpi_lpss_create_device,
+	.handle_children = true,
 };
 
 #endif /* CONFIG_X86_INTEL_LPSS */
@@ -453,6 +454,7 @@ static int acpi_lpss_dummy_attach(struct acpi_device *adev,
 static struct acpi_scan_handler lpss_dummy_handler = {
 	.ids = acpi_lpss_device_ids,
 	.attach = acpi_lpss_dummy_attach,
+	.handle_children = true,
 };
 
 void __init acpi_lpss_init(void)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index c0456e7..cd833f4 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2016,6 +2016,31 @@ static acpi_status acpi_bus_check_add(acpi_handle handle, u32 lvl_not_used,
 	return AE_OK;
 }
 
+static void acpi_do_default_enumeration(struct acpi_device *device)
+{
+	/*
+	 * Do not do enumeration for device object that
+	 * its parent doesn't want to
+	 */
+	if (device->parent && device->parent->flags.no_child_enumeration) {
+		device->flags.no_child_enumeration = 1;
+		return;
+	}
+
+	/* Do not do enumeration for device object with scan handler attached */
+	if (device->handler) {
+		if (device->handler->handle_children)
+			device->flags.no_child_enumeration = 1;
+		return;
+	}
+
+	/* Do not do enumeration for device object w/o platform_id */
+	if (!device->pnp.type.platform_id)
+		return;
+
+	acpi_create_platform_device(device, NULL);
+}
+
 static int acpi_scan_attach_handler(struct acpi_device *device)
 {
 	struct acpi_hardware_id *hwid;
@@ -2035,8 +2060,7 @@ static int acpi_scan_attach_handler(struct acpi_device *device)
 		}
 	}
 end:
-	if (device->pnp.type.platform_id && !device->handler)
-		acpi_create_platform_device(device, NULL);
+	acpi_do_default_enumeration(device);
 	return ret;
 }
 
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index f998746..bc7235f 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -135,6 +135,7 @@ struct acpi_scan_handler {
 	int (*attach)(struct acpi_device *dev, const struct acpi_device_id *id);
 	void (*detach)(struct acpi_device *dev);
 	struct acpi_hotplug_profile hotplug;
+	bool handle_children;
 };
 
 /*
@@ -191,7 +192,8 @@ struct acpi_device_flags {
 	u32 initialized:1;
 	u32 visited:1;
 	u32 no_hotplug:1;
-	u32 reserved:24;
+	u32 no_child_enumeration:1;
+	u32 reserved:23;
 };
 
 /* File System */

Patch
diff mbox series

diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
index dbfe49e..33376a9 100644
--- a/drivers/acpi/acpi_platform.c
+++ b/drivers/acpi/acpi_platform.c
@@ -22,24 +22,6 @@ 
 
 ACPI_MODULE_NAME("platform");
 
-/*
- * The following ACPI IDs are known to be suitable for representing as
- * platform devices.
- */
-static const struct acpi_device_id acpi_platform_device_ids[] = {
-
-	{ "PNP0D40" },
-	{ "ACPI0003" },
-	{ "VPC2004" },
-	{ "BCM4752" },
-
-	/* Intel Smart Sound Technology */
-	{ "INT33C8" },
-	{ "80860F28" },
-
-	{ }
-};
-
 /**
  * acpi_create_platform_device - Create platform device for ACPI device node
  * @adev: ACPI device node to create a platform device for.
@@ -125,13 +107,3 @@  int acpi_create_platform_device(struct acpi_device *adev,
 	kfree(resources);
 	return 1;
 }
-
-static struct acpi_scan_handler platform_handler = {
-	.ids = acpi_platform_device_ids,
-	.attach = acpi_create_platform_device,
-};
-
-void __init acpi_platform_init(void)
-{
-	acpi_scan_add_handler(&platform_handler);
-}
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index e41b5e6..c0456e7 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2028,14 +2028,15 @@  static int acpi_scan_attach_handler(struct acpi_device *device)
 		handler = acpi_scan_match_handler(hwid->id, &devid);
 		if (handler) {
 			ret = handler->attach(device, devid);
-			if (ret > 0) {
+			if (ret > 0)
 				device->handler = handler;
-				break;
-			} else if (ret < 0) {
-				break;
-			}
+			if (ret)
+				goto end;
 		}
 	}
+end:
+	if (device->pnp.type.platform_id && !device->handler)
+		acpi_create_platform_device(device, NULL);
 	return ret;
 }
 
@@ -2191,7 +2192,6 @@  int __init acpi_scan_init(void)
 	acpi_pci_root_init();
 	acpi_pci_link_init();
 	acpi_processor_init();
-	acpi_platform_init();
 	acpi_lpss_init();
 	acpi_cmos_rtc_init();
 	acpi_container_init();