>
> On Thu, Jul 28, 2022 at 07:17:37PM +0200, Łukasz Bartosik wrote:
> > >
> > > 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.
> >
> > Sorry my statement might have been confusing. I tried to change class
> > value with setpci
> > as an experiment to make sure it is read-only and it is ro.
> >
> > > So this is working properly without this patch, right?
> >
> > After thunderbolt is probed its class changes from 00 80 08 -> 40 03 0c
> > and without this patch thunderbolt's pci_dev struct is left holding
> > old class value 00 80 08
> > which is not correct.
>
> Ah, ok, but you might have just gotten lucky about the revision being
> the same. Please also restore that as it comes from the same read
> transaction.
>
I will send v2 which restores revision also.
Thanks,
Lukasz
> thanks,
>
> greg k-h
