ks_dw_pcie_initiate_link_train() question
Hi Murali,
This code looks suspicious. Can you comment?
void ks_dw_pcie_initiate_link_train(struct keystone_pcie *ks_pcie)
{
u32 val;
/* Disable Link training */
val = readl(ks_pcie->va_app_base + CMD_STATUS);
val &= ~LTSSM_EN_VAL;
writel(LTSSM_EN_VAL | val, ks_pcie->va_app_base + CMD_STATUS);Here we cleared the LTSSM_EN_VAL bit in "val", but then we add it right back in before writing it back to CMD_STATUS.
That looks like a cut and paste error to me, but of course I don't know the hardware.
/* Initiate Link Training */
val = readl(ks_pcie->va_app_base + CMD_STATUS);
writel(LTSSM_EN_VAL | val, ks_pcie->va_app_base + CMD_STATUS);
}Quoted 16 lines · Bjorn Helgaas
Hi Murali,
This code looks suspicious. Can you comment?
void ks_dw_pcie_initiate_link_train(struct keystone_pcie *ks_pcie) { u32 val; /* Disable Link training */ val = readl(ks_pcie->va_app_base + CMD_STATUS); val &= ~LTSSM_EN_VAL; writel(LTSSM_EN_VAL | val, ks_pcie->va_app_base + CMD_STATUS);Here we cleared the
LTSSM_EN_VALbit in "val", but then we add it right back in before writing it back toCMD_STATUS.
Quoted 9 lines
That looks like a cut and paste error to me, but of course I don't know the hardware.
/* Initiate Link Training */ val = readl(ks_pcie->va_app_base + CMD_STATUS); writel(LTSSM_EN_VAL | val, ks_pcie->va_app_base + CMD_STATUS); }
Bjorn,
Good catch! That is a cut-n-paste error. Here is the description from device manual
================
Disable link training by de-asserting the LTSSM_EN bit in the PCIESS Command Status Register (CMD_STATUS[LTSSM_EN]=0). Upon reset, the LTSSM_EN is de-asserted automatically by hardware.
Initiate link training can be initiated by asserting LTSSM_EN bit in the CMD_STATUS register (CMD_STATUS[LTSSM_EN]=1).
================================================
Probably it works because it is de-asserted automatically upon reset by hardware. Let me test this and send you a patch?
Signature · 2 lines
Murali Karicheri Linux Kernel, Keystone