LinuxLists.cc - [PATCH v2 1/2] acpiphp: Identify more removable slots

2008-11-13 23:53:16

Subject: [PATCH v2 1/2] acpiphp: Identify more removable slots

From: Matthew Garrett <[email protected]>

According to section 6.3.6 of the ACPI spec, the presence of an _RMV
method that evaluates to 1 is sufficient to indicate that a slot is
removable without needing an eject method. This patch refactors the
ejectable slot detection code a little in order to flag these slots as
ejectable and register them. Acpihp therefore binds to the expresscard
slot on my HP test machine.

Signed-off-by: Matthew Garrett <[email protected]>
---
drivers/pci/hotplug/acpiphp_glue.c | 32 +++++++++++++++++---------------
1 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 955aae4..e8cef99 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -74,7 +74,7 @@ static void handle_hotplug_event_func(acpi_handle handle, u32 type, void *contex
* Ejectable slot should satisfy at least these conditions:
*
* 1. has _ADR method
- * 2. has _EJ0 method
+ * 2. has _EJ0 method or _RMV method
*
* optionally
*
@@ -87,18 +87,25 @@ static int is_ejectable(acpi_handle handle)
{
acpi_status status;
acpi_handle tmp;
+ unsigned long long removable;

status = acpi_get_handle(handle, "_ADR", &tmp);
- if (ACPI_FAILURE(status)) {
+ if (ACPI_FAILURE(status))
return 0;
- }

status = acpi_get_handle(handle, "_EJ0", &tmp);
- if (ACPI_FAILURE(status)) {
- return 0;
+ if (ACPI_SUCCESS(status))
+ return 1;
+
+ status = acpi_get_handle(handle, "_RMV", &tmp);
+ if (ACPI_SUCCESS(status)) {
+ status = acpi_evaluate_integer(handle, "_RMV", NULL,
+ &removable);
+ if (ACPI_SUCCESS(status) && removable)
+ return 1;
}

- return 1;
+ return 0;
}

@@ -185,16 +192,10 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
unsigned long long adr, sun;
int device, function, retval;

- status = acpi_evaluate_integer(handle, "_ADR", NULL, &adr);
-
- if (ACPI_FAILURE(status))
- return AE_OK;
-
- status = acpi_get_handle(handle, "_EJ0", &tmp);
-
- if (ACPI_FAILURE(status) && !(is_dock_device(handle)))
+ if (!is_ejectable(handle) && !is_dock_device(handle))
return AE_OK;

+ acpi_evaluate_integer(handle, "_ADR", NULL, &adr);
device = (adr >> 16) & 0xffff;
function = adr & 0xffff;

@@ -205,7 +206,8 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
INIT_LIST_HEAD(&newfunc->sibling);
newfunc->handle = handle;
newfunc->function = function;
- if (ACPI_SUCCESS(status))
+
+ if (ACPI_SUCCESS(acpi_get_handle(handle, "_EJ0", &tmp)))
newfunc->flags = FUNC_HAS_EJ0;

if (ACPI_SUCCESS(acpi_get_handle(handle, "_STA", &tmp)))

--
Matthew Garrett | [email protected]

2008-11-13 23:54:23

by Matthew Garrett

[permalink] [raw]

Subject: [PATCH v2 2/2] acpiphp: Call _LCK method

From: Matthew Garrett <[email protected]>

Removable ACPI devices may have _LCK methods to indicate a software
controlled lock. These should be locked on device insertion and unlocked
when the user requests the device be ejected. Add support for flagging
devices with a _LCK method and then call appropriately on the add and
eject paths.

Signed-off-by: Matthew Garrett <[email protected]>
---
drivers/pci/hotplug/acpiphp.h | 1 +
drivers/pci/hotplug/acpiphp_glue.c | 45 +++++++++++++++++++++++++++++++++++-
2 files changed, 45 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h
index f9e244d..c07a4e8 100644
--- a/drivers/pci/hotplug/acpiphp.h
+++ b/drivers/pci/hotplug/acpiphp.h
@@ -194,6 +194,7 @@ struct acpiphp_ioapic {
#define FUNC_HAS_PS2 (0x00000040)
#define FUNC_HAS_PS3 (0x00000080)
#define FUNC_HAS_DCK (0x00000100)
+#define FUNC_HAS_LCK (0x00000200)

/* function prototypes */

diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index e8cef99..a83788d 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -222,6 +222,9 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
if (ACPI_SUCCESS(acpi_get_handle(handle, "_DCK", &tmp)))
newfunc->flags |= FUNC_HAS_DCK;

+ if (ACPI_SUCCESS(acpi_get_handle(handle, "_LCK", &tmp)))
+ newfunc->flags |= FUNC_HAS_LCK;
+
status = acpi_evaluate_integer(handle, "_SUN", NULL, &sun);
if (ACPI_FAILURE(status)) {
/*
@@ -1310,6 +1313,21 @@ int acpiphp_eject_slot(struct acpiphp_slot *slot)
list_for_each (l, &slot->funcs) {
func = list_entry(l, struct acpiphp_func, sibling);

+ if ((func->flags & FUNC_HAS_LCK)) {
+ arg_list.count = 1;
+ arg_list.pointer = &arg;
+ arg.type = ACPI_TYPE_INTEGER;
+ arg.integer.value = 0;
+
+ status = acpi_evaluate_object(func->handle, "_LCK",
+ &arg_list, NULL);
+ if (ACPI_FAILURE(status)) {
+ warn("%s: _LCK failed\n", __func__);
+ return -1;
+ } else
+ break;
+ }
+
/* We don't want to call _EJ0 on non-existing functions. */
if ((func->flags & FUNC_HAS_EJ0)) {
/* _EJ0 method take one argument */
@@ -1318,7 +1336,8 @@ int acpiphp_eject_slot(struct acpiphp_slot *slot)
arg.type = ACPI_TYPE_INTEGER;
arg.integer.value = 1;

- status = acpi_evaluate_object(func->handle, "_EJ0", &arg_list, NULL);
+ status = acpi_evaluate_object(func->handle, "_EJ0",
+ &arg_list, NULL);
if (ACPI_FAILURE(status)) {
warn("%s: _EJ0 failed\n", __func__);
return -1;
@@ -1802,6 +1821,11 @@ static int acpiphp_for_each_slot(acpiphp_callback fn, void *data)
int acpiphp_enable_slot(struct acpiphp_slot *slot)
{
int retval;
+ acpi_status status;
+ struct acpiphp_func *func;
+ struct list_head *l;
+ struct acpi_object_list arg_list;
+ union acpi_object arg;

mutex_lock(&slot->crit_sect);

@@ -1810,6 +1834,25 @@ int acpiphp_enable_slot(struct acpiphp_slot *slot)
if (retval)
goto err_exit;

+ list_for_each(l, &slot->funcs) {
+ func = list_entry(l, struct acpiphp_func, sibling);
+
+ if ((func->flags & FUNC_HAS_LCK)) {
+ arg_list.count = 1;
+ arg_list.pointer = &arg;
+ arg.type = ACPI_TYPE_INTEGER;
+ arg.integer.value = 1;
+
+ status = acpi_evaluate_object(func->handle, "_LCK",
+ &arg_list, NULL);
+ if (ACPI_FAILURE(status)) {
+ warn("%s: _LCK failed\n", __func__);
+ return -1;
+ } else
+ break;
+ }
+ }
+
if (get_slot_status(slot) == ACPI_STA_ALL) {
/* configure all functions */
retval = enable_device(slot);
--
Matthew Garrett | [email protected]

2008-11-14 00:20:34

by Matthew Garrett

[permalink] [raw]

Subject: Re: [PATCH v2 2/2] acpiphp: Call _LCK method

On Thu, Nov 13, 2008 at 11:53:54PM +0000, Matthew Garrett wrote:
> From: Matthew Garrett <[email protected]>
>
> Removable ACPI devices may have _LCK methods to indicate a software
> controlled lock. These should be locked on device insertion and unlocked
> when the user requests the device be ejected. Add support for flagging
> devices with a _LCK method and then call appropriately on the add and
> eject paths.

Please ignore this one - I've just realised it'll break on device
reenumeration. The other patch in the series should be good, though.

--
Matthew Garrett | [email protected]

2008-11-17 13:50:13

by Matthew Garrett

[permalink] [raw]

Subject: [PATCH] acpiphp: Identify more removable slots

According to section 6.3.6 of the ACPI spec, the presence of an _RMV
method that evaluates to 1 is sufficient to indicate that a slot is
removable without needing an eject method. This patch refactors the
ejectable slot detection code a little in order to flag these slots as
ejectable and register them. Acpihp then binds to the expresscard slot
on my HP test machine.

Signed-off-by: Matthew Garrett <[email protected]>

---

The _LCK handling code requires some more cleanup and ideally some
hardware that actually implements it

drivers/pci/hotplug/acpiphp_glue.c | 32 +++++++++++++++++---------------
1 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 955aae4..e8cef99 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -74,7 +74,7 @@ static void handle_hotplug_event_func(acpi_handle handle, u32 type, void *contex
* Ejectable slot should satisfy at least these conditions:
*
* 1. has _ADR method
- * 2. has _EJ0 method
+ * 2. has _EJ0 method or _RMV method
*
* optionally
*
@@ -87,18 +87,25 @@ static int is_ejectable(acpi_handle handle)
{
acpi_status status;
acpi_handle tmp;
+ unsigned long long removable;

status = acpi_get_handle(handle, "_ADR", &tmp);
- if (ACPI_FAILURE(status)) {
+ if (ACPI_FAILURE(status))
return 0;
- }

status = acpi_get_handle(handle, "_EJ0", &tmp);
- if (ACPI_FAILURE(status)) {
- return 0;
+ if (ACPI_SUCCESS(status))
+ return 1;
+
+ status = acpi_get_handle(handle, "_RMV", &tmp);
+ if (ACPI_SUCCESS(status)) {
+ status = acpi_evaluate_integer(handle, "_RMV", NULL,
+ &removable);
+ if (ACPI_SUCCESS(status) && removable)
+ return 1;
}

- return 1;
+ return 0;
}

@@ -185,16 +192,10 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
unsigned long long adr, sun;
int device, function, retval;

- status = acpi_evaluate_integer(handle, "_ADR", NULL, &adr);
-
- if (ACPI_FAILURE(status))
- return AE_OK;
-
- status = acpi_get_handle(handle, "_EJ0", &tmp);
-
- if (ACPI_FAILURE(status) && !(is_dock_device(handle)))
+ if (!is_ejectable(handle) && !is_dock_device(handle))
return AE_OK;

+ acpi_evaluate_integer(handle, "_ADR", NULL, &adr);
device = (adr >> 16) & 0xffff;
function = adr & 0xffff;

@@ -205,7 +206,8 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
INIT_LIST_HEAD(&newfunc->sibling);
newfunc->handle = handle;
newfunc->function = function;
- if (ACPI_SUCCESS(status))
+
+ if (ACPI_SUCCESS(acpi_get_handle(handle, "_EJ0", &tmp)))
newfunc->flags = FUNC_HAS_EJ0;

if (ACPI_SUCCESS(acpi_get_handle(handle, "_STA", &tmp)))

--
Matthew Garrett | [email protected]

2008-11-25 21:50:40

by Matthew Garrett

[permalink] [raw]

Subject: [PATCH (resend)] acpiphp: Identify more removable slots

2008-11-25 22:03:22

by Kristen Carlson Accardi

[permalink] [raw]

Subject: Re: [PATCH (resend)] acpiphp: Identify more removable slots

On Tue, 25 Nov 2008 13:48:14 -0800
Matthew Garrett <[email protected]> wrote:

> According to section 6.3.6 of the ACPI spec, the presence of an _RMV
> method that evaluates to 1 is sufficient to indicate that a slot is
> removable without needing an eject method. This patch refactors the
> ejectable slot detection code a little in order to flag these slots as
> ejectable and register them. Acpihp then binds to the expresscard slot
> on my HP test machine.
>
> Signed-off-by: Matthew Garrett <[email protected]>

This looks good to me.

Acked-by: Kristen Carlson Accardi <[email protected]>

>
> ---
>
> The _LCK handling code requires some more cleanup and ideally some
> hardware that actually implements it
>
> drivers/pci/hotplug/acpiphp_glue.c | 32 +++++++++++++++++---------------
> 1 files changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> index 955aae4..e8cef99 100644
> --- a/drivers/pci/hotplug/acpiphp_glue.c
> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> @@ -74,7 +74,7 @@ static void handle_hotplug_event_func(acpi_handle handle, u32 type, void *contex
> * Ejectable slot should satisfy at least these conditions:
> *
> * 1. has _ADR method
> - * 2. has _EJ0 method
> + * 2. has _EJ0 method or _RMV method
> *
> * optionally
> *
> @@ -87,18 +87,25 @@ static int is_ejectable(acpi_handle handle)
> {
> acpi_status status;
> acpi_handle tmp;
> + unsigned long long removable;
>
> status = acpi_get_handle(handle, "_ADR", &tmp);
> - if (ACPI_FAILURE(status)) {
> + if (ACPI_FAILURE(status))
> return 0;
> - }
>
> status = acpi_get_handle(handle, "_EJ0", &tmp);
> - if (ACPI_FAILURE(status)) {
> - return 0;
> + if (ACPI_SUCCESS(status))
> + return 1;
> +
> + status = acpi_get_handle(handle, "_RMV", &tmp);
> + if (ACPI_SUCCESS(status)) {
> + status = acpi_evaluate_integer(handle, "_RMV", NULL,
> + &removable);
> + if (ACPI_SUCCESS(status) && removable)
> + return 1;
> }
>
> - return 1;
> + return 0;
> }
>
>
> @@ -185,16 +192,10 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
> unsigned long long adr, sun;
> int device, function, retval;
>
> - status = acpi_evaluate_integer(handle, "_ADR", NULL, &adr);
> -
> - if (ACPI_FAILURE(status))
> - return AE_OK;
> -
> - status = acpi_get_handle(handle, "_EJ0", &tmp);
> -
> - if (ACPI_FAILURE(status) && !(is_dock_device(handle)))
> + if (!is_ejectable(handle) && !is_dock_device(handle))
> return AE_OK;
>
> + acpi_evaluate_integer(handle, "_ADR", NULL, &adr);
> device = (adr >> 16) & 0xffff;
> function = adr & 0xffff;
>
> @@ -205,7 +206,8 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
> INIT_LIST_HEAD(&newfunc->sibling);
> newfunc->handle = handle;
> newfunc->function = function;
> - if (ACPI_SUCCESS(status))
> +
> + if (ACPI_SUCCESS(acpi_get_handle(handle, "_EJ0", &tmp)))
> newfunc->flags = FUNC_HAS_EJ0;
>
> if (ACPI_SUCCESS(acpi_get_handle(handle, "_STA", &tmp)))
>

2008-12-01 20:40:57

by Jesse Barnes

[permalink] [raw]

Subject: Re: [PATCH (resend)] acpiphp: Identify more removable slots

On Tuesday, November 25, 2008 1:48 pm Matthew Garrett wrote:
> According to section 6.3.6 of the ACPI spec, the presence of an _RMV
> method that evaluates to 1 is sufficient to indicate that a slot is
> removable without needing an eject method. This patch refactors the
> ejectable slot detection code a little in order to flag these slots as
> ejectable and register them. Acpihp then binds to the expresscard slot
> on my HP test machine.
>
> Signed-off-by: Matthew Garrett <[email protected]>

Applied to my linux-next branch, thanks Matthew.

--
Jesse Barnes, Intel Open Source Technology Center