Re: [PATCH 1/4] ACPI: Fix up _PLD methods

Re: [PATCH 1/4] ACPI: Fix up _PLD methods

[Greg KH]

On Mon, Mar 12, 2012 at 07:58:52PM -0400, Matthew Garrett wrote:
> _PLD is defined as returning a package of buffers, but many implementations
> simply return a buffer. Fix this up.
> 
> (Original patch by Bob Moore <robert.moore@intel.com>)
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> ---
>  drivers/acpi/acpica/nsrepair.c |   15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)

I think this series depends on your previous USB core patch, right?  If
so, I'd be glad to take all of these through the USB tree if I get an
ack from Len for the acpi portions.

Otherwise, feel free to take them through the acpi tree, they look good
to me:
	Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 1/4] ACPI: Fix up _PLD methods

[Matthew Garrett]

On Mon, Mar 12, 2012 at 01:17:35PM -0700, Greg KH wrote:
> On Mon, Mar 12, 2012 at 07:58:52PM -0400, Matthew Garrett wrote:
> > _PLD is defined as returning a package of buffers, but many implementations
> > simply return a buffer. Fix this up.
> > 
> > (Original patch by Bob Moore <robert.moore@intel.com>)
> > 
> > Signed-off-by: Matthew Garrett <mjg@redhat.com>
> > ---
> >  drivers/acpi/acpica/nsrepair.c |   15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> I think this series depends on your previous USB core patch, right?  If
> so, I'd be glad to take all of these through the USB tree if I get an
> ack from Len for the acpi portions.

They do, so probably best for you to take them as long as Len approves.

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

Re: [PATCH 3/4] usb: Bind devices to ACPI devices when possible

[Alan Stern]

On Mon, 12 Mar 2012, Matthew Garrett wrote:

> Built-in USB devices will typically have a representation in the system
> ACPI tables. Add support for binding the two together so the USB code can
> make use of the associated methods.

> +static int usb_acpi_find_device(struct device *dev, acpi_handle *handle)
> +{
> +	struct usb_device *udev = to_usb_device(dev);
> +	struct device *parent = dev->parent;
> +	acpi_handle *parent_handle = DEVICE_ACPI_HANDLE(parent);
> +
> +	if (!parent_handle)
> +		return -ENODEV;
> +
> +	*handle = acpi_get_child(parent_handle, udev->portnum);
> +
> +	if (!*handle)
> +		return -ENODEV;
> +
> +	return 0;
> +}
> +
> +static struct acpi_bus_type usb_acpi_bus = {
> +	.bus = &usb_bus_type,
> +	.find_bridge = NULL,
> +	.find_device = usb_acpi_find_device,
> +};

I don't understand exactly how this is supposed to work.  Does this 
mean that the ACPI core will call usb_acpi_find_device() for every ACPI 
device whose underlying real device is registered on the usb_bus_type?  
If so, when does this call take place?

Or does it work some other way?

Bear in mind that USB devices aren't the only things registered with 
usb_bus_type.  We also register device structures representing USB 
interfaces and endpoints.  Hence the to_usb_device() call above might 
not always do what you want.

Alan Stern

Re: [PATCH 3/4] usb: Bind devices to ACPI devices when possible

[Matthew Garrett]

On Mon, Mar 12, 2012 at 04:38:57PM -0400, Alan Stern wrote:
> On Mon, 12 Mar 2012, Matthew Garrett wrote:
> 
> > Built-in USB devices will typically have a representation in the system
> > ACPI tables. Add support for binding the two together so the USB code can
> > make use of the associated methods.
> 
> > +static int usb_acpi_find_device(struct device *dev, acpi_handle *handle)
> > +{
> > +	struct usb_device *udev = to_usb_device(dev);
> > +	struct device *parent = dev->parent;
> > +	acpi_handle *parent_handle = DEVICE_ACPI_HANDLE(parent);
> > +
> > +	if (!parent_handle)
> > +		return -ENODEV;
> > +
> > +	*handle = acpi_get_child(parent_handle, udev->portnum);
> > +
> > +	if (!*handle)
> > +		return -ENODEV;
> > +
> > +	return 0;
> > +}
> > +
> > +static struct acpi_bus_type usb_acpi_bus = {
> > +	.bus = &usb_bus_type,
> > +	.find_bridge = NULL,
> > +	.find_device = usb_acpi_find_device,
> > +};
> 
> I don't understand exactly how this is supposed to work.  Does this 
> mean that the ACPI core will call usb_acpi_find_device() for every ACPI 
> device whose underlying real device is registered on the usb_bus_type?  
> If so, when does this call take place?

