Re: [PATCH v8 13/22] PCI, ACPI: remove acpi_root_bridge in pci_root_hp

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Friday, January 11, 2013 02:40:40 PM Yinghai Lu wrote:
> Tang noticed that hotplug through container will not update acpi_root_bridge
> list.
> 
> After closely checking, we don't need that for struct for tracking and
> could use acpi_pci_root directly.

Well, precisely, so please modify your patches [08-09/22] accordingly instead
of adding this one.

Thanks,
Rafael


> Reported-by: Tang Chen <tangchen@cn.fujitsu.com>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/acpi/pci_root.c |  109 +++++++++--------------------------------------
>  1 file changed, 20 insertions(+), 89 deletions(-)
> 
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 862abcc..697ec65 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -679,73 +679,12 @@ int __init acpi_pci_root_init(void)
>   *	only support root bridge
>   */
>  
> -static LIST_HEAD(acpi_root_bridge_list);
> -struct acpi_root_bridge {
> -	struct list_head list;
> -	acpi_handle handle;
> -	u32 flags;
> -};
> -
>  /* bridge flags */
>  #define ROOT_BRIDGE_HAS_EJ0	(0x00000002)
>  #define ROOT_BRIDGE_HAS_PS3	(0x00000080)
>  
>  #define ACPI_STA_FUNCTIONING	(0x00000008)
>  
> -static struct acpi_root_bridge *acpi_root_handle_to_bridge(acpi_handle handle)
> -{
> -	struct acpi_root_bridge *bridge;
> -
> -	list_for_each_entry(bridge, &acpi_root_bridge_list, list)
> -		if (bridge->handle == handle)
> -			return bridge;
> -
> -	return NULL;
> -}
> -
> -/* allocate and initialize host bridge data structure */
> -static void add_acpi_root_bridge(acpi_handle handle)
> -{
> -	struct acpi_root_bridge *bridge;
> -	acpi_handle dummy_handle;
> -	acpi_status status;
> -
> -	/* if the bridge doesn't have _STA, we assume it is always there */
> -	status = acpi_get_handle(handle, "_STA", &dummy_handle);
> -	if (ACPI_SUCCESS(status)) {
> -		unsigned long long tmp;
> -
> -		status = acpi_evaluate_integer(handle, "_STA", NULL, &tmp);
> -		if (ACPI_FAILURE(status)) {
> -			printk(KERN_DEBUG "%s: _STA evaluation failure\n",
> -						 __func__);
> -			return;
> -		}
> -		if ((tmp & ACPI_STA_FUNCTIONING) == 0)
> -			/* don't register this object */
> -			return;
> -	}
> -
> -	bridge = kzalloc(sizeof(struct acpi_root_bridge), GFP_KERNEL);
> -	if (!bridge)
> -		return;
> -
> -	bridge->handle = handle;
> -
> -	if (ACPI_SUCCESS(acpi_get_handle(handle, "_EJ0", &dummy_handle)))
> -		bridge->flags |= ROOT_BRIDGE_HAS_EJ0;
> -	if (ACPI_SUCCESS(acpi_get_handle(handle, "_PS3", &dummy_handle)))
> -		bridge->flags |= ROOT_BRIDGE_HAS_PS3;
> -
> -	list_add(&bridge->list, &acpi_root_bridge_list);
> -}
> -
> -static void remove_acpi_root_bridge(struct acpi_root_bridge *bridge)
> -{
> -	list_del(&bridge->list);
> -	kfree(bridge);
> -}
> -
>  struct acpi_root_hp_work {
>  	struct work_struct work;
>  	acpi_handle handle;
> @@ -827,28 +766,25 @@ static int acpi_root_evaluate_object(acpi_handle handle, char *cmd, int val)
>  	return 0;
>  }
>  
> -static void handle_root_bridge_removal(acpi_handle handle,
> -		 struct acpi_root_bridge *bridge)
> +static void handle_root_bridge_removal(struct acpi_device *device)
>  {
> +	int ret_val;
>  	u32 flags = 0;
> -	struct acpi_device *device;
> +	acpi_handle dummy_handle;
> +	acpi_handle handle = device->handle;
>  
> -	if (bridge) {
> -		flags = bridge->flags;
> -		remove_acpi_root_bridge(bridge);
> -	}
> +	if (ACPI_SUCCESS(acpi_get_handle(handle, "_EJ0", &dummy_handle)))
> +		flags |= ROOT_BRIDGE_HAS_EJ0;
> +	if (ACPI_SUCCESS(acpi_get_handle(handle, "_PS3", &dummy_handle)))
> +		flags |= ROOT_BRIDGE_HAS_PS3;
>  
> -	if (!acpi_bus_get_device(handle, &device)) {
> -		int ret_val;
> +	/* remove pci devices at first */
> +	ret_val = acpi_bus_trim(device, 0);
> +	printk(KERN_DEBUG "acpi_bus_trim stop return %x\n", ret_val);
>  
> -		/* remove pci devices at first */
> -		ret_val = acpi_bus_trim(device, 0);
> -		printk(KERN_DEBUG "acpi_bus_trim stop return %x\n", ret_val);
> -
> -		/* remove acpi devices */
> -		ret_val = acpi_bus_trim(device, 1);
> -		printk(KERN_DEBUG "acpi_bus_trim remove return %x\n", ret_val);
> -	}
> +	/* remove acpi devices */
> +	ret_val = acpi_bus_trim(device, 1);
> +	printk(KERN_DEBUG "acpi_bus_trim remove return %x\n", ret_val);
>  
>  	if (flags & ROOT_BRIDGE_HAS_PS3) {
>  		acpi_status status;
> @@ -863,7 +799,7 @@ static void handle_root_bridge_removal(acpi_handle handle,
>  
>  static void _handle_hotplug_event_root(struct work_struct *work)
>  {
> -	struct acpi_root_bridge *bridge;
> +	struct acpi_pci_root *root;
>  	char objname[64];
>  	struct acpi_buffer buffer = { .length = sizeof(objname),
>  				      .pointer = objname };
> @@ -875,7 +811,7 @@ static void _handle_hotplug_event_root(struct work_struct *work)
>  	handle = hp_work->handle;
>  	type = hp_work->type;
>  
> -	bridge = acpi_root_handle_to_bridge(handle);
> +	root = acpi_pci_find_root(handle);
>  
>  	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
>  
> @@ -884,10 +820,8 @@ static void _handle_hotplug_event_root(struct work_struct *work)
>  		/* bus enumerate */
>  		printk(KERN_DEBUG "%s: Bus check notify on %s\n", __func__,
>  				 objname);
> -		if (!bridge) {
> +		if (!root)
>  			handle_root_bridge_insertion(handle);
> -			add_acpi_root_bridge(handle);
> -		}
>  
>  		break;
>  
> @@ -895,17 +829,16 @@ static void _handle_hotplug_event_root(struct work_struct *work)
>  		/* device check */
>  		printk(KERN_DEBUG "%s: Device check notify on %s\n", __func__,
>  				 objname);
> -		if (!bridge) {
> +		if (!root)
>  			handle_root_bridge_insertion(handle);
> -			add_acpi_root_bridge(handle);
> -		}
>  		break;
>  
>  	case ACPI_NOTIFY_EJECT_REQUEST:
>  		/* request device eject */
>  		printk(KERN_DEBUG "%s: Device eject notify on %s\n", __func__,
>  				 objname);
> -		handle_root_bridge_removal(handle, bridge);
> +		if (root)
> +			handle_root_bridge_removal(root->device);
>  		break;
>  	default:
>  		printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n",
> @@ -965,8 +898,6 @@ find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
>  		printk(KERN_DEBUG "acpi root: %s notify handler is installed\n",
>  				 objname);
>  
> -	add_acpi_root_bridge(handle);
> -
>  	return AE_OK;
>  }
>  
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.