linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bjorn Helgaas <bhelgaas@google.com>
To: Andreas Noever <andreas.noever@gmail.com>
Cc: linux-kernel@vger.kernel.org,
	Matthew Garrett <mjg59@srcf.ucam.org>,
	Daniel J Blueman <daniel@quora.org>,
	linux-pci@vger.kernel.org,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>
Subject: Re: [PATCH v2 12/14] pci: Suspend/resume support for appel thunderbolt
Date: Tue, 15 Apr 2014 11:37:58 -0600	[thread overview]
Message-ID: <20140415173758.GG16058@google.com> (raw)
In-Reply-To: <1397175901-4023-13-git-send-email-andreas.noever@gmail.com>

[+cc Rafael]

On Fri, Apr 11, 2014 at 02:24:59AM +0200, Andreas Noever wrote:
> This patch makes changes to the pcieport driver to support thunderbolt
> suspend/resume on apple systems. We need to perform two different
> actions during suspend and resume:
> 
> The whole controller has to be powered down before suspend. If this is
> not done then the NHI device will be gone after resume if a thunderbolt
> device was plugged in while suspending. The controller represents itself
> as multiple PCI devices/bridges. To power it down we hook into the
> upstream bridge of the controller and call the magic ACPI methods. Power
> will be restored automatically during resume (by the firmware
> presumably).
> 
> During resume we have to wait for the NHI do reestablish all pci
> tunnels. Since there is no parent-child relationship between the NHI
> and the bridges we have to explicitly wait for them using
> device_pm_wait_for_dev. We do this in the resume_noirq phase of the
> downstream bridges of the controller (which lead into the thunderbolt
> tunnels).
> 
> Signed-off-by: Andreas Noever <andreas.noever@gmail.com>
> ---
>  drivers/pci/pcie/portdrv_pci.c | 117 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 117 insertions(+)
> 
> diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
> index 0d8fdc4..6d33666 100644
> --- a/drivers/pci/pcie/portdrv_pci.c
> +++ b/drivers/pci/pcie/portdrv_pci.c
> @@ -17,6 +17,9 @@
>  #include <linux/aer.h>
>  #include <linux/dmi.h>
>  #include <linux/pci-aspm.h>
> +#include <linux/delay.h>
> +#include <linux/dmi.h>
> +#include <linux/acpi.h>
>  
>  #include "portdrv.h"
>  #include "aer/aerdrv.h"
> @@ -79,6 +82,114 @@ static int pcie_portdrv_restore_config(struct pci_dev *dev)
>  }
>  
>  #ifdef CONFIG_PM
> +
> +#if defined(CONFIG_THUNDERBOLT) || defined(CONFIG_THUNDERBOLT_MODULE)
> +
> +bool is_thunderbolt(struct pci_dev *pdev)
> +{
> +	if (!dmi_match(DMI_PRODUCT_NAME, "MacBookPro10,1"))
> +		return false;
> +	return pdev->vendor == PCI_VENDOR_ID_INTEL && pdev->device == 0x1547;
> +}
> +
> +static void shutdown_thunderbolt_controller(struct pci_dev *pdev)
> +{
> +	/*
> +	 * The thunderbolt controller consists of a pcie switch with downstream
> +	 * bridges leading to the NHI (native host interface) and to the tunnel
> +	 * pci bridges.
> +	 *
> +	 * Without the following quirk the NHI will not resume if a tb device was
> +	 * plugged in before suspend.
> +	 *
> +	 * This quirk cuts power to the whole chip. Therefore we have to apply it
> +	 * during suspend_noirq of the upstream bridge.
> +	 *
> +	 * Power is automagically restored before resume. No action is needed.
> +	 */
> +	acpi_handle bridge, SXIO, SXFP, SXLV;
> +	if (!is_thunderbolt(pdev))
> +		return;
> +	if (pci_pcie_type(pdev) != PCI_EXP_TYPE_UPSTREAM)
> +		return;
> +	bridge = ACPI_HANDLE(&pdev->dev);
> +	if (!bridge)
> +		return;
> +	/*
> +	 * TB bridges in external devices might have the same device id as those
> +	 * on the host, but they will not have the associated ACPI methods. This
> +	 * implicitly checks that we are at the right bridge.
> +	 */
> +	if (ACPI_FAILURE(acpi_get_handle(bridge, "DSB0.NHI0.SXIO", &SXIO))
> +		|| ACPI_FAILURE(acpi_get_handle(bridge, "DSB0.NHI0.SXFP", &SXFP))
> +		|| ACPI_FAILURE(acpi_get_handle(bridge, "DSB0.NHI0.SXLV", &SXLV)))
> +		return;

Hmm, this is pretty ugly because the ACPI names are up to the BIOS writer
so we can't really rely on them.

You could put this sort of thing in a quirk, but I wouldn't be happy
about putting it in a generic place like this.  Same with the magic
sequence below.

Maybe a quirk that sets function pointers that we could call here?

> +	dev_info(&pdev->dev, "cutting power to thunderbolt controller...\n");
> +
> +	/* save registers manually, the pci core won't be able to do later */
> +	pci_save_state(pdev);
> +	pci_prepare_to_sleep(pdev);
> +
> +	/* magic sequence */
> +	acpi_execute_simple_method(SXIO, NULL, 1);
> +	acpi_execute_simple_method(SXFP, NULL, 0);
> +	msleep(300);
> +	acpi_execute_simple_method(SXLV, NULL, 0);
> +	acpi_execute_simple_method(SXIO, NULL, 0);
> +	acpi_execute_simple_method(SXLV, NULL, 0);
> +}
> +
> +static void wait_for_thunderbolt_controller(struct pci_dev *pdev)
> +{
> +	struct pci_dev *sibling = NULL;
> +	struct pci_dev *nhi = NULL;
> +	/*
> +	 * During suspend the thunderbolt controller is reset and all pci
> +	 * tunnels are lost. The NHI driver will try to reestablish all tunnels
> +	 * during resume. We have to manually wait for the NHI since there is
> +	 * no parent child relationship between the NHI and the tunneled
> +	 * bridges.
> +	 */
> +	if (!is_thunderbolt(pdev))
> +		return;
> +	if (pci_pcie_type(pdev) != PCI_EXP_TYPE_DOWNSTREAM)
> +		return;
> +	/*
> +	 * Find the NHI and confirm that we are a bridge on the tb host
> +	 * controller and not on a tb endpoint.
> +	 */
> +	sibling = pci_get_slot(pdev->bus, 0x0);
> +	if (sibling == pdev)
> +		goto out; /* we are the downstream bridge to the NHI */
> +	if (!sibling || !sibling->subordinate)
> +		goto out;
> +	nhi = pci_get_slot(sibling->subordinate, 0x0);
> +	if (!nhi)
> +		goto out;
> +	if (nhi->vendor != PCI_VENDOR_ID_INTEL || nhi->device != 0x1547
> +			|| nhi->subsystem_vendor != 0x2222
> +			|| nhi->subsystem_device != 0x1111)
> +		goto out;
> +	dev_info(&pdev->dev, "wating for thunderbolt to reestablish pci tunnels...\n");
> +	device_pm_wait_for_dev(&pdev->dev, &nhi->dev);
> +out:
> +	pci_dev_put(sibling);
> +	pci_dev_put(nhi);
> +}
> +
> +#else
> +
> +static void shutdown_thunderbolt_controller(struct pci_dev *pdev) { }
> +static void wait_for_thunderbolt_controller(struct pci_dev *pdev) { }
> +
> +#endif
> +
> +int pcie_port_suspend_noirq(struct device *dev)
> +{
> +	shutdown_thunderbolt_controller(to_pci_dev(dev));
> +	return 0;
> +}
> +
>  static int pcie_port_resume_noirq(struct device *dev)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev);
> @@ -90,6 +201,9 @@ static int pcie_port_resume_noirq(struct device *dev)
>  	 */
>  	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT)
>  		pcie_clear_root_pme_status(pdev);
> +
> +	wait_for_thunderbolt_controller(pdev);
> +
>  	return 0;
>  }
>  
> @@ -171,7 +285,10 @@ static const struct dev_pm_ops pcie_portdrv_pm_ops = {
>  	.thaw		= pcie_port_device_resume,
>  	.poweroff	= pcie_port_device_suspend,
>  	.restore	= pcie_port_device_resume,
> +	.suspend_noirq  = pcie_port_suspend_noirq,
>  	.resume_noirq	= pcie_port_resume_noirq,
> +	.poweroff_noirq = pcie_port_suspend_noirq,
> +	.restore_noirq  = pcie_port_resume_noirq,
>  	.runtime_suspend = pcie_port_runtime_suspend,
>  	.runtime_resume = pcie_port_runtime_resume,
>  	.runtime_idle	= pcie_port_runtime_idle,
> -- 
> 1.9.2
> 

  reply	other threads:[~2014-04-15 17:37 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-11  0:24 [Patch v2 00/14] Thunderbolt support for Apple MBP Andreas Noever
2014-04-11  0:24 ` [PATCH v2 01/14] thunderbolt: Add initial cactus ridge NHI support Andreas Noever
2014-04-11  0:24 ` [PATCH v2 02/14] thunderbolt: Add control channel interface Andreas Noever
2014-04-11  0:24 ` [PATCH v2 03/14] thunderbolt: Setup control channel Andreas Noever
2014-04-11  0:24 ` [PATCH v2 04/14] thunderbolt: Add tb_regs.h Andreas Noever
2014-04-11  0:24 ` [PATCH v2 05/14] thunderbolt: Initialize root switch and ports Andreas Noever
2014-04-11  0:24 ` [PATCH v2 06/14] thunderbolt: Add thunderbolt capability handling Andreas Noever
2014-04-11  0:24 ` [PATCH v2 07/14] thunderbolt: Enable plug events Andreas Noever
2014-04-11  0:24 ` [PATCH v2 08/14] thunderbolt: Scan for downstream switches Andreas Noever
2014-04-11  0:24 ` [PATCH v2 09/14] thunderbolt: Handle hotplug events Andreas Noever
2014-04-11  0:24 ` [PATCH v2 10/14] thunderbolt: Add path setup code Andreas Noever
2014-04-11  0:24 ` [PATCH v2 11/14] thunderbolt: Add support for simple pci tunnels Andreas Noever
2014-04-11  0:24 ` [PATCH v2 12/14] pci: Suspend/resume support for appel thunderbolt Andreas Noever
2014-04-15 17:37   ` Bjorn Helgaas [this message]
2014-04-15 23:31     ` Andreas Noever
2014-04-11  0:25 ` [PATCH v2 13/14] thunderbolt: Read switch uid from EEPROM Andreas Noever
2014-04-11  0:25 ` [PATCH v2 14/14] thunderbolt: Add suspend/hibernate support Andreas Noever
2014-04-11 21:06 ` [Patch v2 00/14] Thunderbolt support for Apple MBP Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140415173758.GG16058@google.com \
    --to=bhelgaas@google.com \
    --cc=andreas.noever@gmail.com \
    --cc=daniel@quora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mjg59@srcf.ucam.org \
    --cc=rjw@rjwysocki.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).