ACPI 5.0 introduces _DEP to designate device objects that OSPM should
assign a higher priority in start ordering due to future operation region
accesses.
On Asus T100TA, ACPI battery info are read from a I2C slave device via
I2C operation region. Before I2C operation region handler is installed,
battery _STA always returns 0. There is a _DEP method of designating
start order under battery device node.
This patch is to implement _DEP feature to fix battery issue on the Asus T100TA.
Introducing acpi_dep_list and adding dep_unmet count in the struct
acpi_device. During ACPI namespace scan, create struct acpi_dep_data for a
valid pair of master (device pointed to by _DEP)/slave(device with _DEP), record
master's and slave's ACPI handle in it and put it into acpi_dep_list. The dep_unmet
count will increase by one if there is a device under its _DEP. Driver's probe() should
return EPROBE_DEFER when find dep_unmet is larger than 0. When I2C operation
region handler is installed, remove all struct acpi_dep_data on the acpi_dep_list
whose master is pointed to I2C host controller and decrease slave's dep_unmet.
When dep_unmet decreases to 0, all _DEP conditions are met and then do acpi_bus_attach()
for the device in order to resolve battery _STA issue on the Asus T100TA.
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=69011
Tested-by: Jan-Michael Brummer <[email protected]>
Tested-by: Adam Williamson <[email protected]>
Tested-by: Michael Shigorin <[email protected]>
Acked-by: Wolfram Sang <[email protected]>
Acked-by: Mika Westerberg <[email protected]>
Signed-off-by: Lan Tianyu <[email protected]>
---
Change since V2:
Coding style change
Change since V1:
Remove redundant blank line and some coding style fixs.
drivers/acpi/battery.c | 4 +++
drivers/acpi/scan.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
drivers/i2c/i2c-core.c | 1 +
include/acpi/acpi_bus.h | 1 +
include/linux/acpi.h | 1 +
5 files changed, 92 insertions(+)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 8ec8a89..d98ba43 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -1180,6 +1180,10 @@ static int acpi_battery_add(struct acpi_device *device)
if (!device)
return -EINVAL;
+
+ if (device->dep_unmet)
+ return -EPROBE_DEFER;
+
battery = kzalloc(sizeof(struct acpi_battery), GFP_KERNEL);
if (!battery)
return -ENOMEM;
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 9cb5cca..1b1cf55 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -36,6 +36,8 @@ bool acpi_force_hot_remove;
static const char *dummy_hid = "device";
+static LIST_HEAD(acpi_dep_list);
+static DEFINE_MUTEX(acpi_dep_list_lock);
static LIST_HEAD(acpi_bus_id_list);
static DEFINE_MUTEX(acpi_scan_lock);
static LIST_HEAD(acpi_scan_handlers_list);
@@ -43,6 +45,12 @@ DEFINE_MUTEX(acpi_device_lock);
LIST_HEAD(acpi_wakeup_device_list);
static DEFINE_MUTEX(acpi_hp_context_lock);
+struct acpi_dep_data {
+ struct list_head node;
+ acpi_handle master;
+ acpi_handle slave;
+};
+
struct acpi_device_bus_id{
char bus_id[15];
unsigned int instance_no;
@@ -2193,6 +2201,59 @@ static void acpi_scan_init_hotplug(struct acpi_device *adev)
}
}
+static void acpi_device_dep_initialize(struct acpi_device *adev)
+{
+ struct acpi_dep_data *dep;
+ struct acpi_handle_list dep_devices;
+ acpi_status status;
+ int i;
+
+ if (!acpi_has_method(adev->handle, "_DEP"))
+ return;
+
+ status = acpi_evaluate_reference(adev->handle, "_DEP", NULL,
+ &dep_devices);
+ if (ACPI_FAILURE(status)) {
+ dev_err(&adev->dev, "Failed to evaluate _DEP.\n");
+ return;
+ }
+
+ for (i = 0; i < dep_devices.count; i++) {
+ struct acpi_device_info *info;
+ int skip;
+
+ status = acpi_get_object_info(dep_devices.handles[i], &info);
+ if (ACPI_FAILURE(status)) {
+ dev_err(&adev->dev, "Error reading device info\n");
+ continue;
+ }
+
+ /*
+ * Skip the dependency of Windows System Power
+ * Management Controller
+ */
+ skip = info->valid & ACPI_VALID_HID &&
+ !strcmp(info->hardware_id.string, "INT3396");
+
+ kfree(info);
+
+ if (skip)
+ continue;
+
+ dep = kzalloc(sizeof(struct acpi_dep_data), GFP_KERNEL);
+ if (!dep)
+ return;
+
+ dep->master = dep_devices.handles[i];
+ dep->slave = adev->handle;
+ adev->dep_unmet++;
+
+ mutex_lock(&acpi_dep_list_lock);
+ list_add_tail(&dep->node , &acpi_dep_list);
+ mutex_unlock(&acpi_dep_list_lock);
+ }
+}
+
static acpi_status acpi_bus_check_add(acpi_handle handle, u32 lvl_not_used,
void *not_used, void **return_value)
{
@@ -2219,6 +2280,7 @@ static acpi_status acpi_bus_check_add(acpi_handle handle, u32 lvl_not_used,
return AE_CTRL_DEPTH;
acpi_scan_init_hotplug(device);
+ acpi_device_dep_initialize(device);
out:
if (!*return_value)
@@ -2339,6 +2401,29 @@ static void acpi_bus_attach(struct acpi_device *device)
device->handler->hotplug.notify_online(device);
}
+void acpi_walk_dep_device_list(acpi_handle handle)
+{
+ struct acpi_dep_data *dep, *tmp;
+ struct acpi_device *adev;
+
+ mutex_lock(&acpi_dep_list_lock);
+ list_for_each_entry_safe(dep, tmp, &acpi_dep_list, node) {
+ if (dep->master == handle) {
+ acpi_bus_get_device(dep->slave, &adev);
+ if (!adev)
+ continue;
+
+ adev->dep_unmet--;
+ if (!adev->dep_unmet)
+ acpi_bus_attach(adev);
+ list_del(&dep->node);
+ kfree(dep);
+ }
+ }
+ mutex_unlock(&acpi_dep_list_lock);
+}
+EXPORT_SYMBOL_GPL(acpi_walk_dep_device_list);
+
/**
* acpi_bus_scan - Add ACPI device node objects in a given namespace scope.
* @handle: Root of the namespace scope to scan.
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index f43b4e1..68aeb8e 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -403,6 +403,7 @@ static int acpi_i2c_install_space_handler(struct i2c_adapter *adapter)
return -ENOMEM;
}
+ acpi_walk_dep_device_list(handle);
return 0;
}
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 7d1ce40..d539084 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -372,6 +372,7 @@ struct acpi_device {
void *driver_data;
struct device dev;
unsigned int physical_node_count;
+ unsigned int dep_unmet;
struct list_head physical_node_list;
struct mutex physical_node_lock;
void (*remove)(struct acpi_device *);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 10f2ed9..e09f7f4 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -428,6 +428,7 @@ extern bool acpi_driver_match_device(struct device *dev,
const struct device_driver *drv);
int acpi_device_uevent_modalias(struct device *, struct kobj_uevent_env *);
int acpi_device_modalias(struct device *, char *, int);
+void acpi_walk_dep_device_list(acpi_handle handle);
struct platform_device *acpi_create_platform_device(struct acpi_device *);
#define ACPI_PTR(_ptr) (_ptr)
--
1.8.4.rc0.1.g8f6a3e5.dirty
On 2014/11/23 21:22, Lan Tianyu wrote:
> ACPI 5.0 introduces _DEP to designate device objects that OSPM should
> assign a higher priority in start ordering due to future operation region
> accesses.
>
> On Asus T100TA, ACPI battery info are read from a I2C slave device via
> I2C operation region. Before I2C operation region handler is installed,
> battery _STA always returns 0. There is a _DEP method of designating
> start order under battery device node.
>
> This patch is to implement _DEP feature to fix battery issue on the Asus T100TA.
> Introducing acpi_dep_list and adding dep_unmet count in the struct
> acpi_device. During ACPI namespace scan, create struct acpi_dep_data for a
> valid pair of master (device pointed to by _DEP)/slave(device with _DEP), record
> master's and slave's ACPI handle in it and put it into acpi_dep_list. The dep_unmet
> count will increase by one if there is a device under its _DEP. Driver's probe() should
> return EPROBE_DEFER when find dep_unmet is larger than 0. When I2C operation
> region handler is installed, remove all struct acpi_dep_data on the acpi_dep_list
> whose master is pointed to I2C host controller and decrease slave's dep_unmet.
> When dep_unmet decreases to 0, all _DEP conditions are met and then do acpi_bus_attach()
> for the device in order to resolve battery _STA issue on the Asus T100TA.
Well, Can we explicitly tied this up with ASUS T100TA in the code?
If I understand correctly, the assumption in the patch is that the
battery device only depends on I2C device, which is true on ASUS T100TA,
but may not on the other platforms.
This patch does not work on a box I have, on it _DEP contains I2C and GPIO.
Device (BATC)
{
Name (_HID, EisaId ("PNP0C0A")) // _HID: Hardware ID
--------snip--------
Name (_DEP, Package (0x03) // _DEP: Dependencies
{
I2C1,
GPO2,
GPO0
})
Thanks,
-Aubrey
>
> Reference: https://bugzilla.kernel.org/show_bug.cgi?id=69011
> Tested-by: Jan-Michael Brummer <[email protected]>
> Tested-by: Adam Williamson <[email protected]>
> Tested-by: Michael Shigorin <[email protected]>
> Acked-by: Wolfram Sang <[email protected]>
> Acked-by: Mika Westerberg <[email protected]>
> Signed-off-by: Lan Tianyu <[email protected]>
> ---
> Change since V2:
> Coding style change
> Change since V1:
> Remove redundant blank line and some coding style fixs.
>
>
> drivers/acpi/battery.c | 4 +++
> drivers/acpi/scan.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
> drivers/i2c/i2c-core.c | 1 +
> include/acpi/acpi_bus.h | 1 +
> include/linux/acpi.h | 1 +
> 5 files changed, 92 insertions(+)
>
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index 8ec8a89..d98ba43 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -1180,6 +1180,10 @@ static int acpi_battery_add(struct acpi_device *device)
>
> if (!device)
> return -EINVAL;
> +
> + if (device->dep_unmet)
> + return -EPROBE_DEFER;
> +
> battery = kzalloc(sizeof(struct acpi_battery), GFP_KERNEL);
> if (!battery)
> return -ENOMEM;
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 9cb5cca..1b1cf55 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -36,6 +36,8 @@ bool acpi_force_hot_remove;
>
> static const char *dummy_hid = "device";
>
> +static LIST_HEAD(acpi_dep_list);
> +static DEFINE_MUTEX(acpi_dep_list_lock);
> static LIST_HEAD(acpi_bus_id_list);
> static DEFINE_MUTEX(acpi_scan_lock);
> static LIST_HEAD(acpi_scan_handlers_list);
> @@ -43,6 +45,12 @@ DEFINE_MUTEX(acpi_device_lock);
> LIST_HEAD(acpi_wakeup_device_list);
> static DEFINE_MUTEX(acpi_hp_context_lock);
>
> +struct acpi_dep_data {
> + struct list_head node;
> + acpi_handle master;
> + acpi_handle slave;
> +};
> +
> struct acpi_device_bus_id{
> char bus_id[15];
> unsigned int instance_no;
> @@ -2193,6 +2201,59 @@ static void acpi_scan_init_hotplug(struct acpi_device *adev)
> }
> }
>
> +static void acpi_device_dep_initialize(struct acpi_device *adev)
> +{
> + struct acpi_dep_data *dep;
> + struct acpi_handle_list dep_devices;
> + acpi_status status;
> + int i;
> +
> + if (!acpi_has_method(adev->handle, "_DEP"))
> + return;
> +
> + status = acpi_evaluate_reference(adev->handle, "_DEP", NULL,
> + &dep_devices);
> + if (ACPI_FAILURE(status)) {
> + dev_err(&adev->dev, "Failed to evaluate _DEP.\n");
> + return;
> + }
> +
> + for (i = 0; i < dep_devices.count; i++) {
> + struct acpi_device_info *info;
> + int skip;
> +
> + status = acpi_get_object_info(dep_devices.handles[i], &info);
> + if (ACPI_FAILURE(status)) {
> + dev_err(&adev->dev, "Error reading device info\n");
> + continue;
> + }
> +
> + /*
> + * Skip the dependency of Windows System Power
> + * Management Controller
> + */
> + skip = info->valid & ACPI_VALID_HID &&
> + !strcmp(info->hardware_id.string, "INT3396");
> +
> + kfree(info);
> +
> + if (skip)
> + continue;
> +
> + dep = kzalloc(sizeof(struct acpi_dep_data), GFP_KERNEL);
> + if (!dep)
> + return;
> +
> + dep->master = dep_devices.handles[i];
> + dep->slave = adev->handle;
> + adev->dep_unmet++;
> +
> + mutex_lock(&acpi_dep_list_lock);
> + list_add_tail(&dep->node , &acpi_dep_list);
> + mutex_unlock(&acpi_dep_list_lock);
> + }
> +}
> +
> static acpi_status acpi_bus_check_add(acpi_handle handle, u32 lvl_not_used,
> void *not_used, void **return_value)
> {
> @@ -2219,6 +2280,7 @@ static acpi_status acpi_bus_check_add(acpi_handle handle, u32 lvl_not_used,
> return AE_CTRL_DEPTH;
>
> acpi_scan_init_hotplug(device);
> + acpi_device_dep_initialize(device);
>
> out:
> if (!*return_value)
> @@ -2339,6 +2401,29 @@ static void acpi_bus_attach(struct acpi_device *device)
> device->handler->hotplug.notify_online(device);
> }
>
> +void acpi_walk_dep_device_list(acpi_handle handle)
> +{
> + struct acpi_dep_data *dep, *tmp;
> + struct acpi_device *adev;
> +
> + mutex_lock(&acpi_dep_list_lock);
> + list_for_each_entry_safe(dep, tmp, &acpi_dep_list, node) {
> + if (dep->master == handle) {
> + acpi_bus_get_device(dep->slave, &adev);
> + if (!adev)
> + continue;
> +
> + adev->dep_unmet--;
> + if (!adev->dep_unmet)
> + acpi_bus_attach(adev);
> + list_del(&dep->node);
> + kfree(dep);
> + }
> + }
> + mutex_unlock(&acpi_dep_list_lock);
> +}
> +EXPORT_SYMBOL_GPL(acpi_walk_dep_device_list);
> +
> /**
> * acpi_bus_scan - Add ACPI device node objects in a given namespace scope.
> * @handle: Root of the namespace scope to scan.
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index f43b4e1..68aeb8e 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -403,6 +403,7 @@ static int acpi_i2c_install_space_handler(struct i2c_adapter *adapter)
> return -ENOMEM;
> }
>
> + acpi_walk_dep_device_list(handle);
> return 0;
> }
>
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 7d1ce40..d539084 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -372,6 +372,7 @@ struct acpi_device {
> void *driver_data;
> struct device dev;
> unsigned int physical_node_count;
> + unsigned int dep_unmet;
> struct list_head physical_node_list;
> struct mutex physical_node_lock;
> void (*remove)(struct acpi_device *);
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 10f2ed9..e09f7f4 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -428,6 +428,7 @@ extern bool acpi_driver_match_device(struct device *dev,
> const struct device_driver *drv);
> int acpi_device_uevent_modalias(struct device *, struct kobj_uevent_env *);
> int acpi_device_modalias(struct device *, char *, int);
> +void acpi_walk_dep_device_list(acpi_handle handle);
>
> struct platform_device *acpi_create_platform_device(struct acpi_device *);
> #define ACPI_PTR(_ptr) (_ptr)
>
On Mon, 2014-11-24 at 10:39 +0800, Li, Aubrey wrote:
> On 2014/11/23 21:22, Lan Tianyu wrote:
> > ACPI 5.0 introduces _DEP to designate device objects that OSPM should
> > assign a higher priority in start ordering due to future operation region
> > accesses.
> >
> > On Asus T100TA, ACPI battery info are read from a I2C slave device via
> > I2C operation region. Before I2C operation region handler is installed,
> > battery _STA always returns 0. There is a _DEP method of designating
> > start order under battery device node.
> >
> > This patch is to implement _DEP feature to fix battery issue on the Asus T100TA.
> > Introducing acpi_dep_list and adding dep_unmet count in the struct
> > acpi_device. During ACPI namespace scan, create struct acpi_dep_data for a
> > valid pair of master (device pointed to by _DEP)/slave(device with _DEP), record
> > master's and slave's ACPI handle in it and put it into acpi_dep_list. The dep_unmet
> > count will increase by one if there is a device under its _DEP. Driver's probe() should
> > return EPROBE_DEFER when find dep_unmet is larger than 0. When I2C operation
> > region handler is installed, remove all struct acpi_dep_data on the acpi_dep_list
> > whose master is pointed to I2C host controller and decrease slave's dep_unmet.
> > When dep_unmet decreases to 0, all _DEP conditions are met and then do acpi_bus_attach()
> > for the device in order to resolve battery _STA issue on the Asus T100TA.
>
> Well, Can we explicitly tied this up with ASUS T100TA in the code?
> If I understand correctly, the assumption in the patch is that the
> battery device only depends on I2C device, which is true on ASUS T100TA,
> but may not on the other platforms.
>
> This patch does not work on a box I have, on it _DEP contains I2C and GPIO.
>
> Device (BATC)
> {
> Name (_HID, EisaId ("PNP0C0A")) // _HID: Hardware ID
> --------snip--------
> Name (_DEP, Package (0x03) // _DEP: Dependencies
> {
> I2C1,
> GPO2,
> GPO0
> })
>
> Thanks,
> -Aubrey
It certainly works for more than *just* the T100 - fedlet users have
reported working battery status on at least the Miix 2 and Venue 8 Pro
(I can personally confirm it works on the Venue 8 Pro). However, Bastien
Nocera reported failure on an Onda v975w with a slightly earlier version
of the patch in the bug report -
https://bugzilla.kernel.org/show_bug.cgi?id=69011#c58 , "Tested with the
same patch on a Onda v975w, and it tries very hard to detect the battery
but fails."
--
Adam Williamson
Fedora QA Community Monkey
IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT happyassassin . net
http://www.happyassassin.net
On Mon, Nov 24, 2014 at 10:39:55AM +0800, Li, Aubrey wrote:
> Well, Can we explicitly tied this up with ASUS T100TA in the code?
It works for me on Aava Inari 8 too.
--
?---- WBR, Michael Shigorin / http://altlinux.org
??------ http://opennet.ru / http://anna-news.info
On Monday, November 24, 2014 10:39:55 AM Li, Aubrey wrote:
> On 2014/11/23 21:22, Lan Tianyu wrote:
> > ACPI 5.0 introduces _DEP to designate device objects that OSPM should
> > assign a higher priority in start ordering due to future operation region
> > accesses.
> >
> > On Asus T100TA, ACPI battery info are read from a I2C slave device via
> > I2C operation region. Before I2C operation region handler is installed,
> > battery _STA always returns 0. There is a _DEP method of designating
> > start order under battery device node.
> >
> > This patch is to implement _DEP feature to fix battery issue on the Asus T100TA.
> > Introducing acpi_dep_list and adding dep_unmet count in the struct
> > acpi_device. During ACPI namespace scan, create struct acpi_dep_data for a
> > valid pair of master (device pointed to by _DEP)/slave(device with _DEP), record
> > master's and slave's ACPI handle in it and put it into acpi_dep_list. The dep_unmet
> > count will increase by one if there is a device under its _DEP. Driver's probe() should
> > return EPROBE_DEFER when find dep_unmet is larger than 0. When I2C operation
> > region handler is installed, remove all struct acpi_dep_data on the acpi_dep_list
> > whose master is pointed to I2C host controller and decrease slave's dep_unmet.
> > When dep_unmet decreases to 0, all _DEP conditions are met and then do acpi_bus_attach()
> > for the device in order to resolve battery _STA issue on the Asus T100TA.
>
> Well, Can we explicitly tied this up with ASUS T100TA in the code?
> If I understand correctly, the assumption in the patch is that the
> battery device only depends on I2C device, which is true on ASUS T100TA,
> but may not on the other platforms.
>
> This patch does not work on a box I have, on it _DEP contains I2C and GPIO.
Does the patch break anything for you?
> Device (BATC)
> {
> Name (_HID, EisaId ("PNP0C0A")) // _HID: Hardware ID
> --------snip--------
> Name (_DEP, Package (0x03) // _DEP: Dependencies
> {
> I2C1,
> GPO2,
> GPO0
> })
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
On 2014/11/24 23:27, Rafael J. Wysocki wrote:
> On Monday, November 24, 2014 10:39:55 AM Li, Aubrey wrote:
>> On 2014/11/23 21:22, Lan Tianyu wrote:
>>> ACPI 5.0 introduces _DEP to designate device objects that OSPM should
>>> assign a higher priority in start ordering due to future operation region
>>> accesses.
>>>
>>> On Asus T100TA, ACPI battery info are read from a I2C slave device via
>>> I2C operation region. Before I2C operation region handler is installed,
>>> battery _STA always returns 0. There is a _DEP method of designating
>>> start order under battery device node.
>>>
>>> This patch is to implement _DEP feature to fix battery issue on the Asus T100TA.
>>> Introducing acpi_dep_list and adding dep_unmet count in the struct
>>> acpi_device. During ACPI namespace scan, create struct acpi_dep_data for a
>>> valid pair of master (device pointed to by _DEP)/slave(device with _DEP), record
>>> master's and slave's ACPI handle in it and put it into acpi_dep_list. The dep_unmet
>>> count will increase by one if there is a device under its _DEP. Driver's probe() should
>>> return EPROBE_DEFER when find dep_unmet is larger than 0. When I2C operation
>>> region handler is installed, remove all struct acpi_dep_data on the acpi_dep_list
>>> whose master is pointed to I2C host controller and decrease slave's dep_unmet.
>>> When dep_unmet decreases to 0, all _DEP conditions are met and then do acpi_bus_attach()
>>> for the device in order to resolve battery _STA issue on the Asus T100TA.
>>
>> Well, Can we explicitly tied this up with ASUS T100TA in the code?
>> If I understand correctly, the assumption in the patch is that the
>> battery device only depends on I2C device, which is true on ASUS T100TA,
>> but may not on the other platforms.
>>
>> This patch does not work on a box I have, on it _DEP contains I2C and GPIO.
>
> Does the patch break anything for you?
Not I'm aware so far.
>
>> Device (BATC)
>> {
>> Name (_HID, EisaId ("PNP0C0A")) // _HID: Hardware ID
>> --------snip--------
>> Name (_DEP, Package (0x03) // _DEP: Dependencies
>> {
>> I2C1,
>> GPO2,
>> GPO0
>> })
>>
>
On Tuesday, November 25, 2014 08:18:34 AM Li, Aubrey wrote:
> On 2014/11/24 23:27, Rafael J. Wysocki wrote:
> > On Monday, November 24, 2014 10:39:55 AM Li, Aubrey wrote:
> >> On 2014/11/23 21:22, Lan Tianyu wrote:
> >>> ACPI 5.0 introduces _DEP to designate device objects that OSPM should
> >>> assign a higher priority in start ordering due to future operation region
> >>> accesses.
> >>>
> >>> On Asus T100TA, ACPI battery info are read from a I2C slave device via
> >>> I2C operation region. Before I2C operation region handler is installed,
> >>> battery _STA always returns 0. There is a _DEP method of designating
> >>> start order under battery device node.
> >>>
> >>> This patch is to implement _DEP feature to fix battery issue on the Asus T100TA.
> >>> Introducing acpi_dep_list and adding dep_unmet count in the struct
> >>> acpi_device. During ACPI namespace scan, create struct acpi_dep_data for a
> >>> valid pair of master (device pointed to by _DEP)/slave(device with _DEP), record
> >>> master's and slave's ACPI handle in it and put it into acpi_dep_list. The dep_unmet
> >>> count will increase by one if there is a device under its _DEP. Driver's probe() should
> >>> return EPROBE_DEFER when find dep_unmet is larger than 0. When I2C operation
> >>> region handler is installed, remove all struct acpi_dep_data on the acpi_dep_list
> >>> whose master is pointed to I2C host controller and decrease slave's dep_unmet.
> >>> When dep_unmet decreases to 0, all _DEP conditions are met and then do acpi_bus_attach()
> >>> for the device in order to resolve battery _STA issue on the Asus T100TA.
> >>
> >> Well, Can we explicitly tied this up with ASUS T100TA in the code?
> >> If I understand correctly, the assumption in the patch is that the
> >> battery device only depends on I2C device, which is true on ASUS T100TA,
> >> but may not on the other platforms.
> >>
> >> This patch does not work on a box I have, on it _DEP contains I2C and GPIO.
> >
> > Does the patch break anything for you?
>
> Not I'm aware so far.
OK
It is not a complete solution, but it is an improvement. It evidently helps
people.
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
On Sunday, November 23, 2014 09:22:54 PM Lan Tianyu wrote:
> ACPI 5.0 introduces _DEP to designate device objects that OSPM should
> assign a higher priority in start ordering due to future operation region
> accesses.
>
> On Asus T100TA, ACPI battery info are read from a I2C slave device via
> I2C operation region. Before I2C operation region handler is installed,
> battery _STA always returns 0. There is a _DEP method of designating
> start order under battery device node.
>
> This patch is to implement _DEP feature to fix battery issue on the Asus T100TA.
> Introducing acpi_dep_list and adding dep_unmet count in the struct
> acpi_device. During ACPI namespace scan, create struct acpi_dep_data for a
> valid pair of master (device pointed to by _DEP)/slave(device with _DEP), record
> master's and slave's ACPI handle in it and put it into acpi_dep_list. The dep_unmet
> count will increase by one if there is a device under its _DEP. Driver's probe() should
> return EPROBE_DEFER when find dep_unmet is larger than 0. When I2C operation
> region handler is installed, remove all struct acpi_dep_data on the acpi_dep_list
> whose master is pointed to I2C host controller and decrease slave's dep_unmet.
> When dep_unmet decreases to 0, all _DEP conditions are met and then do acpi_bus_attach()
> for the device in order to resolve battery _STA issue on the Asus T100TA.
>
> Reference: https://bugzilla.kernel.org/show_bug.cgi?id=69011
> Tested-by: Jan-Michael Brummer <[email protected]>
> Tested-by: Adam Williamson <[email protected]>
> Tested-by: Michael Shigorin <[email protected]>
> Acked-by: Wolfram Sang <[email protected]>
> Acked-by: Mika Westerberg <[email protected]>
> Signed-off-by: Lan Tianyu <[email protected]>
Queued up for 3.19-rc1, thanks!
> ---
> Change since V2:
> Coding style change
> Change since V1:
> Remove redundant blank line and some coding style fixs.
>
>
> drivers/acpi/battery.c | 4 +++
> drivers/acpi/scan.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
> drivers/i2c/i2c-core.c | 1 +
> include/acpi/acpi_bus.h | 1 +
> include/linux/acpi.h | 1 +
> 5 files changed, 92 insertions(+)
>
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index 8ec8a89..d98ba43 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -1180,6 +1180,10 @@ static int acpi_battery_add(struct acpi_device *device)
>
> if (!device)
> return -EINVAL;
> +
> + if (device->dep_unmet)
> + return -EPROBE_DEFER;
> +
> battery = kzalloc(sizeof(struct acpi_battery), GFP_KERNEL);
> if (!battery)
> return -ENOMEM;
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 9cb5cca..1b1cf55 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -36,6 +36,8 @@ bool acpi_force_hot_remove;
>
> static const char *dummy_hid = "device";
>
> +static LIST_HEAD(acpi_dep_list);
> +static DEFINE_MUTEX(acpi_dep_list_lock);
> static LIST_HEAD(acpi_bus_id_list);
> static DEFINE_MUTEX(acpi_scan_lock);
> static LIST_HEAD(acpi_scan_handlers_list);
> @@ -43,6 +45,12 @@ DEFINE_MUTEX(acpi_device_lock);
> LIST_HEAD(acpi_wakeup_device_list);
> static DEFINE_MUTEX(acpi_hp_context_lock);
>
> +struct acpi_dep_data {
> + struct list_head node;
> + acpi_handle master;
> + acpi_handle slave;
> +};
> +
> struct acpi_device_bus_id{
> char bus_id[15];
> unsigned int instance_no;
> @@ -2193,6 +2201,59 @@ static void acpi_scan_init_hotplug(struct acpi_device *adev)
> }
> }
>
> +static void acpi_device_dep_initialize(struct acpi_device *adev)
> +{
> + struct acpi_dep_data *dep;
> + struct acpi_handle_list dep_devices;
> + acpi_status status;
> + int i;
> +
> + if (!acpi_has_method(adev->handle, "_DEP"))
> + return;
> +
> + status = acpi_evaluate_reference(adev->handle, "_DEP", NULL,
> + &dep_devices);
> + if (ACPI_FAILURE(status)) {
> + dev_err(&adev->dev, "Failed to evaluate _DEP.\n");
> + return;
> + }
> +
> + for (i = 0; i < dep_devices.count; i++) {
> + struct acpi_device_info *info;
> + int skip;
> +
> + status = acpi_get_object_info(dep_devices.handles[i], &info);
> + if (ACPI_FAILURE(status)) {
> + dev_err(&adev->dev, "Error reading device info\n");
> + continue;
> + }
> +
> + /*
> + * Skip the dependency of Windows System Power
> + * Management Controller
> + */
> + skip = info->valid & ACPI_VALID_HID &&
> + !strcmp(info->hardware_id.string, "INT3396");
> +
> + kfree(info);
> +
> + if (skip)
> + continue;
> +
> + dep = kzalloc(sizeof(struct acpi_dep_data), GFP_KERNEL);
> + if (!dep)
> + return;
> +
> + dep->master = dep_devices.handles[i];
> + dep->slave = adev->handle;
> + adev->dep_unmet++;
> +
> + mutex_lock(&acpi_dep_list_lock);
> + list_add_tail(&dep->node , &acpi_dep_list);
> + mutex_unlock(&acpi_dep_list_lock);
> + }
> +}
> +
> static acpi_status acpi_bus_check_add(acpi_handle handle, u32 lvl_not_used,
> void *not_used, void **return_value)
> {
> @@ -2219,6 +2280,7 @@ static acpi_status acpi_bus_check_add(acpi_handle handle, u32 lvl_not_used,
> return AE_CTRL_DEPTH;
>
> acpi_scan_init_hotplug(device);
> + acpi_device_dep_initialize(device);
>
> out:
> if (!*return_value)
> @@ -2339,6 +2401,29 @@ static void acpi_bus_attach(struct acpi_device *device)
> device->handler->hotplug.notify_online(device);
> }
>
> +void acpi_walk_dep_device_list(acpi_handle handle)
> +{
> + struct acpi_dep_data *dep, *tmp;
> + struct acpi_device *adev;
> +
> + mutex_lock(&acpi_dep_list_lock);
> + list_for_each_entry_safe(dep, tmp, &acpi_dep_list, node) {
> + if (dep->master == handle) {
> + acpi_bus_get_device(dep->slave, &adev);
> + if (!adev)
> + continue;
> +
> + adev->dep_unmet--;
> + if (!adev->dep_unmet)
> + acpi_bus_attach(adev);
> + list_del(&dep->node);
> + kfree(dep);
> + }
> + }
> + mutex_unlock(&acpi_dep_list_lock);
> +}
> +EXPORT_SYMBOL_GPL(acpi_walk_dep_device_list);
> +
> /**
> * acpi_bus_scan - Add ACPI device node objects in a given namespace scope.
> * @handle: Root of the namespace scope to scan.
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index f43b4e1..68aeb8e 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -403,6 +403,7 @@ static int acpi_i2c_install_space_handler(struct i2c_adapter *adapter)
> return -ENOMEM;
> }
>
> + acpi_walk_dep_device_list(handle);
> return 0;
> }
>
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 7d1ce40..d539084 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -372,6 +372,7 @@ struct acpi_device {
> void *driver_data;
> struct device dev;
> unsigned int physical_node_count;
> + unsigned int dep_unmet;
> struct list_head physical_node_list;
> struct mutex physical_node_lock;
> void (*remove)(struct acpi_device *);
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 10f2ed9..e09f7f4 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -428,6 +428,7 @@ extern bool acpi_driver_match_device(struct device *dev,
> const struct device_driver *drv);
> int acpi_device_uevent_modalias(struct device *, struct kobj_uevent_env *);
> int acpi_device_modalias(struct device *, char *, int);
> +void acpi_walk_dep_device_list(acpi_handle handle);
>
> struct platform_device *acpi_create_platform_device(struct acpi_device *);
> #define ACPI_PTR(_ptr) (_ptr)
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.