Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs

From	Lukas Wunner <lukas@wunner.de>
Newsgroups	linux.kernel
Subject	Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs
Date	2017-06-05 14:10 +0200
Message-ID	<tOYZz-2JP-3@gated-at.bofh.it> (permalink)
References	<tNVr4-2dT-5@gated-at.bofh.it> <tNVr4-2dT-29@gated-at.bofh.it> <tOVp0-nU-17@gated-at.bofh.it> <tOWEp-193-7@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Mon, Jun 05, 2017 at 12:32:49PM +0300, Mika Westerberg wrote:
> On Mon, Jun 05, 2017 at 10:14:37AM +0200, Lukas Wunner wrote:
> > On Fri, Jun 02, 2017 at 05:05:16PM +0300, Mika Westerberg wrote:
> > > --- a/drivers/thunderbolt/nhi.h
> > > +++ b/drivers/thunderbolt/nhi.h
> > > @@ -143,4 +143,21 @@ static inline int ring_tx(struct tb_ring *ring, struct ring_frame *frame)
> > >  	return __ring_enqueue(ring, frame);
> > >  }
> > >  
> > > +/*
> > > + * PCI IDs used in this driver from Win Ridge forward. There is no
> > > + * need for the PCI quirk anymore as we will use ICM also on Apple
> > > + * hardware.
> > > + */
> > 
> > I wrote a patch a few months ago which replaces the PCI quirk with
> > device links:
> > 
> > https://github.com/l1k/linux/commit/8e2e7eaa1163
> > 
> > I was going to upstream this sometime this year, it probably would
> > have been better if I had done that already but I wasn't expecting
> > your series.
> > 
> > In any case, all Thunderbolt PCI device IDs can then be moved to a
> > header in drivers/thunderbolt/, except for a few TB1 devices which
> > are referenced by quirk_thunderbolt_hotplug_msi() and
> > quirk_apple_poweroff_thunderbolt().
> 
> OK, but that should be a separate patch, right? On top of your device
> links patch.

Yes.  If you like the device links patch, feel free to include it
in your series or modify it as you see fit.

I forgot to mention, on Alpine Ridge there's an additional downstream
bridge with an XHCI controller below it.  A device link will also be
established from that downstream bridge to the NHI.  I'm not sure if
that is actually necessary, perhaps it's even undesirable.

> > As to using ICM on Apple hardware, I've heard from people with
> > Alpine Ridge MacBook Pros that the native (i.e. non-ICM) driver
> > at least probes fine.  I've yet to hear from folks who have
> > actually tested it with any attached TB3 devices, but my expectation
> > would be that it should work fine in native mode since the protocol
> > seems to be the same.
> 
> I have one Mac with Alpine Ridge (well 4 Thunderbolt ports, 2 Alpine
> Ridges) and it indeed works in native mode but it can tunnel only one
> device, no display port.

Yes.  Those are limitations of the native driver.  It doesn't support
chaining and DP tunnels yet.  If chained devices and DP devices are
present on boot, then the EFI NHI driver (on Macs) will establish the
tunnels and thunderbolt.ko will inherit them.  Once the devices are
unplugged and replugged, the tunnels are gone and cannot be re-established
until the machine is rebooted.

> However, starting ICM on them allows you to connect up to 6 devices
> including display port. It also allows cross-domain connections where we
> can implement things like Amir's networking driver.
> 
> That's the reason we did it this way - to get Thunderbolt working the
> same way in Linux than it works on Macs running OS X :)

