On Thu, 2013-07-18 at 01:32 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>
> The current implementation of acpiphp_check_bridge() is pretty dumb:
> - It enables a slot if it's not enabled and the slot status is
> ACPI_STA_ALL.
> - It disables a slot if it's enabled and the slot status is not
> ACPI_STA_ALL.
>
> This behavior is not sufficient to handle the Thunderbolt daisy
> chaining case properly, however, because in that case the bus
> behind the already enabled slot needs to be rescanned for new
> devices.
>
> For this reason, modify acpiphp_check_bridge() so that slots are
> disabled and stopped if they are not in the ACPI_STA_ALL state.
>
> For slots in the ACPI_STA_ALL state, devices behind them that don't
> respond are trimmed using a new function, trim_stale_devices(),
> introduced specifically for this purpose. That function walks
> the given bus and checks each device on it. If the device doesn't
> respond, it is assumed to be gone and is removed.
>
> Once all of the stale devices directy behind the slot have been
> removed, acpiphp_check_bridge() will start looking for new devices
> that might have appeared on the given bus. It will do that even if
> the slot is already enabled (SLOT_ENABLED is set for it).
>
> In addition to that, make the bus check notification ignore
> SLOT_ENABLED and go for enable_device() directly if bridge is NULL,
> so that devices behind the slot are re-enumerated in that case too.
>
> This change is based on earlier patches from Kirill A Shutemov
> and Mika Westerberg.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> Tested-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> ---
FYI, git bisect landed on this patch as the cause of my serial console
dying on current upstream. Further debugging to come... Thanks,
Alex
> drivers/pci/hotplug/acpiphp_glue.c | 87 +++++++++++++++++++++++++------------
> 1 file changed, 60 insertions(+), 27 deletions(-)
>
> Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c
> ===================================================================
> --- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c
> +++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c
> @@ -46,6 +46,7 @@
> #include <linux/pci.h>
> #include <linux/pci_hotplug.h>
> #include <linux/pci-acpi.h>
> +#include <linux/pm_runtime.h>
> #include <linux/mutex.h>
> #include <linux/slab.h>
> #include <linux/acpi.h>
> @@ -687,47 +688,75 @@ static unsigned int get_slot_status(stru
> }
>
> /**
> + * trim_stale_devices - remove PCI devices that are not responding.
> + * @dev: PCI device to start walking the hierarchy from.
> + */
> +static void trim_stale_devices(struct pci_dev *dev)
> +{
> + acpi_handle handle = ACPI_HANDLE(&dev->dev);
> + struct pci_bus *bus = dev->subordinate;
> + bool alive = false;
> +
> + if (handle) {
> + acpi_status status;
> + unsigned long long sta;
> +
> + status = acpi_evaluate_integer(handle, "_STA", NULL, &sta);
> + alive = ACPI_SUCCESS(status) && sta == ACPI_STA_ALL;
> + }
> + if (!alive) {
> + u32 v;
> +
> + /* Check if the device responds. */
> + alive = pci_bus_read_dev_vendor_id(dev->bus, dev->devfn, &v, 0);
> + }
> + if (!alive) {
> + pci_stop_and_remove_bus_device(dev);
> + if (handle)
> + acpiphp_bus_trim(handle);
> + } else if (bus) {
> + struct pci_dev *child, *tmp;
> +
> + /* The device is a bridge. so check the bus below it. */
> + pm_runtime_get_sync(&dev->dev);
> + list_for_each_entry_safe(child, tmp, &bus->devices, bus_list)
> + trim_stale_devices(child);
> +
> + pm_runtime_put(&dev->dev);
> + }
> +}
> +
> +/**
> * acpiphp_check_bridge - re-enumerate devices
> * @bridge: where to begin re-enumeration
> *
> * Iterate over all slots under this bridge and make sure that if a
> * card is present they are enabled, and if not they are disabled.
> */
> -static int acpiphp_check_bridge(struct acpiphp_bridge *bridge)
> +static void acpiphp_check_bridge(struct acpiphp_bridge *bridge)
> {
> struct acpiphp_slot *slot;
> - int retval = 0;
> - int enabled, disabled;
> -
> - enabled = disabled = 0;
>
> list_for_each_entry(slot, &bridge->slots, node) {
> - unsigned int status = get_slot_status(slot);
> - if (slot->flags & SLOT_ENABLED) {
> - if (status == ACPI_STA_ALL)
> - continue;
> + struct pci_bus *bus = slot->bus;
> + struct pci_dev *dev, *tmp;
>
> - retval = acpiphp_disable_and_eject_slot(slot);
> - if (retval)
> - goto err_exit;
> + mutex_lock(&slot->crit_sect);
> + /* wake up all functions */
> + if (get_slot_status(slot) == ACPI_STA_ALL) {
> + /* remove stale devices if any */
> + list_for_each_entry_safe(dev, tmp, &bus->devices,
> + bus_list)
> + if (PCI_SLOT(dev->devfn) == slot->device)
> + trim_stale_devices(dev);
>
> - disabled++;
> + /* configure all functions */
> + enable_device(slot);
> } else {
> - if (status != ACPI_STA_ALL)
> - continue;
> - retval = acpiphp_enable_slot(slot);
> - if (retval) {
> - err("Error occurred in enabling\n");
> - goto err_exit;
> - }
> - enabled++;
> + disable_device(slot);
> }
> + mutex_unlock(&slot->crit_sect);
> }
> -
> - dbg("%s: %d enabled, %d disabled\n", __func__, enabled, disabled);
> -
> - err_exit:
> - return retval;
> }
>
> static void acpiphp_set_hpp_values(struct pci_bus *bus)
> @@ -828,7 +857,11 @@ static void hotplug_event(acpi_handle ha
> ACPI_UINT32_MAX, check_sub_bridges,
> NULL, NULL, NULL);
> } else {
> - acpiphp_enable_slot(func->slot);
> + struct acpiphp_slot *slot = func->slot;
> +
> + mutex_lock(&slot->crit_sect);
> + enable_device(slot);
> + mutex_unlock(&slot->crit_sect);
> }
> break;
>
>
> --
> 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
--
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
