[PATCH v3 1/3] starfive: pci: Add StarFive JH7110 pcie driver
Pali Rohár
pali at kernel.org
Fri Mar 31 14:59:12 CEST 2023
On Friday 31 March 2023 18:35:00 Minda Chen wrote:
> On 2023/3/30 1:27, Pali Rohár wrote:
> > Hello!
> >
> > On Wednesday 29 March 2023 18:01:41 Minda Chen wrote:
> >> + /* PCIe PCI Standard Configuration Identification Settings. */
> >> + value = (PCI_CLASS_BRIDGE_PCI_NORMAL << IDS_CLASS_CODE_SHIFT) | IDS_REVISION_ID;
> >> + writel(value, priv->reg_base + PCIE_PCI_IDS);
> >
> > This looks like configuration of the PCI_CLASS_REVISION read-only
> > register. Is there any reason why you are removing the original
> > "revision" information by hardcoded IDS_REVISION_ID constant?
> >
> This register is not read-only register, consist resion ID and class code.
> Bit [39:32]: Revision ID
> Bit [63:40]: Class code
I mean that this "priv->reg_base + PCIE_PCI_IDS" read-write register
configures what is visible when reading read-only standard PCI register
PCI_CLASS_REVISION.
PCIe Root Port is by definition of PCI-to-PCI Bridge class and therefore
reading from PCI_CLASS_REVISION must return PCI_CLASS_BRIDGE_PCI_* value.
This is required by PCIe spec.
With above driver init code it looks like that your PCIe controller does
not set correct value into PCI_CLASS_REVISION register after power-on
and manually setting correct value via "priv->reg_base + PCIE_PCI_IDS"
is a workaround.
Hopefully it is a more clear what I mean in my previous email.
Also mvebu controller has this issue and has similar workaround in
pci_mvebu.c driver.
> And the register reset value is zero, Our PCIe version is 2.0. So set value 2.
> Maybe I will add comment to this.
Well, Revision ID (low 8 bits of PCI_CLASS_REVISION) is the extension to
the vendor / device id register. It is not version of PCIe standard.
I would expect that this Vendor ID, Device ID and Revision ID values are
not being changed by the driver as they identify PCI and PCIe devices.
More information about the U-Boot
mailing list