* [PATCH v4 0/4] PCI: designware: LTSSM #define cleanup
@ 2015-10-21 18:42 Bjorn Helgaas
2015-10-21 18:42 ` [PATCH v4 1/4] PCI: designware: Use common LTSSM_STATE_MASK definition Bjorn Helgaas
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Bjorn Helgaas @ 2015-10-21 18:42 UTC (permalink / raw)
To: Fabio Estevam
Cc: Fabio Estevam, linux-pci, pratyush.anand, m-karicheri2, l.stach
This is a revision of Fabio's series:
http://lkml.kernel.org/r/1444664808-16445-1-git-send-email-festevam@gmail.com
Lucas, you reviewed the v3 patches, but I fiddled enough with this that I
didn't want to blindly carry your review forward. I don't *think* I
changed anything substantive, but I might have missed something.
Changes since Fabio's v3:
- Split removal of spear unused #defines to separate patch
- Combine LTSSM_STATE_MASK definition and use changes in one patch
- Combine LTSSM_STATE_RCVRY_LOCK definition and use in one patch
- Use common LTSSM_STATE_L0 definition
---
Bjorn Helgaas (1):
PCI: designware: Use common LTSSM_STATE_L0 definition
Fabio Estevam (3):
PCI: designware: Use common LTSSM_STATE_MASK definition
PCI: designware: Use common LTSSM_STATE_RCVRY_LOCK definition
PCI: spear: Remove unused #defines
drivers/pci/host/pci-imx6.c | 3 +-
drivers/pci/host/pci-keystone-dw.c | 2 -
drivers/pci/host/pci-layerscape.c | 4 +-
drivers/pci/host/pcie-designware.h | 4 ++
drivers/pci/host/pcie-spear13xx.c | 70 ------------------------------------
5 files changed, 6 insertions(+), 77 deletions(-)
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v4 1/4] PCI: designware: Use common LTSSM_STATE_MASK definition
2015-10-21 18:42 [PATCH v4 0/4] PCI: designware: LTSSM #define cleanup Bjorn Helgaas
@ 2015-10-21 18:42 ` Bjorn Helgaas
2015-10-22 14:51 ` Murali Karicheri
2015-10-27 15:15 ` Bjorn Helgaas
2015-10-21 18:42 ` [PATCH v4 2/4] PCI: designware: Use common LTSSM_STATE_RCVRY_LOCK definition Bjorn Helgaas
` (4 subsequent siblings)
5 siblings, 2 replies; 15+ messages in thread
From: Bjorn Helgaas @ 2015-10-21 18:42 UTC (permalink / raw)
To: Fabio Estevam
Cc: Fabio Estevam, linux-pci, pratyush.anand, m-karicheri2, l.stach
From: Fabio Estevam <fabio.estevam@freescale.com>
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.
[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@freescale.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
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;
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v4 2/4] PCI: designware: Use common LTSSM_STATE_RCVRY_LOCK definition
2015-10-21 18:42 [PATCH v4 0/4] PCI: designware: LTSSM #define cleanup Bjorn Helgaas
2015-10-21 18:42 ` [PATCH v4 1/4] PCI: designware: Use common LTSSM_STATE_MASK definition Bjorn Helgaas
@ 2015-10-21 18:42 ` Bjorn Helgaas
2015-10-21 18:43 ` [PATCH v4 3/4] PCI: designware: Use common LTSSM_STATE_L0 definition Bjorn Helgaas
` (3 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Bjorn Helgaas @ 2015-10-21 18:42 UTC (permalink / raw)
To: Fabio Estevam
Cc: Fabio Estevam, linux-pci, pratyush.anand, m-karicheri2, l.stach
From: Fabio Estevam <fabio.estevam@freescale.com>
Add a #define for LTSSM_STATE_RCVRY_LOCK and use it to improve code
readability.
[bhelgaas: include both definition and use for reviewability]
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/host/pci-imx6.c | 2 +-
drivers/pci/host/pcie-designware.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 7b0d120..4c788d27 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -507,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 & LTSSM_STATE_MASK) != 0x0d)
+ if ((debug_r0 & LTSSM_STATE_MASK) != LTSSM_STATE_RCVRY_LOCK)
return 0;
dev_err(pp->dev, "transition to gen2 is stuck, reset PHY!\n");
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index a1a76ff..f3a7583 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -22,6 +22,7 @@
#define MAX_MSI_IRQS 32
#define MAX_MSI_CTRLS (MAX_MSI_IRQS / 32)
+#define LTSSM_STATE_RCVRY_LOCK 0x0d
#define LTSSM_STATE_MASK 0x1f
struct pcie_port {
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v4 3/4] PCI: designware: Use common LTSSM_STATE_L0 definition
2015-10-21 18:42 [PATCH v4 0/4] PCI: designware: LTSSM #define cleanup Bjorn Helgaas
2015-10-21 18:42 ` [PATCH v4 1/4] PCI: designware: Use common LTSSM_STATE_MASK definition Bjorn Helgaas
2015-10-21 18:42 ` [PATCH v4 2/4] PCI: designware: Use common LTSSM_STATE_RCVRY_LOCK definition Bjorn Helgaas
@ 2015-10-21 18:43 ` Bjorn Helgaas
2015-10-22 14:53 ` Murali Karicheri
2015-10-21 18:43 ` [PATCH v4 4/4] PCI: spear: Remove unused #defines Bjorn Helgaas
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Bjorn Helgaas @ 2015-10-21 18:43 UTC (permalink / raw)
To: Fabio Estevam
Cc: Fabio Estevam, linux-pci, pratyush.anand, m-karicheri2, l.stach
Add a common #define for LTSSM_STATE_L0 and use it in all the
DesignWare-based drivers.
Based-on-patch-from: Fabio Estevam <fabio.estevam@freescale.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/host/pci-keystone-dw.c | 1 -
drivers/pci/host/pci-layerscape.c | 3 +--
drivers/pci/host/pcie-designware.h | 1 +
3 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/host/pci-keystone-dw.c b/drivers/pci/host/pci-keystone-dw.c
index 95a8b13..c00ba57 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_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 f02752e..930b193 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_PCIE_L0 0x11 /* L0 state */
/* Symbol Timer Register and Filter Mask Register 1 */
#define PCIE_STRFMR1 0x71c
@@ -55,7 +54,7 @@ static int ls_pcie_link_up(struct pcie_port *pp)
regmap_read(pcie->scfg, SCFG_PEXMSCPORTSR(pcie->index), &state);
state = (state >> LTSSM_STATE_SHIFT) & LTSSM_STATE_MASK;
- if (state < LTSSM_PCIE_L0)
+ if (state < LTSSM_STATE_L0)
return 0;
return 1;
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index f3a7583..33f74a8 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -23,6 +23,7 @@
#define MAX_MSI_CTRLS (MAX_MSI_IRQS / 32)
#define LTSSM_STATE_RCVRY_LOCK 0x0d
+#define LTSSM_STATE_L0 0x11
#define LTSSM_STATE_MASK 0x1f
struct pcie_port {
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v4 4/4] PCI: spear: Remove unused #defines
2015-10-21 18:42 [PATCH v4 0/4] PCI: designware: LTSSM #define cleanup Bjorn Helgaas
` (2 preceding siblings ...)
2015-10-21 18:43 ` [PATCH v4 3/4] PCI: designware: Use common LTSSM_STATE_L0 definition Bjorn Helgaas
@ 2015-10-21 18:43 ` Bjorn Helgaas
2015-10-21 18:51 ` [PATCH v4 0/4] PCI: designware: LTSSM #define cleanup Fabio Estevam
2015-10-22 9:04 ` Lucas Stach
5 siblings, 0 replies; 15+ messages in thread
From: Bjorn Helgaas @ 2015-10-21 18:43 UTC (permalink / raw)
To: Fabio Estevam
Cc: Fabio Estevam, linux-pci, pratyush.anand, m-karicheri2, l.stach
From: Fabio Estevam <fabio.estevam@freescale.com>
Remove unused #defines.
[bhelgaas: split into separate patch]
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/host/pcie-spear13xx.c | 70 -------------------------------------
1 file changed, 70 deletions(-)
diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
index 98d2683..10e33ad 100644
--- a/drivers/pci/host/pcie-spear13xx.c
+++ b/drivers/pci/host/pcie-spear13xx.c
@@ -58,87 +58,17 @@ struct pcie_app_reg {
};
/* CR0 ID */
-#define RX_LANE_FLIP_EN_ID 0
-#define TX_LANE_FLIP_EN_ID 1
-#define SYS_AUX_PWR_DET_ID 2
#define APP_LTSSM_ENABLE_ID 3
-#define SYS_ATTEN_BUTTON_PRESSED_ID 4
-#define SYS_MRL_SENSOR_STATE_ID 5
-#define SYS_PWR_FAULT_DET_ID 6
-#define SYS_MRL_SENSOR_CHGED_ID 7
-#define SYS_PRE_DET_CHGED_ID 8
-#define SYS_CMD_CPLED_INT_ID 9
-#define APP_INIT_RST_0_ID 11
-#define APP_REQ_ENTR_L1_ID 12
-#define APP_READY_ENTR_L23_ID 13
-#define APP_REQ_EXIT_L1_ID 14
-#define DEVICE_TYPE_EP (0 << 25)
-#define DEVICE_TYPE_LEP (1 << 25)
#define DEVICE_TYPE_RC (4 << 25)
-#define SYS_INT_ID 29
#define MISCTRL_EN_ID 30
#define REG_TRANSLATION_ENABLE 31
-/* CR1 ID */
-#define APPS_PM_XMT_TURNOFF_ID 2
-#define APPS_PM_XMT_PME_ID 5
-
/* CR3 ID */
-#define XMLH_LTSSM_STATE_DETECT_QUIET 0x00
-#define XMLH_LTSSM_STATE_DETECT_ACT 0x01
-#define XMLH_LTSSM_STATE_POLL_ACTIVE 0x02
-#define XMLH_LTSSM_STATE_POLL_COMPLIANCE 0x03
-#define XMLH_LTSSM_STATE_POLL_CONFIG 0x04
-#define XMLH_LTSSM_STATE_PRE_DETECT_QUIET 0x05
-#define XMLH_LTSSM_STATE_DETECT_WAIT 0x06
-#define XMLH_LTSSM_STATE_CFG_LINKWD_START 0x07
-#define XMLH_LTSSM_STATE_CFG_LINKWD_ACEPT 0x08
-#define XMLH_LTSSM_STATE_CFG_LANENUM_WAIT 0x09
-#define XMLH_LTSSM_STATE_CFG_LANENUM_ACEPT 0x0A
-#define XMLH_LTSSM_STATE_CFG_COMPLETE 0x0B
-#define XMLH_LTSSM_STATE_CFG_IDLE 0x0C
-#define XMLH_LTSSM_STATE_RCVRY_LOCK 0x0D
-#define XMLH_LTSSM_STATE_RCVRY_SPEED 0x0E
-#define XMLH_LTSSM_STATE_RCVRY_RCVRCFG 0x0F
-#define XMLH_LTSSM_STATE_RCVRY_IDLE 0x10
-#define XMLH_LTSSM_STATE_L0 0x11
-#define XMLH_LTSSM_STATE_L0S 0x12
-#define XMLH_LTSSM_STATE_L123_SEND_EIDLE 0x13
-#define XMLH_LTSSM_STATE_L1_IDLE 0x14
-#define XMLH_LTSSM_STATE_L2_IDLE 0x15
-#define XMLH_LTSSM_STATE_L2_WAKE 0x16
-#define XMLH_LTSSM_STATE_DISABLED_ENTRY 0x17
-#define XMLH_LTSSM_STATE_DISABLED_IDLE 0x18
-#define XMLH_LTSSM_STATE_DISABLED 0x19
-#define XMLH_LTSSM_STATE_LPBK_ENTRY 0x1A
-#define XMLH_LTSSM_STATE_LPBK_ACTIVE 0x1B
-#define XMLH_LTSSM_STATE_LPBK_EXIT 0x1C
-#define XMLH_LTSSM_STATE_LPBK_EXIT_TIMEOUT 0x1D
-#define XMLH_LTSSM_STATE_HOT_RESET_ENTRY 0x1E
-#define XMLH_LTSSM_STATE_HOT_RESET 0x1F
-#define XMLH_LTSSM_STATE_MASK 0x3F
#define XMLH_LINK_UP (1 << 6)
-/* CR4 ID */
-#define CFG_MSI_EN_ID 18
-
/* CR6 */
-#define INTA_CTRL_INT (1 << 7)
-#define INTB_CTRL_INT (1 << 8)
-#define INTC_CTRL_INT (1 << 9)
-#define INTD_CTRL_INT (1 << 10)
#define MSI_CTRL_INT (1 << 26)
-/* CR19 ID */
-#define VEN_MSI_REQ_ID 11
-#define VEN_MSI_FUN_NUM_ID 8
-#define VEN_MSI_TC_ID 5
-#define VEN_MSI_VECTOR_ID 0
-#define VEN_MSI_REQ_EN ((u32)0x1 << VEN_MSI_REQ_ID)
-#define VEN_MSI_FUN_NUM_MASK ((u32)0x7 << VEN_MSI_FUN_NUM_ID)
-#define VEN_MSI_TC_MASK ((u32)0x7 << VEN_MSI_TC_ID)
-#define VEN_MSI_VECTOR_MASK ((u32)0x1F << VEN_MSI_VECTOR_ID)
-
#define EXP_CAP_ID_OFFSET 0x70
#define to_spear13xx_pcie(x) container_of(x, struct spear13xx_pcie, pp)
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v4 0/4] PCI: designware: LTSSM #define cleanup
2015-10-21 18:42 [PATCH v4 0/4] PCI: designware: LTSSM #define cleanup Bjorn Helgaas
` (3 preceding siblings ...)
2015-10-21 18:43 ` [PATCH v4 4/4] PCI: spear: Remove unused #defines Bjorn Helgaas
@ 2015-10-21 18:51 ` Fabio Estevam
2015-10-22 9:04 ` Lucas Stach
5 siblings, 0 replies; 15+ messages in thread
From: Fabio Estevam @ 2015-10-21 18:51 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Fabio Estevam, linux-pci, Pratyush Anand Thakur, m-karicheri2,
Lucas Stach
Hi Bjorn,
On Wed, Oct 21, 2015 at 4:42 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> This is a revision of Fabio's series:
> http://lkml.kernel.org/r/1444664808-16445-1-git-send-email-festevam@gmail.com
>
> Lucas, you reviewed the v3 patches, but I fiddled enough with this that I
> didn't want to blindly carry your review forward. I don't *think* I
> changed anything substantive, but I might have missed something.
>
> Changes since Fabio's v3:
> - Split removal of spear unused #defines to separate patch
> - Combine LTSSM_STATE_MASK definition and use changes in one patch
> - Combine LTSSM_STATE_RCVRY_LOCK definition and use in one patch
> - Use common LTSSM_STATE_L0 definition
Thanks for reworking the series. It looks better now, thanks.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 0/4] PCI: designware: LTSSM #define cleanup
2015-10-21 18:42 [PATCH v4 0/4] PCI: designware: LTSSM #define cleanup Bjorn Helgaas
` (4 preceding siblings ...)
2015-10-21 18:51 ` [PATCH v4 0/4] PCI: designware: LTSSM #define cleanup Fabio Estevam
@ 2015-10-22 9:04 ` Lucas Stach
2015-10-22 15:34 ` Bjorn Helgaas
5 siblings, 1 reply; 15+ messages in thread
From: Lucas Stach @ 2015-10-22 9:04 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Fabio Estevam, Fabio Estevam, linux-pci, pratyush.anand, m-karicheri2
Hi Bjorn,
Am Mittwoch, den 21.10.2015, 13:42 -0500 schrieb Bjorn Helgaas:
> This is a revision of Fabio's series:
> http://lkml.kernel.org/r/1444664808-16445-1-git-send-email-festevam@gmail.com
>
> Lucas, you reviewed the v3 patches, but I fiddled enough with this that I
> didn't want to blindly carry your review forward. I don't *think* I
> changed anything substantive, but I might have missed something.
>
I'm not sure if I like the removal of all the LTSSM state defines, as
not all reference manuals include them and so I liked to have the header
as a reference. But if you prefer to not carry unused defines in the
kernel I won't object strongly to the removal.
Otherwise the series looks fine, which you may take as a Reviewed-by.
Regards,
Lucas
> Changes since Fabio's v3:
> - Split removal of spear unused #defines to separate patch
> - Combine LTSSM_STATE_MASK definition and use changes in one patch
> - Combine LTSSM_STATE_RCVRY_LOCK definition and use in one patch
> - Use common LTSSM_STATE_L0 definition
>
> ---
>
> Bjorn Helgaas (1):
> PCI: designware: Use common LTSSM_STATE_L0 definition
>
> Fabio Estevam (3):
> PCI: designware: Use common LTSSM_STATE_MASK definition
> PCI: designware: Use common LTSSM_STATE_RCVRY_LOCK definition
> PCI: spear: Remove unused #defines
>
>
> drivers/pci/host/pci-imx6.c | 3 +-
> drivers/pci/host/pci-keystone-dw.c | 2 -
> drivers/pci/host/pci-layerscape.c | 4 +-
> drivers/pci/host/pcie-designware.h | 4 ++
> drivers/pci/host/pcie-spear13xx.c | 70 ------------------------------------
> 5 files changed, 6 insertions(+), 77 deletions(-)
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 1/4] PCI: designware: Use common LTSSM_STATE_MASK definition
2015-10-21 18:42 ` [PATCH v4 1/4] PCI: designware: Use common LTSSM_STATE_MASK definition Bjorn Helgaas
@ 2015-10-22 14:51 ` Murali Karicheri
2015-10-27 15:15 ` Bjorn Helgaas
1 sibling, 0 replies; 15+ messages in thread
From: Murali Karicheri @ 2015-10-22 14:51 UTC (permalink / raw)
To: Bjorn Helgaas, Fabio Estevam
Cc: Fabio Estevam, linux-pci, pratyush.anand, l.stach
On 10/21/2015 02:42 PM, Bjorn Helgaas wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> 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.
>
> [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@freescale.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> 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;
>
>
>
For pci-keystone-dw.c
Acked-by: Murali Karicheri <m-karicheri2@ti.com>
--
Murali Karicheri
Linux Kernel, Keystone
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 3/4] PCI: designware: Use common LTSSM_STATE_L0 definition
2015-10-21 18:43 ` [PATCH v4 3/4] PCI: designware: Use common LTSSM_STATE_L0 definition Bjorn Helgaas
@ 2015-10-22 14:53 ` Murali Karicheri
0 siblings, 0 replies; 15+ messages in thread
From: Murali Karicheri @ 2015-10-22 14:53 UTC (permalink / raw)
To: Bjorn Helgaas, Fabio Estevam
Cc: Fabio Estevam, linux-pci, pratyush.anand, l.stach
On 10/21/2015 02:43 PM, Bjorn Helgaas wrote:
> Add a common #define for LTSSM_STATE_L0 and use it in all the
> DesignWare-based drivers.
>
> Based-on-patch-from: Fabio Estevam <fabio.estevam@freescale.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> drivers/pci/host/pci-keystone-dw.c | 1 -
> drivers/pci/host/pci-layerscape.c | 3 +--
> drivers/pci/host/pcie-designware.h | 1 +
> 3 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/host/pci-keystone-dw.c b/drivers/pci/host/pci-keystone-dw.c
> index 95a8b13..c00ba57 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_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 f02752e..930b193 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_PCIE_L0 0x11 /* L0 state */
>
> /* Symbol Timer Register and Filter Mask Register 1 */
> #define PCIE_STRFMR1 0x71c
> @@ -55,7 +54,7 @@ static int ls_pcie_link_up(struct pcie_port *pp)
> regmap_read(pcie->scfg, SCFG_PEXMSCPORTSR(pcie->index), &state);
> state = (state >> LTSSM_STATE_SHIFT) & LTSSM_STATE_MASK;
>
> - if (state < LTSSM_PCIE_L0)
> + if (state < LTSSM_STATE_L0)
> return 0;
>
> return 1;
> diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
> index f3a7583..33f74a8 100644
> --- a/drivers/pci/host/pcie-designware.h
> +++ b/drivers/pci/host/pcie-designware.h
> @@ -23,6 +23,7 @@
> #define MAX_MSI_CTRLS (MAX_MSI_IRQS / 32)
>
> #define LTSSM_STATE_RCVRY_LOCK 0x0d
> +#define LTSSM_STATE_L0 0x11
> #define LTSSM_STATE_MASK 0x1f
>
> struct pcie_port {
>
>
>
for pci-keystone-dw.c
Acked-by: Murali Karicheri <m-karicheri2@ti.com>
--
Murali Karicheri
Linux Kernel, Keystone
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 0/4] PCI: designware: LTSSM #define cleanup
2015-10-22 9:04 ` Lucas Stach
@ 2015-10-22 15:34 ` Bjorn Helgaas
2015-10-23 6:53 ` Pratyush Anand
0 siblings, 1 reply; 15+ messages in thread
From: Bjorn Helgaas @ 2015-10-22 15:34 UTC (permalink / raw)
To: Lucas Stach
Cc: Bjorn Helgaas, Fabio Estevam, Fabio Estevam, linux-pci,
pratyush.anand, m-karicheri2
On Thu, Oct 22, 2015 at 11:04:26AM +0200, Lucas Stach wrote:
> Hi Bjorn,
>
> Am Mittwoch, den 21.10.2015, 13:42 -0500 schrieb Bjorn Helgaas:
> > This is a revision of Fabio's series:
> > http://lkml.kernel.org/r/1444664808-16445-1-git-send-email-festevam@gmail.com
> >
> > Lucas, you reviewed the v3 patches, but I fiddled enough with this that I
> > didn't want to blindly carry your review forward. I don't *think* I
> > changed anything substantive, but I might have missed something.
> >
> I'm not sure if I like the removal of all the LTSSM state defines, as
> not all reference manuals include them and so I liked to have the header
> as a reference. But if you prefer to not carry unused defines in the
> kernel I won't object strongly to the removal.
>
> Otherwise the series looks fine, which you may take as a Reviewed-by.
Thanks, Lucas.
OK, I put the rest of the LTSSM #defines back in
drivers/pci/host/pcie-designware.h as a separate patch (below):
commit 25026d3fcff7dd0e139b04cb52436b9c7e0e545e
Author: Fabio Estevam <fabio.estevam@freescale.com>
Date: Thu Oct 22 10:24:52 2015 -0500
PCI: designware: Add LTSSM state definitions
Add the rest of the LTSSM state definitions. These aren't currently used,
so they're here as documentation.
[bhelgaas: split into separate patch]
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index efe5fca..8cb5725 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -22,8 +22,39 @@
#define MAX_MSI_IRQS 32
#define MAX_MSI_CTRLS (MAX_MSI_IRQS / 32)
+#define LTSSM_STATE_DETECT_QUIET 0x00
+#define LTSSM_STATE_DETECT_ACT 0x01
+#define LTSSM_STATE_POLL_ACTIVE 0x02
+#define LTSSM_STATE_POLL_COMPLIANCE 0x03
+#define LTSSM_STATE_POLL_CONFIG 0x04
+#define LTSSM_STATE_PRE_DETECT_QUIET 0x05
+#define LTSSM_STATE_DETECT_WAIT 0x06
+#define LTSSM_STATE_CFG_LINKWD_START 0x07
+#define LTSSM_STATE_CFG_LINKWD_ACCEPT 0x08
+#define LTSSM_STATE_CFG_LANENUM_WAIT 0x09
+#define LTSSM_STATE_CFG_LANENUM_ACCEPT 0x0a
+#define LTSSM_STATE_CFG_COMPLETE 0x0b
+#define LTSSM_STATE_CFG_IDLE 0x0c
#define LTSSM_STATE_RCVRY_LOCK 0x0d
+#define LTSSM_STATE_RCVRY_SPEED 0x0e
+#define LTSSM_STATE_RCVRY_RCVRCFG 0x0f
+#define LTSSM_STATE_RCVRY_IDLE 0x10
#define LTSSM_STATE_L0 0x11
+#define LTSSM_STATE_L0S 0x12
+#define LTSSM_STATE_L123_SEND_EIDLE 0x13
+#define LTSSM_STATE_L1_IDLE 0x14
+#define LTSSM_STATE_L2_IDLE 0x15
+#define LTSSM_STATE_L2_WAKE 0x16
+#define LTSSM_STATE_DISABLED_ENTRY 0x17
+#define LTSSM_STATE_DISABLED_IDLE 0x18
+#define LTSSM_STATE_DISABLED 0x19
+#define LTSSM_STATE_LPBK_ENTRY 0x1a
+#define LTSSM_STATE_LPBK_ACTIVE 0x1b
+#define LTSSM_STATE_LPBK_EXIT 0x1c
+#define LTSSM_STATE_LPBK_EXIT_TIMEOUT 0x1d
+#define LTSSM_STATE_HOT_RESET_ENTRY 0x1e
+#define LTSSM_STATE_HOT_RESET 0x1f
+
#define LTSSM_STATE_MASK 0x1f
struct pcie_port {
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v4 0/4] PCI: designware: LTSSM #define cleanup
2015-10-22 15:34 ` Bjorn Helgaas
@ 2015-10-23 6:53 ` Pratyush Anand
0 siblings, 0 replies; 15+ messages in thread
From: Pratyush Anand @ 2015-10-23 6:53 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Lucas Stach, Bjorn Helgaas, Fabio Estevam, Fabio Estevam,
linux-pci, Murali Karicheri
On Thu, Oct 22, 2015 at 9:04 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Thu, Oct 22, 2015 at 11:04:26AM +0200, Lucas Stach wrote:
>> I'm not sure if I like the removal of all the LTSSM state defines, as
>> not all reference manuals include them and so I liked to have the header
>> as a reference. But if you prefer to not carry unused defines in the
>> kernel I won't object strongly to the removal.
>>
>> Otherwise the series looks fine, which you may take as a Reviewed-by.
>
> Thanks, Lucas.
>
> OK, I put the rest of the LTSSM #defines back in
> drivers/pci/host/pcie-designware.h as a separate patch (below):
>
Thanks for putting it back. Its useful to debug issues related to link
training, which one finds many a time with new silicon. And as Lucas
pointed, its not mentioned in most of the user manual. I remember, I
had to grep it into RTL code :(.
For the series
Acked-by: Pratyush Anand <pratyush.anand@gmail.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 1/4] PCI: designware: Use common LTSSM_STATE_MASK definition
2015-10-21 18:42 ` [PATCH v4 1/4] PCI: designware: Use common LTSSM_STATE_MASK definition Bjorn Helgaas
2015-10-22 14:51 ` Murali Karicheri
@ 2015-10-27 15:15 ` Bjorn Helgaas
2015-10-27 16:05 ` Fabio Estevam
1 sibling, 1 reply; 15+ messages in thread
From: Bjorn Helgaas @ 2015-10-27 15:15 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Fabio Estevam, Fabio Estevam, linux-pci, pratyush.anand,
m-karicheri2, l.stach, Minghuan Lian
[+cc Minghuan]
On Wed, Oct 21, 2015 at 01:42:50PM -0500, Bjorn Helgaas wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> 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@freescale.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 1/4] PCI: designware: Use common LTSSM_STATE_MASK definition
2015-10-27 15:15 ` Bjorn Helgaas
@ 2015-10-27 16:05 ` Fabio Estevam
2015-10-27 16:20 ` Bjorn Helgaas
0 siblings, 1 reply; 15+ messages in thread
From: Fabio Estevam @ 2015-10-27 16:05 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Bjorn Helgaas, Fabio Estevam, linux-pci, Pratyush Anand Thakur,
m-karicheri2, Lucas Stach, Minghuan Lian
Hi Bjorn,
On Tue, Oct 27, 2015 at 1:15 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> [+cc Minghuan]
>
> On Wed, Oct 21, 2015 at 01:42:50PM -0500, Bjorn Helgaas wrote:
>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>
>> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 1/4] PCI: designware: Use common LTSSM_STATE_MASK definition
2015-10-27 16:05 ` Fabio Estevam
@ 2015-10-27 16:20 ` Bjorn Helgaas
2015-10-27 23:34 ` Fabio Estevam
0 siblings, 1 reply; 15+ messages in thread
From: Bjorn Helgaas @ 2015-10-27 16:20 UTC (permalink / raw)
To: Fabio Estevam
Cc: Bjorn Helgaas, Fabio Estevam, linux-pci, Pratyush Anand Thakur,
m-karicheri2, Lucas Stach, Minghuan Lian
On Tue, Oct 27, 2015 at 02:05:17PM -0200, Fabio Estevam wrote:
> Hi Bjorn,
>
> On Tue, Oct 27, 2015 at 1:15 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > [+cc Minghuan]
> >
> > On Wed, Oct 21, 2015 at 01:42:50PM -0500, Bjorn Helgaas wrote:
> >> From: Fabio Estevam <fabio.estevam@freescale.com>
> >>
> >> 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.
Well, maybe. Are those states documented in the DesignWare spec? I
don't want to put things in pcie-designware.h that are only common by
accident or even by convention. We should only put things there if
they are documented things that users of that IP can rely on.
LTSSM_MASK is documented in the TI Keystone spec, so its definition
probably belongs in pci-keystone-dw.c. The same TI spec also contains
LTSSM state definitions, so I suspect they're in the same boat --
things that might accidentally be the same across devices, but they
don't *have* to be.
So I'm going to drop the following patches from my tree for now:
1ad5fdbc8410 PCI: designware: Add LTSSM state definitions
b09464f77dd2 PCI: designware: Use common LTSSM_STATE_L0 definition
fa15c15fd95d PCI: designware: Use common LTSSM_STATE_RCVRY_LOCK definition
4788fe6ebf45 PCI: designware: Use common LTSSM_STATE_MASK definition
We can add pieces back if they make sense. If we add things to shared
files like pcie-designware.h, I'd like a reference to the DW spec that
justifies the sharing.
Bjorn
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 1/4] PCI: designware: Use common LTSSM_STATE_MASK definition
2015-10-27 16:20 ` Bjorn Helgaas
@ 2015-10-27 23:34 ` Fabio Estevam
0 siblings, 0 replies; 15+ messages in thread
From: Fabio Estevam @ 2015-10-27 23:34 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Bjorn Helgaas, Fabio Estevam, linux-pci, Pratyush Anand Thakur,
m-karicheri2, Lucas Stach, Minghuan Lian
On Tue, Oct 27, 2015 at 2:20 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> Well, maybe. Are those states documented in the DesignWare spec? I
Unfortunately I cannot find them in DesignWare spec that I have access to.
> don't want to put things in pcie-designware.h that are only common by
> accident or even by convention. We should only put things there if
> they are documented things that users of that IP can rely on.
>
> LTSSM_MASK is documented in the TI Keystone spec, so its definition
> probably belongs in pci-keystone-dw.c. The same TI spec also contains
> LTSSM state definitions, so I suspect they're in the same boat --
> things that might accidentally be the same across devices, but they
> don't *have* to be.
>
> So I'm going to drop the following patches from my tree for now:
>
> 1ad5fdbc8410 PCI: designware: Add LTSSM state definitions
> b09464f77dd2 PCI: designware: Use common LTSSM_STATE_L0 definition
> fa15c15fd95d PCI: designware: Use common LTSSM_STATE_RCVRY_LOCK definition
> 4788fe6ebf45 PCI: designware: Use common LTSSM_STATE_MASK definition
>
> We can add pieces back if they make sense. If we add things to shared
> files like pcie-designware.h, I'd like a reference to the DW spec that
> justifies the sharing.
Fair enough, thanks.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-10-27 23:34 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-21 18:42 [PATCH v4 0/4] PCI: designware: LTSSM #define cleanup Bjorn Helgaas
2015-10-21 18:42 ` [PATCH v4 1/4] PCI: designware: Use common LTSSM_STATE_MASK definition Bjorn Helgaas
2015-10-22 14:51 ` Murali Karicheri
2015-10-27 15:15 ` Bjorn Helgaas
2015-10-27 16:05 ` Fabio Estevam
2015-10-27 16:20 ` Bjorn Helgaas
2015-10-27 23:34 ` Fabio Estevam
2015-10-21 18:42 ` [PATCH v4 2/4] PCI: designware: Use common LTSSM_STATE_RCVRY_LOCK definition Bjorn Helgaas
2015-10-21 18:43 ` [PATCH v4 3/4] PCI: designware: Use common LTSSM_STATE_L0 definition Bjorn Helgaas
2015-10-22 14:53 ` Murali Karicheri
2015-10-21 18:43 ` [PATCH v4 4/4] PCI: spear: Remove unused #defines Bjorn Helgaas
2015-10-21 18:51 ` [PATCH v4 0/4] PCI: designware: LTSSM #define cleanup Fabio Estevam
2015-10-22 9:04 ` Lucas Stach
2015-10-22 15:34 ` Bjorn Helgaas
2015-10-23 6:53 ` Pratyush Anand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).