Messages in this thread | | | Subject | Re: [PATCH] PCI: Marvell: Update PCIe fixup | From | Stefan Roese <> | Date | Tue, 2 Nov 2021 18:03:18 +0100 |
| |
On 02.11.21 16:48, Pali Rohár wrote: > On Tuesday 02 November 2021 15:49:29 Pali Rohár wrote: >> On Tuesday 02 November 2021 14:01:41 Maciej W. Rozycki wrote: >>> On Tue, 2 Nov 2021, Pali Rohár wrote: >>> >>>>> 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... >>> >>> Ah, there you go! -- sadly I don't seem to have a copy of the datasheet >>> for the GT-64111, but the GT-64115 has it[1]: >>> >>> Table 158: PCI Class Code and Revision ID, Offset: 0x008 >>> Bits Field name Function Initial Value >>> 7:0 RevID Indicates the GT-64115 PCI Revision 0x01 >>> number. >>> 15:8 Reserved Read only. 0x0 >>> 23:16 SubClass Indicates the GT-64115 Subclass - Mem- 0x80 >>> ory controller. >>> 31:24 BaseClass Indicates the GT-64115 Base Class - 0x05 >>> memory controller. >>> >>> and then: >>> >>> "Device and Vendor ID (0x000), Class Code and Revision ID (0x008), and >>> Header Type (0x00e) fields are read only from the PCI bus. These fields >>> can be modified and read via the CPU bus." >>> >>> Likewise with the GT-64120[2]: >>> >>> Table 208: PCI_0 Class Code and Revision ID, Offset: 0x008 from PCI_0 or CPU; 0x088 from >>> PCI_1 >>> Bits Field name Function Initial Value >>> 7:0 RevID Indicates the GT-64120 PCI_0 revision number. 0x02 >>> 15:8 Reserved Read Only 0. 0x0 >>> 23:16 SubClass Indicates the GT-64120 Subclass Depends on value >>> 0x00 - Host Bridge Device. sampled at reset >>> 0x80 - Memory Device. on BankSel[0]. See >>> Table 44 on page >>> 11-1. >>> 31:24 BaseClass Indicates the GT-64120 Base Class Depends on value >>> 0x06 - Bridge Device. sampled at reset >>> 0x05 - Memory Device. on BankSel[0]. See >>> Table 44 on page >>> 11-1. >>> >>> Table 209: PCI_1 Class Code and Revision ID, Offset: 0x088 from PCI_0 or CPU; 0x008 from >>> PCI_1 >>> Bits Field name Function Initial Value >>> 31:0 Various Same as for PCI_0 Class Code and Revision ID. >>> >>> and then also: >>> >>> "Device and Vendor ID (0x000), Class Code and Revision ID (0x008), and >>> Header Type (0x00e) fields are read only from the PCI bus. These fields >>> can be modified and read via the CPU bus." >>> >>> -- so this is system-specific and an intended chip feature rather than an >>> erratum (or rather it is a system erratum if the reset strap or the boot >>> firmware has got it wrong). >>> >>> The memory device class code is IIUC meant to be typically chosen when >>> the Galileo/Marvell device is used without the CPU interface, i.e. as a >>> PCI memory controller device only[3]. > > I have found on internet some copy of GT64111 datasheet ("GT-64111 > System Controller for RC4640, RM523X and VR4300 CPUs", Galileo > Technology, Product Preview Revision 1.1, FEB 4, 1999) and in section > "17.15 PCI Configuration Registers" there is subsection "Class Code and > Revision ID, Offset: 0x008" with content: > > Bits Field name Function Initial Value > 7:0 RevID Indicates the GT-64111 Revision number. 0x10 > GT-64111-P-0 = 0x10 > 15:8 Reserved 0x0 > 23:16 SubClass Indicates the GT-64111 Subclass (0x80 - other mem- 0x80 > ory controller) > 31:24 BaseClass Indicates the GT-64111 Base Class (0x5 - memory 0x05 > controller). > > And in section "6.5.3 PCI Autoconfiguration at RESET" is following > interesting information: > > Eight PCI registers can be automatically loaded after Rst*. > Autoconfiguration mode is enabled by asserting the DMAReq[3]* LOW on > Rst*. Any PCI transactions targeted for the GT-64111 will be retried > while the loading of the PCI configuration registers is in process. > > It is highly recommended that all PC applications utilize the PCI > Autoconfiguration at RESET feature. The autoload feature can be easily > implemented with a very low cost EPLD. Galileo provides sample EPLD > equations upon request. (You can always pull the EPLD off your final > product if you find there are no issues during testing.) > > NOTE: The GT-64111’s default Class Code is 0x0580 (Memory Controller) > which is a change from the GT-64011. > > The GT-64011 used the Class Code 0x0600 which denotes Host Bridge. Some > PCs refuse to configure host bridges if they are found plugged into a > PCI slot (ask the BIOS vendors why...). The “Memory Controller” Class > Code does not cause a problem for these non-compliant BIOSes, so we used > this as the default in the GT-64111. The Class Code can be reporgrammed > in both devices via autoload or CPU register writes. > > The PCI register values are loaded from the ROM controlled by BootCS* > are shown in Table 21, below. > > TABLE 21. PCI Registers Loaded at RESET > Register Offset Boot Device Address > Device and Vendor ID 0x000 0x1fffffe0 > Class Code and Revision ID 0x008 0x1fffffe4 > Subsystem Device and Vendor ID 0x02c 0x1fffffe8 > Interrupt Pin and Line 0x03c 0x1fffffec > RAS[1:0]* Bank Size 0xc08 0x1ffffff0 > RAS[3:2]* Bank Size 0xc0c 0x1ffffff4 > CS[2:0]* Bank Size 0xc10 0x1ffffff8 > CS[3]* & Boot CS* Bank Size 0xc14 0x1ffffffc > > === > > So the conclusion is that there is also some RESET configuration via > BootCS (I have no idea what it is or was). And default value (when RESET > configuration is not used) is always "Memory controller" due to > existence of "broken PC BIOSes" (probably x86). > > Hence the quirk for GT64111 in kernel is always needed. And Thomas > already confirmed in his pci hexdump that PCI Class code is set to > Memory Controller. > > I hope that now this mystery of this GT64111 quirk is solved :-) I will > update patch with correct comment, why quirk is needed. > > So due to the fact that 20 years ago there were broken x86 BIOSes which > did not like PCI devices with PCI Class code of Host Bridge, Marvell > changed default PCI Class code to Memory Controller and let it in this > state also for future PCIe-based ARM and AR64 SoCs for next 20 years. > Which generally leaded to broken PCIe support in mvebu SoCs. I have no > more comments about it... :-(
If this is really the case, that all this was "copied" in such a bad design state into newer SoC's for that many years, which I don't doubt right now any more, then this is absolutely amazing and pretty sad IMHO.
Pali, many thanks for being persistant enough to dig through this.
Thanks, Stefan
| |