[PATCH v2] thunderbolt: fix PCI device class after powering up

[PATCH v2] thunderbolt: fix PCI device class after powering up

[Łukasz Bartosik]

A thunderbolt
lspci -d 8086:9a1b -vmmknn
Slot:	00:0d.2
Class:	System peripheral [0880]
Vendor:	Intel Corporation [8086]
Device:	Tiger Lake-LP Thunderbolt 4 NHI #0 [9a1b]

presents itself with PCI class 0x088000 after Chromebook boots.
lspci -s 00:0d.2 -xxx
00:0d.2 System peripheral: Intel Corporation Tiger Lake-LP Thunderbolt 4
NHI #0 (rev 01)
00: 86 80 1b 9a 00 00 10 00 01 00 80 08 00 00 00 00
...

However after thunderbolt is powered up in nhi_probe()
its class changes to 0x0c0340
lspci -s 00:0d.2 -xxx
00:0d.2 System peripheral: Intel Corporation Tiger Lake-LP Thunderbolt 4
NHI #0 (rev 01)
00: 86 80 1b 9a 06 04 10 00 01 40 03 0c 00 00 00 00
...

which leaves pci_dev structure with old class value
cat /sys/bus/pci/devices/0000:00:0d.2/class
0x088000

This fix updates PCI device class in pci_dev structure after
thunderbolt is powered up.

Fixes: 3cdb9446a117 ("thunderbolt: Add support for Intel Ice Lake")
Signed-off-by: Łukasz Bartosik <lb@semihalf.com>
---
Changelog v1->v2
* Add restoration of PCI revision id
---
 drivers/thunderbolt/nhi_ops.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/thunderbolt/nhi_ops.c b/drivers/thunderbolt/nhi_ops.c
index 96da07e88c52..7aba47e5b3bd 100644
--- a/drivers/thunderbolt/nhi_ops.c
+++ b/drivers/thunderbolt/nhi_ops.c
@@ -160,12 +160,18 @@ static int icl_nhi_suspend_noirq(struct tb_nhi *nhi, bool wakeup)
 
 static int icl_nhi_resume(struct tb_nhi *nhi)
 {
+	u32 class;
 	int ret;
 
 	ret = icl_nhi_force_power(nhi, true);
 	if (ret)
 		return ret;
 
+	/* Set device class & rev as it might have changed after powering up */
+	pci_read_config_dword(nhi->pdev, PCI_CLASS_REVISION, &class);
+	nhi->pdev->class = class >> 8;
+	nhi->pdev->revision = class & 0xff;
+
 	icl_nhi_set_ltr(nhi);
 	return 0;
 }
-- 
2.37.1.455.g008518b4e5-goog

Re: [PATCH v2] thunderbolt: fix PCI device class after powering up

[Mika Westerberg]

Hi,

On Fri, Jul 29, 2022 at 11:40:22AM +0200, Łukasz Bartosik wrote:
> A thunderbolt
> lspci -d 8086:9a1b -vmmknn
> Slot:	00:0d.2
> Class:	System peripheral [0880]
> Vendor:	Intel Corporation [8086]
> Device:	Tiger Lake-LP Thunderbolt 4 NHI #0 [9a1b]
> 
> presents itself with PCI class 0x088000 after Chromebook boots.
> lspci -s 00:0d.2 -xxx
> 00:0d.2 System peripheral: Intel Corporation Tiger Lake-LP Thunderbolt 4
> NHI #0 (rev 01)
> 00: 86 80 1b 9a 00 00 10 00 01 00 80 08 00 00 00 00
> ...
> 
> However after thunderbolt is powered up in nhi_probe()
> its class changes to 0x0c0340
> lspci -s 00:0d.2 -xxx
> 00:0d.2 System peripheral: Intel Corporation Tiger Lake-LP Thunderbolt 4
> NHI #0 (rev 01)
> 00: 86 80 1b 9a 06 04 10 00 01 40 03 0c 00 00 00 00
> ...
> 
> which leaves pci_dev structure with old class value
> cat /sys/bus/pci/devices/0000:00:0d.2/class
> 0x088000

This is completely unexpected. Which Chromebook this is and have you
tried to upgrade it to the latest?

> This fix updates PCI device class in pci_dev structure after
> thunderbolt is powered up.

