| Message ID | 20211001195856.10081-13-kabel@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: aardvark controller fixes | expand |
On Fri, Oct 01, 2021 at 09:58:55PM +0200, Marek Behún wrote: > From: Pali Rohár <pali@kernel.org> > > Current implementation of advk_pcie_link_up() is wrong as it marks also > link disabled or hot reset states as link up. > > Fix it by marking link up only to those states which are defined in PCIe > Base specification 3.0, Table 4-14: Link Status Mapped to the LTSSM. > > To simplify implementation, Define macros for every LTSSM state which > aardvark hardware can return in CFG_REG register. > > Fix also checking for link training according to the same Table 4-14. > Define a new function advk_pcie_link_training() for this purpose. > > Fixes: 8c39d710363c ("PCI: aardvark: Add Aardvark PCI host controller driver") > Signed-off-by: Pali Rohár <pali@kernel.org> > Reviewed-by: Marek Behún <kabel@kernel.org> > Signed-off-by: Marek Behún <kabel@kernel.org> > Cc: Remi Pommarel <repk@triplefau.lt> > Cc: stable@vger.kernel.org > --- > drivers/pci/controller/pci-aardvark.c | 69 +++++++++++++++++++++++++-- > 1 file changed, 64 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c > index 5387d9cc3eba..9465b630cede 100644 > --- a/drivers/pci/controller/pci-aardvark.c > +++ b/drivers/pci/controller/pci-aardvark.c > @@ -165,7 +165,44 @@ > #define CFG_REG (LMI_BASE_ADDR + 0x0) > #define LTSSM_SHIFT 24 > #define LTSSM_MASK 0x3f > +#define LTSSM_DETECT_QUIET 0x0 > +#define LTSSM_DETECT_ACTIVE 0x1 > +#define LTSSM_POLLING_ACTIVE 0x2 > +#define LTSSM_POLLING_COMPLIANCE 0x3 > +#define LTSSM_POLLING_CONFIGURATION 0x4 > +#define LTSSM_CONFIG_LINKWIDTH_START 0x5 > +#define LTSSM_CONFIG_LINKWIDTH_ACCEPT 0x6 > +#define LTSSM_CONFIG_LANENUM_ACCEPT 0x7 > +#define LTSSM_CONFIG_LANENUM_WAIT 0x8 > +#define LTSSM_CONFIG_COMPLETE 0x9 > +#define LTSSM_CONFIG_IDLE 0xa > +#define LTSSM_RECOVERY_RCVR_LOCK 0xb > +#define LTSSM_RECOVERY_SPEED 0xc > +#define LTSSM_RECOVERY_RCVR_CFG 0xd > +#define LTSSM_RECOVERY_IDLE 0xe > #define LTSSM_L0 0x10 > +#define LTSSM_RX_L0S_ENTRY 0x11 > +#define LTSSM_RX_L0S_IDLE 0x12 > +#define LTSSM_RX_L0S_FTS 0x13 > +#define LTSSM_TX_L0S_ENTRY 0x14 > +#define LTSSM_TX_L0S_IDLE 0x15 > +#define LTSSM_TX_L0S_FTS 0x16 > +#define LTSSM_L1_ENTRY 0x17 > +#define LTSSM_L1_IDLE 0x18 > +#define LTSSM_L2_IDLE 0x19 > +#define LTSSM_L2_TRANSMIT_WAKE 0x1a > +#define LTSSM_DISABLED 0x20 > +#define LTSSM_LOOPBACK_ENTRY_MASTER 0x21 > +#define LTSSM_LOOPBACK_ACTIVE_MASTER 0x22 > +#define LTSSM_LOOPBACK_EXIT_MASTER 0x23 > +#define LTSSM_LOOPBACK_ENTRY_SLAVE 0x24 > +#define LTSSM_LOOPBACK_ACTIVE_SLAVE 0x25 > +#define LTSSM_LOOPBACK_EXIT_SLAVE 0x26 > +#define LTSSM_HOT_RESET 0x27 > +#define LTSSM_RECOVERY_EQUALIZATION_PHASE0 0x28 > +#define LTSSM_RECOVERY_EQUALIZATION_PHASE1 0x29 > +#define LTSSM_RECOVERY_EQUALIZATION_PHASE2 0x2a > +#define LTSSM_RECOVERY_EQUALIZATION_PHASE3 0x2b An enum would be nicer IMO, for readability. Don't repost the series yet. Lorenzo > #define RC_BAR_CONFIG 0x300 > #define VENDOR_ID_REG (LMI_BASE_ADDR + 0x44) > > @@ -258,13 +295,35 @@ static inline u32 advk_readl(struct advk_pcie *pcie, u64 reg) > return readl(pcie->base + reg); > } > > -static int advk_pcie_link_up(struct advk_pcie *pcie) > +static u8 advk_pcie_ltssm_state(struct advk_pcie *pcie) > { > - u32 val, ltssm_state; > + u32 val; > + u8 ltssm_state; > > val = advk_readl(pcie, CFG_REG); > ltssm_state = (val >> LTSSM_SHIFT) & LTSSM_MASK; > - return ltssm_state >= LTSSM_L0; > + return ltssm_state; > +} > + > +static inline bool advk_pcie_link_up(struct advk_pcie *pcie) > +{ > + /* check if LTSSM is in normal operation - some L* state */ > + u8 ltssm_state = advk_pcie_ltssm_state(pcie); > + return ltssm_state >= LTSSM_L0 && ltssm_state < LTSSM_DISABLED; > +} > + > +static inline bool advk_pcie_link_training(struct advk_pcie *pcie) > +{ > + /* > + * According to PCIe Base specification 3.0, Table 4-14: Link > + * Status Mapped to the LTSSM is Link Training mapped to LTSSM > + * Configuration and Recovery states. > + */ > + u8 ltssm_state = advk_pcie_ltssm_state(pcie); > + return ((ltssm_state >= LTSSM_CONFIG_LINKWIDTH_START && > + ltssm_state < LTSSM_L0) || > + (ltssm_state >= LTSSM_RECOVERY_EQUALIZATION_PHASE0 && > + ltssm_state <= LTSSM_RECOVERY_EQUALIZATION_PHASE3)); > } > > static int advk_pcie_wait_for_link(struct advk_pcie *pcie) > @@ -287,7 +346,7 @@ static void advk_pcie_wait_for_retrain(struct advk_pcie *pcie) > size_t retries; > > for (retries = 0; retries < RETRAIN_WAIT_MAX_RETRIES; ++retries) { > - if (!advk_pcie_link_up(pcie)) > + if (advk_pcie_link_training(pcie)) > break; > udelay(RETRAIN_WAIT_USLEEP_US); > } > @@ -706,7 +765,7 @@ advk_pci_bridge_emul_pcie_conf_read(struct pci_bridge_emul *bridge, > /* u32 contains both PCI_EXP_LNKCTL and PCI_EXP_LNKSTA */ > u32 val = advk_readl(pcie, PCIE_CORE_PCIEXP_CAP + reg) & > ~(PCI_EXP_LNKSTA_LT << 16); > - if (!advk_pcie_link_up(pcie)) > + if (advk_pcie_link_training(pcie)) > val |= (PCI_EXP_LNKSTA_LT << 16); > *value = val; > return PCI_BRIDGE_EMUL_HANDLED; > -- > 2.32.0 >
diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c index 5387d9cc3eba..9465b630cede 100644 --- a/drivers/pci/controller/pci-aardvark.c +++ b/drivers/pci/controller/pci-aardvark.c @@ -165,7 +165,44 @@ #define CFG_REG (LMI_BASE_ADDR + 0x0) #define LTSSM_SHIFT 24 #define LTSSM_MASK 0x3f +#define LTSSM_DETECT_QUIET 0x0 +#define LTSSM_DETECT_ACTIVE 0x1 +#define LTSSM_POLLING_ACTIVE 0x2 +#define LTSSM_POLLING_COMPLIANCE 0x3 +#define LTSSM_POLLING_CONFIGURATION 0x4 +#define LTSSM_CONFIG_LINKWIDTH_START 0x5 +#define LTSSM_CONFIG_LINKWIDTH_ACCEPT 0x6 +#define LTSSM_CONFIG_LANENUM_ACCEPT 0x7 +#define LTSSM_CONFIG_LANENUM_WAIT 0x8 +#define LTSSM_CONFIG_COMPLETE 0x9 +#define LTSSM_CONFIG_IDLE 0xa +#define LTSSM_RECOVERY_RCVR_LOCK 0xb +#define LTSSM_RECOVERY_SPEED 0xc +#define LTSSM_RECOVERY_RCVR_CFG 0xd +#define LTSSM_RECOVERY_IDLE 0xe #define LTSSM_L0 0x10 +#define LTSSM_RX_L0S_ENTRY 0x11 +#define LTSSM_RX_L0S_IDLE 0x12 +#define LTSSM_RX_L0S_FTS 0x13 +#define LTSSM_TX_L0S_ENTRY 0x14 +#define LTSSM_TX_L0S_IDLE 0x15 +#define LTSSM_TX_L0S_FTS 0x16 +#define LTSSM_L1_ENTRY 0x17 +#define LTSSM_L1_IDLE 0x18 +#define LTSSM_L2_IDLE 0x19 +#define LTSSM_L2_TRANSMIT_WAKE 0x1a +#define LTSSM_DISABLED 0x20 +#define LTSSM_LOOPBACK_ENTRY_MASTER 0x21 +#define LTSSM_LOOPBACK_ACTIVE_MASTER 0x22 +#define LTSSM_LOOPBACK_EXIT_MASTER 0x23 +#define LTSSM_LOOPBACK_ENTRY_SLAVE 0x24 +#define LTSSM_LOOPBACK_ACTIVE_SLAVE 0x25 +#define LTSSM_LOOPBACK_EXIT_SLAVE 0x26 +#define LTSSM_HOT_RESET 0x27 +#define LTSSM_RECOVERY_EQUALIZATION_PHASE0 0x28 +#define LTSSM_RECOVERY_EQUALIZATION_PHASE1 0x29 +#define LTSSM_RECOVERY_EQUALIZATION_PHASE2 0x2a +#define LTSSM_RECOVERY_EQUALIZATION_PHASE3 0x2b #define RC_BAR_CONFIG 0x300 #define VENDOR_ID_REG (LMI_BASE_ADDR + 0x44) @@ -258,13 +295,35 @@ static inline u32 advk_readl(struct advk_pcie *pcie, u64 reg) return readl(pcie->base + reg); } -static int advk_pcie_link_up(struct advk_pcie *pcie) +static u8 advk_pcie_ltssm_state(struct advk_pcie *pcie) { - u32 val, ltssm_state; + u32 val; + u8 ltssm_state; val = advk_readl(pcie, CFG_REG); ltssm_state = (val >> LTSSM_SHIFT) & LTSSM_MASK; - return ltssm_state >= LTSSM_L0; + return ltssm_state; +} + +static inline bool advk_pcie_link_up(struct advk_pcie *pcie) +{ + /* check if LTSSM is in normal operation - some L* state */ + u8 ltssm_state = advk_pcie_ltssm_state(pcie); + return ltssm_state >= LTSSM_L0 && ltssm_state < LTSSM_DISABLED; +} + +static inline bool advk_pcie_link_training(struct advk_pcie *pcie) +{ + /* + * According to PCIe Base specification 3.0, Table 4-14: Link + * Status Mapped to the LTSSM is Link Training mapped to LTSSM + * Configuration and Recovery states. + */ + u8 ltssm_state = advk_pcie_ltssm_state(pcie); + return ((ltssm_state >= LTSSM_CONFIG_LINKWIDTH_START && + ltssm_state < LTSSM_L0) || + (ltssm_state >= LTSSM_RECOVERY_EQUALIZATION_PHASE0 && + ltssm_state <= LTSSM_RECOVERY_EQUALIZATION_PHASE3)); } static int advk_pcie_wait_for_link(struct advk_pcie *pcie) @@ -287,7 +346,7 @@ static void advk_pcie_wait_for_retrain(struct advk_pcie *pcie) size_t retries; for (retries = 0; retries < RETRAIN_WAIT_MAX_RETRIES; ++retries) { - if (!advk_pcie_link_up(pcie)) + if (advk_pcie_link_training(pcie)) break; udelay(RETRAIN_WAIT_USLEEP_US); } @@ -706,7 +765,7 @@ advk_pci_bridge_emul_pcie_conf_read(struct pci_bridge_emul *bridge, /* u32 contains both PCI_EXP_LNKCTL and PCI_EXP_LNKSTA */ u32 val = advk_readl(pcie, PCIE_CORE_PCIEXP_CAP + reg) & ~(PCI_EXP_LNKSTA_LT << 16); - if (!advk_pcie_link_up(pcie)) + if (advk_pcie_link_training(pcie)) val |= (PCI_EXP_LNKSTA_LT << 16); *value = val; return PCI_BRIDGE_EMUL_HANDLED;