Yes, it's useful to have ICM-based Thunderbolt on Macs and personally
I'm totally fine with making it the default for Macs which support it.
(I can't speak for Andreas, obviously.)

However it would be great to give people the *choice* between ICM versus
native mode, for at least two reasons:

(1) Native mode uses free software.  (I assume the ICM firmware remains
    closed source.)

(2) Native mode allows more versatility, e.g. how PCI tunnels are set up
    to chained devices: PCI fanout or PCI direct routing, see:
    https://developer.apple.com/library/content/documentation/HardwareDrivers/Conceptual/ThunderboltDevGuide/Basics/Basics.html

    Apple supports traffic prioritization to enable audio over Thunderbolt
    with higher accuracy / minimal skew, I assume their choice to use
    native mode was largely motivated by being able to support specialized
    applications like that which are difficult or perhaps impossible to
    implement in firmware:
    http://pdfpiw.uspto.gov/.piw?Docid=09015384

By the way, you wrote in one of the commit messages that ICM is
"a firmware running on the host controller".  Is it really running on
the *controller* (i.e. Thunderbolt chip)?  My understanding was that
ICM is part of the BIOS and executed on the host CPU in System Management
Mode.

Thanks,

Lukas

Thread

[PATCH v3 00/27] Thunderbolt security levels and NVM firmware upgrade Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
  [PATCH v3 05/27] thunderbolt: Add MSI-X support Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
  [PATCH v3 07/27] thunderbolt: Allow passing NULL to tb_ctl_free() Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
  [PATCH v3 18/27] thunderbolt: Rework control channel to be more reliable Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
  [PATCH v3 27/27] MAINTAINERS: Add maintainers for Thunderbolt driver Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
  [PATCH v3 25/27] thunderbolt: Add support for host and device NVM firmware upgrade Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
  [PATCH v3 01/27] thunderbolt: Use const buffer pointer in write operations Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
  [PATCH v3 22/27] thunderbolt: Add support for DMA configuration based mailbox Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
  [PATCH v3 06/27] thunderbolt: Rework capability handling Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
    Re: [PATCH v3 06/27] thunderbolt: Rework capability handling Lukas Wunner <lukas@wunner.de> - 2017-06-04 16:00 +0200
      Re: [PATCH v3 06/27] thunderbolt: Rework capability handling Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-04 17:10 +0200
        Re: [PATCH v3 06/27] thunderbolt: Rework capability handling Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-04 17:10 +0200
          Re: [PATCH v3 06/27] thunderbolt: Rework capability handling Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-06-04 20:20 +0200
            Re: [PATCH v3 06/27] thunderbolt: Rework capability handling Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 08:50 +0200
  [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
    Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Lukas Wunner <lukas@wunner.de> - 2017-06-05 10:20 +0200
      Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 11:40 +0200
        Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Lukas Wunner <lukas@wunner.de> - 2017-06-05 14:10 +0200
          Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 15:00 +0200
            Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 15:40 +0200
              Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Lukas Wunner <lukas@wunner.de> - 2017-06-05 15:50 +0200
                Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 16:00 +0200
              Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-06-05 16:30 +0200
                Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 16:50 +0200
                Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-05 17:30 +0200
                Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 17:50 +0200
                RE: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs <Mario.Limonciello@dell.com> - 2017-06-05 18:00 +0200
                Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 19:00 +0200
  [PATCH v3 17/27] thunderbolt: Let the connection manager handle all notifications Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
  [PATCH v3 04/27] thunderbolt: Do not warn about newer DROM versions Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
  [PATCH v3 16/27] thunderbolt: Expose make_header() to other files Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
  [PATCH v3 26/27] thunderbolt: Add documentation how Thunderbolt bus can be used Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
    Re: [PATCH v3 26/27] thunderbolt: Add documentation how Thunderbolt  bus can be used Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-03 16:20 +0200
  [PATCH v3 10/27] thunderbolt: Fail switch adding operation if reading DROM fails Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
  [PATCH v3 03/27] thunderbolt: Do not try to read UID if DROM offset is read as 0 Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
  [PATCH v3 20/27] thunderbolt: Add support for NHI mailbox Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
  [PATCH v3 08/27] thunderbolt: Introduce thunderbolt bus and connection manager Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:20 +0200
    Re: [PATCH v3 08/27] thunderbolt: Introduce thunderbolt bus and  connection manager Andreas Noever <andreas.noever@gmail.com> - 2017-06-05 17:20 +0200
  [PATCH v3 02/27] thunderbolt: No need to read UID of the root switch on resume Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:20 +0200
  [PATCH v3 14/27] thunderbolt: Move control channel messages to tb_msgs.h Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:20 +0200
  [PATCH v3 13/27] thunderbolt: Read vendor and device name from DROM Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:20 +0200
  [PATCH v3 11/27] thunderbolt: Do not fail if DROM data CRC32 is invalid Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:20 +0200
  [PATCH v3 09/27] thunderbolt: Convert switch to a device Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:20 +0200
    Re: [PATCH v3 09/27] thunderbolt: Convert switch to a device Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-03 16:20 +0200
  [PATCH v3 21/27] thunderbolt: Store Thunderbolt generation in the switch structure Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:20 +0200
  Re: [PATCH v3 00/27] Thunderbolt security levels and NVM firmware  upgrade Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-06-02 20:10 +0200
  Re: [PATCH v3 00/27] Thunderbolt security levels and NVM firmware  upgrade Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-03 16:20 +0200
    Re: [PATCH v3 00/27] Thunderbolt security levels and NVM firmware  upgrade Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 09:20 +0200
      Re: [PATCH v3 00/27] Thunderbolt security levels and NVM firmware upgrade Andreas Noever <andreas.noever@gmail.com> - 2017-06-05 19:10 +0200
        Re: [PATCH v3 00/27] Thunderbolt security levels and NVM firmware  upgrade Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 21:50 +0200
        Re: [PATCH v3 00/27] Thunderbolt security levels and NVM firmware  upgrade Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-06 08:10 +0200
          Re: [PATCH v3 00/27] Thunderbolt security levels and NVM firmware  upgrade Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-06 08:50 +0200

csiph-web