[BUGFIX 1/9] ACPI, DOCK: initialize dock subsystem before scanning PCI root buses

[BUGFIX 1/9] ACPI, DOCK: initialize dock subsystem before scanning PCI root buses

[Jiang Liu]

Changeset "3b63aaa70e1 PCI: acpiphp: Do not use ACPI PCI subdriver
mechanism" causes a regression which breaks ACPI dock support,
please refer to https://bugzilla.kernel.org/show_bug.cgi?id=59501

The root cause is that changeset 3b63aaa70e1 changed the relative
initialization order of ACPI dock subsystem and acpiphp driver,
and acpiphp driver has dependency on ACPI dock subsystem's
initialization result, so that acpiphp can't correctly detect ACPI
dock stations now.

On the other hand, ACPI dock is a built-in driver, so we could
explicitly initialize it before the acpiphp driver is used.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Reported-by: Alexander E. Patrakov <patrakov@gmail.com>
Tested-by: Alexander E. Patrakov <patrakov@gmail.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: linux-acpi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: <stable@vger.kernel.org> # 3.9+
---
 drivers/acpi/dock.c     | 7 +------
 drivers/acpi/internal.h | 5 +++++
 drivers/acpi/scan.c     | 1 +
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
index 4fdea38..02b0563 100644
--- a/drivers/acpi/dock.c
+++ b/drivers/acpi/dock.c
@@ -1033,7 +1033,7 @@ find_dock_and_bay(acpi_handle handle, u32 lvl, void *context, void **rv)
 	return AE_OK;
 }
 
-static int __init dock_init(void)
+int __init acpi_dock_init(void)
 {
 	if (acpi_disabled)
 		return 0;
@@ -1062,9 +1062,4 @@ static void __exit dock_exit(void)
 		dock_remove(dock_station);
 }
 
-/*
- * Must be called before drivers of devices in dock, otherwise we can't know
- * which devices are in a dock
- */
-subsys_initcall(dock_init);
 module_exit(dock_exit);
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 297cbf4..c610a76 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -40,6 +40,11 @@ void acpi_container_init(void);
 #else
 static inline void acpi_container_init(void) {}
 #endif
+#ifdef CONFIG_ACPI_DOCK
+void acpi_dock_init(void);
+#else
+static inline void acpi_dock_init(void) {}
+#endif
 #ifdef CONFIG_ACPI_HOTPLUG_MEMORY
 void acpi_memory_hotplug_init(void);
 #else
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 44225cb..4148163 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2045,6 +2045,7 @@ int __init acpi_scan_init(void)
 	acpi_lpss_init();
 	acpi_container_init();
 	acpi_memory_hotplug_init();
+	acpi_dock_init();
 
 	mutex_lock(&acpi_scan_lock);
 	/*
-- 
1.8.1.2

[BUGFIX 3/9] ACPI, DOCK: clean up unused module related code

[Jiang Liu]

ACPI dock driver can't be built as a module any more, so clean up
module related code.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Cc: Shaohua Li <shaohua.li@intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: linux-acpi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/acpi/dock.c | 41 -----------------------------------------
 1 file changed, 41 deletions(-)

diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
index 79c8d9e..50e38b7 100644
--- a/drivers/acpi/dock.c
+++ b/drivers/acpi/dock.c
@@ -53,12 +53,6 @@ MODULE_PARM_DESC(immediate_undock, "1 (default) will cause the driver to "
 
 static struct atomic_notifier_head dock_notifier_list;
 
-static const struct acpi_device_id dock_device_ids[] = {
-	{"LNXDOCK", 0},
-	{"", 0},
-};
-MODULE_DEVICE_TABLE(acpi, dock_device_ids);
-
 struct dock_station {
 	acpi_handle handle;
 	unsigned long last_dock_time;
@@ -1013,30 +1007,6 @@ err_unregister:
 }
 
 /**
- * dock_remove - free up resources related to the dock station
- */
-static int dock_remove(struct dock_station *ds)
-{
-	struct dock_dependent_device *dd, *tmp;
-	struct platform_device *dock_device = ds->dock_device;
-
-	if (!dock_station_count)
-		return 0;
-
-	/* remove dependent devices */
-	list_for_each_entry_safe(dd, tmp, &ds->dependent_devices, list)
-		kfree(dd);
-
-	list_del(&ds->sibling);
-
-	/* cleanup sysfs */
-	sysfs_remove_group(&dock_device->dev.kobj, &dock_attribute_group);
-	platform_device_unregister(dock_device);
-
-	return 0;
-}
-
-/**
  * find_dock_and_bay - look for dock stations and bays
  * @handle: acpi handle of a device
  * @lvl: unused
@@ -1073,14 +1043,3 @@ int __init acpi_dock_init(void)
 		ACPI_DOCK_DRIVER_DESCRIPTION, dock_station_count);
 	return 0;
 }
-
-static void __exit dock_exit(void)
-{
-	struct dock_station *tmp, *dock_station;
-
-	unregister_acpi_bus_notifier(&dock_acpi_notifier);
-	list_for_each_entry_safe(dock_station, tmp, &dock_stations, sibling)
-		dock_remove(dock_station);
-}
-
-module_exit(dock_exit);
-- 
1.8.1.2

[BUGFIX 4/9] ACPI, DOCK: avoid initializing acpi_dock_notifier_list multiple times

[Jiang Liu]

Initialize acpi_dock_notifier_list from function acpi_dock_init()
instead of dock_add() to avoid initializing it multiple times.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Cc: Shaohua Li <shaohua.li@intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: linux-acpi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/acpi/dock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
index 50e38b7..8e578aa 100644
--- a/drivers/acpi/dock.c
+++ b/drivers/acpi/dock.c
@@ -967,7 +967,6 @@ static int __init dock_add(acpi_handle handle)
 	spin_lock_init(&dock_station->dd_lock);
 	INIT_LIST_HEAD(&dock_station->sibling);
 	INIT_LIST_HEAD(&dock_station->hotplug_devices);
-	ATOMIC_INIT_NOTIFIER_HEAD(&dock_notifier_list);
 	INIT_LIST_HEAD(&dock_station->dependent_devices);
 
 	/* we want the dock device to send uevents */
