Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Fri, 2019-10-04 at 18:16 +0300, Mika Westerberg wrote: > > Is there any harm of also having the 'generation' exposed > > as well? I like the simplicity of the mapping from that value to > > Thunderbolt/USB4 standard version (e.g. I would show that in > > 'boltctl > > list'); 'hw_version' will need a bit more "interpreting". > > If generation is the only thing you need, we can export that now and > forget hw_version :) Sounds good to me, that is should indeed be good enough for bolt.
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Fri, Oct 04, 2019 at 05:02:37PM +0200, Christian Kellner wrote: > Should work. What would the value be for Thunderbolt 3 (and before)? I > guess '0' if I am looking at the right thing (bits 31:24 in > ROUTER_CS_4)? Yes, it would be 0x10 and below that depending on the generation. > Is there any harm of also having the 'generation' exposed > as well? I like the simplicity of the mapping from that value to > Thunderbolt/USB4 standard version (e.g. I would show that in 'boltctl > list'); 'hw_version' will need a bit more "interpreting". If generation is the only thing you need, we can export that now and forget hw_version :)
RE: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
> -Original Message- > From: Limonciello, Mario > Sent: Friday, October 4, 2019 9:06 AM > To: 'Yehezkel Bernat'; Mika Westerberg > Cc: linux-...@vger.kernel.org; Andreas Noever; Michael Jamet; Rajmohan Mani; > nicholas.johnson-opensou...@outlook.com.au; Lukas Wunner; > gre...@linuxfoundation.org; st...@rowland.harvard.edu; Anthony Wong; LKML > Subject: RE: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4 > > > > > On Fri, Oct 4, 2019 at 11:19 AM Mika Westerberg > > wrote: > > > > > > On Fri, Oct 04, 2019 at 11:07:34AM +0300, Yehezkel Bernat wrote: > > > > > Also if you can get the hw_vendor_id and hw_product_id from the kernel > > > > > does that mean you don't need to do the two reads or you still need > > > > > those? > > > > > > > > Are those the chip vendor or the OEM, in case they are different? > > > > > > Those are the actual USB4 hardware maker values, directly from > > > ROUTER_CS_0 (p. 287 in the USB4 spec). This almost certainly differ from > > > the OEM values from DROM we currently expose. > > > > Makes sense to me. Userspace can learn the relevant IDs that their NVM > format > > is > > known. > > > > > > > > > Thinking about it again, I'd guess it shouldn't matter much, if the > > > > chip is from > > > > Intel, the FW supports NVM upgrade, isn't it? > > > > > > So the bottom line is that if the kernel thinks the router supports NVM > > > upgrade it exposes the nvm_active/nvm_non_active files etc. I think > > > fwupd uses this information to display user whether the device can be > > > upgraded or not (for example ICL cannot as the NVM is part of BIOS). > > > > Yes, fwupd already takes this into account, but the question here is how to > > handle cases that NVM is available but the format isn't known to > > userspace (yet). > > Exactly. > > > > > > > > > Exposing hw_vendor_id and hw_product_id may speed up fwupd because it > > > does not need to go over the active NVM to figure out whether the new > > > image is for the correct controller. > > > > It's not about finding the relevant image for upgrade (which must be > > searched > > for by looking in the DROM vendor/product values), but about the question if > the > > NVM format is known to userspace and skip the parsing work if it's anyway > going > > to fail. > > > > So yes, I think exposing vendor ID (and maybe also product ID) can improve > > the > > situation. > > > > Currently at probe time everything comes from udev except for the bit > indicating > running in "native" mode or not. Just enough chunks of the NVM are read to > determine > that (IE no reading up through DROM or jumping around). > > If Christian's patch to export generation is accepted I think that we could > move > that check > to only read -native if generation < 3. > Sorry for the typo; generation < 4. > And if you export the hw_vendor_id and hw_product_id fields then that means > USB4 devices > would require no reading from NVM at "probe" since we don't have to read a - > native bit. > > We would still of course read and analyze the NVM when it comes time to flash > a > new device > though.
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
+Christian On Fri, Oct 04, 2019 at 02:05:46PM +, mario.limoncie...@dell.com wrote: > > > > On Fri, Oct 4, 2019 at 11:19 AM Mika Westerberg > > wrote: > > > > > > On Fri, Oct 04, 2019 at 11:07:34AM +0300, Yehezkel Bernat wrote: > > > > > Also if you can get the hw_vendor_id and hw_product_id from the kernel > > > > > does that mean you don't need to do the two reads or you still need > > > > > those? > > > > > > > > Are those the chip vendor or the OEM, in case they are different? > > > > > > Those are the actual USB4 hardware maker values, directly from > > > ROUTER_CS_0 (p. 287 in the USB4 spec). This almost certainly differ from > > > the OEM values from DROM we currently expose. > > > > Makes sense to me. Userspace can learn the relevant IDs that their NVM > > format > > is > > known. > > > > > > > > > Thinking about it again, I'd guess it shouldn't matter much, if the > > > > chip is from > > > > Intel, the FW supports NVM upgrade, isn't it? > > > > > > So the bottom line is that if the kernel thinks the router supports NVM > > > upgrade it exposes the nvm_active/nvm_non_active files etc. I think > > > fwupd uses this information to display user whether the device can be > > > upgraded or not (for example ICL cannot as the NVM is part of BIOS). > > > > Yes, fwupd already takes this into account, but the question here is how to > > handle cases that NVM is available but the format isn't known to > > userspace (yet). > > Exactly. > > > > > > > > > Exposing hw_vendor_id and hw_product_id may speed up fwupd because it > > > does not need to go over the active NVM to figure out whether the new > > > image is for the correct controller. > > > > It's not about finding the relevant image for upgrade (which must be > > searched > > for by looking in the DROM vendor/product values), but about the question > > if the > > NVM format is known to userspace and skip the parsing work if it's anyway > > going > > to fail. > > > > So yes, I think exposing vendor ID (and maybe also product ID) can improve > > the > > situation. > > > > Currently at probe time everything comes from udev except for the bit > indicating > running in "native" mode or not. Just enough chunks of the NVM are read to > determine > that (IE no reading up through DROM or jumping around). > > If Christian's patch to export generation is accepted I think that we could > move that check > to only read -native if generation < 3. > > And if you export the hw_vendor_id and hw_product_id fields then that means > USB4 devices > would require no reading from NVM at "probe" since we don't have to read a > -native bit. Right. So I'm thinking instead of sw->generation what if we expose three new attributes: hw_vendor_id - Hardware Vendor ID read from ROUTER_CS_0. hw_product_id - Hardware Product ID read from ROUTER_CS_0. hw_version - Hardware USB4 version read from ROUTER_CS_4. This should allow userspace to determine what exactly the device is and which version it is. For example USB4 routers the hw_version is 0x20. @Christian, would this work for bolt as well?
RE: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
> > On Fri, Oct 4, 2019 at 11:19 AM Mika Westerberg > wrote: > > > > On Fri, Oct 04, 2019 at 11:07:34AM +0300, Yehezkel Bernat wrote: > > > > Also if you can get the hw_vendor_id and hw_product_id from the kernel > > > > does that mean you don't need to do the two reads or you still need > > > > those? > > > > > > Are those the chip vendor or the OEM, in case they are different? > > > > Those are the actual USB4 hardware maker values, directly from > > ROUTER_CS_0 (p. 287 in the USB4 spec). This almost certainly differ from > > the OEM values from DROM we currently expose. > > Makes sense to me. Userspace can learn the relevant IDs that their NVM format > is > known. > > > > > > Thinking about it again, I'd guess it shouldn't matter much, if the chip > > > is from > > > Intel, the FW supports NVM upgrade, isn't it? > > > > So the bottom line is that if the kernel thinks the router supports NVM > > upgrade it exposes the nvm_active/nvm_non_active files etc. I think > > fwupd uses this information to display user whether the device can be > > upgraded or not (for example ICL cannot as the NVM is part of BIOS). > > Yes, fwupd already takes this into account, but the question here is how to > handle cases that NVM is available but the format isn't known to > userspace (yet). Exactly. > > > > > Exposing hw_vendor_id and hw_product_id may speed up fwupd because it > > does not need to go over the active NVM to figure out whether the new > > image is for the correct controller. > > It's not about finding the relevant image for upgrade (which must be searched > for by looking in the DROM vendor/product values), but about the question if > the > NVM format is known to userspace and skip the parsing work if it's anyway > going > to fail. > > So yes, I think exposing vendor ID (and maybe also product ID) can improve the > situation. > Currently at probe time everything comes from udev except for the bit indicating running in "native" mode or not. Just enough chunks of the NVM are read to determine that (IE no reading up through DROM or jumping around). If Christian's patch to export generation is accepted I think that we could move that check to only read -native if generation < 3. And if you export the hw_vendor_id and hw_product_id fields then that means USB4 devices would require no reading from NVM at "probe" since we don't have to read a -native bit. We would still of course read and analyze the NVM when it comes time to flash a new device though.
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Fri, Oct 4, 2019 at 11:19 AM Mika Westerberg wrote: > > On Fri, Oct 04, 2019 at 11:07:34AM +0300, Yehezkel Bernat wrote: > > > Also if you can get the hw_vendor_id and hw_product_id from the kernel > > > does that mean you don't need to do the two reads or you still need > > > those? > > > > Are those the chip vendor or the OEM, in case they are different? > > Those are the actual USB4 hardware maker values, directly from > ROUTER_CS_0 (p. 287 in the USB4 spec). This almost certainly differ from > the OEM values from DROM we currently expose. Makes sense to me. Userspace can learn the relevant IDs that their NVM format is known. > > > Thinking about it again, I'd guess it shouldn't matter much, if the chip is > > from > > Intel, the FW supports NVM upgrade, isn't it? > > So the bottom line is that if the kernel thinks the router supports NVM > upgrade it exposes the nvm_active/nvm_non_active files etc. I think > fwupd uses this information to display user whether the device can be > upgraded or not (for example ICL cannot as the NVM is part of BIOS). Yes, fwupd already takes this into account, but the question here is how to handle cases that NVM is available but the format isn't known to userspace (yet). > > Exposing hw_vendor_id and hw_product_id may speed up fwupd because it > does not need to go over the active NVM to figure out whether the new > image is for the correct controller. It's not about finding the relevant image for upgrade (which must be searched for by looking in the DROM vendor/product values), but about the question if the NVM format is known to userspace and skip the parsing work if it's anyway going to fail. So yes, I think exposing vendor ID (and maybe also product ID) can improve the situation. Thanks!
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Fri, Oct 04, 2019 at 11:07:34AM +0300, Yehezkel Bernat wrote: > > Also if you can get the hw_vendor_id and hw_product_id from the kernel > > does that mean you don't need to do the two reads or you still need > > those? > > Are those the chip vendor or the OEM, in case they are different? Those are the actual USB4 hardware maker values, directly from ROUTER_CS_0 (p. 287 in the USB4 spec). This almost certainly differ from the OEM values from DROM we currently expose. > Thinking about it again, I'd guess it shouldn't matter much, if the chip is > from > Intel, the FW supports NVM upgrade, isn't it? So the bottom line is that if the kernel thinks the router supports NVM upgrade it exposes the nvm_active/nvm_non_active files etc. I think fwupd uses this information to display user whether the device can be upgraded or not (for example ICL cannot as the NVM is part of BIOS). Exposing hw_vendor_id and hw_product_id may speed up fwupd because it does not need to go over the active NVM to figure out whether the new image is for the correct controller.
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Fri, Oct 4, 2019 at 10:54 AM Mika Westerberg wrote: > > On Thu, Oct 03, 2019 at 02:41:11PM +, mario.limoncie...@dell.com wrote: > > > -Original Message- > > > From: Mika Westerberg > > > Sent: Thursday, October 3, 2019 3:00 AM > > > To: Limonciello, Mario > > > Cc: yehezkel...@gmail.com; linux-...@vger.kernel.org; > > > andreas.noe...@gmail.com; michael.ja...@intel.com; > > > rajmohan.m...@intel.com; nicholas.johnson-opensou...@outlook.com.au; > > > lu...@wunner.de; gre...@linuxfoundation.org; st...@rowland.harvard.edu; > > > anthony.w...@canonical.com; linux-kernel@vger.kernel.org > > > Subject: Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4 > > > > > > > > > [EXTERNAL EMAIL] > > > > > > On Wed, Oct 02, 2019 at 04:00:55PM +, mario.limoncie...@dell.com > > > wrote: > > > > > It's not even "same location - another meaning", the vendor ID comes > > > > > from > > > the > > > > > DROM section, so it takes a few internal jumps inside the NVM to find > > > > > the > > > > > location. One of the "pointers" or section headers will be broken for > > > > > sure. > > > > > > > > > > And after this, we need to find the NVM in LVFS and it has to pass > > > > > validation > > > in > > > > > a few other locations. The chances are so low that I'd think it isn't > > > > > worth > > > > > worrying about it. > > > > > > > > And now I remember why the back of my mind was having this thought of > > > wanting > > > > sysfs attribute in the first place. The multiple jumps means that a > > > > lot more of > > > the > > > > NVM has to be dumped to get that data, which slows down fwupd startup > > > significantly. > > > > > > IIRC currently fwupd does two reads of total 128 bytes from the active > > > NVM. Is that really slowing down fwupd startup significantly? > > > > Yeah, I timed it with fwupd. Here's the averages: > > > > Without doing the reads to jump to this it's 0:00.06 seconds to probe a > > tree of > > Host controller and dock plugged in. > > > > With doing the reads and just host controller: > > 0:04.40 seconds > > > > With doing the reads and host controller and dock plugged in: > > 0:10.73 seconds > > OK, it clearly takes time to read them. I wonder if this includes > powering up the controller? > > Also if you can get the hw_vendor_id and hw_product_id from the kernel > does that mean you don't need to do the two reads or you still need > those? Are those the chip vendor or the OEM, in case they are different? Thinking about it again, I'd guess it shouldn't matter much, if the chip is from Intel, the FW supports NVM upgrade, isn't it?
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Thu, Oct 03, 2019 at 02:41:11PM +, mario.limoncie...@dell.com wrote: > > -Original Message- > > From: Mika Westerberg > > Sent: Thursday, October 3, 2019 3:00 AM > > To: Limonciello, Mario > > Cc: yehezkel...@gmail.com; linux-...@vger.kernel.org; > > andreas.noe...@gmail.com; michael.ja...@intel.com; > > rajmohan.m...@intel.com; nicholas.johnson-opensou...@outlook.com.au; > > lu...@wunner.de; gre...@linuxfoundation.org; st...@rowland.harvard.edu; > > anthony.w...@canonical.com; linux-kernel@vger.kernel.org > > Subject: Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4 > > > > > > [EXTERNAL EMAIL] > > > > On Wed, Oct 02, 2019 at 04:00:55PM +, mario.limoncie...@dell.com wrote: > > > > It's not even "same location - another meaning", the vendor ID comes > > > > from > > the > > > > DROM section, so it takes a few internal jumps inside the NVM to find > > > > the > > > > location. One of the "pointers" or section headers will be broken for > > > > sure. > > > > > > > > And after this, we need to find the NVM in LVFS and it has to pass > > > > validation > > in > > > > a few other locations. The chances are so low that I'd think it isn't > > > > worth > > > > worrying about it. > > > > > > And now I remember why the back of my mind was having this thought of > > wanting > > > sysfs attribute in the first place. The multiple jumps means that a lot > > > more of > > the > > > NVM has to be dumped to get that data, which slows down fwupd startup > > significantly. > > > > IIRC currently fwupd does two reads of total 128 bytes from the active > > NVM. Is that really slowing down fwupd startup significantly? > > Yeah, I timed it with fwupd. Here's the averages: > > Without doing the reads to jump to this it's 0:00.06 seconds to probe a tree > of > Host controller and dock plugged in. > > With doing the reads and just host controller: > 0:04.40 seconds > > With doing the reads and host controller and dock plugged in: > 0:10.73 seconds OK, it clearly takes time to read them. I wonder if this includes powering up the controller? Also if you can get the hw_vendor_id and hw_product_id from the kernel does that mean you don't need to do the two reads or you still need those?
RE: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
> -Original Message- > From: Mika Westerberg > Sent: Thursday, October 3, 2019 3:00 AM > To: Limonciello, Mario > Cc: yehezkel...@gmail.com; linux-...@vger.kernel.org; > andreas.noe...@gmail.com; michael.ja...@intel.com; > rajmohan.m...@intel.com; nicholas.johnson-opensou...@outlook.com.au; > lu...@wunner.de; gre...@linuxfoundation.org; st...@rowland.harvard.edu; > anthony.w...@canonical.com; linux-kernel@vger.kernel.org > Subject: Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4 > > > [EXTERNAL EMAIL] > > On Wed, Oct 02, 2019 at 04:00:55PM +, mario.limoncie...@dell.com wrote: > > > It's not even "same location - another meaning", the vendor ID comes from > the > > > DROM section, so it takes a few internal jumps inside the NVM to find the > > > location. One of the "pointers" or section headers will be broken for > > > sure. > > > > > > And after this, we need to find the NVM in LVFS and it has to pass > > > validation > in > > > a few other locations. The chances are so low that I'd think it isn't > > > worth > > > worrying about it. > > > > And now I remember why the back of my mind was having this thought of > wanting > > sysfs attribute in the first place. The multiple jumps means that a lot > > more of > the > > NVM has to be dumped to get that data, which slows down fwupd startup > significantly. > > IIRC currently fwupd does two reads of total 128 bytes from the active > NVM. Is that really slowing down fwupd startup significantly? Yeah, I timed it with fwupd. Here's the averages: Without doing the reads to jump to this it's 0:00.06 seconds to probe a tree of Host controller and dock plugged in. With doing the reads and just host controller: 0:04.40 seconds With doing the reads and host controller and dock plugged in: 0:10.73 seconds > > > However the kernel has this information handy already from thunderbolt init > and can > > easily export an attribute which can then come from udev with no startup > penalty. > > Indeed kernel has this information but I'm bit hesitant to add new > attributes if that same information is already available to the > userspace rather easily. IMHO we can always add this to the driver later > as we learn new NVM formats that require more parsing from fwupd side > slowing it down considerably. I guess we can wait until new NVM formats come and cross the bridge at that point. If we encounter a new NVM format that kernel recognizes but old fwupd doesn't it will reject it anyway (just the error won't be super clear why).
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Wed, Oct 02, 2019 at 04:00:55PM +, mario.limoncie...@dell.com wrote: > > It's not even "same location - another meaning", the vendor ID comes from > > the > > DROM section, so it takes a few internal jumps inside the NVM to find the > > location. One of the "pointers" or section headers will be broken for sure. > > > > And after this, we need to find the NVM in LVFS and it has to pass > > validation in > > a few other locations. The chances are so low that I'd think it isn't worth > > worrying about it. > > And now I remember why the back of my mind was having this thought of wanting > sysfs attribute in the first place. The multiple jumps means that a lot more > of the > NVM has to be dumped to get that data, which slows down fwupd startup > significantly. IIRC currently fwupd does two reads of total 128 bytes from the active NVM. Is that really slowing down fwupd startup significantly? > However the kernel has this information handy already from thunderbolt init > and can > easily export an attribute which can then come from udev with no startup > penalty. Indeed kernel has this information but I'm bit hesitant to add new attributes if that same information is already available to the userspace rather easily. IMHO we can always add this to the driver later as we learn new NVM formats that require more parsing from fwupd side slowing it down considerably.
RE: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
> -Original Message- > From: Yehezkel Bernat > Sent: Wednesday, October 2, 2019 10:37 AM > To: Limonciello, Mario > Cc: Mika Westerberg; linux-...@vger.kernel.org; Andreas Noever; Michael > Jamet; Rajmohan Mani; nicholas.johnson-opensou...@outlook.com.au; Lukas > Wunner; gre...@linuxfoundation.org; st...@rowland.harvard.edu; Anthony > Wong; LKML > Subject: Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4 > > > [EXTERNAL EMAIL] > > On Wed, Oct 2, 2019 at 6:09 PM wrote: > > > > > -Original Message- > > > From: Mika Westerberg > > > Sent: Wednesday, October 2, 2019 3:39 AM > > > To: Limonciello, Mario > > > Cc: linux-...@vger.kernel.org; andreas.noe...@gmail.com; > > > michael.ja...@intel.com; yehezkel...@gmail.com; > rajmohan.m...@intel.com; > > > nicholas.johnson-opensou...@outlook.com.au; lu...@wunner.de; > > > gre...@linuxfoundation.org; st...@rowland.harvard.edu; > > > anthony.w...@canonical.com; linux-kernel@vger.kernel.org > > > Subject: Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4 > > > > > > > > > [EXTERNAL EMAIL] > > > > > > On Tue, Oct 01, 2019 at 06:14:23PM +, mario.limoncie...@dell.com > wrote: > > > > One more thought; would you consider exporting to sysfs sw- > > > >config.vendor_id? > > > > Maybe an attribute that is switch_vendor? > > > > > > > > Userland fwupd also does validation on the NVM and will need to follow > this. > > > > The same check will go into fwupd to match the vendor and lack of > > > nvm_non_active0 > > > > to mark the device as not updatable. When the checks in the kernel get > > > relaxed, > > > > some NVM parsing will have to make it over to fwupd too to relax the > > > > check > at > > > that point. > > > > > > The original idea was that the kernel does the basic validation and > > > userspace then does more complex checks. Currently you can compare the > > > two NVM images (active one and the new) and find that information there. > > > I think fwupd is doing just that already. Is that not enough? > > > > I guess for fwupd we can do this without the extra attribute: > > > > 1) If no NVM files or nvm_version: means unsupported vendor -show that > messaging somewhere. > > This means kernel doesn't know the vendor. > > > > 2) If NVM file and nvm_version: means kernel knows it > > *fwupd checks the vendor offset for intel. > > -> intel: good, proceed > > ->something else: fwupd needs to learn the format for the new vendor, show > messaging > > > > There is the unlikely case that kernel knows new vendor format and vendor > NVM happened to have > > same value as Intel vendor ID in same location of Intel NVM but another > meaning. > > Hopefully that doesn't happen :) > > It's not even "same location - another meaning", the vendor ID comes from the > DROM section, so it takes a few internal jumps inside the NVM to find the > location. One of the "pointers" or section headers will be broken for sure. > > And after this, we need to find the NVM in LVFS and it has to pass validation > in > a few other locations. The chances are so low that I'd think it isn't worth > worrying about it. And now I remember why the back of my mind was having this thought of wanting sysfs attribute in the first place. The multiple jumps means that a lot more of the NVM has to be dumped to get that data, which slows down fwupd startup significantly. However the kernel has this information handy already from thunderbolt init and can easily export an attribute which can then come from udev with no startup penalty.
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Wed, Oct 2, 2019 at 6:09 PM wrote: > > > -Original Message- > > From: Mika Westerberg > > Sent: Wednesday, October 2, 2019 3:39 AM > > To: Limonciello, Mario > > Cc: linux-...@vger.kernel.org; andreas.noe...@gmail.com; > > michael.ja...@intel.com; yehezkel...@gmail.com; rajmohan.m...@intel.com; > > nicholas.johnson-opensou...@outlook.com.au; lu...@wunner.de; > > gre...@linuxfoundation.org; st...@rowland.harvard.edu; > > anthony.w...@canonical.com; linux-kernel@vger.kernel.org > > Subject: Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4 > > > > > > [EXTERNAL EMAIL] > > > > On Tue, Oct 01, 2019 at 06:14:23PM +, mario.limoncie...@dell.com wrote: > > > One more thought; would you consider exporting to sysfs sw- > > >config.vendor_id? > > > Maybe an attribute that is switch_vendor? > > > > > > Userland fwupd also does validation on the NVM and will need to follow > > > this. > > > The same check will go into fwupd to match the vendor and lack of > > nvm_non_active0 > > > to mark the device as not updatable. When the checks in the kernel get > > relaxed, > > > some NVM parsing will have to make it over to fwupd too to relax the > > > check at > > that point. > > > > The original idea was that the kernel does the basic validation and > > userspace then does more complex checks. Currently you can compare the > > two NVM images (active one and the new) and find that information there. > > I think fwupd is doing just that already. Is that not enough? > > I guess for fwupd we can do this without the extra attribute: > > 1) If no NVM files or nvm_version: means unsupported vendor -show that > messaging somewhere. > This means kernel doesn't know the vendor. > > 2) If NVM file and nvm_version: means kernel knows it > *fwupd checks the vendor offset for intel. > -> intel: good, proceed > ->something else: fwupd needs to learn the format for the new vendor, show > messaging > > There is the unlikely case that kernel knows new vendor format and vendor NVM > happened to have > same value as Intel vendor ID in same location of Intel NVM but another > meaning. > Hopefully that doesn't happen :) It's not even "same location - another meaning", the vendor ID comes from the DROM section, so it takes a few internal jumps inside the NVM to find the location. One of the "pointers" or section headers will be broken for sure. And after this, we need to find the NVM in LVFS and it has to pass validation in a few other locations. The chances are so low that I'd think it isn't worth worrying about it.
RE: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
> -Original Message- > From: Mika Westerberg > Sent: Wednesday, October 2, 2019 3:39 AM > To: Limonciello, Mario > Cc: linux-...@vger.kernel.org; andreas.noe...@gmail.com; > michael.ja...@intel.com; yehezkel...@gmail.com; rajmohan.m...@intel.com; > nicholas.johnson-opensou...@outlook.com.au; lu...@wunner.de; > gre...@linuxfoundation.org; st...@rowland.harvard.edu; > anthony.w...@canonical.com; linux-kernel@vger.kernel.org > Subject: Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4 > > > [EXTERNAL EMAIL] > > On Tue, Oct 01, 2019 at 06:14:23PM +, mario.limoncie...@dell.com wrote: > > One more thought; would you consider exporting to sysfs sw- > >config.vendor_id? > > Maybe an attribute that is switch_vendor? > > > > Userland fwupd also does validation on the NVM and will need to follow this. > > The same check will go into fwupd to match the vendor and lack of > nvm_non_active0 > > to mark the device as not updatable. When the checks in the kernel get > relaxed, > > some NVM parsing will have to make it over to fwupd too to relax the check > > at > that point. > > The original idea was that the kernel does the basic validation and > userspace then does more complex checks. Currently you can compare the > two NVM images (active one and the new) and find that information there. > I think fwupd is doing just that already. Is that not enough? I guess for fwupd we can do this without the extra attribute: 1) If no NVM files or nvm_version: means unsupported vendor -show that messaging somewhere. This means kernel doesn't know the vendor. 2) If NVM file and nvm_version: means kernel knows it *fwupd checks the vendor offset for intel. -> intel: good, proceed ->something else: fwupd needs to learn the format for the new vendor, show messaging There is the unlikely case that kernel knows new vendor format and vendor NVM happened to have same value as Intel vendor ID in same location of Intel NVM but another meaning. Hopefully that doesn't happen :)
RE: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
> -Original Message-
> From: Mika Westerberg
> Sent: Wednesday, October 2, 2019 3:35 AM
> To: Limonciello, Mario
> Cc: linux-...@vger.kernel.org; andreas.noe...@gmail.com;
> michael.ja...@intel.com; yehezkel...@gmail.com; rajmohan.m...@intel.com;
> nicholas.johnson-opensou...@outlook.com.au; lu...@wunner.de;
> gre...@linuxfoundation.org; st...@rowland.harvard.edu;
> anthony.w...@canonical.com; linux-kernel@vger.kernel.org
> Subject: Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
>
>
> [EXTERNAL EMAIL]
>
> On Tue, Oct 01, 2019 at 05:05:09PM +, mario.limoncie...@dell.com wrote:
> > > @@ -322,9 +398,21 @@ static int tb_switch_nvm_add(struct tb_switch *sw)
> > > u32 val;
> > > int ret;
> > >
> > > - if (!sw->dma_port)
> > > + if (!nvm_readable(sw))
> > > return 0;
> > >
> > > + /*
> > > + * The NVM format of non-Intel hardware is not known so
> > > + * currently restrict NVM upgrade for Intel hardware. We may
> > > + * relax this in the future when we learn other NVM formats.
> > > + */
> > > + if (sw->config.vendor_id != PCI_VENDOR_ID_INTEL) {
> > > + dev_info(&sw->dev,
> > > + "NVM format of vendor %#x is not known, disabling
> NVM
> > > upgrade\n",
> > > + sw->config.vendor_id);
> > > + return 0;
> > > + }
> > > +
> >
> > Don't you actually have an attribute you can use here for this exact purpose
> that you
> > could be setting rather than returning immediately?
> > sw->no_nvm_upgrade
> >
> > Then potentially you can at least let users "dump out" the nvm on !Intel but
> don't let
> > it be written until ready to relax further.
>
> Problem is that we currently read NVM version and size from a known
> location in the NVM. If we don't know the format we can't do that so
> that would mean we need to expose active NVM with some size and hide
> nvm_version. I would rather have this support included in the kernel
> driver and expose standard set of attributes but no strong feelings from
> one way or another.
I agree with you; this is a safer and smarter approach to wait until details of
format
for other vendors is known and export attributes only when it is. This will
also
encourage other vendors to open up their format if the only way to access NVM is
to document the headers.
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Wed, Oct 02, 2019 at 10:39:54AM +0200, Greg Kroah-Hartman wrote: > On Wed, Oct 02, 2019 at 11:30:34AM +0300, Mika Westerberg wrote: > > On Tue, Oct 01, 2019 at 06:27:42PM +0200, Oliver Neukum wrote: > > > Am Dienstag, den 01.10.2019, 18:07 +0300 schrieb Mika Westerberg: > > > > > > Hi, > > > > > > > OK, but does that break existing .configs? I mean if you have already > > > > CONFIG_THUNDERBOLT in your .config/defconfig does it now just get > > > > dropped silently? > > > > > > People will have to look at this new stuff anyway. > > > > > > > For example firewire has CONFIG_FIREWIRE even though the "standard" name > > > > is IEEE 1394. I was thinking maybe we can do the same for > > > > USB4/Thunderbolt > > > > > > USB and Thunderbolt used to be distinct protocols. Whereas Firewire > > > was just a colloquial name for IEEE1394. Please be wordy here. > > > "Unified support for USB4 and Thunderbolt4" > > > > OK. > > > > I've been thinking this bit more and since Thunderbolt will stick around > > as well (it basically implements all the optional USB4 features and > > more) so would it make sense to have the Kconfig option be > > CONFIG_THUNDERBOLT_USB4 (or CONFIG_USB4_THUNDERBOLT)? That should cover > > both. > > I would stick with CONFIG_USB4 but put both in the Kconfig text. Again, > it will be easier to handle this over time. OK, thanks Greg! > > Comments? > > > > Also does anyone have any thoughts about keeping the driver under > > drivers/thunderbolt vs. moving it under usb like > > drivers/usb/thunderbolt? I'm thinking if anyone not familiar with this > > tries to enable support for USB4 so the first place he/she probably > > looks is under "USB support" menuconfig entry. > > You are not sharing/needing any of the drivers/usb/ code just yet, > right? Yes, that's correct. > I imagine that will happen "soon" and when it does, then sure, > moving stuff is fine with me. OK thanks!
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Wed, Oct 02, 2019 at 11:30:34AM +0300, Mika Westerberg wrote: > On Tue, Oct 01, 2019 at 06:27:42PM +0200, Oliver Neukum wrote: > > Am Dienstag, den 01.10.2019, 18:07 +0300 schrieb Mika Westerberg: > > > > Hi, > > > > > OK, but does that break existing .configs? I mean if you have already > > > CONFIG_THUNDERBOLT in your .config/defconfig does it now just get > > > dropped silently? > > > > People will have to look at this new stuff anyway. > > > > > For example firewire has CONFIG_FIREWIRE even though the "standard" name > > > is IEEE 1394. I was thinking maybe we can do the same for > > > USB4/Thunderbolt > > > > USB and Thunderbolt used to be distinct protocols. Whereas Firewire > > was just a colloquial name for IEEE1394. Please be wordy here. > > "Unified support for USB4 and Thunderbolt4" > > OK. > > I've been thinking this bit more and since Thunderbolt will stick around > as well (it basically implements all the optional USB4 features and > more) so would it make sense to have the Kconfig option be > CONFIG_THUNDERBOLT_USB4 (or CONFIG_USB4_THUNDERBOLT)? That should cover > both. I would stick with CONFIG_USB4 but put both in the Kconfig text. Again, it will be easier to handle this over time. > Comments? > > Also does anyone have any thoughts about keeping the driver under > drivers/thunderbolt vs. moving it under usb like > drivers/usb/thunderbolt? I'm thinking if anyone not familiar with this > tries to enable support for USB4 so the first place he/she probably > looks is under "USB support" menuconfig entry. You are not sharing/needing any of the drivers/usb/ code just yet, right? I imagine that will happen "soon" and when it does, then sure, moving stuff is fine with me. thanks, greg k-h
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Tue, Oct 01, 2019 at 06:14:23PM +, mario.limoncie...@dell.com wrote: > One more thought; would you consider exporting to sysfs sw->config.vendor_id? > Maybe an attribute that is switch_vendor? > > Userland fwupd also does validation on the NVM and will need to follow this. > The same check will go into fwupd to match the vendor and lack of > nvm_non_active0 > to mark the device as not updatable. When the checks in the kernel get > relaxed, > some NVM parsing will have to make it over to fwupd too to relax the check at > that point. The original idea was that the kernel does the basic validation and userspace then does more complex checks. Currently you can compare the two NVM images (active one and the new) and find that information there. I think fwupd is doing just that already. Is that not enough?
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Tue, Oct 01, 2019 at 05:05:09PM +, mario.limoncie...@dell.com wrote:
> > @@ -322,9 +398,21 @@ static int tb_switch_nvm_add(struct tb_switch *sw)
> > u32 val;
> > int ret;
> >
> > - if (!sw->dma_port)
> > + if (!nvm_readable(sw))
> > return 0;
> >
> > + /*
> > +* The NVM format of non-Intel hardware is not known so
> > +* currently restrict NVM upgrade for Intel hardware. We may
> > +* relax this in the future when we learn other NVM formats.
> > +*/
> > + if (sw->config.vendor_id != PCI_VENDOR_ID_INTEL) {
> > + dev_info(&sw->dev,
> > +"NVM format of vendor %#x is not known, disabling NVM
> > upgrade\n",
> > +sw->config.vendor_id);
> > + return 0;
> > + }
> > +
>
> Don't you actually have an attribute you can use here for this exact purpose
> that you
> could be setting rather than returning immediately?
> sw->no_nvm_upgrade
>
> Then potentially you can at least let users "dump out" the nvm on !Intel but
> don't let
> it be written until ready to relax further.
Problem is that we currently read NVM version and size from a known
location in the NVM. If we don't know the format we can't do that so
that would mean we need to expose active NVM with some size and hide
nvm_version. I would rather have this support included in the kernel
driver and expose standard set of attributes but no strong feelings from
one way or another.
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Tue, Oct 01, 2019 at 06:27:42PM +0200, Oliver Neukum wrote: > Am Dienstag, den 01.10.2019, 18:07 +0300 schrieb Mika Westerberg: > > Hi, > > > OK, but does that break existing .configs? I mean if you have already > > CONFIG_THUNDERBOLT in your .config/defconfig does it now just get > > dropped silently? > > People will have to look at this new stuff anyway. > > > For example firewire has CONFIG_FIREWIRE even though the "standard" name > > is IEEE 1394. I was thinking maybe we can do the same for > > USB4/Thunderbolt > > USB and Thunderbolt used to be distinct protocols. Whereas Firewire > was just a colloquial name for IEEE1394. Please be wordy here. > "Unified support for USB4 and Thunderbolt4" OK. I've been thinking this bit more and since Thunderbolt will stick around as well (it basically implements all the optional USB4 features and more) so would it make sense to have the Kconfig option be CONFIG_THUNDERBOLT_USB4 (or CONFIG_USB4_THUNDERBOLT)? That should cover both. Comments? Also does anyone have any thoughts about keeping the driver under drivers/thunderbolt vs. moving it under usb like drivers/usb/thunderbolt? I'm thinking if anyone not familiar with this tries to enable support for USB4 so the first place he/she probably looks is under "USB support" menuconfig entry.
RE: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
> -Original Message-
> From: Limonciello, Mario
> Sent: Tuesday, October 1, 2019 12:05 PM
> To: 'Mika Westerberg'; linux-...@vger.kernel.org
> Cc: Andreas Noever; Michael Jamet; Yehezkel Bernat; Rajmohan Mani; Nicholas
> Johnson; Lukas Wunner; Greg Kroah-Hartman; Alan Stern; Anthony Wong; linux-
> ker...@vger.kernel.org
> Subject: RE: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
>
> > @@ -322,9 +398,21 @@ static int tb_switch_nvm_add(struct tb_switch *sw)
> > u32 val;
> > int ret;
> >
> > - if (!sw->dma_port)
> > + if (!nvm_readable(sw))
> > return 0;
> >
> > + /*
> > +* The NVM format of non-Intel hardware is not known so
> > +* currently restrict NVM upgrade for Intel hardware. We may
> > +* relax this in the future when we learn other NVM formats.
> > +*/
> > + if (sw->config.vendor_id != PCI_VENDOR_ID_INTEL) {
> > + dev_info(&sw->dev,
> > +"NVM format of vendor %#x is not known, disabling NVM
> > upgrade\n",
> > +sw->config.vendor_id);
> > + return 0;
> > + }
> > +
>
> Don't you actually have an attribute you can use here for this exact purpose
> that
> you
> could be setting rather than returning immediately?
> sw->no_nvm_upgrade
>
One more thought; would you consider exporting to sysfs sw->config.vendor_id?
Maybe an attribute that is switch_vendor?
Userland fwupd also does validation on the NVM and will need to follow this.
The same check will go into fwupd to match the vendor and lack of
nvm_non_active0
to mark the device as not updatable. When the checks in the kernel get relaxed,
some NVM parsing will have to make it over to fwupd too to relax the check at
that point.
> Then potentially you can at least let users "dump out" the nvm on !Intel but
> don't
> let
> it be written until ready to relax further.
>
> > nvm = kzalloc(sizeof(*nvm), GFP_KERNEL);
> > if (!nvm)
> > return -ENOMEM;
RE: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
> @@ -322,9 +398,21 @@ static int tb_switch_nvm_add(struct tb_switch *sw)
> u32 val;
> int ret;
>
> - if (!sw->dma_port)
> + if (!nvm_readable(sw))
> return 0;
>
> + /*
> + * The NVM format of non-Intel hardware is not known so
> + * currently restrict NVM upgrade for Intel hardware. We may
> + * relax this in the future when we learn other NVM formats.
> + */
> + if (sw->config.vendor_id != PCI_VENDOR_ID_INTEL) {
> + dev_info(&sw->dev,
> + "NVM format of vendor %#x is not known, disabling NVM
> upgrade\n",
> + sw->config.vendor_id);
> + return 0;
> + }
> +
Don't you actually have an attribute you can use here for this exact purpose
that you
could be setting rather than returning immediately?
sw->no_nvm_upgrade
Then potentially you can at least let users "dump out" the nvm on !Intel but
don't let
it be written until ready to relax further.
> nvm = kzalloc(sizeof(*nvm), GFP_KERNEL);
> if (!nvm)
> return -ENOMEM;
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
Am Dienstag, den 01.10.2019, 18:07 +0300 schrieb Mika Westerberg: Hi, > OK, but does that break existing .configs? I mean if you have already > CONFIG_THUNDERBOLT in your .config/defconfig does it now just get > dropped silently? People will have to look at this new stuff anyway. > For example firewire has CONFIG_FIREWIRE even though the "standard" name > is IEEE 1394. I was thinking maybe we can do the same for > USB4/Thunderbolt USB and Thunderbolt used to be distinct protocols. Whereas Firewire was just a colloquial name for IEEE1394. Please be wordy here. "Unified support for USB4 and Thunderbolt4" Regards Oliver
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Tue, Oct 01, 2019 at 05:22:13PM +0200, Greg KH wrote: > On Tue, Oct 01, 2019 at 02:59:06PM +, mario.limoncie...@dell.com wrote: > > > > > > > -Original Message- > > > From: Greg Kroah-Hartman > > > Sent: Tuesday, October 1, 2019 9:54 AM > > > To: Mika Westerberg > > > Cc: linux-...@vger.kernel.org; Andreas Noever; Michael Jamet; Yehezkel > > > Bernat; > > > Rajmohan Mani; Nicholas Johnson; Lukas Wunner; Alan Stern; Limonciello, > > > Mario; > > > Anthony Wong; linux-kernel@vger.kernel.org > > > Subject: Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4 > > > > > > > > > [EXTERNAL EMAIL] > > > > > > On Tue, Oct 01, 2019 at 04:09:05PM +0300, Mika Westerberg wrote: > > > > On Tue, Oct 01, 2019 at 02:47:48PM +0200, Greg Kroah-Hartman wrote: > > > > > > - Thunderbolt Controller driver. This driver is required if you > > > > > > - want to hotplug Thunderbolt devices on Apple hardware or on > > > > > > PCs > > > > > > - with Intel Falcon Ridge or newer. > > > > > > + USB4 (Thunderbolt) driver. USB4 is the public spec based on > > > > > > + Thunderbolt 3 protocol. This driver is required if you want to > > > > > > + hotplug Thunderbolt and USB4 compliant devices on Apple > > > > > > + hardware or on PCs with Intel Falcon Ridge or newer. > > > > > > > > > > Wait, did "old" thunderbolt just get re-branded as USB4? > > > > > > > > Not but the driver started supporting USB4 as well :) > > > > > > > > USB4 is pretty much public spec of Thunderbolt 3 but with some > > > > differences in register layouts (this is because Thunderbolt uses some > > > > vendor specific capabilities which are now moved to more "standard" > > > > places). > > > > > > Ok, then we need to rename the Kconfig option as well, otherwise no one > > > will "know" that this changed, so they will not be prompted for it. > > > > > > > > Because if I have an "old" laptop that needs Thunderbolt support, how > > > > > am > > > > > I going to know it is now called USB4 instead? > > > > > > > > Well the Kconfig option tries to have both names there: > > > > > > > > tristate "USB4 (Thunderbolt) support" > > > > > > > > and then > > > > > > > > USB4 (Thunderbolt) driver. USB4 is the public spec based on > > > > Thunderbolt 3 protocol. This driver is required if you want to hotplug > > > > Thunderbolt and USB4 compliant devices on Apple hardware or on PCs > > > > with Intel Falcon Ridge or newer. > > > > > > > > and the Kconfig option is still CONFIG_THUNDERBOLT. I know this is > > > > confusing but I don't have better ideas how we can advertise both. I > > > > borrowed this "format" from firewire. > > > > > > CONFIG_USB4 instead? > > > > How about CONFIG_USB4_PCIE? > > > > I think that will help align that certain aspects of USB4 can be built > > optionally. > > What aspects? We don't have that here at all. > > I guess the parts of USB4 that are not just this "hook up the PCIe > lane" that need to still be developed? Actually PCIe tunneling is already there in the driver. USB4 has one bit that is needed to be set before PCIe tunneling can happen (we do that in patch 17/22) but other than that the existing PCIe tunneling code in the driver works as is.
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Tue, Oct 01, 2019 at 05:19:35PM +0200, Greg Kroah-Hartman wrote: > On Tue, Oct 01, 2019 at 06:07:34PM +0300, Mika Westerberg wrote: > > On Tue, Oct 01, 2019 at 04:53:54PM +0200, Greg Kroah-Hartman wrote: > > > On Tue, Oct 01, 2019 at 04:09:05PM +0300, Mika Westerberg wrote: > > > > On Tue, Oct 01, 2019 at 02:47:48PM +0200, Greg Kroah-Hartman wrote: > > > > > > - Thunderbolt Controller driver. This driver is required if you > > > > > > - want to hotplug Thunderbolt devices on Apple hardware or on > > > > > > PCs > > > > > > - with Intel Falcon Ridge or newer. > > > > > > + USB4 (Thunderbolt) driver. USB4 is the public spec based on > > > > > > + Thunderbolt 3 protocol. This driver is required if you want to > > > > > > + hotplug Thunderbolt and USB4 compliant devices on Apple > > > > > > + hardware or on PCs with Intel Falcon Ridge or newer. > > > > > > > > > > Wait, did "old" thunderbolt just get re-branded as USB4? > > > > > > > > Not but the driver started supporting USB4 as well :) > > > > > > > > USB4 is pretty much public spec of Thunderbolt 3 but with some > > > > differences in register layouts (this is because Thunderbolt uses some > > > > vendor specific capabilities which are now moved to more "standard" > > > > places). > > > > > > Ok, then we need to rename the Kconfig option as well, otherwise no one > > > will "know" that this changed, so they will not be prompted for it. > > > > > > > > Because if I have an "old" laptop that needs Thunderbolt support, how > > > > > am > > > > > I going to know it is now called USB4 instead? > > > > > > > > Well the Kconfig option tries to have both names there: > > > > > > > > tristate "USB4 (Thunderbolt) support" > > > > > > > > and then > > > > > > > > USB4 (Thunderbolt) driver. USB4 is the public spec based on > > > > Thunderbolt 3 protocol. This driver is required if you want to hotplug > > > > Thunderbolt and USB4 compliant devices on Apple hardware or on PCs > > > > with Intel Falcon Ridge or newer. > > > > > > > > and the Kconfig option is still CONFIG_THUNDERBOLT. I know this is > > > > confusing but I don't have better ideas how we can advertise both. I > > > > borrowed this "format" from firewire. > > > > > > CONFIG_USB4 instead? > > > > OK, but does that break existing .configs? I mean if you have already > > CONFIG_THUNDERBOLT in your .config/defconfig does it now just get > > dropped silently? > > Yup. And then you get asked about CONFIG_USB4 I see. > > For example firewire has CONFIG_FIREWIRE even though the "standard" name > > is IEEE 1394. I was thinking maybe we can do the same for > > USB4/Thunderbolt? > > It comes down to the what do you want to do for the next 20+ years, > explain to people that "to get USB4 support, enable CONFIG_THUNDERBOLT"? That's a very good point indeed :)
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Tue, Oct 01, 2019 at 02:59:06PM +, mario.limoncie...@dell.com wrote: > > > > -Original Message- > > From: Greg Kroah-Hartman > > Sent: Tuesday, October 1, 2019 9:54 AM > > To: Mika Westerberg > > Cc: linux-...@vger.kernel.org; Andreas Noever; Michael Jamet; Yehezkel > > Bernat; > > Rajmohan Mani; Nicholas Johnson; Lukas Wunner; Alan Stern; Limonciello, > > Mario; > > Anthony Wong; linux-kernel@vger.kernel.org > > Subject: Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4 > > > > > > [EXTERNAL EMAIL] > > > > On Tue, Oct 01, 2019 at 04:09:05PM +0300, Mika Westerberg wrote: > > > On Tue, Oct 01, 2019 at 02:47:48PM +0200, Greg Kroah-Hartman wrote: > > > > > - Thunderbolt Controller driver. This driver is required if you > > > > > - want to hotplug Thunderbolt devices on Apple hardware or on > > > > > PCs > > > > > - with Intel Falcon Ridge or newer. > > > > > + USB4 (Thunderbolt) driver. USB4 is the public spec based on > > > > > + Thunderbolt 3 protocol. This driver is required if you want to > > > > > + hotplug Thunderbolt and USB4 compliant devices on Apple > > > > > + hardware or on PCs with Intel Falcon Ridge or newer. > > > > > > > > Wait, did "old" thunderbolt just get re-branded as USB4? > > > > > > Not but the driver started supporting USB4 as well :) > > > > > > USB4 is pretty much public spec of Thunderbolt 3 but with some > > > differences in register layouts (this is because Thunderbolt uses some > > > vendor specific capabilities which are now moved to more "standard" > > > places). > > > > Ok, then we need to rename the Kconfig option as well, otherwise no one > > will "know" that this changed, so they will not be prompted for it. > > > > > > Because if I have an "old" laptop that needs Thunderbolt support, how am > > > > I going to know it is now called USB4 instead? > > > > > > Well the Kconfig option tries to have both names there: > > > > > > tristate "USB4 (Thunderbolt) support" > > > > > > and then > > > > > > USB4 (Thunderbolt) driver. USB4 is the public spec based on > > > Thunderbolt 3 protocol. This driver is required if you want to hotplug > > > Thunderbolt and USB4 compliant devices on Apple hardware or on PCs > > > with Intel Falcon Ridge or newer. > > > > > > and the Kconfig option is still CONFIG_THUNDERBOLT. I know this is > > > confusing but I don't have better ideas how we can advertise both. I > > > borrowed this "format" from firewire. > > > > CONFIG_USB4 instead? > > How about CONFIG_USB4_PCIE? > > I think that will help align that certain aspects of USB4 can be built > optionally. What aspects? We don't have that here at all. I guess the parts of USB4 that are not just this "hook up the PCIe lane" that need to still be developed? thanks, greg k-h
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Tue, Oct 01, 2019 at 06:07:34PM +0300, Mika Westerberg wrote: > On Tue, Oct 01, 2019 at 04:53:54PM +0200, Greg Kroah-Hartman wrote: > > On Tue, Oct 01, 2019 at 04:09:05PM +0300, Mika Westerberg wrote: > > > On Tue, Oct 01, 2019 at 02:47:48PM +0200, Greg Kroah-Hartman wrote: > > > > > - Thunderbolt Controller driver. This driver is required if you > > > > > - want to hotplug Thunderbolt devices on Apple hardware or on > > > > > PCs > > > > > - with Intel Falcon Ridge or newer. > > > > > + USB4 (Thunderbolt) driver. USB4 is the public spec based on > > > > > + Thunderbolt 3 protocol. This driver is required if you want to > > > > > + hotplug Thunderbolt and USB4 compliant devices on Apple > > > > > + hardware or on PCs with Intel Falcon Ridge or newer. > > > > > > > > Wait, did "old" thunderbolt just get re-branded as USB4? > > > > > > Not but the driver started supporting USB4 as well :) > > > > > > USB4 is pretty much public spec of Thunderbolt 3 but with some > > > differences in register layouts (this is because Thunderbolt uses some > > > vendor specific capabilities which are now moved to more "standard" > > > places). > > > > Ok, then we need to rename the Kconfig option as well, otherwise no one > > will "know" that this changed, so they will not be prompted for it. > > > > > > Because if I have an "old" laptop that needs Thunderbolt support, how am > > > > I going to know it is now called USB4 instead? > > > > > > Well the Kconfig option tries to have both names there: > > > > > > tristate "USB4 (Thunderbolt) support" > > > > > > and then > > > > > > USB4 (Thunderbolt) driver. USB4 is the public spec based on > > > Thunderbolt 3 protocol. This driver is required if you want to hotplug > > > Thunderbolt and USB4 compliant devices on Apple hardware or on PCs > > > with Intel Falcon Ridge or newer. > > > > > > and the Kconfig option is still CONFIG_THUNDERBOLT. I know this is > > > confusing but I don't have better ideas how we can advertise both. I > > > borrowed this "format" from firewire. > > > > CONFIG_USB4 instead? > > OK, but does that break existing .configs? I mean if you have already > CONFIG_THUNDERBOLT in your .config/defconfig does it now just get > dropped silently? Yup. And then you get asked about CONFIG_USB4 > For example firewire has CONFIG_FIREWIRE even though the "standard" name > is IEEE 1394. I was thinking maybe we can do the same for > USB4/Thunderbolt? It comes down to the what do you want to do for the next 20+ years, explain to people that "to get USB4 support, enable CONFIG_THUNDERBOLT"? :) thanks, greg k-h
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Tue, Oct 01, 2019 at 02:59:06PM +, mario.limoncie...@dell.com wrote: > > > > -Original Message- > > From: Greg Kroah-Hartman > > Sent: Tuesday, October 1, 2019 9:54 AM > > To: Mika Westerberg > > Cc: linux-...@vger.kernel.org; Andreas Noever; Michael Jamet; Yehezkel > > Bernat; > > Rajmohan Mani; Nicholas Johnson; Lukas Wunner; Alan Stern; Limonciello, > > Mario; > > Anthony Wong; linux-kernel@vger.kernel.org > > Subject: Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4 > > > > > > [EXTERNAL EMAIL] > > > > On Tue, Oct 01, 2019 at 04:09:05PM +0300, Mika Westerberg wrote: > > > On Tue, Oct 01, 2019 at 02:47:48PM +0200, Greg Kroah-Hartman wrote: > > > > > - Thunderbolt Controller driver. This driver is required if you > > > > > - want to hotplug Thunderbolt devices on Apple hardware or on > > > > > PCs > > > > > - with Intel Falcon Ridge or newer. > > > > > + USB4 (Thunderbolt) driver. USB4 is the public spec based on > > > > > + Thunderbolt 3 protocol. This driver is required if you want to > > > > > + hotplug Thunderbolt and USB4 compliant devices on Apple > > > > > + hardware or on PCs with Intel Falcon Ridge or newer. > > > > > > > > Wait, did "old" thunderbolt just get re-branded as USB4? > > > > > > Not but the driver started supporting USB4 as well :) > > > > > > USB4 is pretty much public spec of Thunderbolt 3 but with some > > > differences in register layouts (this is because Thunderbolt uses some > > > vendor specific capabilities which are now moved to more "standard" > > > places). > > > > Ok, then we need to rename the Kconfig option as well, otherwise no one > > will "know" that this changed, so they will not be prompted for it. > > > > > > Because if I have an "old" laptop that needs Thunderbolt support, how am > > > > I going to know it is now called USB4 instead? > > > > > > Well the Kconfig option tries to have both names there: > > > > > > tristate "USB4 (Thunderbolt) support" > > > > > > and then > > > > > > USB4 (Thunderbolt) driver. USB4 is the public spec based on > > > Thunderbolt 3 protocol. This driver is required if you want to hotplug > > > Thunderbolt and USB4 compliant devices on Apple hardware or on PCs > > > with Intel Falcon Ridge or newer. > > > > > > and the Kconfig option is still CONFIG_THUNDERBOLT. I know this is > > > confusing but I don't have better ideas how we can advertise both. I > > > borrowed this "format" from firewire. > > > > CONFIG_USB4 instead? > > How about CONFIG_USB4_PCIE? > > I think that will help align that certain aspects of USB4 can be built > optionally. But this is not about PCIe - it is just one type of a tunnel that is optional with USB4. > > > > Shouldn't there just be a new USB4 option that only enables/builds the > > > > USB4 stuff if selected? Why would I want all of this additional code on > > > > my old system if it's not going to do anything at all? > > > > > > USB4 devices are backward compatible with Thunderbolt 3 so you should be > > > able to plug in USB4 device to your old Thunderbolt 3 laptop for > > > example. It goes the other way as well. Some things are optional but for > > > example USB4 hubs must support also Thunderbolt 3. > > > > > If PCIe tunnels are an optional feature in USB4, how can it be mandatory to > support > Thunderbolt 3? It is mandatory for USB4 hubs. For peripheral devices and hosts Thunderbolt 3 support is optional. So for example you could have USB4 host that does not enter Thunderbolt 3 alternate mode at all so it only supports USB4 devices.
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Tue, Oct 01, 2019 at 04:53:54PM +0200, Greg Kroah-Hartman wrote: > On Tue, Oct 01, 2019 at 04:09:05PM +0300, Mika Westerberg wrote: > > On Tue, Oct 01, 2019 at 02:47:48PM +0200, Greg Kroah-Hartman wrote: > > > > - Thunderbolt Controller driver. This driver is required if you > > > > - want to hotplug Thunderbolt devices on Apple hardware or on > > > > PCs > > > > - with Intel Falcon Ridge or newer. > > > > + USB4 (Thunderbolt) driver. USB4 is the public spec based on > > > > + Thunderbolt 3 protocol. This driver is required if you want to > > > > + hotplug Thunderbolt and USB4 compliant devices on Apple > > > > + hardware or on PCs with Intel Falcon Ridge or newer. > > > > > > Wait, did "old" thunderbolt just get re-branded as USB4? > > > > Not but the driver started supporting USB4 as well :) > > > > USB4 is pretty much public spec of Thunderbolt 3 but with some > > differences in register layouts (this is because Thunderbolt uses some > > vendor specific capabilities which are now moved to more "standard" > > places). > > Ok, then we need to rename the Kconfig option as well, otherwise no one > will "know" that this changed, so they will not be prompted for it. > > > > Because if I have an "old" laptop that needs Thunderbolt support, how am > > > I going to know it is now called USB4 instead? > > > > Well the Kconfig option tries to have both names there: > > > > tristate "USB4 (Thunderbolt) support" > > > > and then > > > > USB4 (Thunderbolt) driver. USB4 is the public spec based on > > Thunderbolt 3 protocol. This driver is required if you want to hotplug > > Thunderbolt and USB4 compliant devices on Apple hardware or on PCs > > with Intel Falcon Ridge or newer. > > > > and the Kconfig option is still CONFIG_THUNDERBOLT. I know this is > > confusing but I don't have better ideas how we can advertise both. I > > borrowed this "format" from firewire. > > CONFIG_USB4 instead? OK, but does that break existing .configs? I mean if you have already CONFIG_THUNDERBOLT in your .config/defconfig does it now just get dropped silently? For example firewire has CONFIG_FIREWIRE even though the "standard" name is IEEE 1394. I was thinking maybe we can do the same for USB4/Thunderbolt?
RE: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
> -Original Message- > From: Greg Kroah-Hartman > Sent: Tuesday, October 1, 2019 9:54 AM > To: Mika Westerberg > Cc: linux-...@vger.kernel.org; Andreas Noever; Michael Jamet; Yehezkel Bernat; > Rajmohan Mani; Nicholas Johnson; Lukas Wunner; Alan Stern; Limonciello, Mario; > Anthony Wong; linux-kernel@vger.kernel.org > Subject: Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4 > > > [EXTERNAL EMAIL] > > On Tue, Oct 01, 2019 at 04:09:05PM +0300, Mika Westerberg wrote: > > On Tue, Oct 01, 2019 at 02:47:48PM +0200, Greg Kroah-Hartman wrote: > > > > - Thunderbolt Controller driver. This driver is required if you > > > > - want to hotplug Thunderbolt devices on Apple hardware or on > > > > PCs > > > > - with Intel Falcon Ridge or newer. > > > > + USB4 (Thunderbolt) driver. USB4 is the public spec based on > > > > + Thunderbolt 3 protocol. This driver is required if you want to > > > > + hotplug Thunderbolt and USB4 compliant devices on Apple > > > > + hardware or on PCs with Intel Falcon Ridge or newer. > > > > > > Wait, did "old" thunderbolt just get re-branded as USB4? > > > > Not but the driver started supporting USB4 as well :) > > > > USB4 is pretty much public spec of Thunderbolt 3 but with some > > differences in register layouts (this is because Thunderbolt uses some > > vendor specific capabilities which are now moved to more "standard" > > places). > > Ok, then we need to rename the Kconfig option as well, otherwise no one > will "know" that this changed, so they will not be prompted for it. > > > > Because if I have an "old" laptop that needs Thunderbolt support, how am > > > I going to know it is now called USB4 instead? > > > > Well the Kconfig option tries to have both names there: > > > > tristate "USB4 (Thunderbolt) support" > > > > and then > > > > USB4 (Thunderbolt) driver. USB4 is the public spec based on > > Thunderbolt 3 protocol. This driver is required if you want to hotplug > > Thunderbolt and USB4 compliant devices on Apple hardware or on PCs > > with Intel Falcon Ridge or newer. > > > > and the Kconfig option is still CONFIG_THUNDERBOLT. I know this is > > confusing but I don't have better ideas how we can advertise both. I > > borrowed this "format" from firewire. > > CONFIG_USB4 instead? How about CONFIG_USB4_PCIE? I think that will help align that certain aspects of USB4 can be built optionally. > > > > Shouldn't there just be a new USB4 option that only enables/builds the > > > USB4 stuff if selected? Why would I want all of this additional code on > > > my old system if it's not going to do anything at all? > > > > USB4 devices are backward compatible with Thunderbolt 3 so you should be > > able to plug in USB4 device to your old Thunderbolt 3 laptop for > > example. It goes the other way as well. Some things are optional but for > > example USB4 hubs must support also Thunderbolt 3. > > If PCIe tunnels are an optional feature in USB4, how can it be mandatory to support Thunderbolt 3? > > Does that clarify? > > Yes, it does, looks like marketing just renamed an old functioning > system into a "brand new one!" :) > > thanks, > > greg k-h
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Tue, Oct 01, 2019 at 04:09:05PM +0300, Mika Westerberg wrote: > On Tue, Oct 01, 2019 at 02:47:48PM +0200, Greg Kroah-Hartman wrote: > > > - Thunderbolt Controller driver. This driver is required if you > > > - want to hotplug Thunderbolt devices on Apple hardware or on PCs > > > - with Intel Falcon Ridge or newer. > > > + USB4 (Thunderbolt) driver. USB4 is the public spec based on > > > + Thunderbolt 3 protocol. This driver is required if you want to > > > + hotplug Thunderbolt and USB4 compliant devices on Apple > > > + hardware or on PCs with Intel Falcon Ridge or newer. > > > > Wait, did "old" thunderbolt just get re-branded as USB4? > > Not but the driver started supporting USB4 as well :) > > USB4 is pretty much public spec of Thunderbolt 3 but with some > differences in register layouts (this is because Thunderbolt uses some > vendor specific capabilities which are now moved to more "standard" > places). Ok, then we need to rename the Kconfig option as well, otherwise no one will "know" that this changed, so they will not be prompted for it. > > Because if I have an "old" laptop that needs Thunderbolt support, how am > > I going to know it is now called USB4 instead? > > Well the Kconfig option tries to have both names there: > > tristate "USB4 (Thunderbolt) support" > > and then > > USB4 (Thunderbolt) driver. USB4 is the public spec based on > Thunderbolt 3 protocol. This driver is required if you want to hotplug > Thunderbolt and USB4 compliant devices on Apple hardware or on PCs > with Intel Falcon Ridge or newer. > > and the Kconfig option is still CONFIG_THUNDERBOLT. I know this is > confusing but I don't have better ideas how we can advertise both. I > borrowed this "format" from firewire. CONFIG_USB4 instead? > > Shouldn't there just be a new USB4 option that only enables/builds the > > USB4 stuff if selected? Why would I want all of this additional code on > > my old system if it's not going to do anything at all? > > USB4 devices are backward compatible with Thunderbolt 3 so you should be > able to plug in USB4 device to your old Thunderbolt 3 laptop for > example. It goes the other way as well. Some things are optional but for > example USB4 hubs must support also Thunderbolt 3. > > Does that clarify? Yes, it does, looks like marketing just renamed an old functioning system into a "brand new one!" :) thanks, greg k-h
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Tue, Oct 01, 2019 at 02:47:48PM +0200, Greg Kroah-Hartman wrote: > > - Thunderbolt Controller driver. This driver is required if you > > - want to hotplug Thunderbolt devices on Apple hardware or on PCs > > - with Intel Falcon Ridge or newer. > > + USB4 (Thunderbolt) driver. USB4 is the public spec based on > > + Thunderbolt 3 protocol. This driver is required if you want to > > + hotplug Thunderbolt and USB4 compliant devices on Apple > > + hardware or on PCs with Intel Falcon Ridge or newer. > > Wait, did "old" thunderbolt just get re-branded as USB4? Not but the driver started supporting USB4 as well :) USB4 is pretty much public spec of Thunderbolt 3 but with some differences in register layouts (this is because Thunderbolt uses some vendor specific capabilities which are now moved to more "standard" places). > Because if I have an "old" laptop that needs Thunderbolt support, how am > I going to know it is now called USB4 instead? Well the Kconfig option tries to have both names there: tristate "USB4 (Thunderbolt) support" and then USB4 (Thunderbolt) driver. USB4 is the public spec based on Thunderbolt 3 protocol. This driver is required if you want to hotplug Thunderbolt and USB4 compliant devices on Apple hardware or on PCs with Intel Falcon Ridge or newer. and the Kconfig option is still CONFIG_THUNDERBOLT. I know this is confusing but I don't have better ideas how we can advertise both. I borrowed this "format" from firewire. > Shouldn't there just be a new USB4 option that only enables/builds the > USB4 stuff if selected? Why would I want all of this additional code on > my old system if it's not going to do anything at all? USB4 devices are backward compatible with Thunderbolt 3 so you should be able to plug in USB4 device to your old Thunderbolt 3 laptop for example. It goes the other way as well. Some things are optional but for example USB4 hubs must support also Thunderbolt 3. Does that clarify?
Re: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
On Tue, Oct 01, 2019 at 02:38:25PM +0300, Mika Westerberg wrote: > USB4 is a public spec based on Thunderbolt protocol. There are some > differences in register layouts and flows. In addition to PCIe and DP > tunneling, USB4 supports tunneling of USB 3.x. USB4 is also backward > compatible with Thunderbolt 3 (and older generations but the spec only > talks about 3rd generation). USB4 compliant devices can be identified by > checking USB4 version field in router configuration space. > > This patch adds initial support for USB4 compliant hosts and devices > which enables following features provided by the existing functionality > in the driver: > > - PCIe tunneling > - Display Port tunneling > - Host and device NVM firmware upgrade > - P2P networking > > This brings the USB4 support to the same level that we already have for > Thunderbolt 1, 2 and 3 devices. > > Note the spec talks about host and device "routers" but in the driver we > still use term "switch" in most places. Both can be used interchangeably. > > This also updates the Kconfig entry accordingly. > > Co-developed-by: Rajmohan Mani > Signed-off-by: Rajmohan Mani > Signed-off-by: Mika Westerberg > --- > drivers/thunderbolt/Kconfig | 9 +- > drivers/thunderbolt/Makefile | 2 +- > drivers/thunderbolt/eeprom.c | 53 ++- > drivers/thunderbolt/nhi.c | 3 + > drivers/thunderbolt/nhi.h | 2 + > drivers/thunderbolt/switch.c | 384 +- > drivers/thunderbolt/tb.c | 20 +- > drivers/thunderbolt/tb.h | 36 ++ > drivers/thunderbolt/tb_regs.h | 36 +- > drivers/thunderbolt/tunnel.c | 10 +- > drivers/thunderbolt/usb4.c| 722 ++ > drivers/thunderbolt/xdomain.c | 6 + > 12 files changed, 1162 insertions(+), 121 deletions(-) > create mode 100644 drivers/thunderbolt/usb4.c > > diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig > index fd9adca898ff..8193ec310bae 100644 > --- a/drivers/thunderbolt/Kconfig > +++ b/drivers/thunderbolt/Kconfig > @@ -1,6 +1,6 @@ > # SPDX-License-Identifier: GPL-2.0-only > menuconfig THUNDERBOLT > - tristate "Thunderbolt support" > + tristate "USB4 (Thunderbolt) support" > depends on PCI > depends on X86 || COMPILE_TEST > select APPLE_PROPERTIES if EFI_STUB && X86 > @@ -9,9 +9,10 @@ menuconfig THUNDERBOLT > select CRYPTO_HASH > select NVMEM > help > - Thunderbolt Controller driver. This driver is required if you > - want to hotplug Thunderbolt devices on Apple hardware or on PCs > - with Intel Falcon Ridge or newer. > + USB4 (Thunderbolt) driver. USB4 is the public spec based on > + Thunderbolt 3 protocol. This driver is required if you want to > + hotplug Thunderbolt and USB4 compliant devices on Apple > + hardware or on PCs with Intel Falcon Ridge or newer. Wait, did "old" thunderbolt just get re-branded as USB4? Because if I have an "old" laptop that needs Thunderbolt support, how am I going to know it is now called USB4 instead? Shouldn't there just be a new USB4 option that only enables/builds the USB4 stuff if selected? Why would I want all of this additional code on my old system if it's not going to do anything at all? Or am I confused? thanks, greg k-h
[RFC PATCH 17/22] thunderbolt: Add initial support for USB4
USB4 is a public spec based on Thunderbolt protocol. There are some
differences in register layouts and flows. In addition to PCIe and DP
tunneling, USB4 supports tunneling of USB 3.x. USB4 is also backward
compatible with Thunderbolt 3 (and older generations but the spec only
talks about 3rd generation). USB4 compliant devices can be identified by
checking USB4 version field in router configuration space.
This patch adds initial support for USB4 compliant hosts and devices
which enables following features provided by the existing functionality
in the driver:
- PCIe tunneling
- Display Port tunneling
- Host and device NVM firmware upgrade
- P2P networking
This brings the USB4 support to the same level that we already have for
Thunderbolt 1, 2 and 3 devices.
Note the spec talks about host and device "routers" but in the driver we
still use term "switch" in most places. Both can be used interchangeably.
This also updates the Kconfig entry accordingly.
Co-developed-by: Rajmohan Mani
Signed-off-by: Rajmohan Mani
Signed-off-by: Mika Westerberg
---
drivers/thunderbolt/Kconfig | 9 +-
drivers/thunderbolt/Makefile | 2 +-
drivers/thunderbolt/eeprom.c | 53 ++-
drivers/thunderbolt/nhi.c | 3 +
drivers/thunderbolt/nhi.h | 2 +
drivers/thunderbolt/switch.c | 384 +-
drivers/thunderbolt/tb.c | 20 +-
drivers/thunderbolt/tb.h | 36 ++
drivers/thunderbolt/tb_regs.h | 36 +-
drivers/thunderbolt/tunnel.c | 10 +-
drivers/thunderbolt/usb4.c| 722 ++
drivers/thunderbolt/xdomain.c | 6 +
12 files changed, 1162 insertions(+), 121 deletions(-)
create mode 100644 drivers/thunderbolt/usb4.c
diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig
index fd9adca898ff..8193ec310bae 100644
--- a/drivers/thunderbolt/Kconfig
+++ b/drivers/thunderbolt/Kconfig
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
menuconfig THUNDERBOLT
- tristate "Thunderbolt support"
+ tristate "USB4 (Thunderbolt) support"
depends on PCI
depends on X86 || COMPILE_TEST
select APPLE_PROPERTIES if EFI_STUB && X86
@@ -9,9 +9,10 @@ menuconfig THUNDERBOLT
select CRYPTO_HASH
select NVMEM
help
- Thunderbolt Controller driver. This driver is required if you
- want to hotplug Thunderbolt devices on Apple hardware or on PCs
- with Intel Falcon Ridge or newer.
+ USB4 (Thunderbolt) driver. USB4 is the public spec based on
+ Thunderbolt 3 protocol. This driver is required if you want to
+ hotplug Thunderbolt and USB4 compliant devices on Apple
+ hardware or on PCs with Intel Falcon Ridge or newer.
To compile this driver a module, choose M here. The module will be
called thunderbolt.
diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile
index 001187c577bf..c0b2fd73dfbd 100644
--- a/drivers/thunderbolt/Makefile
+++ b/drivers/thunderbolt/Makefile
@@ -1,4 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-${CONFIG_THUNDERBOLT} := thunderbolt.o
thunderbolt-objs := nhi.o nhi_ops.o ctl.o tb.o switch.o cap.o path.o tunnel.o
eeprom.o
-thunderbolt-objs += domain.o dma_port.o icm.o property.o xdomain.o lc.o
+thunderbolt-objs += domain.o dma_port.o icm.o property.o xdomain.o lc.o usb4.o
diff --git a/drivers/thunderbolt/eeprom.c b/drivers/thunderbolt/eeprom.c
index 540e0105bcc0..921d164b3f35 100644
--- a/drivers/thunderbolt/eeprom.c
+++ b/drivers/thunderbolt/eeprom.c
@@ -487,6 +487,37 @@ static int tb_drom_copy_nvm(struct tb_switch *sw, u16
*size)
return ret;
}
+static int usb4_copy_host_drom(struct tb_switch *sw, u16 *size)
+{
+ int ret;
+
+ ret = usb4_switch_drom_read(sw, 14, size, sizeof(*size));
+ if (ret)
+ return ret;
+
+ /* Size includes CRC8 + UID + CRC32 */
+ *size += 1 + 8 + 4;
+ sw->drom = kzalloc(*size, GFP_KERNEL);
+ if (!sw->drom)
+ return -ENOMEM;
+
+ ret = usb4_switch_drom_read(sw, 0, sw->drom, *size);
+ if (ret) {
+ kfree(sw->drom);
+ sw->drom = NULL;
+ }
+
+ return ret;
+}
+
+static int tb_drom_read_n(struct tb_switch *sw, u16 offset, u8 *val,
+ size_t count)
+{
+ if (tb_switch_is_usb4(sw))
+ return usb4_switch_drom_read(sw, offset, val, count);
+ return tb_eeprom_read_n(sw, offset, val, count);
+}
+
/**
* tb_drom_read - copy drom to sw->drom and parse it
*/
@@ -512,14 +543,26 @@ int tb_drom_read(struct tb_switch *sw)
goto parse;
/*
-* The root switch contains only a dummy drom (header only,
-* no entries). Hardcode the configuration here.
+* USB4 hosts may support reading DROM through router
+* operations.
*/
- tb_drom_read_uid_only(sw, &sw->uid);
+