I think we need to understand why this happens in the first place before
doing anything else.

Re: [PATCH v2] thunderbolt: fix PCI device class after powering up

[Łukasz Bartosik]

>
> Hi,
>
> On Fri, Jul 29, 2022 at 11:40:22AM +0200, Łukasz Bartosik wrote:
> > A thunderbolt
> > lspci -d 8086:9a1b -vmmknn
> > Slot: 00:0d.2
> > Class:        System peripheral [0880]
> > Vendor:       Intel Corporation [8086]
> > Device:       Tiger Lake-LP Thunderbolt 4 NHI #0 [9a1b]
> >
> > presents itself with PCI class 0x088000 after Chromebook boots.
> > lspci -s 00:0d.2 -xxx
> > 00:0d.2 System peripheral: Intel Corporation Tiger Lake-LP Thunderbolt 4
> > NHI #0 (rev 01)
> > 00: 86 80 1b 9a 00 00 10 00 01 00 80 08 00 00 00 00
> > ...
> >
> > However after thunderbolt is powered up in nhi_probe()
> > its class changes to 0x0c0340
> > lspci -s 00:0d.2 -xxx
> > 00:0d.2 System peripheral: Intel Corporation Tiger Lake-LP Thunderbolt 4
> > NHI #0 (rev 01)
> > 00: 86 80 1b 9a 06 04 10 00 01 40 03 0c 00 00 00 00
> > ...
> >
> > which leaves pci_dev structure with old class value
> > cat /sys/bus/pci/devices/0000:00:0d.2/class
> > 0x088000
>
> This is completely unexpected. Which Chromebook this is and have you
> tried to upgrade it to the latest?
>

This happens on a Tiger Lake based reference Chromebook platform.
The issue also happens on the latest ChromeOS image available for that platform.

> > This fix updates PCI device class in pci_dev structure after
> > thunderbolt is powered up.
>
> I think we need to understand why this happens in the first place before
> doing anything else.

If you have suggestions what else to check apart from what I already
did then please
let me know I will gladly do it.

Thanks,
Lukasz

Re: [PATCH v2] thunderbolt: fix PCI device class after powering up

[Mika Westerberg]

On Tue, Aug 02, 2022 at 02:27:30PM +0200, Łukasz Bartosik wrote:
> >
> > Hi,
> >
> > On Fri, Jul 29, 2022 at 11:40:22AM +0200, Łukasz Bartosik wrote:
> > > A thunderbolt
> > > lspci -d 8086:9a1b -vmmknn
> > > Slot: 00:0d.2
> > > Class:        System peripheral [0880]
> > > Vendor:       Intel Corporation [8086]
> > > Device:       Tiger Lake-LP Thunderbolt 4 NHI #0 [9a1b]
> > >
> > > presents itself with PCI class 0x088000 after Chromebook boots.
> > > lspci -s 00:0d.2 -xxx
> > > 00:0d.2 System peripheral: Intel Corporation Tiger Lake-LP Thunderbolt 4
> > > NHI #0 (rev 01)
> > > 00: 86 80 1b 9a 00 00 10 00 01 00 80 08 00 00 00 00
> > > ...
> > >
> > > However after thunderbolt is powered up in nhi_probe()
> > > its class changes to 0x0c0340
> > > lspci -s 00:0d.2 -xxx
> > > 00:0d.2 System peripheral: Intel Corporation Tiger Lake-LP Thunderbolt 4
> > > NHI #0 (rev 01)
> > > 00: 86 80 1b 9a 06 04 10 00 01 40 03 0c 00 00 00 00
> > > ...
> > >
> > > which leaves pci_dev structure with old class value
> > > cat /sys/bus/pci/devices/0000:00:0d.2/class
> > > 0x088000
> >
> > This is completely unexpected. Which Chromebook this is and have you
> > tried to upgrade it to the latest?
> >
> 
> This happens on a Tiger Lake based reference Chromebook platform.
> The issue also happens on the latest ChromeOS image available for that platform.

Is this something available for purchase? I'm asking because I have Acer
Tiger Lake based Chromebook (740 spin or something) here and the TBT
controller class is "USB controller" all the time, and this is what is
expected. It should not change the class at any point.

Re: [PATCH v2] thunderbolt: fix PCI device class after powering up

