[+cc Minghuan] On Wed, Oct 21, 2015 at 01:42:50PM -0500, Bjorn Helgaas wrote: > From: Fabio Estevam <fabio.estevam@xxxxxxxxxxxxx> > > Add a common #define for LTSSM_STATE_MASK and use it in all the > DesignWare-based drivers. > > Note that this changes LTSSM_STATE_MASK from 6 bits (0x3f) to 5 bits (0x1f) > for i.MX6 and Layerscape. We believe this is safe for all DesignWare-based > PCIe cores. OK, DesignWare experts, what's the story on this LTSSM_STATE_MASK? Minghuan says Layerscape requires a mask of 0x3f and it actually uses states 0x20, 0x21, 0x22, and 0x23: MH> Our LTSSM mask is 0x3f because it includes the following states: MH> 0x20 S_RCVRY_EQ0 MH> 0x21 S_RCVRY_EQ1 MH> 0x22 S_RCVRY_EQ2 MH> 0x23 S_RCVRY_EQ3 MH> And I checked DesignWare Cores PCI Express Controller Databook MH> v4.21a and found the following descriptor: MH> [5:0]: smlh_ltssm_state: LTSSM current state. Encoding is defined in workspace/src/include/cxpl_defs.vh MH> So could we use the mask 0x3f for all SoCs? I couldn't find the "DesignWare Cores PCI Express Controller Databook v4.21a", but I do see the Keystone mask (sec 3.9.11 of http://www.ti.com/lit/ug/sprugs6d/sprugs6d.pdf) is definitely only 5 bits, but that's clearly a TI register, not a generic DesignWare register. So it looks like a mistake to make a common LTSSM_STATE_MASK definition. It looks like this is something with some variation and we shouldn't make assumptions about it being common. Now I'm concerned that the LTSSM state definitions I put in drivers/pci/host/pcie-designware.h might not actually apply to everything derived from DW. For example, the Layerscape S_RCVRY states appear to be Layerscape-specific. I don't want to put misleading stuff in drivers/pci/host/pcie-designware.h if it's not really generic. Better to have it in the individual drivers, with a prefix that indicates that it applies to that driver, e.g., KS_LTSSM_MASK, LS_LTSSM_MASK, etc. Bjorn > [bhelgaas: changelog, move removal of unused SPEAr13xx #defines to separate > patch, add only LTSSM_STATE_MASK and change all users for reviewability] > Signed-off-by: Fabio Estevam <fabio.estevam@xxxxxxxxxxxxx> > Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > --- > drivers/pci/host/pci-imx6.c | 3 +-- > drivers/pci/host/pci-keystone-dw.c | 1 - > drivers/pci/host/pci-layerscape.c | 1 - > drivers/pci/host/pcie-designware.h | 2 ++ > 4 files changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c > index 6f43086..7b0d120 100644 > --- a/drivers/pci/host/pci-imx6.c > +++ b/drivers/pci/host/pci-imx6.c > @@ -55,7 +55,6 @@ struct imx6_pcie { > #define PCIE_PL_PFLR_LINK_STATE_MASK (0x3f << 16) > #define PCIE_PL_PFLR_FORCE_LINK (1 << 15) > #define PCIE_PHY_DEBUG_R0 (PL_OFFSET + 0x28) > -#define PCIE_PHY_DEBUG_R0_LTSSM_MASK (0x3f << 0) > #define PCIE_PHY_DEBUG_R1 (PL_OFFSET + 0x2c) > #define PCIE_PHY_DEBUG_R1_XMLH_LINK_IN_TRAINING (1 << 29) > #define PCIE_PHY_DEBUG_R1_XMLH_LINK_UP (1 << 4) > @@ -508,7 +507,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp) > if (rx_valid & PCIE_PHY_RX_ASIC_OUT_VALID) > return 0; > > - if ((debug_r0 & PCIE_PHY_DEBUG_R0_LTSSM_MASK) != 0x0d) > + if ((debug_r0 & LTSSM_STATE_MASK) != 0x0d) > return 0; > > dev_err(pp->dev, "transition to gen2 is stuck, reset PHY!\n"); > diff --git a/drivers/pci/host/pci-keystone-dw.c b/drivers/pci/host/pci-keystone-dw.c > index e71da99..95a8b13 100644 > --- a/drivers/pci/host/pci-keystone-dw.c > +++ b/drivers/pci/host/pci-keystone-dw.c > @@ -25,7 +25,6 @@ > > /* Application register defines */ > #define LTSSM_EN_VAL 1 > -#define LTSSM_STATE_MASK 0x1f > #define LTSSM_STATE_L0 0x11 > #define DBI_CS2_EN_VAL 0x20 > #define OB_XLAT_EN_VAL 2 > diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c > index b2328ea1..f02752e 100644 > --- a/drivers/pci/host/pci-layerscape.c > +++ b/drivers/pci/host/pci-layerscape.c > @@ -29,7 +29,6 @@ > /* PEX1/2 Misc Ports Status Register */ > #define SCFG_PEXMSCPORTSR(pex_idx) (0x94 + (pex_idx) * 4) > #define LTSSM_STATE_SHIFT 20 > -#define LTSSM_STATE_MASK 0x3f > #define LTSSM_PCIE_L0 0x11 /* L0 state */ > > /* Symbol Timer Register and Filter Mask Register 1 */ > diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h > index d0bbd27..a1a76ff 100644 > --- a/drivers/pci/host/pcie-designware.h > +++ b/drivers/pci/host/pcie-designware.h > @@ -22,6 +22,8 @@ > #define MAX_MSI_IRQS 32 > #define MAX_MSI_CTRLS (MAX_MSI_IRQS / 32) > > +#define LTSSM_STATE_MASK 0x1f > + > struct pcie_port { > struct device *dev; > u8 root_bus_nr; > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
