All of lore.kernel.org
 help / color / mirror / Atom feed
* [Resend][PATCH] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type
@ 2013-02-27 22:06 Rafael J. Wysocki
  2013-02-27 22:20 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2013-02-27 22:06 UTC (permalink / raw)
  To: ACPI Devel Maling List
  Cc: LKML, Greg Kroah-Hartman, Bjorn Helgaas, linux-usb, Tejun Heo,
	linux-ide, Jeff Garzik, Yinghai Lu

From: Rafael J. Wysocki <rafael.j.wysocki@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.wysocki@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;
 	int (*find_device) (struct device *, acpi_handle *);
-	/* For bridges, such as PCI root bridge, IDE controller */
-	int (*find_bridge) (struct device *, acpi_handle *);
 	void (*setup)(struct device *);
 	void (*cleanup)(struct device *);
 };
Index: linux-pm/drivers/ata/libata-acpi.c
===================================================================
--- linux-pm.orig/drivers/ata/libata-acpi.c
+++ linux-pm/drivers/ata/libata-acpi.c
@@ -1144,13 +1144,7 @@ static int ata_acpi_find_device(struct d
 		return -ENODEV;
 }
 
-static int ata_acpi_find_dummy(struct device *dev, acpi_handle *handle)
-{
-	return -ENODEV;
-}
-
 static struct acpi_bus_type ata_acpi_bus = {
-	.find_bridge = ata_acpi_find_dummy,
 	.find_device = ata_acpi_find_device,
 };
 
Index: linux-pm/drivers/usb/core/usb-acpi.c
===================================================================
--- linux-pm.orig/drivers/usb/core/usb-acpi.c
+++ linux-pm/drivers/usb/core/usb-acpi.c
@@ -212,7 +212,7 @@ static int usb_acpi_find_device(struct d
 
 static struct acpi_bus_type usb_acpi_bus = {
 	.bus = &usb_bus_type,
-	.find_bridge = usb_acpi_find_device,
+	.type = &usb_port_device_type,
 	.find_device = usb_acpi_find_device,
 };
 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Resend][PATCH] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type
  2013-02-27 22:06 [Resend][PATCH] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type Rafael J. Wysocki
@ 2013-02-27 22:20 ` Greg Kroah-Hartman
  2013-02-27 23:31   ` Yinghai Lu
  2013-02-28  1:11   ` Rafael J. Wysocki
  0 siblings, 2 replies; 15+ messages in thread
From: Greg Kroah-Hartman @ 2013-02-27 22:20 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: ACPI Devel Maling List, LKML, Bjorn Helgaas, linux-usb,
	Tejun Heo, linux-ide, Jeff Garzik, Yinghai Lu

On Wed, Feb 27, 2013 at 11:06:52PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@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 <gregkh@linuxfoundation.org>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Resend][PATCH] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type
  2013-02-27 22:20 ` Greg Kroah-Hartman
@ 2013-02-27 23:31   ` Yinghai Lu
  2013-02-28  0:23     ` Lan Tianyu
  2013-02-28  1:19     ` Rafael J. Wysocki
  2013-02-28  1:11   ` Rafael J. Wysocki
  1 sibling, 2 replies; 15+ messages in thread
From: Yinghai Lu @ 2013-02-27 23:31 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Lan Tianyu
  Cc: Rafael J. Wysocki, ACPI Devel Maling List, LKML, Bjorn Helgaas,
	linux-usb, Tejun Heo, linux-ide, Jeff Garzik

On Wed, Feb 27, 2013 at 2:20 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Wed, Feb 27, 2013 at 11:06:52PM +0100, Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki <rafael.j.wysocki@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.lan@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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Resend][PATCH] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type
  2013-02-27 23:31   ` Yinghai Lu
@ 2013-02-28  0:23     ` Lan Tianyu
  2013-02-28  1:19     ` Rafael J. Wysocki
  1 sibling, 0 replies; 15+ messages in thread
From: Lan Tianyu @ 2013-02-28  0:23 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, ACPI Devel Maling List,
	LKML, Bjorn Helgaas, linux-usb, Tejun Heo, linux-ide,
	Jeff Garzik

On 2013年02月28日 07:31, Yinghai Lu wrote:
> On Wed, Feb 27, 2013 at 2:20 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>> On Wed, Feb 27, 2013 at 11:06:52PM +0100, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki <rafael.j.wysocki@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.lan@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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Resend][PATCH] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type
  2013-02-27 22:20 ` Greg Kroah-Hartman
  2013-02-27 23:31   ` Yinghai Lu
@ 2013-02-28  1:11   ` Rafael J. Wysocki
  2013-02-28  2:33     ` Greg Kroah-Hartman
  1 sibling, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2013-02-28  1:11 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: ACPI Devel Maling List, LKML, Bjorn Helgaas, linux-usb,
	Tejun Heo, linux-ide, Jeff Garzik, Yinghai Lu

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.wysocki@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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Resend][PATCH] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type
  2013-02-27 23:31   ` Yinghai Lu
  2013-02-28  0:23     ` Lan Tianyu
@ 2013-02-28  1:19     ` Rafael J. Wysocki
  1 sibling, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2013-02-28  1:19 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Greg Kroah-Hartman, Lan Tianyu, ACPI Devel Maling List, LKML,
	Bjorn Helgaas, linux-usb, Tejun Heo, linux-ide, Jeff Garzik

