Thread (63 messages) 63 messages, 8 authors, 2016-08-15

Re: [PATCH v2 00/13] Runtime PM for Thunderbolt on Macs

From: Bjorn Helgaas <helgaas@kernel.org>
Date: 2016-06-14 20:22:34
Also in: linux-pci, lkml

On Tue, Jun 14, 2016 at 09:14:27PM +0200, Andreas Noever wrote:
On Tue, Jun 14, 2016 at 6:37 PM, Bjorn Helgaas [off-list ref] wrote:
quoted
[+cc linux-kernel]

On Sat, May 21, 2016 at 11:48:42AM +0200, Andreas Noever wrote:
quoted
Signed-off-by: Andreas Noever <andreas.noever@gmail.com>

Tested on MacBookPro10,1

On Fri, May 13, 2016 at 1:15 PM, Lukas Wunner [off-list ref] wrote:
quoted
This series powers Thunderbolt controllers on Macs down when nothing is
plugged in, saving 1.7 W on machines with a Light Ridge controller and
reportedly 4 W on Cactus Ridge 4C and Falcon Ridge 4C.

Briefly, a custom ACPI method provided by Apple is used to cut power to
the controller.  A GPE is enabled while the controller is powered down
which side-band signals a plug event, whereupon power is reinstated using
the ACPI method.  Note that even though this mechanism is ACPI-based,
it does not use _PSx methods and is thus entirely nonstandard.
I think the current arrangement was that Andreas would ack Thunderbolt
patches and I would merge them via the PCI tree.  That makes some sense
because Thunderbolt and PCIe are related, but the more I think about
it, the less I'm happy with it.

This series is a good example.  I'm sure it's good work and
worthwhile.  But I can't really say anything about the content of it
because most of it is Thunderbolt-specific and there's no public spec.
It seems like this is basically a collection of reverse-engineered
quirks that happen to work with the current state of Linux PM on
certain Macs.  We don't know what might change on future Macs.  We
don't know what might break when we make changes to Linux PM.

I can't test this series, nor do I want to.  I can't test most of the
patches I merge, but I can at least read the spec and see whether the
patches make sense.  What I would *like* is to have public Thunderbolt
specs and a kernel developer's guide so we know what to expect from
the hardware and the firmware and we can write code that should work
not just on current Macs, but also on non-Macs and future Macs.

I don't think the current situation is really maintainable, and I'm
not comfortable merging code that I can't maintain.
Most of the code is contained within the thunderbolt driver. I think
there is quite some precedence for reverse engineered drivers without
specs being part of the kernel. My understanding was that, since I am
listed in MAINTAINERS, I am responsible for the driver. Now our
changes often need improvements to the pci core, which is why I think
merging through your tree is a good idea (without transferring
responsibility). The changes to the drivers/pci should be supported by
the PCI-spec and make sense without knowing about thunderbolt (but it
might be the case that thunderbolt is the only user of these
features).

Specifically for this series we want to:
 - whitelist thunderbolt bridges for PM. Detecting those bridges is
non-standard but I think this is acceptable, since this
blacklist/whitelist is basically a quirk.
 - Load our portdrv on tb bridges. PCI just sees another portdriver
and all the reverse engineered magic lives inside the driver.
 - Forward more PM callbacks to portdrivers (not tb specific)
 - hotplug D3cold fixes: resume around board_added/remove_board,
ignore interrupts in d3cold (not tb specific and probably a general
bugfix)
 - Make pci not fail if bridges have been put into D3cold by some
external mechanism.

So maybe you could review the pci changes as a solution to the problem
"we want to load a custom portdriver which can put bridges into d3cold
in a device specific way". We certainly to not expect you to take
responsibility for the thunderbolt driver.
That's a fine solution as far as I'm personally concerned.  I think
it's poor for Linux overall, because I think it's fragile, and it's
disappointing that a technology as important as Thunderbolt is so
poorly supported by the promulgators.  But if you're willing to work
in that environment, that's great.

You maintain the thunderbolt code and merge changes, and I'll review
the pieces that touch drivers/pci.  I do have a couple comments on
those pieces, but I don't think they'll be major.

I just want to get out of the business of merging drivers/thunderbolt
code that I can't maintain.

Bjorn
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help