Kind of? platform_notify gets called whenever a device is registered, 
and on ACPi systems that'll be acpi_platform_notify. It'll take the 
registered device and, if there are glue functions for that bus type, 
pass it to the callback functions. The idea is that we have two 
disparate device trees, the Linux one and the ACPI one. This just gives 
us an opportunity to glue some of the devices together.

> Or does it work some other way?
> 
> Bear in mind that USB devices aren't the only things registered with 
> usb_bus_type.  We also register device structures representing USB 
> interfaces and endpoints.  Hence the to_usb_device() call above might 
> not always do what you want.

Ugh. Is there any clean way to distinguish from the device structure? If 
not, the easiest would seem to be to just add another bus type 
structure, even if the members are identical.

-- 
Matthew Garrett | mjg59-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] usb: Bind devices to ACPI devices when possible

[Alan Stern]

On Mon, 12 Mar 2012, Matthew Garrett wrote:

> > > +static struct acpi_bus_type usb_acpi_bus = {
> > > +	.bus = &usb_bus_type,
> > > +	.find_bridge = NULL,
> > > +	.find_device = usb_acpi_find_device,
> > > +};
> > 
> > I don't understand exactly how this is supposed to work.  Does this 
> > mean that the ACPI core will call usb_acpi_find_device() for every ACPI 
> > device whose underlying real device is registered on the usb_bus_type?  
> > If so, when does this call take place?
> 
> Kind of? platform_notify gets called whenever a device is registered, 
> and on ACPi systems that'll be acpi_platform_notify. It'll take the 
> registered device and, if there are glue functions for that bus type, 
> pass it to the callback functions. The idea is that we have two 
> disparate device trees, the Linux one and the ACPI one. This just gives 
> us an opportunity to glue some of the devices together.

Ah, okay.

> > Bear in mind that USB devices aren't the only things registered with 
> > usb_bus_type.  We also register device structures representing USB 
> > interfaces and endpoints.  Hence the to_usb_device() call above might 
> > not always do what you want.
> 
> Ugh. Is there any clean way to distinguish from the device structure? If 
> not, the easiest would seem to be to just add another bus type 
> structure, even if the members are identical.

Easy; just call is_usb_device() (defined in core/usb.h).

Alan Stern

Re: [PATCH 3/4] usb: Bind devices to ACPI devices when possible

[Matthew Garrett]

On Mon, Mar 12, 2012 at 05:03:54PM -0400, Alan Stern wrote:
> On Mon, 12 Mar 2012, Matthew Garrett wrote:
> > Ugh. Is there any clean way to distinguish from the device structure? If 
> > not, the easiest would seem to be to just add another bus type 
> > structure, even if the members are identical.
> 
> Easy; just call is_usb_device() (defined in core/usb.h).

Ah, thanks - I'll add that and repost.

-- 
Matthew Garrett | mjg59-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] ACPI: Fix up _PLD methods

[Matthew Garrett]

_PLD is defined as returning a package of buffers, but many implementations
simply return a buffer. Fix this up.

