Re: [PATCH] PCI: Marvell: Update PCIe fixup

[Pali Rohár]

On Tuesday 02 November 2021 12:35:41 Maciej W. Rozycki wrote:
> On Tue, 2 Nov 2021, Pali Rohár wrote:
> 
> > > > >From all what I saw, I'm sure that this device with this specific
> > > > characteristics is really (non-compliant) Marvell PCIe controller.
> > > 
> > > just nitpicking, it's a Galileo PCI bridge and not PCIe.
> > 
> > Marvell acquired Galileo Technology in the past, so it is possible that
> > this bad design is originated in Galileo. And maybe same for PCIe from
> > PCI. At least PCI vendor id for all (new) PCIe controllers is this one.
> 
>  Umm, PCIe is so different hardware-wise from PCI

Yes hardware is different. But software is same and fully backward
compatible. And base part of config space is same for both PCI an PCIe.
So it is possible to copy+paste HDL code which fills initial values into
config space from old PCI IPs to new PCIe IPs.

> I doubt there's any 
> chance for errata to be carried across.  Plus the MIPS SysAD bus is so 
> different from other CPU buses.  And we're talking 20+ years old Galileo 
> devices here.
> 
>  None of the Galileo system controllers I came across had the class code 
> set incorrectly.

In kernel there is quirk only for one device with id:
PCI_VENDOR_ID_MARVELL (0x11ab) PCI_DEVICE_ID_MARVELL_GT64111 (0x4146)

So for some reasons quirk is needed... Anyway, patch for this quirk just
adds comment as there is no explanation for it. It does not modify quirk
code.

So it is possible that Marvell (or rather Galileo at that time) included
some config space fixup in some products and 0x4146 did not have it.
Just guessing... We can really only guess what could happen at that time
20 years ago...

Running git blame told me that this class code quirk was introduce in:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c4ed38a0c6e2e5c4906296758f816ee71373792f

But there is also no information...

> > > > But I do not have this hardware to verify it.
> > > 
> > > I still have a few Cobalt systems here.
> > 
> > Perfect! It would help if you could provide 'lspci -nn -vv' output from
> > that system. In case you have very old version of lspci on that system
> > you could try to run it with '-xxxx' (or '-xxx') which prints hexdump
> > and I can parse it with local lspci.
> 
>  For the record here's one from a core card used with a Malta (as with 
> arch/mips/pci/fixup-malta.c); it has a newer 64120A chip (marked as an 
> engineering sample BTW):
> 
> 00:00.0 Host bridge: Marvell Technology Group Ltd. GT-64120/64120A/64121A System Controller (rev 11)
> 00: ab 11 20 46 47 01 80 22 11 00 00 06 00 20 00 00
> 10: 00 00 00 00 00 00 00 08 00 00 00 00 00 00 00 00
> 20: 00 00 e0 1b 00 00 00 00 00 00 00 00 00 00 00 00
> 30: 00 00 00 00 00 00 00 00 00 00 00 00 ff 01 00 00
> 40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 
> The lack of a quirk with a platform does not mean it cannot have a certain 
> PCI/e device.

This is 11ab:4620 device an there is no PCIe capability in its config
space (you can inspect it via 'lspci -F dump.txt -nn -vv' but there is
nothing interesting).

>  As I recall various Atlas/Malta core cards had any of the three device 
> variants covered by this vendor:device ID and later batches were actually 
> indeed marked Marvell rather than Galileo.
> 
>   Maciej