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

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

[Matthew Garrett]

From: Matthew Garrett <mjg@redhat.com>

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 <mjg@redhat.com>

---

diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 955aae4..a17984a 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,6 +87,7 @@ static int is_ejectable(acpi_handle handle)
 {
 	acpi_status status;
 	acpi_handle tmp;
+	int ret = 0;
 
 	status = acpi_get_handle(handle, "_ADR", &tmp);
 	if (ACPI_FAILURE(status)) {
@@ -94,11 +95,14 @@ static int is_ejectable(acpi_handle handle)
 	}
 
 	status = acpi_get_handle(handle, "_EJ0", &tmp);
-	if (ACPI_FAILURE(status)) {
-		return 0;
-	}
+	if (ACPI_SUCCESS(status))
+		ret = 1;
+
+	status = acpi_get_handle(handle, "_RMV", &tmp);
+	if (ACPI_SUCCESS(status))
+		ret = 1;
 
-	return 1;
+	return ret;
 }
 
 
@@ -182,17 +186,26 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
 	struct acpiphp_func *newfunc;
 	acpi_handle tmp;
 	acpi_status status = AE_OK;
-	unsigned long long adr, sun;
-	int device, function, retval;
+	unsigned long long adr, sun, removable;
+	int device, function, retval, valid = 0;
 
 	status = acpi_evaluate_integer(handle, "_ADR", NULL, &adr);
 
 	if (ACPI_FAILURE(status))
 		return AE_OK;
 
-	status = acpi_get_handle(handle, "_EJ0", &tmp);
+	status = acpi_get_handle(handle, "_RMV", &tmp);
+	if (ACPI_SUCCESS(status)) {
+		acpi_evaluate_integer(handle, "_RMV", NULL, &removable);
+		if (removable)
+			valid = 1;
+	}
 
-	if (ACPI_FAILURE(status) && !(is_dock_device(handle)))
+	status = acpi_get_handle(handle, "_EJ0", &tmp);
+	if (ACPI_SUCCESS(status) || is_dock_device(handle))
+		valid = 1;
+
+	if (!valid)
 		return AE_OK;
 
 	device = (adr >> 16) & 0xffff;

-- 
Matthew Garrett | mjg59@srcf.ucam.org

[PATCH 2/2] acpiphp: Call _LCK method

[Matthew Garrett]

From: Matthew Garrett <mjg@redhat.com>

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 <mjg@redhat.com>

---

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 955aae4..a17984a 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -220,6 +233,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)) {
 		/*
@@ -1308,6 +1324,20 @@ 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 */
@@ -1800,6 +1830,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);
 
@@ -1808,6 +1843,24 @@ 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 | mjg59@srcf.ucam.org

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

[Matthew Wilcox]

On Thu, Nov 13, 2008 at 08:27:39PM +0000, Matthew Garrett wrote:
> @@ -94,11 +95,14 @@ static int is_ejectable(acpi_handle handle)
>  	}
>  
>  	status = acpi_get_handle(handle, "_EJ0", &tmp);
> -	if (ACPI_FAILURE(status)) {
> -		return 0;
> -	}
> +	if (ACPI_SUCCESS(status))
> +		ret = 1;
> +
> +	status = acpi_get_handle(handle, "_RMV", &tmp);
> +	if (ACPI_SUCCESS(status))
> +		ret = 1;
>  
> -	return 1;
> +	return ret;
>  }

Why not:

 	status = acpi_get_handle(handle, "_EJ0", &tmp);
	if (ACPI_SUCCESS(status))
		return 1;

	status = acpi_get_handle(handle, "_RMV", &tmp);
	if (ACPI_SUCCESS(status))
		return 1;

	return 0;

>  
>  
> @@ -182,17 +186,26 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
>  	struct acpiphp_func *newfunc;
>  	acpi_handle tmp;
>  	acpi_status status = AE_OK;
> -	unsigned long long adr, sun;
> -	int device, function, retval;
> +	unsigned long long adr, sun, removable;
> +	int device, function, retval, valid = 0;
>  
>  	status = acpi_evaluate_integer(handle, "_ADR", NULL, &adr);
>  
>  	if (ACPI_FAILURE(status))
>  		return AE_OK;
>  
> -	status = acpi_get_handle(handle, "_EJ0", &tmp);
> +	status = acpi_get_handle(handle, "_RMV", &tmp);
> +	if (ACPI_SUCCESS(status)) {
> +		acpi_evaluate_integer(handle, "_RMV", NULL, &removable);
> +		if (removable)
> +			valid = 1;
> +	}
>  
> -	if (ACPI_FAILURE(status) && !(is_dock_device(handle)))
> +	status = acpi_get_handle(handle, "_EJ0", &tmp);
> +	if (ACPI_SUCCESS(status) || is_dock_device(handle))
> +		valid = 1;
> +
> +	if (!valid)
>  		return AE_OK;

This looks like almost the same code as above.  New helper function?

>  	device = (adr >> 16) & 0xffff;
> 
> -- 
> Matthew Garrett | mjg59@srcf.ucam.org
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

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

[Matthew Garrett]

On Thu, Nov 13, 2008 at 01:48:59PM -0700, Matthew Wilcox wrote:

> Why not:
> 
>  	status = acpi_get_handle(handle, "_EJ0", &tmp);
> 	if (ACPI_SUCCESS(status))
> 		return 1;
> 
> 	status = acpi_get_handle(handle, "_RMV", &tmp);
> 	if (ACPI_SUCCESS(status))
> 		return 1;
> 
> 	return 0;

Oh, yes, that'd work.

> > +	if (ACPI_SUCCESS(status) || is_dock_device(handle))
> > +		valid = 1;
> > +
> > +	if (!valid)
> >  		return AE_OK;
> 
> This looks like almost the same code as above.  New helper function?

It's not quite the same, but yes, I'll see if it can be reduced.

-- 
Matthew Garrett | mjg59@srcf.ucam.org