* [PATCH v2 1/2] PCI: designware: Move LTSSM state definitions to pcie-designware.h
@ 2015-10-09 1:48 Fabio Estevam
2015-10-09 1:48 ` [PATCH v2 2/2] PCI: imx6: Use define instead of hard coded value Fabio Estevam
2015-10-12 10:19 ` [PATCH v2 1/2] PCI: designware: Move LTSSM state definitions to pcie-designware.h Lucas Stach
0 siblings, 2 replies; 11+ messages in thread
From: Fabio Estevam @ 2015-10-09 1:48 UTC (permalink / raw)
To: bhelgaas; +Cc: pratyush.anand, m-karicheri2, l.stach, linux-pci, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
Move LTSSM state definitions to common pcie-designware.h so that other
drivers can make use of them.
Also, in order to avoid name collision define LTSSM_STATE_MASK_KS, as
keystone uses a different LTSSM mask.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Acked-by: Pratyush Anand <pratyush.anand@gmail.com>
---
Changes since v1:
- Fix build warning noticed by kbuild robot
drivers/pci/host/pci-keystone-dw.c | 4 ++--
drivers/pci/host/pci-layerscape.c | 1 -
drivers/pci/host/pcie-designware.h | 34 ++++++++++++++++++++++++++++++++++
drivers/pci/host/pcie-spear13xx.c | 33 ---------------------------------
4 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/drivers/pci/host/pci-keystone-dw.c b/drivers/pci/host/pci-keystone-dw.c
index 3cf55cd..004b677 100644
--- a/drivers/pci/host/pci-keystone-dw.c
+++ b/drivers/pci/host/pci-keystone-dw.c
@@ -25,7 +25,7 @@
/* Application register defines */
#define LTSSM_EN_VAL 1
-#define LTSSM_STATE_MASK 0x1f
+#define LTSSM_STATE_MASK_KS 0x1f
#define LTSSM_STATE_L0 0x11
#define DBI_CS2_EN_VAL 0x20
#define OB_XLAT_EN_VAL 2
@@ -445,7 +445,7 @@ int ks_dw_pcie_link_up(struct pcie_port *pp)
{
u32 val = readl(pp->dbi_base + DEBUG0);
- return (val & LTSSM_STATE_MASK) == LTSSM_STATE_L0;
+ return (val & LTSSM_STATE_MASK_KS) == LTSSM_STATE_L0;
}
void ks_dw_pcie_initiate_link_train(struct keystone_pcie *ks_pcie)
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 35123d9..97d6558 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -22,6 +22,40 @@
#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_ACEPT 0x08
+#define LTSSM_STATE_CFG_LANENUM_WAIT 0x09
+#define LTSSM_STATE_CFG_LANENUM_ACEPT 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 0x3F
+
struct pcie_port {
struct device *dev;
u8 root_bus_nr;
diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
index 98d2683..920d399 100644
--- a/drivers/pci/host/pcie-spear13xx.c
+++ b/drivers/pci/host/pcie-spear13xx.c
@@ -84,39 +84,6 @@ struct pcie_app_reg {
#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 */
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v2 2/2] PCI: imx6: Use define instead of hard coded value
2015-10-09 1:48 [PATCH v2 1/2] PCI: designware: Move LTSSM state definitions to pcie-designware.h Fabio Estevam
@ 2015-10-09 1:48 ` Fabio Estevam
2015-10-12 10:29 ` Lucas Stach
2015-10-12 10:19 ` [PATCH v2 1/2] PCI: designware: Move LTSSM state definitions to pcie-designware.h Lucas Stach
1 sibling, 1 reply; 11+ messages in thread
From: Fabio Estevam @ 2015-10-09 1:48 UTC (permalink / raw)
To: bhelgaas; +Cc: pratyush.anand, m-karicheri2, l.stach, linux-pci, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
Use LTSSM_STATE_RCVRY_LOCK define instead of hard coded value in order
to improve the code readability;
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
Changes since v1:
- None
drivers/pci/host/pci-imx6.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 6f43086..4e7b577 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivePCIE_PHY_DEBUG_R0_LTSSM_MASKrs/pci/host/pci-imx6.c
@@ -508,7 +508,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 & PCIE_PHY_DEBUG_R0_LTSSM_MASK) != LTSSM_STATE_RCVRY_LOCK)
return 0;
dev_err(pp->dev, "transition to gen2 is stuck, reset PHY!\n");
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] PCI: imx6: Use define instead of hard coded value
2015-10-09 1:48 ` [PATCH v2 2/2] PCI: imx6: Use define instead of hard coded value Fabio Estevam
@ 2015-10-12 10:29 ` Lucas Stach
0 siblings, 0 replies; 11+ messages in thread
From: Lucas Stach @ 2015-10-12 10:29 UTC (permalink / raw)
To: Fabio Estevam
Cc: bhelgaas, pratyush.anand, m-karicheri2, linux-pci, Fabio Estevam
Am Donnerstag, den 08.10.2015, 22:48 -0300 schrieb Fabio Estevam:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> Use LTSSM_STATE_RCVRY_LOCK define instead of hard coded value in order
> to improve the code readability;
>
Normally we end sentences to be parsed by humans with a full stop
instead of a semicolon. ;)
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> Changes since v1:
> - None
>
> drivers/pci/host/pci-imx6.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index 6f43086..4e7b577 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivePCIE_PHY_DEBUG_R0_LTSSM_MASKrs/pci/host/pci-imx6.c
> @@ -508,7 +508,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 & PCIE_PHY_DEBUG_R0_LTSSM_MASK) != LTSSM_STATE_RCVRY_LOCK)
> return 0;
>
> dev_err(pp->dev, "transition to gen2 is stuck, reset PHY!\n");
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] PCI: designware: Move LTSSM state definitions to pcie-designware.h
2015-10-09 1:48 [PATCH v2 1/2] PCI: designware: Move LTSSM state definitions to pcie-designware.h Fabio Estevam
2015-10-09 1:48 ` [PATCH v2 2/2] PCI: imx6: Use define instead of hard coded value Fabio Estevam
@ 2015-10-12 10:19 ` Lucas Stach
2015-10-12 10:28 ` Lucas Stach
2015-10-12 12:26 ` Fabio Estevam
1 sibling, 2 replies; 11+ messages in thread
From: Lucas Stach @ 2015-10-12 10:19 UTC (permalink / raw)
To: Fabio Estevam
Cc: bhelgaas, pratyush.anand, m-karicheri2, linux-pci, Fabio Estevam
Am Donnerstag, den 08.10.2015, 22:48 -0300 schrieb Fabio Estevam:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> Move LTSSM state definitions to common pcie-designware.h so that other
> drivers can make use of them.
>
> Also, in order to avoid name collision define LTSSM_STATE_MASK_KS, as
> keystone uses a different LTSSM mask.
>
Why? Is this some kind of difference between the DW PCIe core revisions?
I don't see the other drivers using any LTSSM state value that would not
fit inside the KS mask. Can we just use the narrower KS mask, even if
the others have one bit more allocated in the register? This would spare
us this needless differentiation in the software.
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> Acked-by: Pratyush Anand <pratyush.anand@gmail.com>
> ---
> Changes since v1:
> - Fix build warning noticed by kbuild robot
>
> drivers/pci/host/pci-keystone-dw.c | 4 ++--
> drivers/pci/host/pci-layerscape.c | 1 -
> drivers/pci/host/pcie-designware.h | 34 ++++++++++++++++++++++++++++++++++
> drivers/pci/host/pcie-spear13xx.c | 33 ---------------------------------
> 4 files changed, 36 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/pci/host/pci-keystone-dw.c b/drivers/pci/host/pci-keystone-dw.c
> index 3cf55cd..004b677 100644
> --- a/drivers/pci/host/pci-keystone-dw.c
> +++ b/drivers/pci/host/pci-keystone-dw.c
> @@ -25,7 +25,7 @@
>
> /* Application register defines */
> #define LTSSM_EN_VAL 1
> -#define LTSSM_STATE_MASK 0x1f
> +#define LTSSM_STATE_MASK_KS 0x1f
> #define LTSSM_STATE_L0 0x11
> #define DBI_CS2_EN_VAL 0x20
> #define OB_XLAT_EN_VAL 2
> @@ -445,7 +445,7 @@ int ks_dw_pcie_link_up(struct pcie_port *pp)
> {
> u32 val = readl(pp->dbi_base + DEBUG0);
>
> - return (val & LTSSM_STATE_MASK) == LTSSM_STATE_L0;
> + return (val & LTSSM_STATE_MASK_KS) == LTSSM_STATE_L0;
> }
>
> void ks_dw_pcie_initiate_link_train(struct keystone_pcie *ks_pcie)
> 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 35123d9..97d6558 100644
> --- a/drivers/pci/host/pcie-designware.h
> +++ b/drivers/pci/host/pcie-designware.h
> @@ -22,6 +22,40 @@
> #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_ACEPT 0x08
> +#define LTSSM_STATE_CFG_LANENUM_WAIT 0x09
> +#define LTSSM_STATE_CFG_LANENUM_ACEPT 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 0x3F
> +
I know this is just copy-and-paste, but can we use lowercase letters for
the hex values please? This would make this fit in better with the rest
of the header and is much more like any other part of the kernel.
Regards,
Lucas
> struct pcie_port {
> struct device *dev;
> u8 root_bus_nr;
> diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
> index 98d2683..920d399 100644
> --- a/drivers/pci/host/pcie-spear13xx.c
> +++ b/drivers/pci/host/pcie-spear13xx.c
> @@ -84,39 +84,6 @@ struct pcie_app_reg {
> #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 */
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 1/2] PCI: designware: Move LTSSM state definitions to pcie-designware.h
2015-10-12 10:19 ` [PATCH v2 1/2] PCI: designware: Move LTSSM state definitions to pcie-designware.h Lucas Stach
@ 2015-10-12 10:28 ` Lucas Stach
2015-10-12 12:31 ` Fabio Estevam
2015-10-12 12:26 ` Fabio Estevam
1 sibling, 1 reply; 11+ messages in thread
From: Lucas Stach @ 2015-10-12 10:28 UTC (permalink / raw)
To: Fabio Estevam
Cc: bhelgaas, pratyush.anand, m-karicheri2, linux-pci, Fabio Estevam
Am Montag, den 12.10.2015, 12:19 +0200 schrieb Lucas Stach:
> Am Donnerstag, den 08.10.2015, 22:48 -0300 schrieb Fabio Estevam:
> > From: Fabio Estevam <fabio.estevam@freescale.com>
> >
> > Move LTSSM state definitions to common pcie-designware.h so that other
> > drivers can make use of them.
> >
> > Also, in order to avoid name collision define LTSSM_STATE_MASK_KS, as
> > keystone uses a different LTSSM mask.
> >
> Why? Is this some kind of difference between the DW PCIe core revisions?
>
> I don't see the other drivers using any LTSSM state value that would not
> fit inside the KS mask. Can we just use the narrower KS mask, even if
> the others have one bit more allocated in the register? This would spare
> us this needless differentiation in the software.
>
Actually the TRM states that i.MX6 is using the same narrower mask of
0x1f for the LTSSM state, so another reason to just use that.
> > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> > Acked-by: Pratyush Anand <pratyush.anand@gmail.com>
> > ---
> > Changes since v1:
> > - Fix build warning noticed by kbuild robot
> >
> > drivers/pci/host/pci-keystone-dw.c | 4 ++--
> > drivers/pci/host/pci-layerscape.c | 1 -
> > drivers/pci/host/pcie-designware.h | 34 ++++++++++++++++++++++++++++++++++
> > drivers/pci/host/pcie-spear13xx.c | 33 ---------------------------------
> > 4 files changed, 36 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/pci/host/pci-keystone-dw.c b/drivers/pci/host/pci-keystone-dw.c
> > index 3cf55cd..004b677 100644
> > --- a/drivers/pci/host/pci-keystone-dw.c
> > +++ b/drivers/pci/host/pci-keystone-dw.c
> > @@ -25,7 +25,7 @@
> >
> > /* Application register defines */
> > #define LTSSM_EN_VAL 1
> > -#define LTSSM_STATE_MASK 0x1f
> > +#define LTSSM_STATE_MASK_KS 0x1f
> > #define LTSSM_STATE_L0 0x11
> > #define DBI_CS2_EN_VAL 0x20
> > #define OB_XLAT_EN_VAL 2
> > @@ -445,7 +445,7 @@ int ks_dw_pcie_link_up(struct pcie_port *pp)
> > {
> > u32 val = readl(pp->dbi_base + DEBUG0);
> >
> > - return (val & LTSSM_STATE_MASK) == LTSSM_STATE_L0;
> > + return (val & LTSSM_STATE_MASK_KS) == LTSSM_STATE_L0;
> > }
> >
> > void ks_dw_pcie_initiate_link_train(struct keystone_pcie *ks_pcie)
> > 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 35123d9..97d6558 100644
> > --- a/drivers/pci/host/pcie-designware.h
> > +++ b/drivers/pci/host/pcie-designware.h
> > @@ -22,6 +22,40 @@
> > #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_ACEPT 0x08
> > +#define LTSSM_STATE_CFG_LANENUM_WAIT 0x09
> > +#define LTSSM_STATE_CFG_LANENUM_ACEPT 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 0x3F
> > +
> I know this is just copy-and-paste, but can we use lowercase letters for
> the hex values please? This would make this fit in better with the rest
> of the header and is much more like any other part of the kernel.
>
> Regards,
> Lucas
>
> > struct pcie_port {
> > struct device *dev;
> > u8 root_bus_nr;
> > diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
> > index 98d2683..920d399 100644
> > --- a/drivers/pci/host/pcie-spear13xx.c
> > +++ b/drivers/pci/host/pcie-spear13xx.c
> > @@ -84,39 +84,6 @@ struct pcie_app_reg {
> > #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 */
>
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 1/2] PCI: designware: Move LTSSM state definitions to pcie-designware.h
2015-10-12 10:28 ` Lucas Stach
@ 2015-10-12 12:31 ` Fabio Estevam
2015-10-12 13:34 ` Lucas Stach
0 siblings, 1 reply; 11+ messages in thread
From: Fabio Estevam @ 2015-10-12 12:31 UTC (permalink / raw)
To: Lucas Stach
Cc: Bjorn Helgaas, Pratyush Anand Thakur, m-karicheri2,
linux-pci@vger.kernel.org, Fabio Estevam
On Mon, Oct 12, 2015 at 7:28 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Actually the TRM states that i.MX6 is using the same narrower mask of
> 0x1f for the LTSSM state, so another reason to just use that.
The manual I have seems to tell a different value :-)
Please check: http://cache.freescale.com/files/32bit/doc/ref_manual/IMX6DQRM.pdf
"48.12.11
Debug Register 0 (PCIE_PL_DEBUG0)"
says "[5:0]: xmlh_ltssm_state LTSSM current state. See source for encodings"
Regards,
Fabio Estevam
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] PCI: designware: Move LTSSM state definitions to pcie-designware.h
2015-10-12 12:31 ` Fabio Estevam
@ 2015-10-12 13:34 ` Lucas Stach
2015-10-12 14:04 ` Fabio Estevam
0 siblings, 1 reply; 11+ messages in thread
From: Lucas Stach @ 2015-10-12 13:34 UTC (permalink / raw)
To: Fabio Estevam
Cc: Bjorn Helgaas, Pratyush Anand Thakur, m-karicheri2,
linux-pci@vger.kernel.org, Fabio Estevam
Am Montag, den 12.10.2015, 09:31 -0300 schrieb Fabio Estevam:
> On Mon, Oct 12, 2015 at 7:28 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
>
> > Actually the TRM states that i.MX6 is using the same narrower mask of
> > 0x1f for the LTSSM state, so another reason to just use that.
>
> The manual I have seems to tell a different value :-)
>
> Please check: http://cache.freescale.com/files/32bit/doc/ref_manual/IMX6DQRM.pdf
>
> "48.12.11
> Debug Register 0 (PCIE_PL_DEBUG0)"
>
> says "[5:0]: xmlh_ltssm_state LTSSM current state. See source for encodings"
>
Urgh. Seems we got confused by all the different defines.
In fact [5:0] is 0x1f, so i.MX6 uses the same mask as Keystone, yet the
common and shared header now states a mask of 0x3f with Keystone being
the exception. That doesn't reflect reality which isn't nice IMHO. I
think it would be better to just use 0x1f as the LTSSM mask in the
shared header and use this for all drivers until a need arises for using
the extra bit on SPEAR.
Regards,
Lucas
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] PCI: designware: Move LTSSM state definitions to pcie-designware.h
2015-10-12 13:34 ` Lucas Stach
@ 2015-10-12 14:04 ` Fabio Estevam
2015-10-12 14:11 ` Lucas Stach
0 siblings, 1 reply; 11+ messages in thread
From: Fabio Estevam @ 2015-10-12 14:04 UTC (permalink / raw)
To: Lucas Stach
Cc: Bjorn Helgaas, Pratyush Anand Thakur, m-karicheri2,
linux-pci@vger.kernel.org, Fabio Estevam
On Mon, Oct 12, 2015 at 10:34 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Montag, den 12.10.2015, 09:31 -0300 schrieb Fabio Estevam:
>> On Mon, Oct 12, 2015 at 7:28 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
>>
>> > Actually the TRM states that i.MX6 is using the same narrower mask of
>> > 0x1f for the LTSSM state, so another reason to just use that.
>>
>> The manual I have seems to tell a different value :-)
>>
>> Please check: http://cache.freescale.com/files/32bit/doc/ref_manual/IMX6DQRM.pdf
>>
>> "48.12.11
>> Debug Register 0 (PCIE_PL_DEBUG0)"
>>
>> says "[5:0]: xmlh_ltssm_state LTSSM current state. See source for encodings"
>>
> Urgh. Seems we got confused by all the different defines.
>
> In fact [5:0] is 0x1f, so i.MX6 uses the same mask as Keystone, yet the
No, bits [5..0] means 0x3f. What am I missing here?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] PCI: designware: Move LTSSM state definitions to pcie-designware.h
2015-10-12 14:04 ` Fabio Estevam
@ 2015-10-12 14:11 ` Lucas Stach
2015-10-12 15:47 ` Fabio Estevam
0 siblings, 1 reply; 11+ messages in thread
From: Lucas Stach @ 2015-10-12 14:11 UTC (permalink / raw)
To: Fabio Estevam
Cc: Bjorn Helgaas, Pratyush Anand Thakur, m-karicheri2,
linux-pci@vger.kernel.org, Fabio Estevam
Am Montag, den 12.10.2015, 11:04 -0300 schrieb Fabio Estevam:
> On Mon, Oct 12, 2015 at 10:34 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> > Am Montag, den 12.10.2015, 09:31 -0300 schrieb Fabio Estevam:
> >> On Mon, Oct 12, 2015 at 7:28 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> >>
> >> > Actually the TRM states that i.MX6 is using the same narrower mask of
> >> > 0x1f for the LTSSM state, so another reason to just use that.
> >>
> >> The manual I have seems to tell a different value :-)
> >>
> >> Please check: http://cache.freescale.com/files/32bit/doc/ref_manual/IMX6DQRM.pdf
> >>
> >> "48.12.11
> >> Debug Register 0 (PCIE_PL_DEBUG0)"
> >>
> >> says "[5:0]: xmlh_ltssm_state LTSSM current state. See source for encodings"
> >>
> > Urgh. Seems we got confused by all the different defines.
> >
> > In fact [5:0] is 0x1f, so i.MX6 uses the same mask as Keystone, yet the
>
> No, bits [5..0] means 0x3f. What am I missing here?
That you have to deal with people who are apparently unable to count to
5. *me puts on the brown paper bag*
Still I would love to have a definition in the common header that's
usable for all drivers. So unless there is a reason to use 0x3f (and we
don't have any LTSSM state defines which would use the extra bit) keep
it at 0x1f. Simpler, cleaner... IMHO.
Regards,
Lucas
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] PCI: designware: Move LTSSM state definitions to pcie-designware.h
2015-10-12 14:11 ` Lucas Stach
@ 2015-10-12 15:47 ` Fabio Estevam
0 siblings, 0 replies; 11+ messages in thread
From: Fabio Estevam @ 2015-10-12 15:47 UTC (permalink / raw)
To: Lucas Stach
Cc: Bjorn Helgaas, Pratyush Anand Thakur, m-karicheri2,
linux-pci@vger.kernel.org, Fabio Estevam
On Mon, Oct 12, 2015 at 11:11 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Still I would love to have a definition in the common header that's
> usable for all drivers. So unless there is a reason to use 0x3f (and we
> don't have any LTSSM state defines which would use the extra bit) keep
> it at 0x1f. Simpler, cleaner... IMHO.
Sure, just did as suggested in v3. Thanks
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] PCI: designware: Move LTSSM state definitions to pcie-designware.h
2015-10-12 10:19 ` [PATCH v2 1/2] PCI: designware: Move LTSSM state definitions to pcie-designware.h Lucas Stach
2015-10-12 10:28 ` Lucas Stach
@ 2015-10-12 12:26 ` Fabio Estevam
1 sibling, 0 replies; 11+ messages in thread
From: Fabio Estevam @ 2015-10-12 12:26 UTC (permalink / raw)
To: Lucas Stach
Cc: Bjorn Helgaas, Pratyush Anand Thakur, m-karicheri2,
linux-pci@vger.kernel.org, Fabio Estevam
Hi Lucas,
On Mon, Oct 12, 2015 at 7:19 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Donnerstag, den 08.10.2015, 22:48 -0300 schrieb Fabio Estevam:
>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>
>> Move LTSSM state definitions to common pcie-designware.h so that other
>> drivers can make use of them.
>>
>> Also, in order to avoid name collision define LTSSM_STATE_MASK_KS, as
>> keystone uses a different LTSSM mask.
>>
> Why? Is this some kind of difference between the DW PCIe core revisions?
This patch does not alter the LTSSM masks that are used currently and
they match the ones stated in the SoC manuals.
For keystone:
http://www.ti.com/lit/ug/sprugs6d/sprugs6d.pdf
Section "48.9.11 Debug Register 0 (PCIE_PL_DEBUG0)" shows that LTSSM
current state has 6 bits.
For i.MX6:
Section "3.9.11 Debug 0 Register (DEBUG0)" shows that LTSSM_STATE
field has 5 bits.
Regards,
Fabio Estevam
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-10-12 15:47 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-09 1:48 [PATCH v2 1/2] PCI: designware: Move LTSSM state definitions to pcie-designware.h Fabio Estevam
2015-10-09 1:48 ` [PATCH v2 2/2] PCI: imx6: Use define instead of hard coded value Fabio Estevam
2015-10-12 10:29 ` Lucas Stach
2015-10-12 10:19 ` [PATCH v2 1/2] PCI: designware: Move LTSSM state definitions to pcie-designware.h Lucas Stach
2015-10-12 10:28 ` Lucas Stach
2015-10-12 12:31 ` Fabio Estevam
2015-10-12 13:34 ` Lucas Stach
2015-10-12 14:04 ` Fabio Estevam
2015-10-12 14:11 ` Lucas Stach
2015-10-12 15:47 ` Fabio Estevam
2015-10-12 12:26 ` Fabio Estevam
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).