Re: Port power off: ACPI _PLD and other ideas
On Thu, 9 Aug 2012, Lan Tianyu wrote: > > Alan also suggested that the default for unconnectable USB ports should > > be the "auto" setting instead of the "on" setting. His point was that > > it's hard to add more aggressive power savings later on, and we should > > just turn on the power savings and see if anyone complains. Then we'll > > get more real user testing with systems that have ACPI tables from other > > BIOS vendors, and we can see if the ACPI tables are sane. > I think we needs others' opinions. > hi Grep,Alan,Oliver: > Do you have some comments? I think Alan's suggestion would be okay. The most likely way it could fail is if a supposedly unconnectable port actually does have a device connected, which we will test for. Also, if the default turns out to be wrong on some systems, users can switch the power policy back to "on" by hand ... until the default is fixed. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Port power off: ACPI _PLD and other ideas
On 2012/8/9 5:30, Moore, Robert wrote:
I have a comment about the existing _PLD support in Linux.
It uses bitfields, and these are known to not be reliable when using them to
extract bits from a predefined bit-packed data structure. (different compilers
implement bitfields differently, and there are also endian considerations.)
We are working on a patch to remove these bitfield definitions and also create
a function that returns a useable struct from _PLD.
OK. So our work should base on your new patchs.
Bob
+struct acpi_pld {
+ unsigned int revision:7; /* 0 */
+ unsigned int ignore_colour:1; /* 7 */
+ unsigned int colour:24; /* 8 */
--
Best Regards
Tianyu Lan
linux kernel enabling team
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Port power off: ACPI _PLD and other ideas
On 2012/8/9 5:22, Sarah Sharp wrote:
Hi Tianyu,
I was talking to Alan Cox about the port power off mechanism, and he
suggested it might be good to expose the power resource information to
userspace via sysfs. Then the user could figure out which of their
devices are connected to which power resources. For example, users
could move devices that we can't safely power off to the same power
resource, instead of connecting them to several power resources.
Ok. But the name of power resource is defined by the bios randomly,
usr space app should do something to identify, categorize and show
this to user. Current the power resource is acpi specific. Is there
a generic way to present it or we should add power resource architecture
in the usb core?
Alan also suggested that the default for unconnectable USB ports should
be the "auto" setting instead of the "on" setting. His point was that
it's hard to add more aggressive power savings later on, and we should
just turn on the power savings and see if anyone complains. Then we'll
get more real user testing with systems that have ACPI tables from other
BIOS vendors, and we can see if the ACPI tables are sane.
I think we needs others' opinions.
hi Grep,Alan,Oliver:
Do you have some comments?
Also, Bob Moore pointed out that we could use the _PLD ACPI method to
give users a better idea of the physical location of ports associated
with a power resource. I note that this patch adds support for the
physical device location data. Is that exposed to userspace via sysfs?
No. This is only used to identify whether the device is removable and
export to user space visa attribute removable and no other information
(e.g visible and connectable)
If not, maybe we should expose the _PLD values along with the "User
Visiable" and "Port is Connectable" ACPI values in the new port
directory.
Yeah. I agree.
Sarah Sharp
On Fri, May 04, 2012 at 11:06:37AM +0800, Lan Tianyu wrote:
From: Matthew Garrett
Add a simple helper function to allow drivers to obtain the physical
device location data.
Signed-off-by: Matthew Garrett
---
drivers/acpi/utils.c| 29 +
include/acpi/acpi_bus.h | 31 +++
2 files changed, 60 insertions(+), 0 deletions(-)
diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index b002a47..5c320a0 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -382,3 +382,32 @@ acpi_evaluate_reference(acpi_handle handle,
}
EXPORT_SYMBOL(acpi_evaluate_reference);
+
+acpi_status
+acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld *pld)
+{
+ acpi_status status;
+ struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+ union acpi_object *output;
+
+ status = acpi_evaluate_object(handle, "_PLD", NULL, &buffer);
+
+ if (ACPI_FAILURE(status))
+ return status;
+
+ output = buffer.pointer;
+
+ if (!output || output->type != ACPI_TYPE_PACKAGE
+ || !output->package.count
+ || output->package.elements[0].type != ACPI_TYPE_BUFFER
+ || output->package.elements[0].buffer.length > sizeof(*pld)) {
+ status = AE_TYPE;
+ goto out;
+ }
+
+ memcpy(pld, output->package.elements[0].buffer.pointer,
+ output->package.elements[0].buffer.length);
+out:
+ kfree(buffer.pointer);
+ return status;
+}
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index f1c8ca6..2642744 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -51,6 +51,37 @@ acpi_evaluate_reference(acpi_handle handle,
struct acpi_object_list *arguments,
struct acpi_handle_list *list);
+struct acpi_pld {
+ unsigned int revision:7; /* 0 */
+ unsigned int ignore_colour:1; /* 7 */
+ unsigned int colour:24; /* 8 */
+ unsigned int width:16; /* 32 */
+ unsigned int height:16; /* 48 */
+ unsigned int user_visible:1; /* 64 */
+ unsigned int dock:1; /* 65 */
+ unsigned int lid:1; /* 66 */
+ unsigned int panel:3; /* 67 */
+ unsigned int vertical_pos:2; /* 70 */
+ unsigned int horizontal_pos:2; /* 72 */
+ unsigned int shape:4; /* 74 */
+ unsigned int group_orientation:1; /* 78 */
+ unsigned int group_token:8; /* 79 */
+ unsigned int group_position:8; /* 87 */
+ unsigned int bay:1; /* 95 */
+ unsigned int ejectable:1; /* 96 */
+ unsigned int ospm_eject_required:1; /* 97 */
+ unsigned int cabinet_number:8; /* 98 */
+ unsigned int card_cage_number:8; /* 106 */
+ unsigned int reference:1; /* 114 */
+ unsigned int rotation:4; /* 115 */
+ unsigned int order:5; /* 119 */
+ unsigned int reserved:4; /* 124 */
+ unsigned int vertical_offset:16; /* 128 */
+ unsigned int horizontal_offset:16; /* 144 */
+};
+
+acpi_status
+acpi_get_physical_device_location
RE: Port power off: ACPI _PLD and other ideas
I have a comment about the existing _PLD support in Linux.
It uses bitfields, and these are known to not be reliable when using them to
extract bits from a predefined bit-packed data structure. (different compilers
implement bitfields differently, and there are also endian considerations.)
We are working on a patch to remove these bitfield definitions and also create
a function that returns a useable struct from _PLD.
Bob
> +struct acpi_pld {
> + unsigned int revision:7; /* 0 */
> + unsigned int ignore_colour:1; /* 7 */
> + unsigned int colour:24; /* 8 */
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Port power off: ACPI _PLD and other ideas
Hi Tianyu,
I was talking to Alan Cox about the port power off mechanism, and he
suggested it might be good to expose the power resource information to
userspace via sysfs. Then the user could figure out which of their
devices are connected to which power resources. For example, users
could move devices that we can't safely power off to the same power
resource, instead of connecting them to several power resources.
Alan also suggested that the default for unconnectable USB ports should
be the "auto" setting instead of the "on" setting. His point was that
it's hard to add more aggressive power savings later on, and we should
just turn on the power savings and see if anyone complains. Then we'll
get more real user testing with systems that have ACPI tables from other
BIOS vendors, and we can see if the ACPI tables are sane.
Also, Bob Moore pointed out that we could use the _PLD ACPI method to
give users a better idea of the physical location of ports associated
with a power resource. I note that this patch adds support for the
physical device location data. Is that exposed to userspace via sysfs?
If not, maybe we should expose the _PLD values along with the "User
Visiable" and "Port is Connectable" ACPI values in the new port
directory.
Sarah Sharp
On Fri, May 04, 2012 at 11:06:37AM +0800, Lan Tianyu wrote:
> From: Matthew Garrett
>
> Add a simple helper function to allow drivers to obtain the physical
> device location data.
>
> Signed-off-by: Matthew Garrett
> ---
> drivers/acpi/utils.c| 29 +
> include/acpi/acpi_bus.h | 31 +++
> 2 files changed, 60 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
> index b002a47..5c320a0 100644
> --- a/drivers/acpi/utils.c
> +++ b/drivers/acpi/utils.c
> @@ -382,3 +382,32 @@ acpi_evaluate_reference(acpi_handle handle,
> }
>
> EXPORT_SYMBOL(acpi_evaluate_reference);
> +
> +acpi_status
> +acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld *pld)
> +{
> + acpi_status status;
> + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> + union acpi_object *output;
> +
> + status = acpi_evaluate_object(handle, "_PLD", NULL, &buffer);
> +
> + if (ACPI_FAILURE(status))
> + return status;
> +
> + output = buffer.pointer;
> +
> + if (!output || output->type != ACPI_TYPE_PACKAGE
> + || !output->package.count
> + || output->package.elements[0].type != ACPI_TYPE_BUFFER
> + || output->package.elements[0].buffer.length > sizeof(*pld)) {
> + status = AE_TYPE;
> + goto out;
> + }
> +
> + memcpy(pld, output->package.elements[0].buffer.pointer,
> +output->package.elements[0].buffer.length);
> +out:
> + kfree(buffer.pointer);
> + return status;
> +}
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index f1c8ca6..2642744 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -51,6 +51,37 @@ acpi_evaluate_reference(acpi_handle handle,
> struct acpi_object_list *arguments,
> struct acpi_handle_list *list);
>
> +struct acpi_pld {
> + unsigned int revision:7; /* 0 */
> + unsigned int ignore_colour:1; /* 7 */
> + unsigned int colour:24; /* 8 */
> + unsigned int width:16; /* 32 */
> + unsigned int height:16; /* 48 */
> + unsigned int user_visible:1; /* 64 */
> + unsigned int dock:1; /* 65 */
> + unsigned int lid:1; /* 66 */
> + unsigned int panel:3; /* 67 */
> + unsigned int vertical_pos:2; /* 70 */
> + unsigned int horizontal_pos:2; /* 72 */
> + unsigned int shape:4; /* 74 */
> + unsigned int group_orientation:1; /* 78 */
> + unsigned int group_token:8; /* 79 */
> + unsigned int group_position:8; /* 87 */
> + unsigned int bay:1; /* 95 */
> + unsigned int ejectable:1; /* 96 */
> + unsigned int ospm_eject_required:1; /* 97 */
> + unsigned int cabinet_number:8; /* 98 */
> + unsigned int card_cage_number:8; /* 106 */
> + unsigned int reference:1; /* 114 */
> + unsigned int rotation:4; /* 115 */
> + unsigned int order:5; /* 119 */
> + unsigned int reserved:4; /* 124 */
> + unsigned int vertical_offset:16; /* 128 */
> + unsigned int horizontal_offset:16; /* 144 */
> +};
> +
> +acpi_status
> +acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld *pld);
> #ifdef CONFIG_ACPI
>
> #include
> --
> 1.7.6.rc2.8.g28eb
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
