Re: [PATCH v5 7/8] thunderbolt: Power down controller when idle
On Sun, Feb 12, 2017 at 05:31:30PM +0100, Lukas Wunner wrote:
> On Sat, Jan 28, 2017 at 05:32:37PM -0600, Bjorn Helgaas wrote:
> > On Sun, Jan 15, 2017 at 09:03:45PM +0100, Lukas Wunner wrote:
> > > +#define pr_fmt(fmt) KBUILD_MODNAME " %s: " fmt, dev_name(dev)
> [...]
> > > + /* prevent interrupts during system sleep transition */
> > > + if (ACPI_FAILURE(acpi_disable_gpe(NULL, power->wake_gpe))) {
> > > + pr_err("cannot disable wake GPE, resuming\n");
> >
> > Can you use dev_err() here and below? This is related to a specific
> > device, and it'd be nice to know which one.
>
> See above, the device name is included in pr_fmt. The reason to use
> pr_err() instead of dev_err() is to get the error message prefixed
> with "thunderbolt" instead of "pcieport". Recall that this function
> is executed in the context of the upstream bridge, whose driver name
> is pcieport. I would like to prevent people from grepping the portdrv
> code for the error message. You're the second person to raise this
> question (Andy Shevchenko made the same comment on an earlier version
> of this patch), so I've now added a comment to explain it.
Oh, right, I missed the sneaky dev_name(dev) in pr_fmt(). I guess we
may have a mix of "pcieport" and "thunderbolt" messages related to
the same device, which is sort of sub-optimal, but maybe the best we
can do for now.
Bjorn
Re: [PATCH v5 7/8] thunderbolt: Power down controller when idle
On Sun, Feb 12, 2017 at 05:31:30PM +0100, Lukas Wunner wrote:
> On Sat, Jan 28, 2017 at 05:32:37PM -0600, Bjorn Helgaas wrote:
> > On Sun, Jan 15, 2017 at 09:03:45PM +0100, Lukas Wunner wrote:
> > > +#define pr_fmt(fmt) KBUILD_MODNAME " %s: " fmt, dev_name(dev)
> [...]
> > > + /* prevent interrupts during system sleep transition */
> > > + if (ACPI_FAILURE(acpi_disable_gpe(NULL, power->wake_gpe))) {
> > > + pr_err("cannot disable wake GPE, resuming\n");
> >
> > Can you use dev_err() here and below? This is related to a specific
> > device, and it'd be nice to know which one.
>
> See above, the device name is included in pr_fmt. The reason to use
> pr_err() instead of dev_err() is to get the error message prefixed
> with "thunderbolt" instead of "pcieport". Recall that this function
> is executed in the context of the upstream bridge, whose driver name
> is pcieport. I would like to prevent people from grepping the portdrv
> code for the error message. You're the second person to raise this
> question (Andy Shevchenko made the same comment on an earlier version
> of this patch), so I've now added a comment to explain it.
Oh, right, I missed the sneaky dev_name(dev) in pr_fmt(). I guess we
may have a mix of "pcieport" and "thunderbolt" messages related to
the same device, which is sort of sub-optimal, but maybe the best we
can do for now.
Bjorn
Re: [PATCH v5 7/8] thunderbolt: Power down controller when idle
On Sat, Jan 28, 2017 at 05:32:37PM -0600, Bjorn Helgaas wrote:
> On Sun, Jan 15, 2017 at 09:03:45PM +0100, Lukas Wunner wrote:
> > A Thunderbolt controller appears to the OS as a set of virtual devices:
> > One upstream bridge, multiple downstream bridges and one NHI (Native
> > Host Interface). The upstream and downstream bridges represent a PCIe
> > switch (see definition of a switch in the PCIe spec). The NHI device is
> > used to manage the switch fabric. Hotplugged devices appear behind the
> > downstream bridges:
> >
> > (Root Port) Upstream Bridge --+-- Downstream Bridge 0 NHI
> > +-- Downstream Bridge 1 --
> > +-- Downstream Bridge 2 --
> > ...
> >
> > Power is cut to the entire set of devices. The Linux pm model is
> > hierarchical and assumes that a child cannot resume before its parent.
> > To conform to this model, power control must be governed by the
> > Thunderbolt controller's topmost device, which is the upstream bridge.
> > The NHI and downstream bridges go to D3hot independently and the
> > upstream bridge goes to D3cold once all its children have suspended.
> > This commit only adds runtime pm for the upstream bridge. Runtime pm
> > for the NHI is added in a separate commit to signify its independence.
> > Runtime pm for the downstream bridges is handled by the pcieport driver.
> >
> > Because Apple's ACPI methods are nonstandard, a struct dev_pm_domain is
> > used to override the PCI bus pm_ops. The thunderbolt driver binds to
>
> What are the default PCI bus pm_ops? I looked briefly for it to see
> if there was some way to use hooks there instead of using
> dev_pm_domain_set() with its weird out-of-orderness.
The default PCI bus pm_ops are defined in drivers/pci/pci-driver.c as
struct dev_pm_ops pci_dev_pm_ops.
I did hook into those callbacks in an earlier version of this series
but it required more patches and more modifications to the PCI core
and PCIe port services driver to get Thunderbolt runpm to work:
https://www.spinics.net/lists/linux-pci/msg51158.html
Using dev_pm_domain_set() is the de facto standard to solve this,
vga_switcheroo does the same, that's why I settled on this approach.
(see drivers/gpu/vga/vga_switcheroo.c:vga_switcheroo_init_domain_pm_ops())
> I guess what you do in thunderbolt_power_init() is copy the upstream
> device's bus's ops, then override a few of them? Seems like we then
> have the problem of keeping the Thunderbolt ones in sync with the
> generic ones, e.g., if something got added to the generic one,
> somebody should look at the Thunderbolt one to see if it's also need
> there?
Not a problem. upstream_runtime_suspend() essentially does this:
// call pci_dev_pm_ops ->runtime_suspend hook:
ret = dev->bus->pm->runtime_suspend(dev);
// power down:
acpi_execute_simple_method(power->set, NULL, 0)
// enable wake interrupt:
acpi_enable_gpe(NULL, power->wake_gpe)
So any changes to the pci_dev_pm_ops are inherited. Again,
vga_switcheroo does the same (see vga_switcheroo_runtime_suspend()).
> > +#define pr_fmt(fmt) KBUILD_MODNAME " %s: " fmt, dev_name(dev)
[...]
> > + /* prevent interrupts during system sleep transition */
> > + if (ACPI_FAILURE(acpi_disable_gpe(NULL, power->wake_gpe))) {
> > + pr_err("cannot disable wake GPE, resuming\n");
>
> Can you use dev_err() here and below? This is related to a specific
> device, and it'd be nice to know which one.
See above, the device name is included in pr_fmt. The reason to use
pr_err() instead of dev_err() is to get the error message prefixed
with "thunderbolt" instead of "pcieport". Recall that this function
is executed in the context of the upstream bridge, whose driver name
is pcieport. I would like to prevent people from grepping the portdrv
code for the error message. You're the second person to raise this
question (Andy Shevchenko made the same comment on an earlier version
of this patch), so I've now added a comment to explain it.
> > +void thunderbolt_power_init(struct tb *tb)
> > +{
> > + struct device *upstream_dev, *nhi_dev = >nhi->pdev->dev;
> > + struct tb_power *power = NULL;
>
> Unnecessary initialization.
Good point.
Thanks,
Lukas
Re: [PATCH v5 7/8] thunderbolt: Power down controller when idle
On Sat, Jan 28, 2017 at 05:32:37PM -0600, Bjorn Helgaas wrote:
> On Sun, Jan 15, 2017 at 09:03:45PM +0100, Lukas Wunner wrote:
> > A Thunderbolt controller appears to the OS as a set of virtual devices:
> > One upstream bridge, multiple downstream bridges and one NHI (Native
> > Host Interface). The upstream and downstream bridges represent a PCIe
> > switch (see definition of a switch in the PCIe spec). The NHI device is
> > used to manage the switch fabric. Hotplugged devices appear behind the
> > downstream bridges:
> >
> > (Root Port) Upstream Bridge --+-- Downstream Bridge 0 NHI
> > +-- Downstream Bridge 1 --
> > +-- Downstream Bridge 2 --
> > ...
> >
> > Power is cut to the entire set of devices. The Linux pm model is
> > hierarchical and assumes that a child cannot resume before its parent.
> > To conform to this model, power control must be governed by the
> > Thunderbolt controller's topmost device, which is the upstream bridge.
> > The NHI and downstream bridges go to D3hot independently and the
> > upstream bridge goes to D3cold once all its children have suspended.
> > This commit only adds runtime pm for the upstream bridge. Runtime pm
> > for the NHI is added in a separate commit to signify its independence.
> > Runtime pm for the downstream bridges is handled by the pcieport driver.
> >
> > Because Apple's ACPI methods are nonstandard, a struct dev_pm_domain is
> > used to override the PCI bus pm_ops. The thunderbolt driver binds to
>
> What are the default PCI bus pm_ops? I looked briefly for it to see
> if there was some way to use hooks there instead of using
> dev_pm_domain_set() with its weird out-of-orderness.
The default PCI bus pm_ops are defined in drivers/pci/pci-driver.c as
struct dev_pm_ops pci_dev_pm_ops.
I did hook into those callbacks in an earlier version of this series
but it required more patches and more modifications to the PCI core
and PCIe port services driver to get Thunderbolt runpm to work:
https://www.spinics.net/lists/linux-pci/msg51158.html
Using dev_pm_domain_set() is the de facto standard to solve this,
vga_switcheroo does the same, that's why I settled on this approach.
(see drivers/gpu/vga/vga_switcheroo.c:vga_switcheroo_init_domain_pm_ops())
> I guess what you do in thunderbolt_power_init() is copy the upstream
> device's bus's ops, then override a few of them? Seems like we then
> have the problem of keeping the Thunderbolt ones in sync with the
> generic ones, e.g., if something got added to the generic one,
> somebody should look at the Thunderbolt one to see if it's also need
> there?
Not a problem. upstream_runtime_suspend() essentially does this:
// call pci_dev_pm_ops ->runtime_suspend hook:
ret = dev->bus->pm->runtime_suspend(dev);
// power down:
acpi_execute_simple_method(power->set, NULL, 0)
// enable wake interrupt:
acpi_enable_gpe(NULL, power->wake_gpe)
So any changes to the pci_dev_pm_ops are inherited. Again,
vga_switcheroo does the same (see vga_switcheroo_runtime_suspend()).
> > +#define pr_fmt(fmt) KBUILD_MODNAME " %s: " fmt, dev_name(dev)
[...]
> > + /* prevent interrupts during system sleep transition */
> > + if (ACPI_FAILURE(acpi_disable_gpe(NULL, power->wake_gpe))) {
> > + pr_err("cannot disable wake GPE, resuming\n");
>
> Can you use dev_err() here and below? This is related to a specific
> device, and it'd be nice to know which one.
See above, the device name is included in pr_fmt. The reason to use
pr_err() instead of dev_err() is to get the error message prefixed
with "thunderbolt" instead of "pcieport". Recall that this function
is executed in the context of the upstream bridge, whose driver name
is pcieport. I would like to prevent people from grepping the portdrv
code for the error message. You're the second person to raise this
question (Andy Shevchenko made the same comment on an earlier version
of this patch), so I've now added a comment to explain it.
> > +void thunderbolt_power_init(struct tb *tb)
> > +{
> > + struct device *upstream_dev, *nhi_dev = >nhi->pdev->dev;
> > + struct tb_power *power = NULL;
>
> Unnecessary initialization.
Good point.
Thanks,
Lukas
Re: [PATCH v5 7/8] thunderbolt: Power down controller when idle
On Sun, Jan 15, 2017 at 09:03:45PM +0100, Lukas Wunner wrote:
> Document and implement Apple's ACPI-based (but nonstandard) pm mechanism
> for Thunderbolt. Briefly, an ACPI method provided by Apple is used to
> cut power to the controller. A GPE is enabled while the controller is
> powered down which sideband-signals a plug event, whereupon we reinstate
> power using the ACPI method.
>
> This saves 1.7 W on machines with a Light Ridge controller and is
> reported to save 4 W on Cactus Ridge 4C and Falcon Ridge 4C. (I believe
> 4 W includes the bus power drawn by Apple's Gigabit Ethernet adapter.)
> It fixes (at least partially) a power regression introduced in 3.17 by
> commit 7bc5a2bad0b8 ("ACPI: Support _OSI("Darwin") correctly").
>
> A Thunderbolt controller appears to the OS as a set of virtual devices:
> One upstream bridge, multiple downstream bridges and one NHI (Native
> Host Interface). The upstream and downstream bridges represent a PCIe
> switch (see definition of a switch in the PCIe spec). The NHI device is
> used to manage the switch fabric. Hotplugged devices appear behind the
> downstream bridges:
>
> (Root Port) Upstream Bridge --+-- Downstream Bridge 0 NHI
> +-- Downstream Bridge 1 --
> +-- Downstream Bridge 2 --
> ...
>
> Power is cut to the entire set of devices. The Linux pm model is
> hierarchical and assumes that a child cannot resume before its parent.
> To conform to this model, power control must be governed by the
> Thunderbolt controller's topmost device, which is the upstream bridge.
> The NHI and downstream bridges go to D3hot independently and the
> upstream bridge goes to D3cold once all its children have suspended.
> This commit only adds runtime pm for the upstream bridge. Runtime pm
> for the NHI is added in a separate commit to signify its independence.
> Runtime pm for the downstream bridges is handled by the pcieport driver.
>
> Because Apple's ACPI methods are nonstandard, a struct dev_pm_domain is
> used to override the PCI bus pm_ops. The thunderbolt driver binds to
What are the default PCI bus pm_ops? I looked briefly for it to see
if there was some way to use hooks there instead of using
dev_pm_domain_set() with its weird out-of-orderness.
I guess what you do in thunderbolt_power_init() is copy the upstream
device's bus's ops, then override a few of them? Seems like we then
have the problem of keeping the Thunderbolt ones in sync with the
generic ones, e.g., if something got added to the generic one,
somebody should look at the Thunderbolt one to see if it's also need
there?
A couple minor code comments below.
> the NHI, thus the dev_pm_domain is assigned to the upstream bridge when
> its grandchild ->probes and evicted when it ->removes.
>
> There are no Thunderbolt specs publicly available from Intel or Apple,
> so I've included documentation to the extent that I was able to reverse-
> engineer things. Documentation on the Go2Sx and Ok2Go2Sx pins is
> tentative as those are missing on my Light Ridge. Apple only uses them
> on Cactus Ridge 4C. Someone with such a controller needs to find out
> through experimentation if the documentation is accurate and amend it if
> necessary.
>
> To maximize power saving, the controller utilizes the PM core's direct-
> complete procedure, i.e. it stays suspended during the system sleep
> process.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=92111
> Cc: Andreas Noever
> Signed-off-by: Lukas Wunner
> ---
> drivers/thunderbolt/Kconfig | 3 +-
> drivers/thunderbolt/Makefile | 4 +-
> drivers/thunderbolt/nhi.c| 3 +
> drivers/thunderbolt/power.c | 346
> +++
> drivers/thunderbolt/power.h | 37 +
> drivers/thunderbolt/tb.h | 2 +
> 6 files changed, 392 insertions(+), 3 deletions(-)
> create mode 100644 drivers/thunderbolt/power.c
> create mode 100644 drivers/thunderbolt/power.h
>
> diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig
> index d35db16aa43f..41625cf3f461 100644
> --- a/drivers/thunderbolt/Kconfig
> +++ b/drivers/thunderbolt/Kconfig
> @@ -1,9 +1,10 @@
> menuconfig THUNDERBOLT
> tristate "Thunderbolt support for Apple devices"
> - depends on PCI
> + depends on PCI && ACPI
> depends on X86 || COMPILE_TEST
> select APPLE_PROPERTIES if EFI_STUB && X86
> select CRC32
> + select PM
> help
> Cactus Ridge Thunderbolt Controller driver
> This driver is required if you want to hotplug Thunderbolt devices on
> diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile
> index 5d1053cdfa54..b22082596c42 100644
> --- a/drivers/thunderbolt/Makefile
> +++ b/drivers/thunderbolt/Makefile
> @@ -1,3 +1,3 @@
> obj-${CONFIG_THUNDERBOLT} := thunderbolt.o
> -thunderbolt-objs :=
Re: [PATCH v5 7/8] thunderbolt: Power down controller when idle
On Sun, Jan 15, 2017 at 09:03:45PM +0100, Lukas Wunner wrote:
> Document and implement Apple's ACPI-based (but nonstandard) pm mechanism
> for Thunderbolt. Briefly, an ACPI method provided by Apple is used to
> cut power to the controller. A GPE is enabled while the controller is
> powered down which sideband-signals a plug event, whereupon we reinstate
> power using the ACPI method.
>
> This saves 1.7 W on machines with a Light Ridge controller and is
> reported to save 4 W on Cactus Ridge 4C and Falcon Ridge 4C. (I believe
> 4 W includes the bus power drawn by Apple's Gigabit Ethernet adapter.)
> It fixes (at least partially) a power regression introduced in 3.17 by
> commit 7bc5a2bad0b8 ("ACPI: Support _OSI("Darwin") correctly").
>
> A Thunderbolt controller appears to the OS as a set of virtual devices:
> One upstream bridge, multiple downstream bridges and one NHI (Native
> Host Interface). The upstream and downstream bridges represent a PCIe
> switch (see definition of a switch in the PCIe spec). The NHI device is
> used to manage the switch fabric. Hotplugged devices appear behind the
> downstream bridges:
>
> (Root Port) Upstream Bridge --+-- Downstream Bridge 0 NHI
> +-- Downstream Bridge 1 --
> +-- Downstream Bridge 2 --
> ...
>
> Power is cut to the entire set of devices. The Linux pm model is
> hierarchical and assumes that a child cannot resume before its parent.
> To conform to this model, power control must be governed by the
> Thunderbolt controller's topmost device, which is the upstream bridge.
> The NHI and downstream bridges go to D3hot independently and the
> upstream bridge goes to D3cold once all its children have suspended.
> This commit only adds runtime pm for the upstream bridge. Runtime pm
> for the NHI is added in a separate commit to signify its independence.
> Runtime pm for the downstream bridges is handled by the pcieport driver.
>
> Because Apple's ACPI methods are nonstandard, a struct dev_pm_domain is
> used to override the PCI bus pm_ops. The thunderbolt driver binds to
What are the default PCI bus pm_ops? I looked briefly for it to see
if there was some way to use hooks there instead of using
dev_pm_domain_set() with its weird out-of-orderness.
I guess what you do in thunderbolt_power_init() is copy the upstream
device's bus's ops, then override a few of them? Seems like we then
have the problem of keeping the Thunderbolt ones in sync with the
generic ones, e.g., if something got added to the generic one,
somebody should look at the Thunderbolt one to see if it's also need
there?
A couple minor code comments below.
> the NHI, thus the dev_pm_domain is assigned to the upstream bridge when
> its grandchild ->probes and evicted when it ->removes.
>
> There are no Thunderbolt specs publicly available from Intel or Apple,
> so I've included documentation to the extent that I was able to reverse-
> engineer things. Documentation on the Go2Sx and Ok2Go2Sx pins is
> tentative as those are missing on my Light Ridge. Apple only uses them
> on Cactus Ridge 4C. Someone with such a controller needs to find out
> through experimentation if the documentation is accurate and amend it if
> necessary.
>
> To maximize power saving, the controller utilizes the PM core's direct-
> complete procedure, i.e. it stays suspended during the system sleep
> process.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=92111
> Cc: Andreas Noever
> Signed-off-by: Lukas Wunner
> ---
> drivers/thunderbolt/Kconfig | 3 +-
> drivers/thunderbolt/Makefile | 4 +-
> drivers/thunderbolt/nhi.c| 3 +
> drivers/thunderbolt/power.c | 346
> +++
> drivers/thunderbolt/power.h | 37 +
> drivers/thunderbolt/tb.h | 2 +
> 6 files changed, 392 insertions(+), 3 deletions(-)
> create mode 100644 drivers/thunderbolt/power.c
> create mode 100644 drivers/thunderbolt/power.h
>
> diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig
> index d35db16aa43f..41625cf3f461 100644
> --- a/drivers/thunderbolt/Kconfig
> +++ b/drivers/thunderbolt/Kconfig
> @@ -1,9 +1,10 @@
> menuconfig THUNDERBOLT
> tristate "Thunderbolt support for Apple devices"
> - depends on PCI
> + depends on PCI && ACPI
> depends on X86 || COMPILE_TEST
> select APPLE_PROPERTIES if EFI_STUB && X86
> select CRC32
> + select PM
> help
> Cactus Ridge Thunderbolt Controller driver
> This driver is required if you want to hotplug Thunderbolt devices on
> diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile
> index 5d1053cdfa54..b22082596c42 100644
> --- a/drivers/thunderbolt/Makefile
> +++ b/drivers/thunderbolt/Makefile
> @@ -1,3 +1,3 @@
> obj-${CONFIG_THUNDERBOLT} := thunderbolt.o
> -thunderbolt-objs := nhi.o ctl.o tb.o switch.o cap.o path.o
[PATCH v5 7/8] thunderbolt: Power down controller when idle
Document and implement Apple's ACPI-based (but nonstandard) pm mechanism
for Thunderbolt. Briefly, an ACPI method provided by Apple is used to
cut power to the controller. A GPE is enabled while the controller is
powered down which sideband-signals a plug event, whereupon we reinstate
power using the ACPI method.
This saves 1.7 W on machines with a Light Ridge controller and is
reported to save 4 W on Cactus Ridge 4C and Falcon Ridge 4C. (I believe
4 W includes the bus power drawn by Apple's Gigabit Ethernet adapter.)
It fixes (at least partially) a power regression introduced in 3.17 by
commit 7bc5a2bad0b8 ("ACPI: Support _OSI("Darwin") correctly").
A Thunderbolt controller appears to the OS as a set of virtual devices:
One upstream bridge, multiple downstream bridges and one NHI (Native
Host Interface). The upstream and downstream bridges represent a PCIe
switch (see definition of a switch in the PCIe spec). The NHI device is
used to manage the switch fabric. Hotplugged devices appear behind the
downstream bridges:
(Root Port) Upstream Bridge --+-- Downstream Bridge 0 NHI
+-- Downstream Bridge 1 --
+-- Downstream Bridge 2 --
...
Power is cut to the entire set of devices. The Linux pm model is
hierarchical and assumes that a child cannot resume before its parent.
To conform to this model, power control must be governed by the
Thunderbolt controller's topmost device, which is the upstream bridge.
The NHI and downstream bridges go to D3hot independently and the
upstream bridge goes to D3cold once all its children have suspended.
This commit only adds runtime pm for the upstream bridge. Runtime pm
for the NHI is added in a separate commit to signify its independence.
Runtime pm for the downstream bridges is handled by the pcieport driver.
Because Apple's ACPI methods are nonstandard, a struct dev_pm_domain is
used to override the PCI bus pm_ops. The thunderbolt driver binds to
the NHI, thus the dev_pm_domain is assigned to the upstream bridge when
its grandchild ->probes and evicted when it ->removes.
There are no Thunderbolt specs publicly available from Intel or Apple,
so I've included documentation to the extent that I was able to reverse-
engineer things. Documentation on the Go2Sx and Ok2Go2Sx pins is
tentative as those are missing on my Light Ridge. Apple only uses them
on Cactus Ridge 4C. Someone with such a controller needs to find out
through experimentation if the documentation is accurate and amend it if
necessary.
To maximize power saving, the controller utilizes the PM core's direct-
complete procedure, i.e. it stays suspended during the system sleep
process.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=92111
Cc: Andreas Noever
Signed-off-by: Lukas Wunner
---
drivers/thunderbolt/Kconfig | 3 +-
drivers/thunderbolt/Makefile | 4 +-
drivers/thunderbolt/nhi.c| 3 +
drivers/thunderbolt/power.c | 346 +++
drivers/thunderbolt/power.h | 37 +
drivers/thunderbolt/tb.h | 2 +
6 files changed, 392 insertions(+), 3 deletions(-)
create mode 100644 drivers/thunderbolt/power.c
create mode 100644 drivers/thunderbolt/power.h
diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig
index d35db16aa43f..41625cf3f461 100644
--- a/drivers/thunderbolt/Kconfig
+++ b/drivers/thunderbolt/Kconfig
@@ -1,9 +1,10 @@
menuconfig THUNDERBOLT
tristate "Thunderbolt support for Apple devices"
- depends on PCI
+ depends on PCI && ACPI
depends on X86 || COMPILE_TEST
select APPLE_PROPERTIES if EFI_STUB && X86
select CRC32
+ select PM
help
Cactus Ridge Thunderbolt Controller driver
This driver is required if you want to hotplug Thunderbolt devices on
diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile
index 5d1053cdfa54..b22082596c42 100644
--- a/drivers/thunderbolt/Makefile
+++ b/drivers/thunderbolt/Makefile
@@ -1,3 +1,3 @@
obj-${CONFIG_THUNDERBOLT} := thunderbolt.o
-thunderbolt-objs := nhi.o ctl.o tb.o switch.o cap.o path.o tunnel_pci.o
eeprom.o
-
+thunderbolt-objs := nhi.o ctl.o tb.o switch.o cap.o path.o tunnel_pci.o \
+ eeprom.o power.o
diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index a8c20413dbda..88fb2fb8cf4e 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -605,6 +605,8 @@ static int nhi_probe(struct pci_dev *pdev, const struct
pci_device_id *id)
}
pci_set_drvdata(pdev, tb);
+ thunderbolt_power_init(tb);
+
return 0;
}
@@ -612,6 +614,7 @@ static void nhi_remove(struct pci_dev *pdev)
{
struct tb *tb = pci_get_drvdata(pdev);
struct tb_nhi *nhi = tb->nhi;
+ thunderbolt_power_fini(tb);
thunderbolt_shutdown_and_free(tb);
[PATCH v5 7/8] thunderbolt: Power down controller when idle
Document and implement Apple's ACPI-based (but nonstandard) pm mechanism
for Thunderbolt. Briefly, an ACPI method provided by Apple is used to
cut power to the controller. A GPE is enabled while the controller is
powered down which sideband-signals a plug event, whereupon we reinstate
power using the ACPI method.
This saves 1.7 W on machines with a Light Ridge controller and is
reported to save 4 W on Cactus Ridge 4C and Falcon Ridge 4C. (I believe
4 W includes the bus power drawn by Apple's Gigabit Ethernet adapter.)
It fixes (at least partially) a power regression introduced in 3.17 by
commit 7bc5a2bad0b8 ("ACPI: Support _OSI("Darwin") correctly").
A Thunderbolt controller appears to the OS as a set of virtual devices:
One upstream bridge, multiple downstream bridges and one NHI (Native
Host Interface). The upstream and downstream bridges represent a PCIe
switch (see definition of a switch in the PCIe spec). The NHI device is
used to manage the switch fabric. Hotplugged devices appear behind the
downstream bridges:
(Root Port) Upstream Bridge --+-- Downstream Bridge 0 NHI
+-- Downstream Bridge 1 --
+-- Downstream Bridge 2 --
...
Power is cut to the entire set of devices. The Linux pm model is
hierarchical and assumes that a child cannot resume before its parent.
To conform to this model, power control must be governed by the
Thunderbolt controller's topmost device, which is the upstream bridge.
The NHI and downstream bridges go to D3hot independently and the
upstream bridge goes to D3cold once all its children have suspended.
This commit only adds runtime pm for the upstream bridge. Runtime pm
for the NHI is added in a separate commit to signify its independence.
Runtime pm for the downstream bridges is handled by the pcieport driver.
Because Apple's ACPI methods are nonstandard, a struct dev_pm_domain is
used to override the PCI bus pm_ops. The thunderbolt driver binds to
the NHI, thus the dev_pm_domain is assigned to the upstream bridge when
its grandchild ->probes and evicted when it ->removes.
There are no Thunderbolt specs publicly available from Intel or Apple,
so I've included documentation to the extent that I was able to reverse-
engineer things. Documentation on the Go2Sx and Ok2Go2Sx pins is
tentative as those are missing on my Light Ridge. Apple only uses them
on Cactus Ridge 4C. Someone with such a controller needs to find out
through experimentation if the documentation is accurate and amend it if
necessary.
To maximize power saving, the controller utilizes the PM core's direct-
complete procedure, i.e. it stays suspended during the system sleep
process.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=92111
Cc: Andreas Noever
Signed-off-by: Lukas Wunner
---
drivers/thunderbolt/Kconfig | 3 +-
drivers/thunderbolt/Makefile | 4 +-
drivers/thunderbolt/nhi.c| 3 +
drivers/thunderbolt/power.c | 346 +++
drivers/thunderbolt/power.h | 37 +
drivers/thunderbolt/tb.h | 2 +
6 files changed, 392 insertions(+), 3 deletions(-)
create mode 100644 drivers/thunderbolt/power.c
create mode 100644 drivers/thunderbolt/power.h
diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig
index d35db16aa43f..41625cf3f461 100644
--- a/drivers/thunderbolt/Kconfig
+++ b/drivers/thunderbolt/Kconfig
@@ -1,9 +1,10 @@
menuconfig THUNDERBOLT
tristate "Thunderbolt support for Apple devices"
- depends on PCI
+ depends on PCI && ACPI
depends on X86 || COMPILE_TEST
select APPLE_PROPERTIES if EFI_STUB && X86
select CRC32
+ select PM
help
Cactus Ridge Thunderbolt Controller driver
This driver is required if you want to hotplug Thunderbolt devices on
diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile
index 5d1053cdfa54..b22082596c42 100644
--- a/drivers/thunderbolt/Makefile
+++ b/drivers/thunderbolt/Makefile
@@ -1,3 +1,3 @@
obj-${CONFIG_THUNDERBOLT} := thunderbolt.o
-thunderbolt-objs := nhi.o ctl.o tb.o switch.o cap.o path.o tunnel_pci.o
eeprom.o
-
+thunderbolt-objs := nhi.o ctl.o tb.o switch.o cap.o path.o tunnel_pci.o \
+ eeprom.o power.o
diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index a8c20413dbda..88fb2fb8cf4e 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -605,6 +605,8 @@ static int nhi_probe(struct pci_dev *pdev, const struct
pci_device_id *id)
}
pci_set_drvdata(pdev, tb);
+ thunderbolt_power_init(tb);
+
return 0;
}
@@ -612,6 +614,7 @@ static void nhi_remove(struct pci_dev *pdev)
{
struct tb *tb = pci_get_drvdata(pdev);
struct tb_nhi *nhi = tb->nhi;
+ thunderbolt_power_fini(tb);
thunderbolt_shutdown_and_free(tb);
nhi_shutdown(nhi);
}
diff --git