(Original patch by Bob Moore <robert.moore@intel.com>)

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/acpi/acpica/nsrepair.c |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/acpica/nsrepair.c b/drivers/acpi/acpica/nsrepair.c
index 9c35d20..6217cfd 100644
--- a/drivers/acpi/acpica/nsrepair.c
+++ b/drivers/acpi/acpica/nsrepair.c
@@ -151,9 +151,18 @@ acpi_ns_repair_object(struct acpi_predefined_data *data,
 		}
 	}
 	if (expected_btypes & ACPI_RTYPE_PACKAGE) {
-		status = acpi_ns_convert_to_package(return_object, &new_object);
-		if (ACPI_SUCCESS(status)) {
-			goto object_repaired;
+		if (return_object->common.type == ACPI_TYPE_BUFFER) {
+			status = acpi_ns_repair_package_list(data, &return_object);
+			if (ACPI_SUCCESS(status)) {
+				*return_object_ptr = return_object;
+				data->flags |= ACPI_OBJECT_REPAIRED;
+				return (AE_OK);
+			}
+		} else {
+			status = acpi_ns_convert_to_package(return_object, &new_object);
+			if (ACPI_SUCCESS(status)) {
+				goto object_repaired;
+			}
 		}
 	}
 
-- 
1.7.9.3

[PATCH 2/4] ACPI: Add _PLD support

[Matthew Garrett]

Add a simple helper function to allow drivers to obtain the physical
device location data.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/acpi/utils.c    |   29 +++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h |   31 +++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)

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 6cd5b64..39d7de9 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 <linux/proc_fs.h>
-- 
1.7.9.3

[PATCH 3/4] usb: Bind devices to ACPI devices when possible

[Matthew Garrett]

Built-in USB devices will typically have a representation in the system
ACPI tables. Add support for binding the two together so the USB code can
make use of the associated methods.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/usb/core/Makefile   |    1 +
 drivers/usb/core/usb-acpi.c |   53 +++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/core/usb.c      |    6 +++++
 drivers/usb/core/usb.h      |    7 ++++++
 4 files changed, 67 insertions(+)
 create mode 100644 drivers/usb/core/usb-acpi.c

diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
index 507a4e1..9268ddb 100644
--- a/drivers/usb/core/Makefile
+++ b/drivers/usb/core/Makefile
@@ -10,5 +10,6 @@ usbcore-y += devio.o notify.o generic.o quirks.o devices.o
 
 usbcore-$(CONFIG_PCI)		+= hcd-pci.o
 usbcore-$(CONFIG_USB_DEVICEFS)	+= inode.o
+usbcore-$(CONFIG_ACPI)		+= usb-acpi.o
 
 obj-$(CONFIG_USB)		+= usbcore.o
diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
new file mode 100644
index 0000000..1224c50
--- /dev/null
+++ b/drivers/usb/core/usb-acpi.c
@@ -0,0 +1,53 @@
+/*
+ * USB-ACPI glue code
+ *
+ * Copyright 2011 Red Hat <mjg@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation, version 2.
+ *
+ */
+#include <linux/module.h>
+#include <linux/usb.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/acpi.h>
+#include <linux/pci.h>
+#include <acpi/acpi_bus.h>
+
+#include "usb.h"
+
+static int usb_acpi_find_device(struct device *dev, acpi_handle *handle)
+{
+	struct usb_device *udev = to_usb_device(dev);
+	struct device *parent = dev->parent;
+	acpi_handle *parent_handle = DEVICE_ACPI_HANDLE(parent);
+
+	if (!parent_handle)
+		return -ENODEV;
+
+	*handle = acpi_get_child(parent_handle, udev->portnum);
+
+	if (!*handle)
+		return -ENODEV;
+
+	return 0;
+}
+
+static struct acpi_bus_type usb_acpi_bus = {
+	.bus = &usb_bus_type,
+	.find_bridge = NULL,
+	.find_device = usb_acpi_find_device,
+};
+
+int usb_acpi_register(void)
+{
+	return register_acpi_bus_type(&usb_acpi_bus);
+}
+
+void usb_acpi_unregister(void)
+{
+	unregister_acpi_bus_type(&usb_acpi_bus);
+}
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 8ca9f99..ce47e3f 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1015,6 +1015,9 @@ static int __init usb_init(void)
 	if (retval)
 		goto out;
 