@@ -1038,6 +1037,7 @@ int __init acpi_dock_init(void)
 		return 0;
 	}
 
+	ATOMIC_INIT_NOTIFIER_HEAD(&dock_notifier_list);
 	register_acpi_bus_notifier(&dock_acpi_notifier);
 	pr_info(PREFIX "%s: %d docks/bays found\n",
 		ACPI_DOCK_DRIVER_DESCRIPTION, dock_station_count);
-- 
1.8.1.2

[BUGFIX 5/9] ACPI, DOCK: kill redundant spin lock in dock device object

[Jiang Liu]

All dock device related data structures are created during driver
initialization, so kill the redundant spin lock in dock device object.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Cc: Shaohua Li <shaohua.li@intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: linux-acpi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/acpi/dock.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
index 8e578aa..cd2d5df 100644
--- a/drivers/acpi/dock.c
+++ b/drivers/acpi/dock.c
@@ -57,7 +57,6 @@ struct dock_station {
 	acpi_handle handle;
 	unsigned long last_dock_time;
 	u32 flags;
-	spinlock_t dd_lock;
 	struct mutex hp_lock;
 	struct task_struct *owner;
 	struct list_head dependent_devices;
@@ -107,10 +106,7 @@ add_dock_dependent_device(struct dock_station *ds, acpi_handle handle)
 	dd->handle = handle;
 	INIT_LIST_HEAD(&dd->list);
 	INIT_LIST_HEAD(&dd->hotplug_list);
-
-	spin_lock(&ds->dd_lock);
 	list_add_tail(&dd->list, &ds->dependent_devices);
-	spin_unlock(&ds->dd_lock);
 
 	return 0;
 }
@@ -168,14 +164,10 @@ find_dock_dependent_device(struct dock_station *ds, acpi_handle handle)
 {
 	struct dock_dependent_device *dd;
 
-	spin_lock(&ds->dd_lock);
-	list_for_each_entry(dd, &ds->dependent_devices, list) {
-		if (handle == dd->handle) {
-			spin_unlock(&ds->dd_lock);
+	list_for_each_entry(dd, &ds->dependent_devices, list)
+		if (handle == dd->handle)
 			return dd;
-		}
-	}
-	spin_unlock(&ds->dd_lock);
+
 	return NULL;
 }
 
@@ -964,7 +956,6 @@ static int __init dock_add(acpi_handle handle)
 	dock_station->last_dock_time = jiffies - HZ;
 
 	mutex_init(&dock_station->hp_lock);
-	spin_lock_init(&dock_station->dd_lock);
 	INIT_LIST_HEAD(&dock_station->sibling);
 	INIT_LIST_HEAD(&dock_station->hotplug_devices);
 	INIT_LIST_HEAD(&dock_station->dependent_devices);
-- 
1.8.1.2

[BUGFIX 6/9] ACPI, DOCK: mark initialization functions with __init

[Jiang Liu]

