[PATCH 0/3] ACPI / bind: Use struct acpi_device pointers instead of ACPI handles

[PATCH 0/3] ACPI / bind: Use struct acpi_device pointers instead of ACPI handles

[Rafael J. Wysocki]

Hi,

Now that we store a pointer to struct acpi_device as the ACPI companion in
struct device, the code making associations between "physical" devices and
ACPI device objects can be modified to work with struct acpi_device pointers
instead of ACPI handles too.  The first two of the following patches make these
changes and the third one is just a related cleanup.

[1/3] ACPI / bind: Replace .find_device in struct acpi_bus_type with .find_companion
[2/3] ACPI / bind: Modify acpi_bind_one() to take a struct acpi_device pointer as
      the second argument.
[3/3] ACPI / bind: Move acpi_get_child() to ide-acpi.c which is the only remaining
      user of that function.

The patches are on top of linux-pm.git/linux-next.  Please let me know if you see
any problems in them.

Thanks!

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

[PATCH 1/3] ACPI / bind: Rework struct acpi_bus_type

[Rafael J. Wysocki]

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

Replace the .find_device function pointer in struct acpi_bus_type
with a new one, .find_copmanion, 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 <rafael.j.wysocki@intel.com>
---
 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);
+	return dev ? to_acpi_device(dev) : NULL;
 }
 
 /* complete initialization of a PNPACPI device includes having
@@ -356,7 +347,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)

Re: [PATCH 1/3] ACPI / bind: Rework struct acpi_bus_type

[Lan Tianyu]

On 2013年11月29日 08:37, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> 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

> 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 <rafael.j.wysocki@intel.com>
> ---
>  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.

> +	return dev ? to_acpi_device(dev) : NULL;
>  }
>  
>  /* complete initialization of a PNPACPI device includes having
> @@ -356,7 +347,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)
> 


-- 
Best regards
Tianyu Lan

Re: [PATCH 1/3] ACPI / bind: Rework struct acpi_bus_type

[Rafael J. Wysocki]

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 <rafael.j.wysocki@intel.com>
> > 
> > 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 <rafael.j.wysocki@intel.com>
> > ---
> >  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 <rafael.j.wysocki@intel.com>
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 <rafael.j.wysocki@intel.com>
---
 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(-)

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)

[PATCH 2/3] ACPI / bind: Pass struct acpi_device pointer to acpi_bind_one()

[Rafael J. Wysocki]

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

There is no reason to pass an ACPI handle to acpi_bind_one() instead
of a struct acpi_device pointer to the target device object, so
modify that function to take a struct acpi_device pointer as its
second argument and update all code depending on it accordingly.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_memhotplug.c |   15 +++++++--------
 drivers/acpi/acpi_processor.c  |    2 +-
 drivers/acpi/glue.c            |    9 +++------
 drivers/acpi/internal.h        |    2 +-
 4 files changed, 12 insertions(+), 16 deletions(-)

Index: linux-pm/drivers/acpi/glue.c
===================================================================
--- linux-pm.orig/drivers/acpi/glue.c
+++ linux-pm/drivers/acpi/glue.c
@@ -172,9 +172,8 @@ static void acpi_physnode_link_name(char
 		strcpy(buf, PHYSICAL_NODE_STRING);
 }
 
-int acpi_bind_one(struct device *dev, acpi_handle handle)
+int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
 {
-	struct acpi_device *acpi_dev = NULL;
 	struct acpi_device_physical_node *physical_node, *pn;
 	char physical_node_name[PHYSICAL_NODE_NAME_SIZE];
 	struct list_head *physnode_list;
@@ -182,14 +181,12 @@ int acpi_bind_one(struct device *dev, ac
 	int retval = -EINVAL;
 
 	if (ACPI_COMPANION(dev)) {
-		if (handle) {
+		if (acpi_dev) {
 			dev_warn(dev, "ACPI companion already set\n");
 			return -EINVAL;
 		} else {
 			acpi_dev = ACPI_COMPANION(dev);
 		}
-	} else {
-		acpi_bus_get_device(handle, &acpi_dev);
 	}
 	if (!acpi_dev)
 		return -EINVAL;
@@ -314,7 +311,7 @@ static int acpi_platform_notify(struct d
 			ret = -ENODEV;
 			goto out;
 		}
-		ret = acpi_bind_one(dev, adev->handle);
+		ret = acpi_bind_one(dev, adev);
 		if (ret)
 			goto out;
 	}
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -86,7 +86,7 @@ void acpi_init_device_object(struct acpi
 			     int type, unsigned long long sta);
 void acpi_device_add_finalize(struct acpi_device *device);
 void acpi_free_pnp_ids(struct acpi_device_pnp *pnp);
-int acpi_bind_one(struct device *dev, acpi_handle handle);
+int acpi_bind_one(struct device *dev, struct acpi_device *adev);
 int acpi_unbind_one(struct device *dev);
 bool acpi_device_is_present(struct acpi_device *adev);
 
Index: linux-pm/drivers/acpi/acpi_processor.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_processor.c
+++ linux-pm/drivers/acpi/acpi_processor.c
@@ -395,7 +395,7 @@ static int acpi_processor_add(struct acp
 		goto err;
 	}
 
-	result = acpi_bind_one(dev, pr->handle);
+	result = acpi_bind_one(dev, device);
 	if (result)
 		goto err;
 
Index: linux-pm/drivers/acpi/acpi_memhotplug.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_memhotplug.c
+++ linux-pm/drivers/acpi/acpi_memhotplug.c
@@ -180,14 +180,14 @@ static unsigned long acpi_meminfo_end_pf
 
 static int acpi_bind_memblk(struct memory_block *mem, void *arg)
 {
-	return acpi_bind_one(&mem->dev, (acpi_handle)arg);
+	return acpi_bind_one(&mem->dev, arg);
 }
 
 static int acpi_bind_memory_blocks(struct acpi_memory_info *info,
-				   acpi_handle handle)
+				   struct acpi_device *adev)
 {
 	return walk_memory_range(acpi_meminfo_start_pfn(info),
-				 acpi_meminfo_end_pfn(info), (void *)handle,
+				 acpi_meminfo_end_pfn(info), adev,
 				 acpi_bind_memblk);
 }
 
@@ -197,8 +197,7 @@ static int acpi_unbind_memblk(struct mem
 	return 0;
 }
 
-static void acpi_unbind_memory_blocks(struct acpi_memory_info *info,
-				      acpi_handle handle)
+static void acpi_unbind_memory_blocks(struct acpi_memory_info *info)
 {
 	walk_memory_range(acpi_meminfo_start_pfn(info),
 			  acpi_meminfo_end_pfn(info), NULL, acpi_unbind_memblk);
@@ -242,9 +241,9 @@ static int acpi_memory_enable_device(str
 		if (result && result != -EEXIST)
 			continue;
 
-		result = acpi_bind_memory_blocks(info, handle);
+		result = acpi_bind_memory_blocks(info, mem_device->device);
 		if (result) {
-			acpi_unbind_memory_blocks(info, handle);
+			acpi_unbind_memory_blocks(info);
 			return -ENODEV;
 		}
 
@@ -285,7 +284,7 @@ static void acpi_memory_remove_memory(st
 		if (nid == NUMA_NO_NODE)
 			nid = memory_add_physaddr_to_nid(info->start_addr);
 
-		acpi_unbind_memory_blocks(info, handle);
+		acpi_unbind_memory_blocks(info);
 		remove_memory(nid, info->start_addr, info->length);
 		list_del(&info->list);
 		kfree(info);

[PATCH 3/3] ACPI / bind: Move acpi_get_child() to drivers/ide/ide-acpi.c

[Rafael J. Wysocki]

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

Since drivers/ide/ide-acpi.c is the only remaining user of
acpi_get_child(), move that function into that file as a static
routine.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/glue.c     |   12 ------------
 drivers/ide/ide-acpi.c  |   11 +++++++++++
 include/acpi/acpi_bus.h |    1 -
 3 files changed, 11 insertions(+), 13 deletions(-)

Index: linux-pm/drivers/acpi/glue.c
===================================================================
--- linux-pm.orig/drivers/acpi/glue.c
+++ linux-pm/drivers/acpi/glue.c
@@ -151,18 +151,6 @@ struct acpi_device *acpi_find_child_devi
 }
 EXPORT_SYMBOL_GPL(acpi_find_child_device);
 
-acpi_handle acpi_get_child(acpi_handle handle, u64 addr)
-{
-	struct acpi_device *adev;
-
-	if (!handle || acpi_bus_get_device(handle, &adev))
-		return NULL;
-
-	adev = acpi_find_child_device(adev, addr, false);
-	return adev ? adev->handle : NULL;
-}
-EXPORT_SYMBOL_GPL(acpi_get_child);
-
 static void acpi_physnode_link_name(char *buf, unsigned int node_id)
 {
 	if (node_id > 0)
Index: linux-pm/drivers/ide/ide-acpi.c
===================================================================
--- linux-pm.orig/drivers/ide/ide-acpi.c
+++ linux-pm/drivers/ide/ide-acpi.c
@@ -98,6 +98,17 @@ bool ide_port_acpi(ide_hwif_t *hwif)
 	return ide_noacpi == 0 && hwif->acpidata;
 }
 
+static acpi_handle acpi_get_child(acpi_handle handle, u64 addr)
+{
+	struct acpi_device *adev;
+
+	if (!handle || acpi_bus_get_device(handle, &adev))
+		return NULL;
+
+	adev = acpi_find_child_device(adev, addr, false);
+	return adev ? adev->handle : NULL;
+}
+
 /**
  * ide_get_dev_handle - finds acpi_handle and PCI device.function
  * @dev: device to locate
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -436,7 +436,6 @@ struct acpi_pci_root {
 
 struct acpi_device *acpi_find_child_device(struct acpi_device *parent,
 					   u64 address, bool check_children);
-acpi_handle acpi_get_child(acpi_handle handle, u64 addr);
 int acpi_is_root_bridge(acpi_handle);
 struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle);
 

Re: [PATCH 0/3] ACPI / bind: Use struct acpi_device pointers instead of ACPI handles

[Lan Tianyu]

On 2013年11月29日 08:36, Rafael J. Wysocki wrote:
> Hi,
> 
> Now that we store a pointer to struct acpi_device as the ACPI companion in
> struct device, the code making associations between "physical" devices and
> ACPI device objects can be modified to work with struct acpi_device pointers
> instead of ACPI handles too.  The first two of the following patches make these
> changes and the third one is just a related cleanup.
> 
> [1/3] ACPI / bind: Replace .find_device in struct acpi_bus_type with .find_companion
> [2/3] ACPI / bind: Modify acpi_bind_one() to take a struct acpi_device pointer as
>       the second argument.
> [3/3] ACPI / bind: Move acpi_get_child() to ide-acpi.c which is the only remaining
>       user of that function.
> 

Test usb/ACPI bind part, it works normally.

Tested-by: Lan Tianyu <tianyu.lan@intel.com>

> The patches are on top of linux-pm.git/linux-next.  Please let me know if you see
> any problems in them.
> 
> Thanks!
> 


-- 
Best regards
Tianyu Lan