Re: [RFC PATCH 00/22] thunderbolt: Add support for USB4

From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org,
	Andreas Noever <andreas.noever@gmail.com>,
	Michael Jamet <michael.jamet@intel.com>,
	Yehezkel Bernat <YehezkelShB@gmail.com>,
	Rajmohan Mani <rajmohan.mani@intel.com>,
	Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>,
	Lukas Wunner <lukas@wunner.de>,
	Alan Stern <stern@rowland.harvard.edu>,
	Mario.Limonciello@dell.com,
	Anthony Wong <anthony.wong@canonical.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 00/22] thunderbolt: Add support for USB4
Date: Tue, 1 Oct 2019 16:42:47 +0300	[thread overview]
Message-ID: <20191001134247.GT2714@lahna.fi.intel.com> (raw)
In-Reply-To: <20191001124954.GI2954373@kroah.com>

On Tue, Oct 01, 2019 at 02:49:54PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Oct 01, 2019 at 02:38:08PM +0300, Mika Westerberg wrote:
> > Hi all,
> > 
> > I'm sending this as RFC because the series is still missing important
> > features such as power management so not ready for merging. However, I
> > think it is good to get any early feedback from the community. We are
> > working to add support for the missing features.
> > 
> > USB4 is the public specification of Thunderbolt 3 protocol and can be
> > downloaded here:
> > 
> >   https://www.usb.org/sites/default/files/USB4%20Specification_1.zip
> > 
> > USB4 is about tunneling different protocols over a single cable (in the
> > same way as Thunderbolt). The current USB4 spec supports PCIe, Display Port
> > and USB 3.x, and also software based protocols such as networking between
> > domains (hosts).
> > 
> > So far PCs have been using firmware based Connection Manager and Apple
> > systems have been using software based one. A Connection Manager is the
> > entity that handles creation of different tunnel types through the USB4
> > (and Thunderbolt) fabric. With USB4 the plan is to have software based
> > Connection Manager everywhere but some early systems will also support
> > firmware to allow OS downgrade for example.
> > 
> > Current Linux Thunderbolt driver supports both "modes" and can detect which
> > one to use dynamically.
> > 
> > This series first adds support for Thunderbolt 3 devices to the software
> > connection manager and then extends that to support USB4 compliant hosts
> > and devices (this applies to both firmware and software based connection
> > managers). With this series the following features are supported also for
> > USB4 compliant devices:
> > 
> >   - PCIe tunneling
> >   - Display Port tunneling
> >   - USB 3.x tunneling
> >   - P2P networking (implemented in drivers/net/thunderbolt.c)
> >   - Host and device NVM firmware upgrade
> > 
> > We also add two new sysfs attributes under each device that expose link
> > speed and width to userspace. The rest of the userspace ABI stays the same.
> > 
> > I'm including Linux USB folks as well because USB4 is officially coming
> > from USB-IF which puts us on same boat now.
> > 
> > While I changed the user visible Kconfig string to mention "USB4" (the
> > Kconfig option is still CONFIG_THUNDERBOLT), I'm wondering whether we
> > should move the whole Thunderbolt driver under drivers/usb/thunderbolt as
> > well? 
> 
> Looks "interesting", nice work!

Thanks!

> I stopped at patch "Add initial support for USB4" as I don't think we
> want to add USB4 code to a system that we know does not have it, right?

You can connect a USB4 device to Thunderbolt 3 system. USB4 hubs
specifically are required to support this by the spec. Of course then it
is in Thunderbolt 3 alternate mode (not in USB4 native mode) but the
USB4 register set is still there. So for example we still need to
configure TMU (Time Management Unit) and access the DROM via router
operations and so on.

Do you mean we don't want that?

> Everything up to then is just "normal" thunderbolt, and with the
> exception of a few minor comments, all look fine to me.
> 
> I didn't actually read the USB4 patch just yet, as I figured we needed
> to argue about that some more :)

Heh, sure and thanks for the feedback so far :)