Re: [Resend][PATCH] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type

2013-02-28 Thread Rafael J. Wysocki
On Wednesday, February 27, 2013 06:33:13 PM Greg Kroah-Hartman wrote:
 On Thu, Feb 28, 2013 at 02:11:58AM +0100, Rafael J. Wysocki wrote:
  On Wednesday, February 27, 2013 02:20:32 PM Greg Kroah-Hartman wrote:
   On Wed, Feb 27, 2013 at 11:06:52PM +0100, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com

After PCI has stopped using the .find_bridge() callback in
struct acpi_bus_type, the only remaining users of it are SATA and
USB.  However, SATA only pretends to be a user, because it points
that callback to a stub always returning -ENODEV, and USB uses it
incorrectly, because as a result of the way it is used by USB every
device in the system that doesn't have a bus type or parent is
passed to usb_acpi_find_device() for inspection.

What USB actually needs, though, is to call usb_acpi_find_device()
for USB ports that don't have a bus type defined, but have
usb_port_device_type as their device type.
   
   Ick, that's not good.  Can you have the original creator of that code
   (someone else from Intel, I can't remember at the moment), fix that up
   properly and send me patches?
  
  That won't be necessary afer this patch.  Or do you want to fix up USB
  separately first?
 
 No, sorry, my misunderstanding, I was assuming we still needed to do
 other USB work after this.  If not, that's an even better reason to
 accept this patch :)

Heh, thanks!

Still, it seems that we can do a bit better that this.

The two patches that will follow should be almost functionally equivalent to
the $subject one, but the resulting code looks somewhat cleaner to me.

[1/2] Add .macth() callback to struct acpi_bus_type (instead of the bus 
pointer).
[2/2] Drop .find_bridge() from struct acpi_bus_type

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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


[Resend][PATCH] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type

2013-02-27 Thread Rafael J. Wysocki
From: Rafael J. Wysocki rafael.j.wyso...@intel.com

After PCI has stopped using the .find_bridge() callback in
struct acpi_bus_type, the only remaining users of it are SATA and
USB.  However, SATA only pretends to be a user, because it points
that callback to a stub always returning -ENODEV, and USB uses it
incorrectly, because as a result of the way it is used by USB every
device in the system that doesn't have a bus type or parent is
passed to usb_acpi_find_device() for inspection.

What USB actually needs, though, is to call usb_acpi_find_device()
for USB ports that don't have a bus type defined, but have
usb_port_device_type as their device type.

To fix that add a device type field to struct acpi_bus_type and
make acpi_get_bus_type() compare it with the device type fields of
the device objects it inspects in addition to checking their bus
types.  Next, make USB set the new type field in usb_acpi_bus to
point to usb_port_device_type and stop abusing the .find_bridge()
callback.

In addition to that drop the SATA's dummy .find_bridge() callback,
and remove .find_bridge(), which is not used any more, from struct
acpi_bus_type entirely.

Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---

I send this as an RFC before the merge window, but since it depends on changes
in both PM and PCI trees, it couldn't be applied before.  Now that they both
have been merged, here it goes again.

Please let me know if there are any objections.

Thanks,
Rafael

---
 drivers/acpi/glue.c |   39 ++-
 drivers/ata/libata-acpi.c   |6 --
 drivers/usb/core/usb-acpi.c |2 +-
 include/acpi/acpi_bus.h |4 +---
 4 files changed, 8 insertions(+), 43 deletions(-)

Index: linux-pm/drivers/acpi/glue.c
===
--- linux-pm.orig/drivers/acpi/glue.c
+++ linux-pm/drivers/acpi/glue.c
@@ -64,16 +64,17 @@ int unregister_acpi_bus_type(struct acpi
 }
 EXPORT_SYMBOL_GPL(unregister_acpi_bus_type);
 