Mark all initialization functions with __init to reduce memory
consumption at runtime.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Cc: Shaohua Li <shaohua.li@intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: linux-acpi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/acpi/dock.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
index cd2d5df..da3314d 100644
--- a/drivers/acpi/dock.c
+++ b/drivers/acpi/dock.c
@@ -94,7 +94,7 @@ struct dock_dependent_device {
  *
  * Add the dependent device to the dock's dependent device list.
  */
-static int
+static int __init
 add_dock_dependent_device(struct dock_station *ds, acpi_handle handle)
 {
 	struct dock_dependent_device *dd;
@@ -192,7 +192,7 @@ static int is_dock(acpi_handle handle)
 	return 1;
 }
 
-static int is_ejectable(acpi_handle handle)
+static int __init is_ejectable(acpi_handle handle)
 {
 	acpi_status status;
 	acpi_handle tmp;
@@ -203,7 +203,7 @@ static int is_ejectable(acpi_handle handle)
 	return 1;
 }
 
-static int is_ata(acpi_handle handle)
+static int __init is_ata(acpi_handle handle)
 {
 	acpi_handle tmp;
 
@@ -216,7 +216,7 @@ static int is_ata(acpi_handle handle)
 	return 0;
 }
 
-static int is_battery(acpi_handle handle)
+static int __init is_battery(acpi_handle handle)
 {
 	struct acpi_device_info *info;
 	int ret = 1;
@@ -232,7 +232,7 @@ static int is_battery(acpi_handle handle)
 	return ret;
 }
 
-static int is_ejectable_bay(acpi_handle handle)
+static int __init is_ejectable_bay(acpi_handle handle)
 {
 	acpi_handle phandle;
 
@@ -809,7 +809,7 @@ static struct notifier_block dock_acpi_notifier = {
  * check to see if an object has an _EJD method.  If it does, then it
  * will see if it is dependent on the dock station.
  */
-static acpi_status
+static acpi_status __init
 find_dock_devices(acpi_handle handle, u32 lvl, void *context, void **rv)
 {
 	acpi_status status;
-- 
1.8.1.2

[BUGFIX 7/9] ACPI, DOCK: simplify implementation of dock_create_acpi_device()

[Jiang Liu]

The return value of dock_create_acpi_device() is unused, so simplify
it by returning void.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Cc: Shaohua Li <shaohua.li@intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: linux-acpi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/acpi/dock.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
index da3314d..72cf97e 100644
--- a/drivers/acpi/dock.c
+++ b/drivers/acpi/dock.c
@@ -299,10 +299,8 @@ static int dock_present(struct dock_station *ds)
  *  handle if one does not exist already.  This should cause
  *  acpi to scan for drivers for the given devices, and call
  *  matching driver's add routine.
- *
- *  Returns a pointer to the acpi_device corresponding to the handle.
  */
-static struct acpi_device * dock_create_acpi_device(acpi_handle handle)
+static void dock_create_acpi_device(acpi_handle handle)
 {
 	struct acpi_device *device;
 	int ret;
@@ -315,10 +313,7 @@ static struct acpi_device * dock_create_acpi_device(acpi_handle handle)
 		ret = acpi_bus_scan(handle);
 		if (ret)
 			pr_debug("error adding bus, %x\n", -ret);
-
-		acpi_bus_get_device(handle, &device);
 	}
-	return device;
 }
 
 /**
-- 
1.8.1.2

Re: [BUGFIX 1/9] ACPI, DOCK: initialize dock subsystem before scanning PCI root buses

[Rafael J. Wysocki]

On Friday, June 14, 2013 12:32:24 AM Jiang Liu wrote:
> Changeset "3b63aaa70e1 PCI: acpiphp: Do not use ACPI PCI subdriver
> mechanism" causes a regression which breaks ACPI dock support,
> please refer to https://bugzilla.kernel.org/show_bug.cgi?id=59501
> 
> The root cause is that changeset 3b63aaa70e1 changed the relative
> initialization order of ACPI dock subsystem and acpiphp driver,
> and acpiphp driver has dependency on ACPI dock subsystem's
> initialization result, so that acpiphp can't correctly detect ACPI
> dock stations now.
> 
> On the other hand, ACPI dock is a built-in driver, so we could
> explicitly initialize it before the acpiphp driver is used.
> 
> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> Reported-by: Alexander E. Patrakov <patrakov@gmail.com>
> Tested-by: Alexander E. Patrakov <patrakov@gmail.com>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: <stable@vger.kernel.org> # 3.9+

Can you please send the whole series to linux-acpi next time?

First, because I believe it should go in through the ACPI tree.  Second,
because it is kind of useful to have all of the patches in context.


Thanks,
Rafael


> ---
>  drivers/acpi/dock.c     | 7 +------
>  drivers/acpi/internal.h | 5 +++++
>  drivers/acpi/scan.c     | 1 +
>  3 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
> index 4fdea38..02b0563 100644
> --- a/drivers/acpi/dock.c
> +++ b/drivers/acpi/dock.c
> @@ -1033,7 +1033,7 @@ find_dock_and_bay(acpi_handle handle, u32 lvl, void *context, void **rv)
>  	return AE_OK;
>  }
>  
> -static int __init dock_init(void)
> +int __init acpi_dock_init(void)
>  {
>  	if (acpi_disabled)
>  		return 0;
> @@ -1062,9 +1062,4 @@ static void __exit dock_exit(void)
>  		dock_remove(dock_station);
>  }
>  
> -/*
> - * Must be called before drivers of devices in dock, otherwise we can't know
> - * which devices are in a dock
> - */
> -subsys_initcall(dock_init);
>  module_exit(dock_exit);
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index 297cbf4..c610a76 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -40,6 +40,11 @@ void acpi_container_init(void);
>  #else
>  static inline void acpi_container_init(void) {}
>  #endif
> +#ifdef CONFIG_ACPI_DOCK
> +void acpi_dock_init(void);
> +#else
> +static inline void acpi_dock_init(void) {}
> +#endif
>  #ifdef CONFIG_ACPI_HOTPLUG_MEMORY
>  void acpi_memory_hotplug_init(void);
>  #else
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 44225cb..4148163 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2045,6 +2045,7 @@ int __init acpi_scan_init(void)
>  	acpi_lpss_init();
>  	acpi_container_init();
>  	acpi_memory_hotplug_init();
> +	acpi_dock_init();
>  
>  	mutex_lock(&acpi_scan_lock);
>  	/*
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [BUGFIX 1/9] ACPI, DOCK: initialize dock subsystem before scanning PCI root buses

[Rafael J. Wysocki]

On Friday, June 14, 2013 12:32:24 AM Jiang Liu wrote:
> Changeset "3b63aaa70e1 PCI: acpiphp: Do not use ACPI PCI subdriver
> mechanism" causes a regression which breaks ACPI dock support,
> please refer to https://bugzilla.kernel.org/show_bug.cgi?id=59501
> 
> The root cause is that changeset 3b63aaa70e1 changed the relative
> initialization order of ACPI dock subsystem and acpiphp driver,
> and acpiphp driver has dependency on ACPI dock subsystem's
> initialization result, so that acpiphp can't correctly detect ACPI
> dock stations now.
> 
> On the other hand, ACPI dock is a built-in driver, so we could
> explicitly initialize it before the acpiphp driver is used.

This change makes sense to me.

Thanks,
Rafael


> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> Reported-by: Alexander E. Patrakov <patrakov@gmail.com>
> Tested-by: Alexander E. Patrakov <patrakov@gmail.com>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: <stable@vger.kernel.org> # 3.9+
> ---
>  drivers/acpi/dock.c     | 7 +------
>  drivers/acpi/internal.h | 5 +++++
>  drivers/acpi/scan.c     | 1 +
>  3 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
> index 4fdea38..02b0563 100644
> --- a/drivers/acpi/dock.c
> +++ b/drivers/acpi/dock.c
> @@ -1033,7 +1033,7 @@ find_dock_and_bay(acpi_handle handle, u32 lvl, void *context, void **rv)
>  	return AE_OK;
>  }
>  
> -static int __init dock_init(void)
> +int __init acpi_dock_init(void)
>  {
>  	if (acpi_disabled)
>  		return 0;
> @@ -1062,9 +1062,4 @@ static void __exit dock_exit(void)
>  		dock_remove(dock_station);
>  }
>  
> -/*
> - * Must be called before drivers of devices in dock, otherwise we can't know
> - * which devices are in a dock
> - */
> -subsys_initcall(dock_init);
>  module_exit(dock_exit);
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index 297cbf4..c610a76 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -40,6 +40,11 @@ void acpi_container_init(void);
>  #else
>  static inline void acpi_container_init(void) {}
>  #endif
> +#ifdef CONFIG_ACPI_DOCK
> +void acpi_dock_init(void);
> +#else
> +static inline void acpi_dock_init(void) {}
> +#endif
>  #ifdef CONFIG_ACPI_HOTPLUG_MEMORY
>  void acpi_memory_hotplug_init(void);
>  #else
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 44225cb..4148163 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2045,6 +2045,7 @@ int __init acpi_scan_init(void)
>  	acpi_lpss_init();
>  	acpi_container_init();
>  	acpi_memory_hotplug_init();
> +	acpi_dock_init();
>  
>  	mutex_lock(&acpi_scan_lock);
>  	/*
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [BUGFIX 3/9] ACPI, DOCK: clean up unused module related code

[Rafael J. Wysocki]

On Friday, June 14, 2013 12:32:26 AM Jiang Liu wrote:
> ACPI dock driver can't be built as a module any more, so clean up
> module related code.
> 
> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> Cc: Shaohua Li <shaohua.li@intel.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org

How exactly does this depend on [2/9]?  If it doesn't at all, it should go
after [1/9].

> ---
>  drivers/acpi/dock.c | 41 -----------------------------------------
>  1 file changed, 41 deletions(-)
> 
> diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
> index 79c8d9e..50e38b7 100644
> --- a/drivers/acpi/dock.c
> +++ b/drivers/acpi/dock.c
> @@ -53,12 +53,6 @@ MODULE_PARM_DESC(immediate_undock, "1 (default) will cause the driver to "
>  
>  static struct atomic_notifier_head dock_notifier_list;
>  
> -static const struct acpi_device_id dock_device_ids[] = {
> -	{"LNXDOCK", 0},
> -	{"", 0},
> -};
> -MODULE_DEVICE_TABLE(acpi, dock_device_ids);
> -

Don't we actually need the device IDs?

>  struct dock_station {
>  	acpi_handle handle;
>  	unsigned long last_dock_time;
> @@ -1013,30 +1007,6 @@ err_unregister:
>  }
>  
>  /**
> - * dock_remove - free up resources related to the dock station
> - */
> -static int dock_remove(struct dock_station *ds)
> -{
> -	struct dock_dependent_device *dd, *tmp;
> -	struct platform_device *dock_device = ds->dock_device;
> -
> -	if (!dock_station_count)
> -		return 0;
> -
> -	/* remove dependent devices */
> -	list_for_each_entry_safe(dd, tmp, &ds->dependent_devices, list)
> -		kfree(dd);
> -
> -	list_del(&ds->sibling);
> -
> -	/* cleanup sysfs */
> -	sysfs_remove_group(&dock_device->dev.kobj, &dock_attribute_group);
> -	platform_device_unregister(dock_device);
> -
> -	return 0;
> -}
> -
> -/**
>   * find_dock_and_bay - look for dock stations and bays
>   * @handle: acpi handle of a device
>   * @lvl: unused
> @@ -1073,14 +1043,3 @@ int __init acpi_dock_init(void)
>  		ACPI_DOCK_DRIVER_DESCRIPTION, dock_station_count);
>  	return 0;
>  }
> -
> -static void __exit dock_exit(void)
> -{
> -	struct dock_station *tmp, *dock_station;
> -
> -	unregister_acpi_bus_notifier(&dock_acpi_notifier);
> -	list_for_each_entry_safe(dock_station, tmp, &dock_stations, sibling)
> -		dock_remove(dock_station);
> -}
> -
> -module_exit(dock_exit);

The other changes look OK to me.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [BUGFIX 4/9] ACPI, DOCK: avoid initializing acpi_dock_notifier_list multiple times

[Rafael J. Wysocki]

On Friday, June 14, 2013 12:32:27 AM Jiang Liu wrote:
> Initialize acpi_dock_notifier_list from function acpi_dock_init()
> instead of dock_add() to avoid initializing it multiple times.

Well, makes sense.

Thanks,
Rafael


> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> Cc: Shaohua Li <shaohua.li@intel.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/acpi/dock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
> index 50e38b7..8e578aa 100644
> --- a/drivers/acpi/dock.c
> +++ b/drivers/acpi/dock.c
> @@ -967,7 +967,6 @@ static int __init dock_add(acpi_handle handle)
>  	spin_lock_init(&dock_station->dd_lock);
>  	INIT_LIST_HEAD(&dock_station->sibling);
>  	INIT_LIST_HEAD(&dock_station->hotplug_devices);
> -	ATOMIC_INIT_NOTIFIER_HEAD(&dock_notifier_list);
>  	INIT_LIST_HEAD(&dock_station->dependent_devices);
>  
>  	/* we want the dock device to send uevents */
> @@ -1038,6 +1037,7 @@ int __init acpi_dock_init(void)
>  		return 0;
>  	}
>  
> +	ATOMIC_INIT_NOTIFIER_HEAD(&dock_notifier_list);
>  	register_acpi_bus_notifier(&dock_acpi_notifier);
>  	pr_info(PREFIX "%s: %d docks/bays found\n",
>  		ACPI_DOCK_DRIVER_DESCRIPTION, dock_station_count);
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [BUGFIX 5/9] ACPI, DOCK: kill redundant spin lock in dock device object

[Rafael J. Wysocki]

On Friday, June 14, 2013 12:32:28 AM Jiang Liu wrote:
> All dock device related data structures are created during driver
> initialization, so kill the redundant spin lock in dock device object.

As a cleanup, it definitely makes sense, but does it fix anything?

Rafael


> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> Cc: Shaohua Li <shaohua.li@intel.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/acpi/dock.c | 15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
> index 8e578aa..cd2d5df 100644
> --- a/drivers/acpi/dock.c
> +++ b/drivers/acpi/dock.c
> @@ -57,7 +57,6 @@ struct dock_station {
>  	acpi_handle handle;
>  	unsigned long last_dock_time;
>  	u32 flags;
> -	spinlock_t dd_lock;
>  	struct mutex hp_lock;
>  	struct task_struct *owner;
>  	struct list_head dependent_devices;
> @@ -107,10 +106,7 @@ add_dock_dependent_device(struct dock_station *ds, acpi_handle handle)
>  	dd->handle = handle;
>  	INIT_LIST_HEAD(&dd->list);
>  	INIT_LIST_HEAD(&dd->hotplug_list);
> -
> -	spin_lock(&ds->dd_lock);
>  	list_add_tail(&dd->list, &ds->dependent_devices);
> -	spin_unlock(&ds->dd_lock);
>  
>  	return 0;
>  }
> @@ -168,14 +164,10 @@ find_dock_dependent_device(struct dock_station *ds, acpi_handle handle)
>  {
>  	struct dock_dependent_device *dd;
>  
> -	spin_lock(&ds->dd_lock);
> -	list_for_each_entry(dd, &ds->dependent_devices, list) {
> -		if (handle == dd->handle) {
> -			spin_unlock(&ds->dd_lock);
> +	list_for_each_entry(dd, &ds->dependent_devices, list)
> +		if (handle == dd->handle)
>  			return dd;
> -		}
> -	}
> -	spin_unlock(&ds->dd_lock);
> +
>  	return NULL;
>  }
>  
> @@ -964,7 +956,6 @@ static int __init dock_add(acpi_handle handle)
>  	dock_station->last_dock_time = jiffies - HZ;
>  
>  	mutex_init(&dock_station->hp_lock);
> -	spin_lock_init(&dock_station->dd_lock);
>  	INIT_LIST_HEAD(&dock_station->sibling);
>  	INIT_LIST_HEAD(&dock_station->hotplug_devices);
>  	INIT_LIST_HEAD(&dock_station->dependent_devices);
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [BUGFIX 6/9] ACPI, DOCK: mark initialization functions with __init

[Rafael J. Wysocki]

On Friday, June 14, 2013 12:32:29 AM Jiang Liu wrote:
> Mark all initialization functions with __init to reduce memory
> consumption at runtime.

Again, this is a *cleanup*, not a fix.  Please don't mix cleanups with fixes,
especially with ones you want to go into 3.10, because the cleanups aren't
3.10 material for sure.

Thanks,
Rafael


> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> Cc: Shaohua Li <shaohua.li@intel.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/acpi/dock.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
> index cd2d5df..da3314d 100644
> --- a/drivers/acpi/dock.c
> +++ b/drivers/acpi/dock.c
> @@ -94,7 +94,7 @@ struct dock_dependent_device {
>   *
>   * Add the dependent device to the dock's dependent device list.
>   */
> -static int
> +static int __init
>  add_dock_dependent_device(struct dock_station *ds, acpi_handle handle)
>  {
>  	struct dock_dependent_device *dd;
> @@ -192,7 +192,7 @@ static int is_dock(acpi_handle handle)
>  	return 1;
>  }
>  
> -static int is_ejectable(acpi_handle handle)
> +static int __init is_ejectable(acpi_handle handle)
>  {
>  	acpi_status status;
>  	acpi_handle tmp;
> @@ -203,7 +203,7 @@ static int is_ejectable(acpi_handle handle)
>  	return 1;
>  }
>  
> -static int is_ata(acpi_handle handle)
> +static int __init is_ata(acpi_handle handle)
>  {
>  	acpi_handle tmp;
>  
> @@ -216,7 +216,7 @@ static int is_ata(acpi_handle handle)
>  	return 0;
>  }
>  
> -static int is_battery(acpi_handle handle)
> +static int __init is_battery(acpi_handle handle)
>  {
>  	struct acpi_device_info *info;
>  	int ret = 1;
> @@ -232,7 +232,7 @@ static int is_battery(acpi_handle handle)
>  	return ret;
>  }
>  
> -static int is_ejectable_bay(acpi_handle handle)
> +static int __init is_ejectable_bay(acpi_handle handle)
>  {
>  	acpi_handle phandle;
>  
> @@ -809,7 +809,7 @@ static struct notifier_block dock_acpi_notifier = {
>   * check to see if an object has an _EJD method.  If it does, then it
>   * will see if it is dependent on the dock station.
>   */
> -static acpi_status
> +static acpi_status __init
>  find_dock_devices(acpi_handle handle, u32 lvl, void *context, void **rv)
>  {
>  	acpi_status status;
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [BUGFIX 3/9] ACPI, DOCK: clean up unused module related code

[Rafael J. Wysocki]

On Thursday, June 13, 2013 08:26:10 PM Rafael J. Wysocki wrote:
> On Friday, June 14, 2013 12:32:26 AM Jiang Liu wrote:
> > ACPI dock driver can't be built as a module any more, so clean up
> > module related code.
> > 
> > Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> > Cc: Shaohua Li <shaohua.li@intel.com>
> > Cc: Len Brown <lenb@kernel.org>
> > Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> > Cc: linux-acpi@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> 
> How exactly does this depend on [2/9]?  If it doesn't at all, it should go
> after [1/9].

However, this isn't even 3.10 material, so please stop sending it for now.

Thanks,
Rafael


> > ---
> >  drivers/acpi/dock.c | 41 -----------------------------------------
> >  1 file changed, 41 deletions(-)
> > 
> > diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
> > index 79c8d9e..50e38b7 100644
> > --- a/drivers/acpi/dock.c
> > +++ b/drivers/acpi/dock.c
> > @@ -53,12 +53,6 @@ MODULE_PARM_DESC(immediate_undock, "1 (default) will cause the driver to "
> >  
> >  static struct atomic_notifier_head dock_notifier_list;
> >  
> > -static const struct acpi_device_id dock_device_ids[] = {
> > -	{"LNXDOCK", 0},
> > -	{"", 0},
> > -};
> > -MODULE_DEVICE_TABLE(acpi, dock_device_ids);
> > -
> 
> Don't we actually need the device IDs?
> 
> >  struct dock_station {
> >  	acpi_handle handle;
> >  	unsigned long last_dock_time;
> > @@ -1013,30 +1007,6 @@ err_unregister:
> >  }
> >  
> >  /**
> > - * dock_remove - free up resources related to the dock station
> > - */
> > -static int dock_remove(struct dock_station *ds)
> > -{
> > -	struct dock_dependent_device *dd, *tmp;
> > -	struct platform_device *dock_device = ds->dock_device;
> > -
> > -	if (!dock_station_count)
> > -		return 0;
> > -
> > -	/* remove dependent devices */
> > -	list_for_each_entry_safe(dd, tmp, &ds->dependent_devices, list)
> > -		kfree(dd);
> > -
> > -	list_del(&ds->sibling);
> > -
> > -	/* cleanup sysfs */
> > -	sysfs_remove_group(&dock_device->dev.kobj, &dock_attribute_group);
> > -	platform_device_unregister(dock_device);
> > -
> > -	return 0;
> > -}
> > -
> > -/**
> >   * find_dock_and_bay - look for dock stations and bays
> >   * @handle: acpi handle of a device
> >   * @lvl: unused
> > @@ -1073,14 +1043,3 @@ int __init acpi_dock_init(void)
> >  		ACPI_DOCK_DRIVER_DESCRIPTION, dock_station_count);
> >  	return 0;
> >  }
> > -
> > -static void __exit dock_exit(void)
> > -{
> > -	struct dock_station *tmp, *dock_station;
> > -
> > -	unregister_acpi_bus_notifier(&dock_acpi_notifier);
> > -	list_for_each_entry_safe(dock_station, tmp, &dock_stations, sibling)
> > -		dock_remove(dock_station);
> > -}
> > -
> > -module_exit(dock_exit);
> 
> The other changes look OK to me.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [BUGFIX 3/9] ACPI, DOCK: clean up unused module related code

[Jiang Liu]

On 06/14/2013 02:26 AM, Rafael J. Wysocki wrote:
> On Friday, June 14, 2013 12:32:26 AM Jiang Liu wrote:
>> ACPI dock driver can't be built as a module any more, so clean up
>> module related code.
>>
>> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
>> Cc: Shaohua Li <shaohua.li@intel.com>
>> Cc: Len Brown <lenb@kernel.org>
>> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
>> Cc: linux-acpi@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
> 
> How exactly does this depend on [2/9]?  If it doesn't at all, it should go
> after [1/9].
> 
>> ---
>>  drivers/acpi/dock.c | 41 -----------------------------------------
>>  1 file changed, 41 deletions(-)
>>
>> diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
>> index 79c8d9e..50e38b7 100644
>> --- a/drivers/acpi/dock.c
>> +++ b/drivers/acpi/dock.c
>> @@ -53,12 +53,6 @@ MODULE_PARM_DESC(immediate_undock, "1 (default) will cause the driver to "
>>  
>>  static struct atomic_notifier_head dock_notifier_list;
>>  
>> -static const struct acpi_device_id dock_device_ids[] = {
>> -	{"LNXDOCK", 0},
>> -	{"", 0},
>> -};
>> -MODULE_DEVICE_TABLE(acpi, dock_device_ids);
>> -
> 
> Don't we actually need the device IDs?
Now dock driver could only be built as built-in, and it doesn't really
bind to ACPI dock devices, so I think the device ids are not used any
more. Not sure whether any userspace tool has dependency on the device
IDs.

> 
>>  struct dock_station {
>>  	acpi_handle handle;
>>  	unsigned long last_dock_time;
>> @@ -1013,30 +1007,6 @@ err_unregister:
>>  }
>>  
>>  /**
>> - * dock_remove - free up resources related to the dock station
>> - */
>> -static int dock_remove(struct dock_station *ds)
>> -{
>> -	struct dock_dependent_device *dd, *tmp;
>> -	struct platform_device *dock_device = ds->dock_device;
>> -
>> -	if (!dock_station_count)
>> -		return 0;
>> -
>> -	/* remove dependent devices */
>> -	list_for_each_entry_safe(dd, tmp, &ds->dependent_devices, list)
>> -		kfree(dd);
>> -
>> -	list_del(&ds->sibling);
>> -
>> -	/* cleanup sysfs */
>> -	sysfs_remove_group(&dock_device->dev.kobj, &dock_attribute_group);
>> -	platform_device_unregister(dock_device);
>> -
>> -	return 0;
>> -}
>> -
>> -/**
>>   * find_dock_and_bay - look for dock stations and bays
>>   * @handle: acpi handle of a device
>>   * @lvl: unused
>> @@ -1073,14 +1043,3 @@ int __init acpi_dock_init(void)
>>  		ACPI_DOCK_DRIVER_DESCRIPTION, dock_station_count);
>>  	return 0;
>>  }
>> -
>> -static void __exit dock_exit(void)
>> -{
>> -	struct dock_station *tmp, *dock_station;
>> -
>> -	unregister_acpi_bus_notifier(&dock_acpi_notifier);
>> -	list_for_each_entry_safe(dock_station, tmp, &dock_stations, sibling)
>> -		dock_remove(dock_station);
>> -}
>> -
>> -module_exit(dock_exit);
> 
> The other changes look OK to me.
Thanks for review.

> 
> Thanks,
> Rafael
> 
> 

Re: [BUGFIX 5/9] ACPI, DOCK: kill redundant spin lock in dock device object

[Jiang Liu]

On 06/14/2013 02:28 AM, Rafael J. Wysocki wrote:
> On Friday, June 14, 2013 12:32:28 AM Jiang Liu wrote:
>> All dock device related data structures are created during driver
>> initialization, so kill the redundant spin lock in dock device object.
> 
> As a cleanup, it definitely makes sense, but does it fix anything?
> 
> Rafael
Patch 3-9 are code cleanup only, I will resend them as separate patchset
later.

> 
> 
>> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
>> Cc: Shaohua Li <shaohua.li@intel.com>
>> Cc: Len Brown <lenb@kernel.org>
>> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
>> Cc: linux-acpi@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> ---
>>  drivers/acpi/dock.c | 15 +++------------
>>  1 file changed, 3 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
>> index 8e578aa..cd2d5df 100644
>> --- a/drivers/acpi/dock.c
>> +++ b/drivers/acpi/dock.c
>> @@ -57,7 +57,6 @@ struct dock_station {
>>  	acpi_handle handle;
>>  	unsigned long last_dock_time;
>>  	u32 flags;
>> -	spinlock_t dd_lock;
>>  	struct mutex hp_lock;
>>  	struct task_struct *owner;
>>  	struct list_head dependent_devices;
>> @@ -107,10 +106,7 @@ add_dock_dependent_device(struct dock_station *ds, acpi_handle handle)
>>  	dd->handle = handle;
>>  	INIT_LIST_HEAD(&dd->list);
>>  	INIT_LIST_HEAD(&dd->hotplug_list);
>> -
>> -	spin_lock(&ds->dd_lock);
>>  	list_add_tail(&dd->list, &ds->dependent_devices);
>> -	spin_unlock(&ds->dd_lock);
>>  
>>  	return 0;
>>  }
>> @@ -168,14 +164,10 @@ find_dock_dependent_device(struct dock_station *ds, acpi_handle handle)
>>  {
>>  	struct dock_dependent_device *dd;
>>  
>> -	spin_lock(&ds->dd_lock);
>> -	list_for_each_entry(dd, &ds->dependent_devices, list) {
>> -		if (handle == dd->handle) {
>> -			spin_unlock(&ds->dd_lock);
>> +	list_for_each_entry(dd, &ds->dependent_devices, list)
>> +		if (handle == dd->handle)
>>  			return dd;
>> -		}
>> -	}
>> -	spin_unlock(&ds->dd_lock);
>> +
>>  	return NULL;
>>  }
>>  
>> @@ -964,7 +956,6 @@ static int __init dock_add(acpi_handle handle)
>>  	dock_station->last_dock_time = jiffies - HZ;
>>  
>>  	mutex_init(&dock_station->hp_lock);
>> -	spin_lock_init(&dock_station->dd_lock);
>>  	INIT_LIST_HEAD(&dock_station->sibling);
>>  	INIT_LIST_HEAD(&dock_station->hotplug_devices);
>>  	INIT_LIST_HEAD(&dock_station->dependent_devices);
>>

Re: [BUGFIX 3/9] ACPI, DOCK: clean up unused module related code

[Rafael J. Wysocki]

On Friday, June 14, 2013 10:04:01 PM Jiang Liu wrote:
> On 06/14/2013 02:26 AM, Rafael J. Wysocki wrote:
> > On Friday, June 14, 2013 12:32:26 AM Jiang Liu wrote:
> >> ACPI dock driver can't be built as a module any more, so clean up
> >> module related code.
> >>
> >> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> >> Cc: Shaohua Li <shaohua.li@intel.com>
> >> Cc: Len Brown <lenb@kernel.org>
> >> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> >> Cc: linux-acpi@vger.kernel.org
> >> Cc: linux-kernel@vger.kernel.org
> > 
> > How exactly does this depend on [2/9]?  If it doesn't at all, it should go
> > after [1/9].
> > 
> >> ---
> >>  drivers/acpi/dock.c | 41 -----------------------------------------
> >>  1 file changed, 41 deletions(-)
> >>
> >> diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
> >> index 79c8d9e..50e38b7 100644
> >> --- a/drivers/acpi/dock.c
> >> +++ b/drivers/acpi/dock.c
> >> @@ -53,12 +53,6 @@ MODULE_PARM_DESC(immediate_undock, "1 (default) will cause the driver to "
> >>  
> >>  static struct atomic_notifier_head dock_notifier_list;
> >>  
> >> -static const struct acpi_device_id dock_device_ids[] = {
> >> -	{"LNXDOCK", 0},
> >> -	{"", 0},
> >> -};
> >> -MODULE_DEVICE_TABLE(acpi, dock_device_ids);
> >> -
> > 
> > Don't we actually need the device IDs?
> Now dock driver could only be built as built-in, and it doesn't really
> bind to ACPI dock devices, so I think the device ids are not used any
> more. Not sure whether any userspace tool has dependency on the device
> IDs.

I see.  OK

Thanks,
Rafael


> > 
> >>  struct dock_station {
> >>  	acpi_handle handle;
> >>  	unsigned long last_dock_time;
> >> @@ -1013,30 +1007,6 @@ err_unregister:
> >>  }
> >>  
> >>  /**
> >> - * dock_remove - free up resources related to the dock station
> >> - */
> >> -static int dock_remove(struct dock_station *ds)
> >> -{
> >> -	struct dock_dependent_device *dd, *tmp;
> >> -	struct platform_device *dock_device = ds->dock_device;
> >> -
> >> -	if (!dock_station_count)
> >> -		return 0;
> >> -
> >> -	/* remove dependent devices */
> >> -	list_for_each_entry_safe(dd, tmp, &ds->dependent_devices, list)
> >> -		kfree(dd);
> >> -
> >> -	list_del(&ds->sibling);
> >> -
> >> -	/* cleanup sysfs */
> >> -	sysfs_remove_group(&dock_device->dev.kobj, &dock_attribute_group);
> >> -	platform_device_unregister(dock_device);
> >> -
> >> -	return 0;
> >> -}
> >> -
> >> -/**
> >>   * find_dock_and_bay - look for dock stations and bays
> >>   * @handle: acpi handle of a device
> >>   * @lvl: unused
> >> @@ -1073,14 +1043,3 @@ int __init acpi_dock_init(void)
> >>  		ACPI_DOCK_DRIVER_DESCRIPTION, dock_station_count);
> >>  	return 0;
> >>  }
> >> -
> >> -static void __exit dock_exit(void)
> >> -{
> >> -	struct dock_station *tmp, *dock_station;
> >> -
> >> -	unregister_acpi_bus_notifier(&dock_acpi_notifier);
> >> -	list_for_each_entry_safe(dock_station, tmp, &dock_stations, sibling)
> >> -		dock_remove(dock_station);
> >> -}
> >> -
> >> -module_exit(dock_exit);
> > 
> > The other changes look OK to me.
> Thanks for review.
> 
> > 
> > Thanks,
> > Rafael
> > 
> > 
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [BUGFIX 5/9] ACPI, DOCK: kill redundant spin lock in dock device object

[Rafael J. Wysocki]

On Friday, June 14, 2013 10:05:47 PM Jiang Liu wrote:
> On 06/14/2013 02:28 AM, Rafael J. Wysocki wrote:
> > On Friday, June 14, 2013 12:32:28 AM Jiang Liu wrote:
> >> All dock device related data structures are created during driver
> >> initialization, so kill the redundant spin lock in dock device object.
> > 
> > As a cleanup, it definitely makes sense, but does it fix anything?
> > 
> > Rafael
> Patch 3-9 are code cleanup only, I will resend them as separate patchset
> later.

Cool, thanks!

Rafael


> >> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> >> Cc: Shaohua Li <shaohua.li@intel.com>
> >> Cc: Len Brown <lenb@kernel.org>
> >> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> >> Cc: linux-acpi@vger.kernel.org
> >> Cc: linux-kernel@vger.kernel.org
> >> ---
> >>  drivers/acpi/dock.c | 15 +++------------
> >>  1 file changed, 3 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
> >> index 8e578aa..cd2d5df 100644
> >> --- a/drivers/acpi/dock.c
> >> +++ b/drivers/acpi/dock.c
> >> @@ -57,7 +57,6 @@ struct dock_station {
> >>  	acpi_handle handle;
> >>  	unsigned long last_dock_time;
> >>  	u32 flags;
> >> -	spinlock_t dd_lock;
> >>  	struct mutex hp_lock;
> >>  	struct task_struct *owner;
> >>  	struct list_head dependent_devices;
> >> @@ -107,10 +106,7 @@ add_dock_dependent_device(struct dock_station *ds, acpi_handle handle)
> >>  	dd->handle = handle;
> >>  	INIT_LIST_HEAD(&dd->list);
> >>  	INIT_LIST_HEAD(&dd->hotplug_list);
> >> -
> >> -	spin_lock(&ds->dd_lock);
> >>  	list_add_tail(&dd->list, &ds->dependent_devices);
> >> -	spin_unlock(&ds->dd_lock);
> >>  
> >>  	return 0;
> >>  }
> >> @@ -168,14 +164,10 @@ find_dock_dependent_device(struct dock_station *ds, acpi_handle handle)
> >>  {
> >>  	struct dock_dependent_device *dd;
> >>  
> >> -	spin_lock(&ds->dd_lock);
> >> -	list_for_each_entry(dd, &ds->dependent_devices, list) {
> >> -		if (handle == dd->handle) {
> >> -			spin_unlock(&ds->dd_lock);
> >> +	list_for_each_entry(dd, &ds->dependent_devices, list)
> >> +		if (handle == dd->handle)
> >>  			return dd;
> >> -		}
> >> -	}
> >> -	spin_unlock(&ds->dd_lock);
> >> +
> >>  	return NULL;
> >>  }
> >>  
> >> @@ -964,7 +956,6 @@ static int __init dock_add(acpi_handle handle)
> >>  	dock_station->last_dock_time = jiffies - HZ;
> >>  
> >>  	mutex_init(&dock_station->hp_lock);
> >> -	spin_lock_init(&dock_station->dd_lock);
> >>  	INIT_LIST_HEAD(&dock_station->sibling);
> >>  	INIT_LIST_HEAD(&dock_station->hotplug_devices);
> >>  	INIT_LIST_HEAD(&dock_station->dependent_devices);
> >>
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.