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

From: Lukas Wunner <lukas@wunner.de>
To: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andreas Noever <andreas.noever@gmail.com>,
	Michael Jamet <michael.jamet@intel.com>,
	Yehezkel Bernat <yehezkel.bernat@intel.com>,
	Amir Levy <amir.jer.levy@intel.com>,
	Andy Lutomirski <luto@kernel.org>,
	Mario.Limonciello@dell.com, Jared.Dominguez@dell.com,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs
Date: Mon, 5 Jun 2017 10:14:37 +0200	[thread overview]
Message-ID: <20170605081437.GA7519@wunner.de> (raw)
In-Reply-To: <20170602140524.23367-20-mika.westerberg@linux.intel.com>

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().

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 think I saw somewhere that you reset the root switch when reconfiguring
the chip to run in ICM mode.  That's a problem because on Apple hardware,
there's a (native, non-ICM) EFI NHI driver which sets up tunnels to any
devices which are already attached on boot.  This can be used to boot
from Thunderbolt-attached harddisks.  I imagine that resetting the chip
to switch to ICM-mode will cause those already established tunnels to
go away, essentially breaking booting from TB-attached disks.  That's
a regression which we should try to avoid.

Ideally, the user should be able to choose whether to use ICM-mode or
native mode.  This could be implemented via a module parameter.
I imagine it might even be possible to use native mode on non-Macs
and this should be allowed as well.

Thanks,

Lukas