-static struct acpi_bus_type *acpi_get_bus_type(struct bus_type *type)
+static struct acpi_bus_type *acpi_get_bus_type(struct device *dev)
 {
struct acpi_bus_type *tmp, *ret = NULL;
 
-   if (!type)
+   if (!dev-bus  !dev-type)
return NULL;
 
down_read(bus_type_sem);
list_for_each_entry(tmp, bus_type_list, list) {
-   if (tmp-bus == type) {
+   if ((tmp-bus  tmp-bus == dev-bus)
+   || (tmp-type  tmp-type == dev-type)) {
ret = tmp;
break;
}
@@ -82,22 +83,6 @@ static struct acpi_bus_type *acpi_get_bu
return ret;
 }
 
-static int acpi_find_bridge_device(struct device *dev, acpi_handle * handle)
-{
-   struct acpi_bus_type *tmp;
-   int ret = -ENODEV;
-
-   down_read(bus_type_sem);
-   list_for_each_entry(tmp, bus_type_list, list) {
-   if (tmp-find_bridge  !tmp-find_bridge(dev, handle)) {
-   ret = 0;
-   break;
-   }
-   }
-   up_read(bus_type_sem);
-   return ret;
-}
-
 static acpi_status do_acpi_find_child(acpi_handle handle, u32 lvl_not_used,
  void *addr_p, void **ret_p)
 {
@@ -261,22 +246,11 @@ err:
 
 static int acpi_platform_notify(struct device *dev)
 {
-   struct acpi_bus_type *type;
+   struct acpi_bus_type *type = acpi_get_bus_type(dev);
acpi_handle handle;
int ret;
 
ret = acpi_bind_one(dev, NULL);
-   if (ret  (!dev-bus || !dev-parent)) {
-   /* bridge devices genernally haven't bus or parent */
-   ret = acpi_find_bridge_device(dev, handle);
-   if (!ret) {
-   ret = acpi_bind_one(dev, handle);
-   if (ret)
-   goto out;
-   }
-   }
-
-   type = acpi_get_bus_type(dev-bus);
if (ret) {
if (!type || !type-find_device) {
DBG(No ACPI bus support for %s\n, dev_name(dev));
@@ -293,7 +267,6 @@ static int acpi_platform_notify(struct d
if (ret)
goto out;
}
-
if (type  type-setup)
type-setup(dev);
 
@@ -316,7 +289,7 @@ static int acpi_platform_notify_remove(s
 {
struct acpi_bus_type *type;
 
-   type = acpi_get_bus_type(dev-bus);
+   type = acpi_get_bus_type(dev);
if (type  type-cleanup)
type-cleanup(dev);
 
Index: linux-pm/include/acpi/acpi_bus.h
===
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -438,10 +438,8 @@ void acpi_remove_dir(struct acpi_device
 struct acpi_bus_type {
struct list_head list;
struct bus_type *bus;
-   /* For general devices under the bus */
+   struct device_type *type;

Re: [Resend][PATCH] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type

2013-02-27 Thread Greg Kroah-Hartman
On Wed, Feb 27, 2013 at 11:06:52PM +0100, Rafael J. Wysocki wrote:
 From: Rafael J. Wysocki rafael.j.wyso...@intel.com
 
 After PCI has stopped using the .find_bridge() callback in
 struct acpi_bus_type, the only remaining users of it are SATA and
 USB.  However, SATA only pretends to be a user, because it points
 that callback to a stub always returning -ENODEV, and USB uses it
 incorrectly, because as a result of the way it is used by USB every
 device in the system that doesn't have a bus type or parent is
 passed to usb_acpi_find_device() for inspection.
 
 What USB actually needs, though, is to call usb_acpi_find_device()
 for USB ports that don't have a bus type defined, but have
 usb_port_device_type as their device type.

Ick, that's not good.  Can you have the original creator of that code
(someone else from Intel, I can't remember at the moment), fix that up
properly and send me patches?

 Please let me know if there are any objections.

No objection from me:

Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org

--
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: [Resend][PATCH] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type

2013-02-27 Thread Yinghai Lu
On Wed, Feb 27, 2013 at 2:20 PM, Greg Kroah-Hartman
gre...@linuxfoundation.org wrote:
 On Wed, Feb 27, 2013 at 11:06:52PM +0100, Rafael J. Wysocki wrote:
 From: Rafael J. Wysocki rafael.j.wyso...@intel.com

 After PCI has stopped using the .find_bridge() callback in
 struct acpi_bus_type, the only remaining users of it are SATA and
 USB.  However, SATA only pretends to be a user, because it points
 that callback to a stub always returning -ENODEV, and USB uses it
 incorrectly, because as a result of the way it is used by USB every
 device in the system that doesn't have a bus type or parent is
 passed to usb_acpi_find_device() for inspection.

 What USB actually needs, though, is to call usb_acpi_find_device()
 for USB ports that don't have a bus type defined, but have
 usb_port_device_type as their device type.

 Ick, that's not good.  Can you have the original creator of that code
 (someone else from Intel, I can't remember at the moment), fix that up
 properly and send me patches?

[Add To: Lan Tianyu tianyu@intel.com]


 Please let me know if there are any objections.

I still prefer to ask USB to add bus_type instead at first.

Thanks

Yinghai
--
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: [Resend][PATCH] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type

2013-02-27 Thread Lan Tianyu
On 2013年02月28日 07:31, Yinghai Lu wrote:
 On Wed, Feb 27, 2013 at 2:20 PM, Greg Kroah-Hartman
 gre...@linuxfoundation.org wrote:
 On Wed, Feb 27, 2013 at 11:06:52PM +0100, Rafael J. Wysocki wrote:
 From: Rafael J. Wysocki rafael.j.wyso...@intel.com

 After PCI has stopped using the .find_bridge() callback in
 struct acpi_bus_type, the only remaining users of it are SATA and
 USB.  However, SATA only pretends to be a user, because it points
 that callback to a stub always returning -ENODEV, and USB uses it
 incorrectly, because as a result of the way it is used by USB every
 device in the system that doesn't have a bus type or parent is
 passed to usb_acpi_find_device() for inspection.

 What USB actually needs, though, is to call usb_acpi_find_device()
 for USB ports that don't have a bus type defined, but have
 usb_port_device_type as their device type.

 Ick, that's not good.  Can you have the original creator of that code
 (someone else from Intel, I can't remember at the moment), fix that up
 properly and send me patches?
 
 [Add To: Lan Tianyu tianyu@intel.com]
Ok. I will fix it later.
 

 Please let me know if there are any objections.
 
 I still prefer to ask USB to add bus_type instead at first.
 
 Thanks
 
 Yinghai
 


-- 
Best regards
Tianyu Lan
--
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: [Resend][PATCH] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type

2013-02-27 Thread Rafael J. Wysocki
On Wednesday, February 27, 2013 02:20:32 PM Greg Kroah-Hartman wrote:
 On Wed, Feb 27, 2013 at 11:06:52PM +0100, Rafael J. Wysocki wrote:
  From: Rafael J. Wysocki rafael.j.wyso...@intel.com
  
  After PCI has stopped using the .find_bridge() callback in
  struct acpi_bus_type, the only remaining users of it are SATA and
  USB.  However, SATA only pretends to be a user, because it points
  that callback to a stub always returning -ENODEV, and USB uses it
  incorrectly, because as a result of the way it is used by USB every
  device in the system that doesn't have a bus type or parent is
  passed to usb_acpi_find_device() for inspection.
  
  What USB actually needs, though, is to call usb_acpi_find_device()
  for USB ports that don't have a bus type defined, but have
  usb_port_device_type as their device type.
 
 Ick, that's not good.  Can you have the original creator of that code
 (someone else from Intel, I can't remember at the moment), fix that up
 properly and send me patches?

That won't be necessary afer this patch.  Or do you want to fix up USB
separately first?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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: [Resend][PATCH] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type

2013-02-27 Thread Rafael J. Wysocki
On Wednesday, February 27, 2013 03:31:08 PM Yinghai Lu wrote:
 On Wed, Feb 27, 2013 at 2:20 PM, Greg Kroah-Hartman
 gre...@linuxfoundation.org wrote:
  On Wed, Feb 27, 2013 at 11:06:52PM +0100, Rafael J. Wysocki wrote:
  From: Rafael J. Wysocki rafael.j.wyso...@intel.com
 
  After PCI has stopped using the .find_bridge() callback in
  struct acpi_bus_type, the only remaining users of it are SATA and
  USB.  However, SATA only pretends to be a user, because it points
  that callback to a stub always returning -ENODEV, and USB uses it
  incorrectly, because as a result of the way it is used by USB every
  device in the system that doesn't have a bus type or parent is
  passed to usb_acpi_find_device() for inspection.
 
  What USB actually needs, though, is to call usb_acpi_find_device()
  for USB ports that don't have a bus type defined, but have
  usb_port_device_type as their device type.
 
  Ick, that's not good.  Can you have the original creator of that code
  (someone else from Intel, I can't remember at the moment), fix that up
  properly and send me patches?
 
 [Add To: Lan Tianyu tianyu@intel.com]
 
 
  Please let me know if there are any objections.
 
 I still prefer to ask USB to add bus_type instead at first.

IIRC, You want USB to register an *additional* ACPI bus type for
usb_port_device_type, which quite frankly will be very confusing and I don't
see any actual benefits from doing that.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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: [Resend][PATCH] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type

2013-02-27 Thread Greg Kroah-Hartman
On Thu, Feb 28, 2013 at 02:11:58AM +0100, Rafael J. Wysocki wrote:
 On Wednesday, February 27, 2013 02:20:32 PM Greg Kroah-Hartman wrote:
  On Wed, Feb 27, 2013 at 11:06:52PM +0100, Rafael J. Wysocki wrote:
   From: Rafael J. Wysocki rafael.j.wyso...@intel.com
   
   After PCI has stopped using the .find_bridge() callback in
   struct acpi_bus_type, the only remaining users of it are SATA and
   USB.  However, SATA only pretends to be a user, because it points
   that callback to a stub always returning -ENODEV, and USB uses it
   incorrectly, because as a result of the way it is used by USB every
   device in the system that doesn't have a bus type or parent is
   passed to usb_acpi_find_device() for inspection.
   
   What USB actually needs, though, is to call usb_acpi_find_device()
   for USB ports that don't have a bus type defined, but have
   usb_port_device_type as their device type.
  
  Ick, that's not good.  Can you have the original creator of that code
  (someone else from Intel, I can't remember at the moment), fix that up
  properly and send me patches?
 
 That won't be necessary afer this patch.  Or do you want to fix up USB
 separately first?

No, sorry, my misunderstanding, I was assuming we still needed to do
other USB work after this.  If not, that's an even better reason to
accept this patch :)

thanks,

greg k-h
--
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