linux-kernel - Re: [RFC PATCH] ACPI: Add _DEP(Operation Region Dependencies) support to fix battery issue on the Asus T100TA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5424F851.1020809@intel.com>
Date:	Fri, 26 Sep 2014 13:23:29 +0800
From:	Lan Tianyu <tianyu.lan@...el.com>
To:	"Rafael J. Wysocki" <rjw@...ysocki.net>
CC:	lenb@...nel.org, mika.westerberg@...ux.intel.com,
	wsa@...-dreams.de, robert.moore@...el.com, lv.zheng@...el.com,
	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-i2c@...r.kernel.org
Subject: Re: [RFC PATCH] ACPI: Add _DEP(Operation Region Dependencies) support
 to fix battery issue on the Asus T100TA

On 2014年09月26日 03:27, Rafael J. Wysocki wrote:
> I'm not sure what you mean.  "Dependent" means "depending on something", so the
> question reads "This requires the devices with _DEP to have a list of devices
> that depend on them" which is probably not what you meant.
> 

Sorry, I didn't say clearly. The "dependent device" I meant is device
pointed to by _DEP(the master you mentioned at the bottom). I thought
master also needed a list to find its slave(device with _DEP).

> For each device with _DEP we have dep_devices, so if you pass a pointer
> (opregion_adev) to the device that has just installed an operation region
> handler to acpi_walk_dep_device_list() as an argument, then you can do
> 
> 	for (i = 0; i < adev->dep_devices.count; i++)
> 		if (opregion_adev->handle == adev->dep_devices.handles[i]) {
> 			adev->dep_unmet = false;
> 			acpi_bus_attach(adev);
> 			list_del(&dep->node);
> 			kfree(dep);
> 		}
> 
> and of course appropriate locking needs to be there in case this races with
> enumeration during hotplug after loading a new ACPI table on demand).
> 

Yes, we can scan all devices on the list and match the opregion_adev
with adev->dep_devices. This is comparatively simple solution.

> I think you can even define
>   
> struct acpi_dep_data {
> 	struct list_head node;
> 	struct acpi_device *master;
> 	struct acpi_device *slave;
> };
> 
> and create that for every valid pair of master (device pointed to by _DEP)/slave
> (device with _DEP) and create a list of these.  Then, you won't need dep_devices
> in struct acpi_device any more and your acpi_walk_dep_device_list() will only
> need to walk the list until it finds the matching master/slave pair.

One question is that when create struct acpi_dep_data for the dependency
relationship between master and slave. If do this when slave's ACPI
device is created during ACPI namespace scan, master's ACPI device maybe
not created at that point. So acpi_handle maybe more suitable than
struct acpi_device here.

> 
> That will handle the case when one device depends on multiple other devices too
> I think.
> 
>>>> > >> +		dep_adev = acpi_bus_get_acpi_device(
>>>> > >> +				adev->dep_devices.handles[i]);
>> > 


-- 
Best regards
Tianyu Lan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/