On Wednesday, February 27, 2013 03:31:08 PM Yinghai Lu wrote:
> On Wed, Feb 27, 2013 at 2:20 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Wed, Feb 27, 2013 at 11:06:52PM +0100, Rafael J. Wysocki wrote:
> >> From: Rafael J. Wysocki <rafael.j.wysocki@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.lan@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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Resend][PATCH] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type
  2013-02-28  1:11   ` Rafael J. Wysocki
@ 2013-02-28  2:33     ` Greg Kroah-Hartman
  2013-02-28 21:49       ` Rafael J. Wysocki
  0 siblings, 1 reply; 15+ messages in thread
From: Greg Kroah-Hartman @ 2013-02-28  2:33 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: ACPI Devel Maling List, LKML, Bjorn Helgaas, linux-usb,
	Tejun Heo, linux-ide, Jeff Garzik, Yinghai Lu

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.wysocki@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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Resend][PATCH] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type
  2013-02-28  2:33     ` Greg Kroah-Hartman
@ 2013-02-28 21:49       ` Rafael J. Wysocki
  2013-02-28 21:53         ` [PATCH 1/2] ACPI / glue: Add .match() callback to " Rafael J. Wysocki
  2013-02-28 21:53         ` [PATCH 2/2] ACPI / glue: Drop .find_bridge() callback from " Rafael J. Wysocki
  0 siblings, 2 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2013-02-28 21:49 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: ACPI Devel Maling List, LKML, Bjorn Helgaas, linux-usb,
	Tejun Heo, linux-ide, Jeff Garzik, Yinghai Lu

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.wysocki@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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/2] ACPI / glue: Add .match() callback to struct acpi_bus_type
  2013-02-28 21:49       ` Rafael J. Wysocki
@ 2013-02-28 21:53         ` Rafael J. Wysocki
  2013-02-28 22:29           ` Yinghai Lu
  2013-03-01  5:07           ` Greg Kroah-Hartman
  2013-02-28 21:53         ` [PATCH 2/2] ACPI / glue: Drop .find_bridge() callback from " Rafael J. Wysocki
  1 sibling, 2 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2013-02-28 21:53 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: ACPI Devel Maling List, LKML, Bjorn Helgaas, linux-usb,
	Tejun Heo, linux-ide, Jeff Garzik, Yinghai Lu

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

USB uses the .find_bridge() callback from struct acpi_bus_type
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, as well as for USB
devices.