+	retval = usb_acpi_register();
+	if (retval)
+		goto acpi_register_failed;
 	retval = bus_register(&usb_bus_type);
 	if (retval)
 		goto bus_register_failed;
@@ -1054,6 +1057,8 @@ major_init_failed:
 bus_notifier_failed:
 	bus_unregister(&usb_bus_type);
 bus_register_failed:
+	usb_acpi_unregister();
+acpi_register_failed:
 	usb_debugfs_cleanup();
 out:
 	return retval;
@@ -1076,6 +1081,7 @@ static void __exit usb_exit(void)
 	usb_hub_cleanup();
 	bus_unregister_notifier(&usb_bus_type, &usb_bus_nb);
 	bus_unregister(&usb_bus_type);
+	usb_acpi_unregister();
 	usb_debugfs_cleanup();
 }
 
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 45e8479..636d98e 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -155,3 +155,10 @@ extern void usb_notify_remove_device(struct usb_device *udev);
 extern void usb_notify_add_bus(struct usb_bus *ubus);
 extern void usb_notify_remove_bus(struct usb_bus *ubus);
 
+#ifdef CONFIG_ACPI
+extern int usb_acpi_register(void);
+extern void usb_acpi_unregister(void);
+#else
+static inline int usb_acpi_register(void) { return 0; };
+static inline void usb_acpi_unregister(void) { };
+#endif
-- 
1.7.9.3

[PATCH 4/4] usb: Set device removable state based on ACPI USB data

[Matthew Garrett]

ACPI offers two methods that allow us to infer whether or not a USB port
is removable. The _PLD method gives us information on whether the port is
"user visible" or not. If that's not present then we can fall back to the
_UPC method which tells us whether or not a port is connectable.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/usb/core/usb-acpi.c |   56 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index 1224c50..18bc4e7 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -19,6 +19,53 @@
 
 #include "usb.h"
 
+static int usb_acpi_check_upc(struct usb_device *udev, acpi_handle handle)
+{
+	acpi_status status;
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *upc;
+	int ret = 0;
+
+	status = acpi_evaluate_object(handle, "_UPC", NULL, &buffer);
+
+	if (ACPI_FAILURE(status))
+		return -ENODEV;
+
+	upc = buffer.pointer;
+
+	if (!upc || (upc->type != ACPI_TYPE_PACKAGE) || upc->package.count != 4) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (upc->package.elements[0].integer.value)
+		udev->removable = USB_DEVICE_REMOVABLE;
+	else
+		udev->removable = USB_DEVICE_FIXED;
+
+out:
+	kfree(upc);
+	return ret;
+}
+
+static int usb_acpi_check_pld(struct usb_device *udev, acpi_handle handle)
+{
+	acpi_status status;
+	struct acpi_pld pld;
+
+	status = acpi_get_physical_device_location(handle, &pld);
+
+	if (ACPI_FAILURE(status))
+		return -ENODEV;
+
+	if (pld.user_visible)
+		udev->removable = USB_DEVICE_REMOVABLE;
+	else
+		udev->removable = USB_DEVICE_FIXED;
+
+	return 0;
+}
+
 static int usb_acpi_find_device(struct device *dev, acpi_handle *handle)
 {
 	struct usb_device *udev = to_usb_device(dev);
@@ -33,6 +80,15 @@ static int usb_acpi_find_device(struct device *dev, acpi_handle *handle)
 	if (!*handle)
 		return -ENODEV;
 
+	/*
+	 * PLD will tell us whether a port is removable to the user or
+	 * not. If we don't get an answer from PLD (it's not present
+	 * or it's malformed) then try to infer it from UPC. If a
+	 * device isn't connectable then it's probably not removable.
+	 */
+	if (usb_acpi_check_pld(udev, *handle) != 0)
+		usb_acpi_check_upc(udev, *handle);
+
 	return 0;
 }
 
-- 
1.7.9.3