On Thu, Jul 28, 2022 at 06:31:47PM +0200, Łukasz Bartosik wrote:
> >
> > On Thu, Jul 28, 2022 at 03:16:08PM +0200, Lukasz Bartosik wrote:
> > > From: Łukasz Bartosik <lb@xxxxxxxxxxxx>
> > >
> > > 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@xxxxxxxxxxxx>
> > > ---
> > > drivers/thunderbolt/nhi_ops.c | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/thunderbolt/nhi_ops.c b/drivers/thunderbolt/nhi_ops.c
> > > index 96da07e88c52..6a343d7e3f90 100644
> > > --- a/drivers/thunderbolt/nhi_ops.c
> > > +++ b/drivers/thunderbolt/nhi_ops.c
> > > @@ -160,12 +160,17 @@ 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 code as it might have changed after powering up */
> > > + pci_read_config_dword(nhi->pdev, PCI_CLASS_REVISION, &class);
> > > + nhi->pdev->class = class >> 8;
> >
> > What about the revision field, why not set that as well:
> > nhi->pdev->revision = class & 0xff;
> >
> > If the value is overwritten for 3 of the bytes, why not the 4th?
>
> Fair point but I observed class change, revision stayed the same.
> I read class and revision before and after icl_nhi_force_power() with
> pci_read_config_dword(nhi->pdev, PCI_CLASS_REVISION, &class);
> It changed from 0x8800001 -> 0xc034001
>
> > Also this feels odd, what is changing the bytes here? Why only the
> > class? What else changed and what caused it?
>
> I compared 64 bytes of config space before and after modprobing
> thunderbolt module
> Before modprobe
> lspci -s 00:0d.2 -x
> 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
> 10: 04 00 a0 80 02 00 00 00 04 80 a4 80 02 00 00 00
> 20: 00 00 00 00 00 00 00 00 00 00 00 00 22 22 11 11
> 30: 00 00 00 00 80 00 00 00 00 00 00 00 ff 01 00 00
>
> After modprobe
> lspci -s 00:0d.2 -x
> 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
> 10: 04 00 a0 80 02 00 00 00 04 80 a4 80 02 00 00 00
> 20: 00 00 00 00 00 00 00 00 00 00 00 00 22 22 11 11
> 30: 00 00 00 00 80 00 00 00 00 00 00 00 ff 01 00 00
>
> The diff is in class 00 80 08 -> 40 03 0c
> and command 00 00 -> 06 04
>
> The value 40 03 0c is defined as PCI_CLASS_SERIAL_USB_USB4 in
> drivers/thunderbolt/nhi.h
>
> I think the device itself changed the class because I tried to change
> class value with setpci command and it seems to be read-only.
Wait huh? You can't change the class of a device in the configuration,
that is read-only.
So this is working properly without this patch, right?
thanks,
greg k-h
