From patchwork Fri Nov 29 12:19:24 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rafael J. Wysocki" X-Patchwork-Id: 3257621 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id EB2D29F1F4 for ; Fri, 29 Nov 2013 12:07:05 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 66643205FD for ; Fri, 29 Nov 2013 12:07:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 44885205FF for ; Fri, 29 Nov 2013 12:06:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752819Ab3K2MGf (ORCPT ); Fri, 29 Nov 2013 07:06:35 -0500 Received: from v094114.home.net.pl ([79.96.170.134]:61702 "HELO v094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1753191Ab3K2MGd convert rfc822-to-8bit (ORCPT ); Fri, 29 Nov 2013 07:06:33 -0500 Received: from afch246.neoplus.adsl.tpnet.pl [95.49.59.246] (HELO vostro.rjw.lan) by serwer1319399.home.pl [79.96.170.134] with SMTP (IdeaSmtpServer v0.80) id 3c5dd5bf789ca769; Fri, 29 Nov 2013 13:06:30 +0100 From: "Rafael J. Wysocki" To: Lan Tianyu Cc: ACPI Devel Maling List , Linux PCI , LKML , Greg Kroah-Hartman , Aaron Lu , Toshi Kani , Bjorn Helgaas , linux-ide@vger.kernel.org, linux-usb@vger.kernel.org Subject: Re: [PATCH 1/3] ACPI / bind: Rework struct acpi_bus_type Date: Fri, 29 Nov 2013 13:19:24 +0100 Message-ID: <1930953.ZacyFLFM6D@vostro.rjw.lan> User-Agent: KMail/4.10.5 (Linux/3.12.0-rc6+; KDE/4.10.5; x86_64; ; ) In-Reply-To: <5297F3FB.6030808@intel.com> References: <4501334.ovACrGogrl@vostro.rjw.lan> <2273591.1tQV675JFy@vostro.rjw.lan> <5297F3FB.6030808@intel.com> MIME-Version: 1.0 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Friday, November 29, 2013 09:55:07 AM Lan Tianyu wrote: > On 2013?11?29? 08:37, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki > > > > Replace the .find_device function pointer in struct acpi_bus_type > > with a new one, .find_copmanion, that is supposed to point to a > -------------------------^ > A typo Yeah, thanks! > > function returning struct acpi_device pointer (instead of an int) > > and takes one argument (instead of two). This way the role of > > this callback is more clear and the implementation of it can > > be more straightforward. > > > > Update all of the users of struct acpi_bus_type (PCI, PNP/ACPI and > > USB) to reflect the structure change. > > > > Signed-off-by: Rafael J. Wysocki > > --- > > drivers/acpi/glue.c | 12 +++++++----- > > drivers/pci/pci-acpi.c | 12 +++--------- > > drivers/pnp/pnpacpi/core.c | 19 +++++-------------- > > drivers/usb/core/usb-acpi.c | 40 ++++++++++++++++++++-------------------- > > include/acpi/acpi_bus.h | 2 +- > > 5 files changed, 36 insertions(+), 49 deletions(-) > > > > Index: linux-pm/include/acpi/acpi_bus.h > > =================================================================== > > --- linux-pm.orig/include/acpi/acpi_bus.h > > +++ linux-pm/include/acpi/acpi_bus.h > > @@ -414,7 +414,7 @@ struct acpi_bus_type { > > struct list_head list; > > const char *name; > > bool (*match)(struct device *dev); > > - int (*find_device) (struct device *, acpi_handle *); > > + struct acpi_device * (*find_companion)(struct device *); > > 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 > > @@ -37,7 +37,7 @@ int register_acpi_bus_type(struct acpi_b > > { > > if (acpi_disabled) > > return -ENODEV; > > - if (type && type->match && type->find_device) { > > + if (type && type->match && type->find_companion) { > > down_write(&bus_type_sem); > > list_add_tail(&type->list, &bus_type_list); > > up_write(&bus_type_sem); > > @@ -302,17 +302,19 @@ EXPORT_SYMBOL_GPL(acpi_unbind_one); > > static int acpi_platform_notify(struct device *dev) > > { > > struct acpi_bus_type *type = acpi_get_bus_type(dev); > > - acpi_handle handle; > > int ret; > > > > ret = acpi_bind_one(dev, NULL); > > if (ret && type) { > > - ret = type->find_device(dev, &handle); > > - if (ret) { > > + struct acpi_device *adev; > > + > > + adev = type->find_companion(dev); > > + if (!adev) { > > DBG("Unable to get handle for %s\n", dev_name(dev)); > > + ret = -ENODEV; > > goto out; > > } > > - ret = acpi_bind_one(dev, handle); > > + ret = acpi_bind_one(dev, adev->handle); > > if (ret) > > goto out; > > } > > Index: linux-pm/drivers/pci/pci-acpi.c > > =================================================================== > > --- linux-pm.orig/drivers/pci/pci-acpi.c > > +++ linux-pm/drivers/pci/pci-acpi.c > > @@ -306,10 +306,9 @@ void acpi_pci_remove_bus(struct pci_bus > > } > > > > /* ACPI bus type */ > > -static int acpi_pci_find_device(struct device *dev, acpi_handle *handle) > > +static struct acpi_device *acpi_pci_find_companion(struct device *dev) > > { > > struct pci_dev *pci_dev = to_pci_dev(dev); > > - struct acpi_device *adev; > > bool check_children; > > u64 addr; > > > > @@ -322,13 +321,8 @@ static int acpi_pci_find_device(struct d > > || pci_dev->hdr_type == PCI_HEADER_TYPE_CARDBUS; > > /* Please ref to ACPI spec for the syntax of _ADR */ > > addr = (PCI_SLOT(pci_dev->devfn) << 16) | PCI_FUNC(pci_dev->devfn); > > - adev = acpi_find_child_device(ACPI_COMPANION(dev->parent), addr, > > + return acpi_find_child_device(ACPI_COMPANION(dev->parent), addr, > > check_children); > > - if (adev) { > > - *handle = adev->handle; > > - return 0; > > - } > > - return -ENODEV; > > } > > > > static void pci_acpi_setup(struct device *dev) > > @@ -368,7 +362,7 @@ static bool pci_acpi_bus_match(struct de > > static struct acpi_bus_type acpi_pci_bus = { > > .name = "PCI", > > .match = pci_acpi_bus_match, > > - .find_device = acpi_pci_find_device, > > + .find_companion = acpi_pci_find_companion, > > .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 > > @@ -329,20 +329,11 @@ static int __init acpi_pnp_match(struct > > && compare_pnp_id(pnp->id, acpi_device_hid(acpi)); > > } > > > > -static int __init acpi_pnp_find_device(struct device *dev, acpi_handle * handle) > > +static struct acpi_device * __init acpi_pnp_find_companion(struct device *dev) > > { > > - struct device *adev; > > - struct acpi_device *acpi; > > - > > - adev = bus_find_device(&acpi_bus_type, NULL, > > - to_pnp_dev(dev), acpi_pnp_match); > > - if (!adev) > > - return -ENODEV; > > - > > - acpi = to_acpi_device(adev); > > - *handle = acpi->handle; > > - put_device(adev); > > - return 0; > > + dev = bus_find_device(&acpi_bus_type, NULL, to_pnp_dev(dev), > > + acpi_pnp_match); > > Why remove put_device here? > bus_find_device() increase dev's reference count when a dev is matched. By mistake. Thanks for catching this! Update patch follows. Thanks, Rafael --- From: Rafael J. Wysocki Subject: ACPI / bind: Rework struct acpi_bus_type Replace the .find_device function pointer in struct acpi_bus_type with a new one, .find_companion, that is supposed to point to a function returning struct acpi_device pointer (instead of an int) and takes one argument (instead of two). This way the role of this callback is more clear and the implementation of it can be more straightforward. Update all of the users of struct acpi_bus_type (PCI, PNP/ACPI and USB) to reflect the structure change. Signed-off-by: Rafael J. Wysocki --- drivers/acpi/glue.c | 12 +++++++----- drivers/pci/pci-acpi.c | 12 +++--------- drivers/pnp/pnpacpi/core.c | 21 ++++++++------------- drivers/usb/core/usb-acpi.c | 40 ++++++++++++++++++++-------------------- include/acpi/acpi_bus.h | 2 +- 5 files changed, 39 insertions(+), 48 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Index: linux-pm/include/acpi/acpi_bus.h =================================================================== --- linux-pm.orig/include/acpi/acpi_bus.h +++ linux-pm/include/acpi/acpi_bus.h @@ -414,7 +414,7 @@ struct acpi_bus_type { struct list_head list; const char *name; bool (*match)(struct device *dev); - int (*find_device) (struct device *, acpi_handle *); + struct acpi_device * (*find_companion)(struct device *); 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 @@ -37,7 +37,7 @@ int register_acpi_bus_type(struct acpi_b { if (acpi_disabled) return -ENODEV; - if (type && type->match && type->find_device) { + if (type && type->match && type->find_companion) { down_write(&bus_type_sem); list_add_tail(&type->list, &bus_type_list); up_write(&bus_type_sem); @@ -302,17 +302,19 @@ EXPORT_SYMBOL_GPL(acpi_unbind_one); static int acpi_platform_notify(struct device *dev) { struct acpi_bus_type *type = acpi_get_bus_type(dev); - acpi_handle handle; int ret; ret = acpi_bind_one(dev, NULL); if (ret && type) { - ret = type->find_device(dev, &handle); - if (ret) { + struct acpi_device *adev; + + adev = type->find_companion(dev); + if (!adev) { DBG("Unable to get handle for %s\n", dev_name(dev)); + ret = -ENODEV; goto out; } - ret = acpi_bind_one(dev, handle); + ret = acpi_bind_one(dev, adev->handle); if (ret) goto out; } Index: linux-pm/drivers/pci/pci-acpi.c =================================================================== --- linux-pm.orig/drivers/pci/pci-acpi.c +++ linux-pm/drivers/pci/pci-acpi.c @@ -306,10 +306,9 @@ void acpi_pci_remove_bus(struct pci_bus } /* ACPI bus type */ -static int acpi_pci_find_device(struct device *dev, acpi_handle *handle) +static struct acpi_device *acpi_pci_find_companion(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); - struct acpi_device *adev; bool check_children; u64 addr; @@ -322,13 +321,8 @@ static int acpi_pci_find_device(struct d || pci_dev->hdr_type == PCI_HEADER_TYPE_CARDBUS; /* Please ref to ACPI spec for the syntax of _ADR */ addr = (PCI_SLOT(pci_dev->devfn) << 16) | PCI_FUNC(pci_dev->devfn); - adev = acpi_find_child_device(ACPI_COMPANION(dev->parent), addr, + return acpi_find_child_device(ACPI_COMPANION(dev->parent), addr, check_children); - if (adev) { - *handle = adev->handle; - return 0; - } - return -ENODEV; } static void pci_acpi_setup(struct device *dev) @@ -368,7 +362,7 @@ static bool pci_acpi_bus_match(struct de static struct acpi_bus_type acpi_pci_bus = { .name = "PCI", .match = pci_acpi_bus_match, - .find_device = acpi_pci_find_device, + .find_companion = acpi_pci_find_companion, .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 @@ -329,20 +329,15 @@ static int __init acpi_pnp_match(struct && compare_pnp_id(pnp->id, acpi_device_hid(acpi)); } -static int __init acpi_pnp_find_device(struct device *dev, acpi_handle * handle) +static struct acpi_device * __init acpi_pnp_find_companion(struct device *dev) { - struct device *adev; - struct acpi_device *acpi; + dev = bus_find_device(&acpi_bus_type, NULL, to_pnp_dev(dev), + acpi_pnp_match); + if (!dev) + return NULL; - adev = bus_find_device(&acpi_bus_type, NULL, - to_pnp_dev(dev), acpi_pnp_match); - if (!adev) - return -ENODEV; - - acpi = to_acpi_device(adev); - *handle = acpi->handle; - put_device(adev); - return 0; + put_device(dev); + return to_acpi_device(dev); } /* complete initialization of a PNPACPI device includes having @@ -356,7 +351,7 @@ static bool acpi_pnp_bus_match(struct de static struct acpi_bus_type __initdata acpi_pnp_bus = { .name = "PNP", .match = acpi_pnp_bus_match, - .find_device = acpi_pnp_find_device, + .find_companion = acpi_pnp_find_companion, }; int pnpacpi_disabled __initdata; 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 @@ -127,7 +127,7 @@ out: return ret; } -static int usb_acpi_find_device(struct device *dev, acpi_handle *handle) +static struct acpi_device *usb_acpi_find_companion(struct device *dev) { struct usb_device *udev; acpi_handle *parent_handle; @@ -169,16 +169,15 @@ static int usb_acpi_find_device(struct d break; } - return -ENODEV; + return NULL; } /* root hub's parent is the usb hcd. */ - parent_handle = ACPI_HANDLE(dev->parent); - *handle = acpi_get_child(parent_handle, udev->portnum); - if (!*handle) - return -ENODEV; - return 0; + return acpi_find_child_device(ACPI_COMPANION(dev->parent), + udev->portnum, false); } else if (is_usb_port(dev)) { + struct acpi_device *adev = NULL; + sscanf(dev_name(dev), "port%d", &port_num); /* Get the struct usb_device point of port's hub */ udev = to_usb_device(dev->parent->parent); @@ -194,26 +193,27 @@ static int usb_acpi_find_device(struct d raw_port_num = usb_hcd_find_raw_port_number(hcd, port_num); - *handle = acpi_get_child(ACPI_HANDLE(&udev->dev), - raw_port_num); - if (!*handle) - return -ENODEV; + adev = acpi_find_child_device(ACPI_COMPANION(&udev->dev), + raw_port_num, false); + if (!adev) + return NULL; } else { parent_handle = usb_get_hub_port_acpi_handle(udev->parent, udev->portnum); if (!parent_handle) - return -ENODEV; + return NULL; - *handle = acpi_get_child(parent_handle, port_num); - if (!*handle) - return -ENODEV; + acpi_bus_get_device(parent_handle, &adev); + adev = acpi_find_child_device(adev, port_num, false); + if (!adev) + return NULL; } - usb_acpi_check_port_connect_type(udev, *handle, port_num); - } else - return -ENODEV; + usb_acpi_check_port_connect_type(udev, adev->handle, port_num); + return adev; + } - return 0; + return NULL; } static bool usb_acpi_bus_match(struct device *dev) @@ -224,7 +224,7 @@ static bool usb_acpi_bus_match(struct de static struct acpi_bus_type usb_acpi_bus = { .name = "USB", .match = usb_acpi_bus_match, - .find_device = usb_acpi_find_device, + .find_companion = usb_acpi_find_companion, }; int usb_acpi_register(void)