AM6442: The common clock for PCIe cannot be enabled. (continued)

Hi Liu-san,

Thank you for your reply.

It appears the lspci command gets the slotclk information from the PCI config space. However, I looked the kernel PCIe driver, it doesn't seem the AM64x PCI refclk type gets populated to the PCI config space.

Is this a current specification limitation of the Linux provided by TI?

Hi Masanobu-san,

I searched all the PCI controller drivers in Linux kernel, only two drivers manipulate this "SlotClk" bit in the PCI Config Space, the majority of the PCI controller drivers do not touch this bit at all, so I don't have further comments about the SlotClk information in the lspci command report.

But my question is why you need to check this information using lspci command? Is that you already know the PCI common clock configuration once the system hw/sw design is finished and it doesn't change?

Hello Bin,

Bin Liu said:

But my question is why you need to check this information using lspci command? Is that you already know the PCI common clock configuration once the system hw/sw design is finished and it doesn't change?

I suppose it makes sense to wait for Masanobu-san's response, but generally speaking a host could (and from my experience does) behave differently based on this bit. The PCIe spec says that system firmware is supposed this bit to determine how to set the common clock configuration (bit 6 in link control).

I've seen communication errors in a setup with an Intel CPU on a MSI mainboard and an AM64x as an EP depending on the value reported in the Slot Clock Configuration bit. IIRC this was related to spread spectrum clocking. To complicate matters these communication errors might not be immediately visible since they're normally correctable (but of course affect throughput and latency).

The PCIe spec also mentions that a electromechanical form factor specification might require use of a common clock. The PCIe express card electromechanical specification (i.e. what you'd normally consider a PCIe card to plug into a PCIe slot) does have that requirement.

Regards,

Dominic

Hi Liu-san, Rath-san,

Thank you for your replay.

Liu-san said: 

But my question is why you need to check this information using lspci command? Is that you already know the PCI common clock configuration once the system hw/sw design is finished and it doesn't change?

I want to check that the PCIe clock settings in the device tree are being applied, but is there no other way than to directly read the PCIe registers or physically measure them?
Also, as Rath-san says, does the information displayed in the lspci results affect the operation of other kernel modules and applications? I think that the interface that lspci uses to obtain information returns results that do not use the common clock, but is that okay?

Hi Masanobu-san,

masanobu.tsuchiya said:

I want to check that the PCIe clock settings in the device tree are being applied, but is there no other way than to directly read the PCIe registers or physically measure them?

If I really wanted to know the clock setting, I would either read the register, or check the devicetree setting, or physically measure them.

However Dominic made a good point - if the PCIe RC requires the EP clock setting to act accordingly. I don't know the answer, but I will check with our development team, and get back to you.

Hi Masanobu-san,

The follow kernel patch sets the Slot Clock Configuration in the Root Complex Config space. With this patch lspci command should show "SlotClk+".

diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
index 3ab5ffc76167..08585fa488c2 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
@@ -191,6 +191,7 @@ static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc)
        struct cdns_pcie *pcie = &rc->pcie;
        u32 value, ctrl;
        u32 id;
+       u16 lnk_stat;
 
        /*
         * Set the root complex BAR configuration register:
@@ -223,6 +224,12 @@ static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc)
        cdns_pcie_rp_writeb(pcie, PCI_CLASS_PROG, 0);
        cdns_pcie_rp_writew(pcie, PCI_CLASS_DEVICE, PCI_CLASS_BRIDGE_PCI);
 
+       /* set Slot Clock Configuration */
+       lnk_stat = cdns_pcie_rp_readw(pcie, CDNS_PCIE_RP_CAP_OFFSET + PCI_EXP_LNKSTA);
+       lnk_stat |= PCI_EXP_LNKSTA_SLC;
+       cdns_pcie_rp_writew(pcie, CDNS_PCIE_RP_CAP_OFFSET + PCI_EXP_LNKSTA, lnk_stat);
+
+
        return 0;
 }

Hi Liu-san

Thank you for your reply.

I will give it a try this patch.

masanobu.tsuchiya said:

Also, as Rath-san says, does the information displayed in the lspci results affect the operation of other kernel modules and applications? I think that the interface that lspci uses to obtain information returns results that do not use the common clock, but is that okay?

What I'm saying is that as per the PCIe specification an RC might use that information and adjust its behavior. On an x86 that typically happens in the UEFI BIOS already. Not sure if the Linux kernel would do anything differently depending on that bit.

My point is: The specification describes this bit, I know from BIOS source code that they're evaluating this bit, and not setting it correctly is therefore asking for trouble.

Regarding your intention:

masanobu.tsuchiya said:

I want to check that the PCIe clock settings in the device tree are being applied

As I understand it Bin's patch unconditionally sets this bit. That means you still can't use that bit to check whether your device tree configured the common reference clock or not.

I'd look for a difference in the SERDES_RST register bit [24]. That register should be at offset 0x40c (https://github.com/TexasInstruments/linux-kernel/blob/ti-mainline/drivers/phy/ti/phy-j721e-wiz.c#L38), the SerDes is at 0x0F000000, so address 0x0f00040c. Unfortunately the SerDes registers are not documented for the AM64x. Maybe Bin could help out with that.

Regards,

Dominic

Hi Dominic,

I would have to dig into the SerDes internal documentation for the register details, however the problem here is that the solution is across two "irrelevant" kernel drivers - the refclk config is in the SerDes driver, but the PCI Config Space is modified in the PCIe driver. These two drivers do not directly talk to each other. Of cause there are solutions, for example, to define a link in devicetree through which the PCIe driver can query the SerDes registers, but since I didn't find any other kernel PCI controller driver modify the PCI Config Space for "SlotClk" information, I figured to just use the solution I posted above, since we already know the SerDes devicetree is already changed to use external refclk, and it won't be changed at Linux runtime.