On Sun, Jan 27, 2013 at 2:32 PM, Rafael J. Wysocki <rjw@xxxxxxx> wrote:
> On Sunday, January 27, 2013 11:23:31 AM Yinghai Lu wrote:
>> only device that does not have bus_type, will go to that path...
>
> While I agree that the comment doesn't make sense any more, I also think that
> changing the comment alone is not sufficient, because what USB does with
> .find_bridge() is really disgusting to me and USB is the only real user of it
> (SATA just pretends to be one).
agree. that is really like hack for usb.
>
> So, instead of this change, I'd prefer to apply the appended patch some time
> in the second part of the 3.9 merge window, after integrating all of the
> ACPI and PCI changes already queued up.
Good, I will drop patch 4 but will keep patch3 about libata part with
ack from Jeff Garzik.
>
> Thanks,
> Rafael
>
>
> ---
> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> Subject: ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type
>
> 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@xxxxxxxxx>
> ---
> 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
> @@ -412,10 +412,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
> @@ -1167,13 +1167,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,
> };
Can we add one acpi_bus_type for port directly?
Thanks
Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
