Re: [PATCH 2/2] PCI Hotplug: convert acpi_pci_detect_ejectable() to take an acpi_handle

[Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>]

Alex Chiang wrote:
> acpi_pci_detect_ejectable() goes through effort to convert its
> struct pci_bus arg to an acpi_handle, but every time we use this
> interface, we already have the handle available.
> 
> So let's just use the handle instead of converting back and forth.
> 
> Cc: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> Signed-off-by: Alex Chiang <achiang@hp.com>
> ---
> 
>  drivers/pci/hotplug/acpi_pcihp.c   |    9 +++------
>  drivers/pci/hotplug/acpiphp_glue.c |   33 +++++++++++----------------------
>  drivers/pci/hotplug/pciehp_acpi.c  |    5 +++--
>  include/linux/pci_hotplug.h        |    2 +-
>  4 files changed, 18 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/acpi_pcihp.c b/drivers/pci/hotplug/acpi_pcihp.c
> index eb15958..80af75c 100644
> --- a/drivers/pci/hotplug/acpi_pcihp.c
> +++ b/drivers/pci/hotplug/acpi_pcihp.c
> @@ -500,18 +500,15 @@ check_hotplug(acpi_handle handle, u32 lvl, void *context, void **rv)
>  
>  /**
>   * acpi_pci_detect_ejectable - check if the PCI bus has ejectable slots
> - * @pbus - PCI bus to scan
> + * @handle - handle of the PCI bus to scan
>   *
>   * Returns 1 if the PCI bus has ACPI based ejectable slots, 0 otherwise.
>   */
> -int acpi_pci_detect_ejectable(struct pci_bus *pbus)
> +int acpi_pci_detect_ejectable(acpi_handle handle)
>  {
> -	acpi_handle handle;
>  	int found = 0;
>  
> -	if (!(handle = acpi_pci_get_bridge_handle(pbus)))
> -		return 0;
> -	acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,

I think we need NULL check here.

	if (!handle)
		return 0;

(snip.)

> diff --git a/drivers/pci/hotplug/pciehp_acpi.c b/drivers/pci/hotplug/pciehp_acpi.c
> index 9604801..0c6aa99 100644
> --- a/drivers/pci/hotplug/pciehp_acpi.c
> +++ b/drivers/pci/hotplug/pciehp_acpi.c
> @@ -47,7 +47,7 @@ int pciehp_acpi_slot_detection_check(struct pci_dev *dev)
>  {
>  	if (slot_detection_mode != PCIEHP_DETECT_ACPI)
>  		return 0;
> -	if (acpi_pci_detect_ejectable(dev->subordinate))
> +	if (acpi_pci_detect_ejectable(dev->dev.archdata.acpi_handle))
>  		return 0;

How about:

	handle = DEVICE_ACPI_HANDLE(&dev->dev);
	if (acpi_pci_detect_ejectable(handle))
		return 0;


>  	return -ENODEV;
>  }
> @@ -94,7 +94,8 @@ static int __init dummy_probe(struct pcie_device *dev)
>  			dup_slot_id++;
>  	}
>  	list_add_tail(&slot->slot_list, &dummy_slots);
> -	if (!acpi_slot_detected && acpi_pci_detect_ejectable(pbus))
> +	if (!acpi_slot_detected &&
> +		acpi_pci_detect_ejectable(pbus->self->dev.archdata.acpi_handle))
>  		acpi_slot_detected = 1;

How about:

	handle = DEVICE_ACPI_HANDLE(&pdev->dev);
	if (!acpi_slot_detected && acpi_pci_detect_ejectable(handle))

And the following line at the head of dummy_probe() seems no longer
needed.

	struct pci_bus *pbus = pdev->subordinate;

Thanks,
Kenji Kaneshige