[Łukasz Bartosik]

>
> On Tue, Aug 02, 2022 at 02:27:30PM +0200, Łukasz Bartosik wrote:
> > >
> > > Hi,
> > >
> > > On Fri, Jul 29, 2022 at 11:40:22AM +0200, Łukasz Bartosik wrote:
> > > > A thunderbolt
> > > > lspci -d 8086:9a1b -vmmknn
> > > > Slot: 00:0d.2
> > > > Class:        System peripheral [0880]
> > > > Vendor:       Intel Corporation [8086]
> > > > Device:       Tiger Lake-LP Thunderbolt 4 NHI #0 [9a1b]
> > > >
> > > > presents itself with PCI class 0x088000 after Chromebook boots.
> > > > lspci -s 00:0d.2 -xxx
> > > > 00:0d.2 System peripheral: Intel Corporation Tiger Lake-LP Thunderbolt 4
> > > > NHI #0 (rev 01)
> > > > 00: 86 80 1b 9a 00 00 10 00 01 00 80 08 00 00 00 00
> > > > ...
> > > >
> > > > However after thunderbolt is powered up in nhi_probe()
> > > > its class changes to 0x0c0340
> > > > lspci -s 00:0d.2 -xxx
> > > > 00:0d.2 System peripheral: Intel Corporation Tiger Lake-LP Thunderbolt 4
> > > > NHI #0 (rev 01)
> > > > 00: 86 80 1b 9a 06 04 10 00 01 40 03 0c 00 00 00 00
> > > > ...
> > > >
> > > > which leaves pci_dev structure with old class value
> > > > cat /sys/bus/pci/devices/0000:00:0d.2/class
> > > > 0x088000
> > >
> > > This is completely unexpected. Which Chromebook this is and have you
> > > tried to upgrade it to the latest?
> > >
> >
> > This happens on a Tiger Lake based reference Chromebook platform.
> > The issue also happens on the latest ChromeOS image available for that platform.
>
> Is this something available for purchase? I'm asking because I have Acer
> Tiger Lake based Chromebook (740 spin or something) here and the TBT
> controller class is "USB controller" all the time, and this is what is
> expected. It should not change the class at any point.

Sorry this platform is not available on the market.

I compared the platform where I see the issue with another platform
where thunderbolt is "usb controller" all the time
and I noticed one difference in function icl_nhi_force_power() in
drivers/thunderbolt/nhi_ops.c I observed the value of VS_CAP_22
after being read and before being written again with additional bits
set. And on the platform where thunderbolt is "usb controller" all the
time
this value was 0x22061002 after reading and 0x22061002 before being
written. The value has not changed
which suggest that thunderbolt was already powered up during probe.

On the platform where I observe the issue where thunderbolt class
changes the observed
read value was 0x6061000 and then 0x22061002 before being written.
That's the difference I spotted.

Anyway I can provide logs or run experiments to shed more light on the
issue. Just need
to know what might be useful ?

Thanks,
Lukasz

Re: [PATCH v2] thunderbolt: fix PCI device class after powering up

[Mika Westerberg]

Hi,

On Tue, Aug 02, 2022 at 05:06:40PM +0200, Łukasz Bartosik wrote:
> > Is this something available for purchase? I'm asking because I have Acer
> > Tiger Lake based Chromebook (740 spin or something) here and the TBT
> > controller class is "USB controller" all the time, and this is what is
> > expected. It should not change the class at any point.
> 
> Sorry this platform is not available on the market.

I don't think the mainline Linux needs to have this kind of a quirk for
a device that is not available for general public.

> I compared the platform where I see the issue with another platform
> where thunderbolt is "usb controller" all the time
> and I noticed one difference in function icl_nhi_force_power() in
> drivers/thunderbolt/nhi_ops.c I observed the value of VS_CAP_22
> after being read and before being written again with additional bits
> set. And on the platform where thunderbolt is "usb controller" all the
> time
> this value was 0x22061002 after reading and 0x22061002 before being
> written. The value has not changed
> which suggest that thunderbolt was already powered up during probe.

