Hi Bjorn, On Tue, Oct 27, 2015 at 1:15 PM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > [+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. Yes, what if we keep the original behaviour: define a common LTSSM_STATE_MASK as 0x3f and then add KS_LTSSM_STATE_MASK as 0x1f for Keystone only? > > 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. The LTSSM states we put in drivers/pci/host/pcie-designware.h are from 0-1f, so they are common. Would you like to drop this series from your tree and then I can resend it with the proposed changes above if everyone agrees? Thanks, Fabio Estevam -- 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