To fix that replace the struct bus_type pointer in struct
acpi_bus_type used for matching devices to specific subsystems
with a .match() callback to be used for this purpose and update
the users of struct acpi_bus_type, including USB, accordingly.
Define the .match() callback routine for USB, usb_acpi_bus_match(),
in such a way that it will cover both USB devices and USB ports
and remove the now redundant .find_bridge() callback pointer from
usb_acpi_bus.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/glue.c         |   39 +++++++++++++--------------------------
 drivers/ata/libata-acpi.c   |    1 +
 drivers/pci/pci-acpi.c      |    8 +++++++-
 drivers/pnp/pnpacpi/core.c  |    8 +++++++-
 drivers/scsi/scsi_lib.c     |    7 ++++++-
 drivers/usb/core/usb-acpi.c |    9 +++++++--
 include/acpi/acpi_bus.h     |    3 ++-
 7 files changed, 43 insertions(+), 32 deletions(-)

Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -437,7 +437,8 @@ void acpi_remove_dir(struct acpi_device
  */
 struct acpi_bus_type {
 	struct list_head list;
-	struct bus_type *bus;
+	const char *name;
+	bool (*match)(struct device *dev);
 	/* For general devices under the bus */
 	int (*find_device) (struct device *, acpi_handle *);
 	/* For bridges, such as PCI root bridge, IDE controller */
Index: linux-pm/drivers/acpi/glue.c
===================================================================
--- linux-pm.orig/drivers/acpi/glue.c
+++ linux-pm/drivers/acpi/glue.c
@@ -36,12 +36,11 @@ int register_acpi_bus_type(struct acpi_b
 {
 	if (acpi_disabled)
 		return -ENODEV;
-	if (type && type->bus && type->find_device) {
+	if (type && type->match && type->find_device) {
 		down_write(&bus_type_sem);
 		list_add_tail(&type->list, &bus_type_list);
 		up_write(&bus_type_sem);
-		printk(KERN_INFO PREFIX "bus type %s registered\n",
-		       type->bus->name);
+		printk(KERN_INFO PREFIX "bus type %s registered\n", type->name);
 		return 0;
 	}
 	return -ENODEV;
@@ -56,24 +55,21 @@ int unregister_acpi_bus_type(struct acpi
 		down_write(&bus_type_sem);
 		list_del_init(&type->list);
 		up_write(&bus_type_sem);
-		printk(KERN_INFO PREFIX "ACPI bus type %s unregistered\n",
-		       type->bus->name);
+		printk(KERN_INFO PREFIX "bus type %s unregistered\n",
+		       type->name);
 		return 0;
 	}
 	return -ENODEV;
 }
 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)
-		return NULL;
-
 	down_read(&bus_type_sem);
 	list_for_each_entry(tmp, &bus_type_list, list) {
-		if (tmp->bus == type) {
+		if (tmp->match(dev)) {
 			ret = tmp;
 			break;
 		}
@@ -261,26 +257,17 @@ 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));
-			ret = -EINVAL;
+		if (!type) {
+			ret = acpi_find_bridge_device(dev, &handle);
+			if (!ret)
+				ret = acpi_bind_one(dev, handle);
+
 			goto out;
 		}
 
@@ -316,7 +303,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/drivers/scsi/scsi_lib.c
===================================================================
--- linux-pm.orig/drivers/scsi/scsi_lib.c
+++ linux-pm/drivers/scsi/scsi_lib.c
@@ -71,9 +71,14 @@ struct kmem_cache *scsi_sdb_cache;
 #ifdef CONFIG_ACPI
 #include <acpi/acpi_bus.h>
 
+static bool acpi_scsi_bus_match(struct device *dev)
+{
+	return dev->bus == &scsi_bus_type;
+}
+
 int scsi_register_acpi_bus_type(struct acpi_bus_type *bus)
 {
-        bus->bus = &scsi_bus_type;
+        bus->match = acpi_scsi_bus_match;
         return register_acpi_bus_type(bus);
 }
 EXPORT_SYMBOL_GPL(scsi_register_acpi_bus_type);
Index: linux-pm/drivers/pci/pci-acpi.c
===================================================================
--- linux-pm.orig/drivers/pci/pci-acpi.c
+++ linux-pm/drivers/pci/pci-acpi.c
@@ -331,8 +331,14 @@ static void pci_acpi_cleanup(struct devi
 	}
 }
 
+static bool pci_acpi_bus_match(struct device *dev)
+{
+	return dev->bus == &pci_bus_type;
+}
+
 static struct acpi_bus_type acpi_pci_bus = {
-	.bus = &pci_bus_type,
+	.name = "PCI",
+	.match = pci_acpi_bus_match,
 	.find_device = acpi_pci_find_device,
 	.setup = pci_acpi_setup,
 	.cleanup = pci_acpi_cleanup,
Index: linux-pm/drivers/pnp/pnpacpi/core.c
===================================================================
--- linux-pm.orig/drivers/pnp/pnpacpi/core.c
+++ linux-pm/drivers/pnp/pnpacpi/core.c
@@ -353,8 +353,14 @@ static int __init acpi_pnp_find_device(s
 /* complete initialization of a PNPACPI device includes having
  * pnpdev->dev.archdata.acpi_handle point to its ACPI sibling.
  */
+static bool acpi_pnp_bus_match(struct device *dev)
+{
+	return dev->bus == &pnp_bus_type;
+}
+
 static struct acpi_bus_type __initdata acpi_pnp_bus = {
-	.bus	     = &pnp_bus_type,
+	.name	     = "PNP",
+	.match	     = acpi_pnp_bus_match,
 	.find_device = acpi_pnp_find_device,
 };
 
Index: linux-pm/drivers/usb/core/usb-acpi.c
===================================================================
--- linux-pm.orig/drivers/usb/core/usb-acpi.c
+++ linux-pm/drivers/usb/core/usb-acpi.c
@@ -210,9 +210,14 @@ static int usb_acpi_find_device(struct d
 	return 0;
 }
 
+static bool usb_acpi_bus_match(struct device *dev)
+{
+	return is_usb_device(dev) || is_usb_port(dev);
+}
+
 static struct acpi_bus_type usb_acpi_bus = {
-	.bus = &usb_bus_type,
-	.find_bridge = usb_acpi_find_device,
+	.name = "USB",
+	.match = usb_acpi_bus_match,
 	.find_device = usb_acpi_find_device,
 };
 
Index: linux-pm/drivers/ata/libata-acpi.c
===================================================================
--- linux-pm.orig/drivers/ata/libata-acpi.c
+++ linux-pm/drivers/ata/libata-acpi.c
@@ -1150,6 +1150,7 @@ static int ata_acpi_find_dummy(struct de
 }
 
 static struct acpi_bus_type ata_acpi_bus = {
+	.name = "ATA",
 	.find_bridge = ata_acpi_find_dummy,
 	.find_device = ata_acpi_find_device,
 };

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/2] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type
  2013-02-28 21:49       ` Rafael J. Wysocki
  2013-02-28 21:53         ` [PATCH 1/2] ACPI / glue: Add .match() callback to " Rafael J. Wysocki
@ 2013-02-28 21:53         ` Rafael J. Wysocki
  2013-02-28 22:13           ` Jeff Garzik
  1 sibling, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2013-02-28 21:53 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: ACPI Devel Maling List, LKML, Bjorn Helgaas, linux-usb,
	Tejun Heo, linux-ide, Jeff Garzik, Yinghai Lu

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

After PCI and USB have stopped using the .find_bridge() callback in
struct acpi_bus_type, the only remaining user of it is SATA, but SATA
only pretends to be a user, because it points that callback to a stub
always returning -ENODEV.

For this reason, 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.wysocki@intel.com>
---
 drivers/acpi/glue.c       |   26 +-------------------------
 drivers/ata/libata-acpi.c |    6 ------
 include/acpi/acpi_bus.h   |    3 ---
 3 files changed, 1 insertion(+), 34 deletions(-)

Index: linux-pm/drivers/ata/libata-acpi.c
===================================================================
--- linux-pm.orig/drivers/ata/libata-acpi.c
+++ linux-pm/drivers/ata/libata-acpi.c
@@ -1144,14 +1144,8 @@ static int ata_acpi_find_device(struct d
 		return -ENODEV;
 }
 
-static int ata_acpi_find_dummy(struct device *dev, acpi_handle *handle)
-{
-	return -ENODEV;
-}
-
 static struct acpi_bus_type ata_acpi_bus = {
 	.name = "ATA",
-	.find_bridge = ata_acpi_find_dummy,
 	.find_device = ata_acpi_find_device,
 };
 
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -439,10 +439,7 @@ struct acpi_bus_type {
 	struct list_head list;
 	const char *name;
 	bool (*match)(struct device *dev);
-	/* For general devices under the bus */
 	int (*find_device) (struct device *, acpi_handle *);
-	/* For bridges, such as PCI root bridge, IDE controller */
-	int (*find_bridge) (struct device *, acpi_handle *);
 	void (*setup)(struct device *);
 	void (*cleanup)(struct device *);
 };
Index: linux-pm/drivers/acpi/glue.c
===================================================================
--- linux-pm.orig/drivers/acpi/glue.c
+++ linux-pm/drivers/acpi/glue.c
@@ -78,22 +78,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)
 {
@@ -262,15 +246,7 @@ static int acpi_platform_notify(struct d
 	int ret;
 
 	ret = acpi_bind_one(dev, NULL);
-	if (ret) {
-		if (!type) {
-			ret = acpi_find_bridge_device(dev, &handle);
-			if (!ret)
-				ret = acpi_bind_one(dev, handle);
-
-			goto out;
-		}
-
+	if (ret && type) {
 		ret = type->find_device(dev, &handle);
 		if (ret) {
 			DBG("Unable to get handle for %s\n", dev_name(dev));

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type
  2013-02-28 21:53         ` [PATCH 2/2] ACPI / glue: Drop .find_bridge() callback from " Rafael J. Wysocki
@ 2013-02-28 22:13           ` Jeff Garzik
  2013-02-28 23:37             ` Rafael J. Wysocki
  0 siblings, 1 reply; 15+ messages in thread
From: Jeff Garzik @ 2013-02-28 22:13 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Greg Kroah-Hartman, ACPI Devel Maling List, LKML, Bjorn Helgaas,
	linux-usb, Tejun Heo, linux-ide, Yinghai Lu

On 02/28/2013 04:53 PM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> After PCI and USB have stopped using the .find_bridge() callback in
> struct acpi_bus_type, the only remaining user of it is SATA, but SATA
> only pretends to be a user, because it points that callback to a stub
> always returning -ENODEV.
>
> For this reason, 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.wysocki@intel.com>
> ---
>   drivers/acpi/glue.c       |   26 +-------------------------
>   drivers/ata/libata-acpi.c |    6 ------
>   include/acpi/acpi_bus.h   |    3 ---
>   3 files changed, 1 insertion(+), 34 deletions(-)

patches 1-2 Acked-by: Jeff Garzik <jgarzik@pobox.com>




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] ACPI / glue: Add .match() callback to struct acpi_bus_type
  2013-02-28 21:53         ` [PATCH 1/2] ACPI / glue: Add .match() callback to " Rafael J. Wysocki
@ 2013-02-28 22:29           ` Yinghai Lu
  2013-02-28 23:37             ` Rafael J. Wysocki
  2013-03-01  5:07           ` Greg Kroah-Hartman
  1 sibling, 1 reply; 15+ messages in thread
From: Yinghai Lu @ 2013-02-28 22:29 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Greg Kroah-Hartman, ACPI Devel Maling List, LKML, Bjorn Helgaas,
	linux-usb, Tejun Heo, linux-ide, Jeff Garzik

On Thu, Feb 28, 2013 at 1:53 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> USB uses the .find_bridge() callback from struct acpi_bus_type
> 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, as well as for USB
> devices.
>
> To fix that replace the struct bus_type pointer in struct
> acpi_bus_type used for matching devices to specific subsystems
> with a .match() callback to be used for this purpose and update
> the users of struct acpi_bus_type, including USB, accordingly.
> Define the .match() callback routine for USB, usb_acpi_bus_match(),
> in such a way that it will cover both USB devices and USB ports
> and remove the now redundant .find_bridge() callback pointer from
> usb_acpi_bus.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/acpi/glue.c         |   39 +++++++++++++--------------------------
>  drivers/ata/libata-acpi.c   |    1 +
>  drivers/pci/pci-acpi.c      |    8 +++++++-
>  drivers/pnp/pnpacpi/core.c  |    8 +++++++-
>  drivers/scsi/scsi_lib.c     |    7 ++++++-
>  drivers/usb/core/usb-acpi.c |    9 +++++++--
>  include/acpi/acpi_bus.h     |    3 ++-
>  7 files changed, 43 insertions(+), 32 deletions(-)
>
> Index: linux-pm/include/acpi/acpi_bus.h
> ===================================================================
> --- linux-pm.orig/include/acpi/acpi_bus.h
> +++ linux-pm/include/acpi/acpi_bus.h
> @@ -437,7 +437,8 @@ void acpi_remove_dir(struct acpi_device
>   */
>  struct acpi_bus_type {
>         struct list_head list;
> -       struct bus_type *bus;
> +       const char *name;
> +       bool (*match)(struct device *dev);
>         /* For general devices under the bus */
>         int (*find_device) (struct device *, acpi_handle *);
>         /* For bridges, such as PCI root bridge, IDE controller */
> Index: linux-pm/drivers/acpi/glue.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/glue.c
> +++ linux-pm/drivers/acpi/glue.c
> @@ -36,12 +36,11 @@ int register_acpi_bus_type(struct acpi_b
>  {
>         if (acpi_disabled)
>                 return -ENODEV;
> -       if (type && type->bus && type->find_device) {
> +       if (type && type->match && type->find_device) {
>                 down_write(&bus_type_sem);
>                 list_add_tail(&type->list, &bus_type_list);
>                 up_write(&bus_type_sem);
> -               printk(KERN_INFO PREFIX "bus type %s registered\n",
> -                      type->bus->name);
> +               printk(KERN_INFO PREFIX "bus type %s registered\n", type->name);
>                 return 0;
>         }
>         return -ENODEV;
> @@ -56,24 +55,21 @@ int unregister_acpi_bus_type(struct acpi
>                 down_write(&bus_type_sem);
>                 list_del_init(&type->list);
>                 up_write(&bus_type_sem);
> -               printk(KERN_INFO PREFIX "ACPI bus type %s unregistered\n",
> -                      type->bus->name);
> +               printk(KERN_INFO PREFIX "bus type %s unregistered\n",
> +                      type->name);
>                 return 0;
>         }
>         return -ENODEV;
>  }
>  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)
> -               return NULL;
> -
>         down_read(&bus_type_sem);
>         list_for_each_entry(tmp, &bus_type_list, list) {
> -               if (tmp->bus == type) {
> +               if (tmp->match(dev)) {
>                         ret = tmp;
>                         break;
>                 }
> @@ -261,26 +257,17 @@ 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));
> -                       ret = -EINVAL;
> +               if (!type) {
> +                       ret = acpi_find_bridge_device(dev, &handle);
> +                       if (!ret)
> +                               ret = acpi_bind_one(dev, handle);
> +
>                         goto out;
>                 }
>
> @@ -316,7 +303,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/drivers/scsi/scsi_lib.c
> ===================================================================
> --- linux-pm.orig/drivers/scsi/scsi_lib.c
> +++ linux-pm/drivers/scsi/scsi_lib.c
> @@ -71,9 +71,14 @@ struct kmem_cache *scsi_sdb_cache;
>  #ifdef CONFIG_ACPI
>  #include <acpi/acpi_bus.h>
>
> +static bool acpi_scsi_bus_match(struct device *dev)
> +{
> +       return dev->bus == &scsi_bus_type;
> +}
> +
>  int scsi_register_acpi_bus_type(struct acpi_bus_type *bus)
>  {
> -        bus->bus = &scsi_bus_type;
> +        bus->match = acpi_scsi_bus_match;
>          return register_acpi_bus_type(bus);
>  }
>  EXPORT_SYMBOL_GPL(scsi_register_acpi_bus_type);
> Index: linux-pm/drivers/pci/pci-acpi.c
> ===================================================================
> --- linux-pm.orig/drivers/pci/pci-acpi.c
> +++ linux-pm/drivers/pci/pci-acpi.c
> @@ -331,8 +331,14 @@ static void pci_acpi_cleanup(struct devi
>         }
>  }
>
> +static bool pci_acpi_bus_match(struct device *dev)
> +{
> +       return dev->bus == &pci_bus_type;
> +}
> +
>  static struct acpi_bus_type acpi_pci_bus = {
> -       .bus = &pci_bus_type,
> +       .name = "PCI",
> +       .match = pci_acpi_bus_match,
>         .find_device = acpi_pci_find_device,
>         .setup = pci_acpi_setup,
>         .cleanup = pci_acpi_cleanup,
> Index: linux-pm/drivers/pnp/pnpacpi/core.c
> ===================================================================
> --- linux-pm.orig/drivers/pnp/pnpacpi/core.c
> +++ linux-pm/drivers/pnp/pnpacpi/core.c
> @@ -353,8 +353,14 @@ static int __init acpi_pnp_find_device(s
>  /* complete initialization of a PNPACPI device includes having
>   * pnpdev->dev.archdata.acpi_handle point to its ACPI sibling.
>   */
> +static bool acpi_pnp_bus_match(struct device *dev)
> +{
> +       return dev->bus == &pnp_bus_type;
> +}
> +
>  static struct acpi_bus_type __initdata acpi_pnp_bus = {
> -       .bus         = &pnp_bus_type,
> +       .name        = "PNP",
> +       .match       = acpi_pnp_bus_match,
>         .find_device = acpi_pnp_find_device,
>  };
>
> Index: linux-pm/drivers/usb/core/usb-acpi.c
> ===================================================================
> --- linux-pm.orig/drivers/usb/core/usb-acpi.c
> +++ linux-pm/drivers/usb/core/usb-acpi.c
> @@ -210,9 +210,14 @@ static int usb_acpi_find_device(struct d
>         return 0;
>  }
>
> +static bool usb_acpi_bus_match(struct device *dev)
> +{
> +       return is_usb_device(dev) || is_usb_port(dev);
> +}
> +
>  static struct acpi_bus_type usb_acpi_bus = {
> -       .bus = &usb_bus_type,
> -       .find_bridge = usb_acpi_find_device,
> +       .name = "USB",
> +       .match = usb_acpi_bus_match,
>         .find_device = usb_acpi_find_device,
>  };

yes, much clean.

Don't need to add extra bus type for usb port device.

For 1-2.
Acked-by: Yinghai Lu <yinghai@kernel.org>

Thanks

Yinghai


>
> Index: linux-pm/drivers/ata/libata-acpi.c
> ===================================================================
> --- linux-pm.orig/drivers/ata/libata-acpi.c
> +++ linux-pm/drivers/ata/libata-acpi.c
> @@ -1150,6 +1150,7 @@ static int ata_acpi_find_dummy(struct de
>  }
>
>  static struct acpi_bus_type ata_acpi_bus = {
> +       .name = "ATA",
>         .find_bridge = ata_acpi_find_dummy,
>         .find_device = ata_acpi_find_device,
>  };

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] ACPI / glue: Add .match() callback to struct acpi_bus_type
  2013-02-28 22:29           ` Yinghai Lu
@ 2013-02-28 23:37             ` Rafael J. Wysocki
  0 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2013-02-28 23:37 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Greg Kroah-Hartman, ACPI Devel Maling List, LKML, Bjorn Helgaas,
	linux-usb, Tejun Heo, linux-ide, Jeff Garzik

On Thursday, February 28, 2013 02:29:56 PM Yinghai Lu wrote:
> On Thu, Feb 28, 2013 at 1:53 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > USB uses the .find_bridge() callback from struct acpi_bus_type
> > 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, as well as for USB
> > devices.
> >
> > To fix that replace the struct bus_type pointer in struct
> > acpi_bus_type used for matching devices to specific subsystems
> > with a .match() callback to be used for this purpose and update
> > the users of struct acpi_bus_type, including USB, accordingly.
> > Define the .match() callback routine for USB, usb_acpi_bus_match(),
> > in such a way that it will cover both USB devices and USB ports
> > and remove the now redundant .find_bridge() callback pointer from
> > usb_acpi_bus.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  drivers/acpi/glue.c         |   39 +++++++++++++--------------------------
> >  drivers/ata/libata-acpi.c   |    1 +
> >  drivers/pci/pci-acpi.c      |    8 +++++++-
> >  drivers/pnp/pnpacpi/core.c  |    8 +++++++-
> >  drivers/scsi/scsi_lib.c     |    7 ++++++-
> >  drivers/usb/core/usb-acpi.c |    9 +++++++--
> >  include/acpi/acpi_bus.h     |    3 ++-
> >  7 files changed, 43 insertions(+), 32 deletions(-)
> >
> > Index: linux-pm/include/acpi/acpi_bus.h
> > ===================================================================
> > --- linux-pm.orig/include/acpi/acpi_bus.h
> > +++ linux-pm/include/acpi/acpi_bus.h
> > @@ -437,7 +437,8 @@ void acpi_remove_dir(struct acpi_device
> >   */
> >  struct acpi_bus_type {
> >         struct list_head list;
> > -       struct bus_type *bus;
> > +       const char *name;
> > +       bool (*match)(struct device *dev);
> >         /* For general devices under the bus */
> >         int (*find_device) (struct device *, acpi_handle *);
> >         /* For bridges, such as PCI root bridge, IDE controller */
> > Index: linux-pm/drivers/acpi/glue.c
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/glue.c
> > +++ linux-pm/drivers/acpi/glue.c
> > @@ -36,12 +36,11 @@ int register_acpi_bus_type(struct acpi_b
> >  {
> >         if (acpi_disabled)
> >                 return -ENODEV;
> > -       if (type && type->bus && type->find_device) {
> > +       if (type && type->match && type->find_device) {
> >                 down_write(&bus_type_sem);
> >                 list_add_tail(&type->list, &bus_type_list);
> >                 up_write(&bus_type_sem);
> > -               printk(KERN_INFO PREFIX "bus type %s registered\n",
> > -                      type->bus->name);
> > +               printk(KERN_INFO PREFIX "bus type %s registered\n", type->name);
> >                 return 0;
> >         }
> >         return -ENODEV;
> > @@ -56,24 +55,21 @@ int unregister_acpi_bus_type(struct acpi
> >                 down_write(&bus_type_sem);
> >                 list_del_init(&type->list);
> >                 up_write(&bus_type_sem);
> > -               printk(KERN_INFO PREFIX "ACPI bus type %s unregistered\n",
> > -                      type->bus->name);
> > +               printk(KERN_INFO PREFIX "bus type %s unregistered\n",
> > +                      type->name);
> >                 return 0;
> >         }
> >         return -ENODEV;
> >  }
> >  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)
> > -               return NULL;
> > -
> >         down_read(&bus_type_sem);
> >         list_for_each_entry(tmp, &bus_type_list, list) {
> > -               if (tmp->bus == type) {
> > +               if (tmp->match(dev)) {
> >                         ret = tmp;
> >                         break;
> >                 }
> > @@ -261,26 +257,17 @@ 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));
> > -                       ret = -EINVAL;
> > +               if (!type) {
> > +                       ret = acpi_find_bridge_device(dev, &handle);
> > +                       if (!ret)
> > +                               ret = acpi_bind_one(dev, handle);
> > +
> >                         goto out;
> >                 }
> >
> > @@ -316,7 +303,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/drivers/scsi/scsi_lib.c
> > ===================================================================
> > --- linux-pm.orig/drivers/scsi/scsi_lib.c
> > +++ linux-pm/drivers/scsi/scsi_lib.c
> > @@ -71,9 +71,14 @@ struct kmem_cache *scsi_sdb_cache;
> >  #ifdef CONFIG_ACPI
> >  #include <acpi/acpi_bus.h>
> >
> > +static bool acpi_scsi_bus_match(struct device *dev)
> > +{
> > +       return dev->bus == &scsi_bus_type;
> > +}
> > +
> >  int scsi_register_acpi_bus_type(struct acpi_bus_type *bus)
> >  {
> > -        bus->bus = &scsi_bus_type;
> > +        bus->match = acpi_scsi_bus_match;
> >          return register_acpi_bus_type(bus);
> >  }
> >  EXPORT_SYMBOL_GPL(scsi_register_acpi_bus_type);
> > Index: linux-pm/drivers/pci/pci-acpi.c
> > ===================================================================
> > --- linux-pm.orig/drivers/pci/pci-acpi.c
> > +++ linux-pm/drivers/pci/pci-acpi.c
> > @@ -331,8 +331,14 @@ static void pci_acpi_cleanup(struct devi
> >         }
> >  }
> >
> > +static bool pci_acpi_bus_match(struct device *dev)
> > +{
> > +       return dev->bus == &pci_bus_type;
> > +}
> > +
> >  static struct acpi_bus_type acpi_pci_bus = {
> > -       .bus = &pci_bus_type,
> > +       .name = "PCI",
> > +       .match = pci_acpi_bus_match,
> >         .find_device = acpi_pci_find_device,
> >         .setup = pci_acpi_setup,
> >         .cleanup = pci_acpi_cleanup,
> > Index: linux-pm/drivers/pnp/pnpacpi/core.c
> > ===================================================================
> > --- linux-pm.orig/drivers/pnp/pnpacpi/core.c
> > +++ linux-pm/drivers/pnp/pnpacpi/core.c
> > @@ -353,8 +353,14 @@ static int __init acpi_pnp_find_device(s
> >  /* complete initialization of a PNPACPI device includes having
> >   * pnpdev->dev.archdata.acpi_handle point to its ACPI sibling.
> >   */
> > +static bool acpi_pnp_bus_match(struct device *dev)
> > +{
> > +       return dev->bus == &pnp_bus_type;
> > +}
> > +
> >  static struct acpi_bus_type __initdata acpi_pnp_bus = {
> > -       .bus         = &pnp_bus_type,
> > +       .name        = "PNP",
> > +       .match       = acpi_pnp_bus_match,
> >         .find_device = acpi_pnp_find_device,
> >  };
> >
> > Index: linux-pm/drivers/usb/core/usb-acpi.c
> > ===================================================================
> > --- linux-pm.orig/drivers/usb/core/usb-acpi.c
> > +++ linux-pm/drivers/usb/core/usb-acpi.c
> > @@ -210,9 +210,14 @@ static int usb_acpi_find_device(struct d
> >         return 0;
> >  }
> >
> > +static bool usb_acpi_bus_match(struct device *dev)
> > +{
> > +       return is_usb_device(dev) || is_usb_port(dev);
> > +}
> > +
> >  static struct acpi_bus_type usb_acpi_bus = {
> > -       .bus = &usb_bus_type,
> > -       .find_bridge = usb_acpi_find_device,
> > +       .name = "USB",
> > +       .match = usb_acpi_bus_match,
> >         .find_device = usb_acpi_find_device,
> >  };
> 
> yes, much clean.
> 
> Don't need to add extra bus type for usb port device.
> 
> For 1-2.
> Acked-by: Yinghai Lu <yinghai@kernel.org>

Thanks!

Rafael


> > Index: linux-pm/drivers/ata/libata-acpi.c
> > ===================================================================
> > --- linux-pm.orig/drivers/ata/libata-acpi.c
> > +++ linux-pm/drivers/ata/libata-acpi.c
> > @@ -1150,6 +1150,7 @@ static int ata_acpi_find_dummy(struct de
> >  }
> >
> >  static struct acpi_bus_type ata_acpi_bus = {
> > +       .name = "ATA",
> >         .find_bridge = ata_acpi_find_dummy,
> >         .find_device = ata_acpi_find_device,
> >  };
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type
  2013-02-28 22:13           ` Jeff Garzik
@ 2013-02-28 23:37             ` Rafael J. Wysocki
  0 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2013-02-28 23:37 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Greg Kroah-Hartman, ACPI Devel Maling List, LKML, Bjorn Helgaas,
	linux-usb, Tejun Heo, linux-ide, Yinghai Lu

On Thursday, February 28, 2013 05:13:27 PM Jeff Garzik wrote:
> On 02/28/2013 04:53 PM, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > After PCI and USB have stopped using the .find_bridge() callback in
> > struct acpi_bus_type, the only remaining user of it is SATA, but SATA
> > only pretends to be a user, because it points that callback to a stub
> > always returning -ENODEV.
> >
> > For this reason, 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.wysocki@intel.com>
> > ---
> >   drivers/acpi/glue.c       |   26 +-------------------------
> >   drivers/ata/libata-acpi.c |    6 ------
> >   include/acpi/acpi_bus.h   |    3 ---
> >   3 files changed, 1 insertion(+), 34 deletions(-)
> 
> patches 1-2 Acked-by: Jeff Garzik <jgarzik@pobox.com>

Thanks!


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] ACPI / glue: Add .match() callback to struct acpi_bus_type
  2013-02-28 21:53         ` [PATCH 1/2] ACPI / glue: Add .match() callback to " Rafael J. Wysocki
  2013-02-28 22:29           ` Yinghai Lu
@ 2013-03-01  5:07           ` Greg Kroah-Hartman
  1 sibling, 0 replies; 15+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-01  5:07 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: ACPI Devel Maling List, LKML, Bjorn Helgaas, linux-usb,
	Tejun Heo, linux-ide, Jeff Garzik, Yinghai Lu

On Thu, Feb 28, 2013 at 10:53:21PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> USB uses the .find_bridge() callback from struct acpi_bus_type
> 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, as well as for USB
> devices.
> 
> To fix that replace the struct bus_type pointer in struct
> acpi_bus_type used for matching devices to specific subsystems
> with a .match() callback to be used for this purpose and update
> the users of struct acpi_bus_type, including USB, accordingly.
> Define the .match() callback routine for USB, usb_acpi_bus_match(),
> in such a way that it will cover both USB devices and USB ports
> and remove the now redundant .find_bridge() callback pointer from
> usb_acpi_bus.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2013-03-01  5:07 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-27 22:06 [Resend][PATCH] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type Rafael J. Wysocki
2013-02-27 22:20 ` Greg Kroah-Hartman
2013-02-27 23:31   ` Yinghai Lu
2013-02-28  0:23     ` Lan Tianyu
2013-02-28  1:19     ` Rafael J. Wysocki
2013-02-28  1:11   ` Rafael J. Wysocki
2013-02-28  2:33     ` Greg Kroah-Hartman
2013-02-28 21:49       ` Rafael J. Wysocki
2013-02-28 21:53         ` [PATCH 1/2] ACPI / glue: Add .match() callback to " Rafael J. Wysocki
2013-02-28 22:29           ` Yinghai Lu
2013-02-28 23:37             ` Rafael J. Wysocki
2013-03-01  5:07           ` Greg Kroah-Hartman
2013-02-28 21:53         ` [PATCH 2/2] ACPI / glue: Drop .find_bridge() callback from " Rafael J. Wysocki
2013-02-28 22:13           ` Jeff Garzik
2013-02-28 23:37             ` Rafael J. Wysocki

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.