It is being set also if you boot with device connected but in any case
the class code should not change ever. It may be that this is some older
spin of the Tiger Lake silicon that still had the wrong class but it got
fixed in later spins (or firmware, I don't remember which).

Re: [PATCH v2] thunderbolt: fix PCI device class after powering up

[Łukasz Bartosik]

>
> Hi,
>
> On Tue, Aug 02, 2022 at 05:06:40PM +0200, Łukasz Bartosik wrote:
> > > Is this something available for purchase? I'm asking because I have Acer
> > > Tiger Lake based Chromebook (740 spin or something) here and the TBT
> > > controller class is "USB controller" all the time, and this is what is
> > > expected. It should not change the class at any point.
> >
> > Sorry this platform is not available on the market.
>
> I don't think the mainline Linux needs to have this kind of a quirk for
> a device that is not available for general public.
>

The reference Chromebook platform is not available on the market now
however there will be Chromebooks based on that platform available for
purchase in the future.
We'd prefer not to carry a private patch for this issue.

Thanks,
Lukasz

> > I compared the platform where I see the issue with another platform
> > where thunderbolt is "usb controller" all the time
> > and I noticed one difference in function icl_nhi_force_power() in
> > drivers/thunderbolt/nhi_ops.c I observed the value of VS_CAP_22
> > after being read and before being written again with additional bits
> > set. And on the platform where thunderbolt is "usb controller" all the
> > time
> > this value was 0x22061002 after reading and 0x22061002 before being
> > written. The value has not changed
> > which suggest that thunderbolt was already powered up during probe.
>
> It is being set also if you boot with device connected but in any case
> the class code should not change ever. It may be that this is some older
> spin of the Tiger Lake silicon that still had the wrong class but it got
> fixed in later spins (or firmware, I don't remember which).

Re: [PATCH v2] thunderbolt: fix PCI device class after powering up

[Mika Westerberg]

On Wed, Aug 03, 2022 at 11:30:09AM +0200, Łukasz Bartosik wrote:
> >
> > Hi,
> >
> > On Tue, Aug 02, 2022 at 05:06:40PM +0200, Łukasz Bartosik wrote:
> > > > Is this something available for purchase? I'm asking because I have Acer
> > > > Tiger Lake based Chromebook (740 spin or something) here and the TBT
> > > > controller class is "USB controller" all the time, and this is what is
> > > > expected. It should not change the class at any point.
> > >
> > > Sorry this platform is not available on the market.
> >
> > I don't think the mainline Linux needs to have this kind of a quirk for
> > a device that is not available for general public.
> >
> 
> The reference Chromebook platform is not available on the market now
> however there will be Chromebooks based on that platform available for
> purchase in the future.

Right, and do you know if those have this issue? Like I said my
Chromebook, that is also based on the reference Chromebook, does not
have this problem so probably something firmware/hardware related that
does not appear when the production systems are ready. And this really
should not happen at all. I also suggest to contact the Intel TBT folks
(assuming your company is making these Chromebooks to make sure you have
the latest silicon/firmwares).

Re: [PATCH v2] thunderbolt: fix PCI device class after powering up

[Łukasz Bartosik]

>
> On Wed, Aug 03, 2022 at 11:30:09AM +0200, Łukasz Bartosik wrote:
> > >
> > > Hi,
> > >
> > > On Tue, Aug 02, 2022 at 05:06:40PM +0200, Łukasz Bartosik wrote:
> > > > > Is this something available for purchase? I'm asking because I have Acer
> > > > > Tiger Lake based Chromebook (740 spin or something) here and the TBT
> > > > > controller class is "USB controller" all the time, and this is what is
> > > > > expected. It should not change the class at any point.
> > > >
> > > > Sorry this platform is not available on the market.
> > >
> > > I don't think the mainline Linux needs to have this kind of a quirk for
> > > a device that is not available for general public.
> > >
> >
> > The reference Chromebook platform is not available on the market now
> > however there will be Chromebooks based on that platform available for
> > purchase in the future.
>
> Right, and do you know if those have this issue? Like I said my
> Chromebook, that is also based on the reference Chromebook, does not
> have this problem so probably something firmware/hardware related that
> does not appear when the production systems are ready. And this really
> should not happen at all. I also suggest to contact the Intel TBT folks
> (assuming your company is making these Chromebooks to make sure you have
> the latest silicon/firmwares).

Ok. Thanks for the hint.

Regards,
Lukasz