[PATCH V3 0/3] Add ASPM-L1 Substates support for Tegra

[PATCH V3 0/3] Add ASPM-L1 Substates support for Tegra

[Vidya Sagar]

Tegra chips T210 and T186 support ASPM-L1 Substates (i.e. L1.1 and L1.2)
This patch series
- applies fixups to reflect correct capability values for
  T_cmrt (Common Mode Restore Time) and
  T_pwr_on (Power On)
  and adjusts counter values for 19.2 MHz of clk_m
- applies fixup specific to T210 to avoid unnecessary wake ups from L1.2 state
- enables advertisement of ASPM-L1 support in capability register

NOTE:- This version of patch series drops a patch (that was present till V2 series)
that introduces an API for host controller drivers to specify LTR_L1.2_THRESHOLD
value as more details and better understanding of spec is required to program
LTR_L1.2_THRESHOLD value.
It also reorders patches to appropriately have software fixups and advertisement
of capabilities

PCIe - ASPM L1 Sub States spec
https://pcisig.com/sites/default/files/specification_documents/ECN_L1_PM_Substates_with_CLKREQ_31_May_2013_Rev10a.pdf

Testing Done on T210 and T186
- ASPM-L1:
  Verified ASPM-L1 enablement by selecting PCIEASPM_POWERSAVE config
  With the help of Tegra rootport's internal counter registers, confirmed
  link entry in and out of ASPM-L1 state using USB3.0 add-on card, NVMe and NIC
  cards
- ASPM-L1 SubStates:
  Verified ASPM-L1 Substates enablement by selecting PCIEASPM_POWER_SUPERSAVE config
  Confirmed link's entry into L1SS using Westren Digital NVMe card (with Sandisk
  Controller) using Tegra rootport's internal counter registers

Vidya Sagar (3):
  PCI: tegra: Apply sw fixups to support ASPM-L1 Sub-States
  PCI: tegra: fixups to avoid unnecessary wakeup from ASPM-L1.2
  PCI: tegra: Enable ASPM-L1 capability advertisement

 drivers/pci/host/pci-tegra.c | 93 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

-- 
2.7.4

[PATCH V3 1/3] PCI: tegra: Apply sw fixups to support ASPM-L1 Sub-States

[Vidya Sagar]

Programs T_cmrt (Commmon Mode Restore Time) and T_pwr_on (Power On)
values to get them reflected in ASPM-L1 Sub-States capability registers
Also adjusts internal counter values according to 19.2 MHz clk_m value

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
V2:
* no change in this patch

V3:
* no change in this patch

 drivers/pci/host/pci-tegra.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index adae03d671ab..6d68f49f152e 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -40,6 +40,7 @@
 #include <linux/of_pci.h>
 #include <linux/of_platform.h>
 #include <linux/pci.h>
+#include <linux/pci-aspm.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/reset.h>
@@ -191,6 +192,27 @@
 #define RP_PRIV_XP_DL	0x494
 #define  RP_PRIV_XP_DL_GEN2_UPD_FC_TSHOLD	(0x1ff << 1)
 
+#define RP_L1_PM_SUBSTATES_CTL				0xC00
+#define RP_L1_PM_SUBSTATES_CTL_CM_RTIME_MASK		(0xFF << 8)
+#define RP_L1_PM_SUBSTATES_CTL_CM_RTIME_SHIFT		8
+#define RP_L1_PM_SUBSTATES_CTL_T_PWRN_SCL_MASK		(0x3 << 16)
+#define RP_L1_PM_SUBSTATES_CTL_T_PWRN_SCL_SHIFT		16
+#define RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_MASK		(0x1F << 19)
+#define RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_SHIFT		19
+#define RP_L1_PM_SUBSTATES_CTL_HIDE_CAP			(0x1 << 24)
+
+#define RP_L1_PM_SUBSTATES_1_CTL			0xC04
+#define RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK	0x1FFF
+#define RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY		0x26
+
+#define RP_L1_PM_SUBSTATES_2_CTL			0xC08
+#define RP_L1_PM_SUBSTATES_2_CTL_T_L1_2_DLY_MASK	0x1FFF
+#define RP_L1_PM_SUBSTATES_2_CTL_T_L1_2_DLY		0x4D
+#define RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND_MASK	(0xFF << 13)
+#define RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND		(0x13 << 13)
+#define RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND_COMP_MASK	(0xF << 21)
+#define RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND_COMP	(0x2 << 21)
+
 #define RP_RX_HDR_LIMIT	0xe00
 #define  RP_RX_HDR_LIMIT_PW_MASK	(0xff << 8)
 #define  RP_RX_HDR_LIMIT_PW		(0x0e << 8)
@@ -327,6 +349,7 @@ struct tegra_pcie_soc {
 	bool RAW_violation_fixup;
 	bool program_deskew_time;
 	bool updateFC_threshold;
+	bool has_aspm_l1ss;
 };
 
 static inline struct tegra_msi *to_tegra_msi(struct msi_controller *chip)
@@ -2251,6 +2274,37 @@ static void tegra_pcie_apply_sw_fixup(struct tegra_pcie_port *port)
 		value |= RP_VEND_XP_UPDATE_FC_THRESHOLD_T210;
 		writel(value, port->base + RP_VEND_XP);
 	}
+
+	if (soc->has_aspm_l1ss) {
+		/* Set Common Mode Restore Time to 30us */
+		value = readl(port->base + RP_L1_PM_SUBSTATES_CTL);
+		value &= ~RP_L1_PM_SUBSTATES_CTL_CM_RTIME_MASK;
+		value |= (0x1E << RP_L1_PM_SUBSTATES_CTL_CM_RTIME_SHIFT);
+		writel(value, port->base + RP_L1_PM_SUBSTATES_CTL);
+
+		/* set T_Power_On to 70us */
+		value = readl(port->base + RP_L1_PM_SUBSTATES_CTL);
+		value &= ~(RP_L1_PM_SUBSTATES_CTL_T_PWRN_SCL_MASK |
+			RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_MASK);
+		value |= (1 << RP_L1_PM_SUBSTATES_CTL_T_PWRN_SCL_SHIFT) |
+			(7 << RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_SHIFT);
+		writel(value, port->base + RP_L1_PM_SUBSTATES_CTL);
+
+		/* Following is based on clk_m being 19.2 MHz */
+		value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
+		value &= ~RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK;
+		value |= RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY;
+		writel(value, port->base + RP_L1_PM_SUBSTATES_1_CTL);
+
+		value = readl(port->base + RP_L1_PM_SUBSTATES_2_CTL);
+		value &= ~RP_L1_PM_SUBSTATES_2_CTL_T_L1_2_DLY_MASK;
+		value |= RP_L1_PM_SUBSTATES_2_CTL_T_L1_2_DLY;
+		value &= ~RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND_MASK;
+		value |= RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND;
+		value &= ~RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND_COMP_MASK;
+		value |= RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND_COMP;
+		writel(value, port->base + RP_L1_PM_SUBSTATES_2_CTL);
+	}
 }
 /*
  * FIXME: If there are no PCIe cards attached, then calling this function
@@ -2391,6 +2445,7 @@ static const struct tegra_pcie_soc tegra20_pcie = {
 	.RAW_violation_fixup = false,
 	.program_deskew_time = false,
 	.updateFC_threshold = false,
+	.has_aspm_l1ss = false,
 };
 
 static const struct tegra_pcie_soc tegra30_pcie = {
@@ -2412,6 +2467,7 @@ static const struct tegra_pcie_soc tegra30_pcie = {
 	.RAW_violation_fixup = false,
 	.program_deskew_time = false,
 	.updateFC_threshold = false,
+	.has_aspm_l1ss = false,
 };
 
 static const struct tegra_pcie_soc tegra124_pcie = {
@@ -2432,6 +2488,7 @@ static const struct tegra_pcie_soc tegra124_pcie = {
 	.RAW_violation_fixup = true,
 	.program_deskew_time = false,
 	.updateFC_threshold = false,
+	.has_aspm_l1ss = false,
 };
 
 static const struct tegra_pcie_soc tegra210_pcie = {
@@ -2460,6 +2517,7 @@ static const struct tegra_pcie_soc tegra210_pcie = {
 	.RAW_violation_fixup = false,
 	.program_deskew_time = true,
 	.updateFC_threshold = true,
+	.has_aspm_l1ss = true,
 };
 
 static const struct tegra_pcie_soc tegra186_pcie = {
@@ -2481,6 +2539,7 @@ static const struct tegra_pcie_soc tegra186_pcie = {
 	.RAW_violation_fixup = false,
 	.program_deskew_time = false,
 	.updateFC_threshold = false,
+	.has_aspm_l1ss = true,
 };
 
 static const struct of_device_id tegra_pcie_of_match[] = {
-- 
2.7.4

[PATCH V3 2/3] PCI: tegra: fixups to avoid unnecessary wakeup from ASPM-L1.2

[Vidya Sagar]

sets CLKREQ asserted delay to a higher value to avoid
unnecessary wake up from L1.2_ENTRY state for Tegra210

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
V2:
* no change in this patch

V3:
* no change in this patch

 drivers/pci/host/pci-tegra.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 6d68f49f152e..29ee4bb0b7c6 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -204,6 +204,8 @@
 #define RP_L1_PM_SUBSTATES_1_CTL			0xC04
 #define RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK	0x1FFF
 #define RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY		0x26
+#define RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY_MASK		(0x1FF << 13)
+#define RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY		(0x27 << 13)
 
 #define RP_L1_PM_SUBSTATES_2_CTL			0xC08
 #define RP_L1_PM_SUBSTATES_2_CTL_T_L1_2_DLY_MASK	0x1FFF
@@ -350,6 +352,7 @@ struct tegra_pcie_soc {
 	bool program_deskew_time;
 	bool updateFC_threshold;
 	bool has_aspm_l1ss;
+	bool l1ss_rp_wake_fixup;
 };
 
 static inline struct tegra_msi *to_tegra_msi(struct msi_controller *chip)
@@ -2290,6 +2293,16 @@ static void tegra_pcie_apply_sw_fixup(struct tegra_pcie_port *port)
 			(7 << RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_SHIFT);
 		writel(value, port->base + RP_L1_PM_SUBSTATES_CTL);
 
+		if (soc->l1ss_rp_wake_fixup) {
+			/* Set CLKREQ asserted delay greater than Power_Off
+			 * time (2us) to avoid RP wakeup in L1.2_ENTRY
+			 */
+			value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
+			value &= ~RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY_MASK;
+			value |= RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY;
+			writel(value, port->base + RP_L1_PM_SUBSTATES_1_CTL);
+		}
+
 		/* Following is based on clk_m being 19.2 MHz */
 		value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
 		value &= ~RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK;
@@ -2446,6 +2459,7 @@ static const struct tegra_pcie_soc tegra20_pcie = {
 	.program_deskew_time = false,
 	.updateFC_threshold = false,
 	.has_aspm_l1ss = false,
+	.l1ss_rp_wake_fixup = false,
 };
 
 static const struct tegra_pcie_soc tegra30_pcie = {
@@ -2468,6 +2482,7 @@ static const struct tegra_pcie_soc tegra30_pcie = {
 	.program_deskew_time = false,
 	.updateFC_threshold = false,
 	.has_aspm_l1ss = false,
+	.l1ss_rp_wake_fixup = false,
 };
 
 static const struct tegra_pcie_soc tegra124_pcie = {
@@ -2489,6 +2504,7 @@ static const struct tegra_pcie_soc tegra124_pcie = {
 	.program_deskew_time = false,
 	.updateFC_threshold = false,
 	.has_aspm_l1ss = false,
+	.l1ss_rp_wake_fixup = false,
 };
 
 static const struct tegra_pcie_soc tegra210_pcie = {
@@ -2518,6 +2534,7 @@ static const struct tegra_pcie_soc tegra210_pcie = {
 	.program_deskew_time = true,
 	.updateFC_threshold = true,
 	.has_aspm_l1ss = true,
+	.l1ss_rp_wake_fixup = true,
 };
 
 static const struct tegra_pcie_soc tegra186_pcie = {
@@ -2540,6 +2557,7 @@ static const struct tegra_pcie_soc tegra186_pcie = {
 	.program_deskew_time = false,
 	.updateFC_threshold = false,
 	.has_aspm_l1ss = true,
+	.l1ss_rp_wake_fixup = false,
 };
 
 static const struct of_device_id tegra_pcie_of_match[] = {
-- 
2.7.4

[PATCH V3 3/3] PCI: tegra: Enable ASPM-L1 capability advertisement

[Vidya Sagar]

Enables advertisement of ASPM-L1 support in capability
registers of applicable Tegra chips

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
V2:
* no change in this patch

V3:
* no change in this patch

 drivers/pci/host/pci-tegra.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 29ee4bb0b7c6..fb61202ee60f 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -250,6 +250,9 @@
 #define  RP_VEND_XP_UPDATE_FC_THRESHOLD_MASK	(0xff << 18)
 #define  RP_VEND_XP_UPDATE_FC_THRESHOLD_T210	(0x60 << 18)
 
+#define RP_VEND_XP1	0xf04
+#define RP_VEND_XP1_LINK_PVT_CTL_L1_ASPM_SUPPORT	BIT(21)
+
 #define RP_VEND_CTL0	0xf44
 #define  RP_VEND_CTL0_DSK_RST_PULSE_WIDTH_MASK	(0xf << 12)
 #define  RP_VEND_CTL0_DSK_RST_PULSE_WIDTH	(0x9 << 12)
@@ -351,6 +354,7 @@ struct tegra_pcie_soc {
 	bool RAW_violation_fixup;
 	bool program_deskew_time;
 	bool updateFC_threshold;
+	bool has_aspm_l1;
 	bool has_aspm_l1ss;
 	bool l1ss_rp_wake_fixup;
 };
@@ -2214,6 +2218,13 @@ static void tegra_pcie_enable_rp_features(struct tegra_pcie_port *port)
 	value = readl(port->base + RP_VEND_CTL1);
 	value |= RP_VEND_CTL1_ERPT;
 	writel(value, port->base + RP_VEND_CTL1);
+
+	if (port->pcie->soc->has_aspm_l1) {
+		/* Advertise ASPM-L1 state capability*/
+		value = readl(port->base + RP_VEND_XP1);
+		value |= RP_VEND_XP1_LINK_PVT_CTL_L1_ASPM_SUPPORT;
+		writel(value, port->base + RP_VEND_XP1);
+	}
 }
 
 static void tegra_pcie_apply_sw_fixup(struct tegra_pcie_port *port)
@@ -2458,6 +2469,7 @@ static const struct tegra_pcie_soc tegra20_pcie = {
 	.RAW_violation_fixup = false,
 	.program_deskew_time = false,
 	.updateFC_threshold = false,
+	.has_aspm_l1 = false,
 	.has_aspm_l1ss = false,
 	.l1ss_rp_wake_fixup = false,
 };
@@ -2481,6 +2493,7 @@ static const struct tegra_pcie_soc tegra30_pcie = {
 	.RAW_violation_fixup = false,
 	.program_deskew_time = false,
 	.updateFC_threshold = false,
+	.has_aspm_l1 = true,
 	.has_aspm_l1ss = false,
 	.l1ss_rp_wake_fixup = false,
 };
@@ -2503,6 +2516,7 @@ static const struct tegra_pcie_soc tegra124_pcie = {
 	.RAW_violation_fixup = true,
 	.program_deskew_time = false,
 	.updateFC_threshold = false,
+	.has_aspm_l1 = true,
 	.has_aspm_l1ss = false,
 	.l1ss_rp_wake_fixup = false,
 };
@@ -2533,6 +2547,7 @@ static const struct tegra_pcie_soc tegra210_pcie = {
 	.RAW_violation_fixup = false,
 	.program_deskew_time = true,
 	.updateFC_threshold = true,
+	.has_aspm_l1 = true,
 	.has_aspm_l1ss = true,
 	.l1ss_rp_wake_fixup = true,
 };
@@ -2556,6 +2571,7 @@ static const struct tegra_pcie_soc tegra186_pcie = {
 	.RAW_violation_fixup = false,
 	.program_deskew_time = false,
 	.updateFC_threshold = false,
+	.has_aspm_l1 = true,
 	.has_aspm_l1ss = true,
 	.l1ss_rp_wake_fixup = false,
 };
-- 
2.7.4

Re: [PATCH V3 1/3] PCI: tegra: Apply sw fixups to support ASPM-L1 Sub-States

[Bjorn Helgaas]

On Sun, Nov 12, 2017 at 06:47:52PM +0530, Vidya Sagar wrote:
> Programs T_cmrt (Commmon Mode Restore Time) and T_pwr_on (Power On)

Please use the exact names used in the spec.  In this case, I think
that would be "Port Common_Mode_Restore_Time" and "Port T_POWER_ON"
(from the L1 PM Substates Capabilities Register, sec 7.33.2).
Searching the spec for "T_cmrt" isn't going to find anything useful.

These look like things that maybe should be in the DT because they
depend on external component values, so you might need different
values depending on what board the Tegra is on?  Same for clk_m?

> values to get them reflected in ASPM-L1 Sub-States capability registers
> Also adjusts internal counter values according to 19.2 MHz clk_m value
> 
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> V2:
> * no change in this patch
> 
> V3:
> * no change in this patch
> 
>  drivers/pci/host/pci-tegra.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> index adae03d671ab..6d68f49f152e 100644
> --- a/drivers/pci/host/pci-tegra.c
> +++ b/drivers/pci/host/pci-tegra.c
> @@ -40,6 +40,7 @@
>  #include <linux/of_pci.h>
>  #include <linux/of_platform.h>
>  #include <linux/pci.h>
> +#include <linux/pci-aspm.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
>  #include <linux/reset.h>
> @@ -191,6 +192,27 @@
>  #define RP_PRIV_XP_DL	0x494
>  #define  RP_PRIV_XP_DL_GEN2_UPD_FC_TSHOLD	(0x1ff << 1)
>  
> +#define RP_L1_PM_SUBSTATES_CTL				0xC00
> +#define RP_L1_PM_SUBSTATES_CTL_CM_RTIME_MASK		(0xFF << 8)
> +#define RP_L1_PM_SUBSTATES_CTL_CM_RTIME_SHIFT		8
> +#define RP_L1_PM_SUBSTATES_CTL_T_PWRN_SCL_MASK		(0x3 << 16)
> +#define RP_L1_PM_SUBSTATES_CTL_T_PWRN_SCL_SHIFT		16
> +#define RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_MASK		(0x1F << 19)
> +#define RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_SHIFT		19
> +#define RP_L1_PM_SUBSTATES_CTL_HIDE_CAP			(0x1 << 24)
> +
> +#define RP_L1_PM_SUBSTATES_1_CTL			0xC04
> +#define RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK	0x1FFF
> +#define RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY		0x26
> +
> +#define RP_L1_PM_SUBSTATES_2_CTL			0xC08
> +#define RP_L1_PM_SUBSTATES_2_CTL_T_L1_2_DLY_MASK	0x1FFF
> +#define RP_L1_PM_SUBSTATES_2_CTL_T_L1_2_DLY		0x4D
> +#define RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND_MASK	(0xFF << 13)
> +#define RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND		(0x13 << 13)
> +#define RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND_COMP_MASK	(0xF << 21)
> +#define RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND_COMP	(0x2 << 21)
> +
>  #define RP_RX_HDR_LIMIT	0xe00
>  #define  RP_RX_HDR_LIMIT_PW_MASK	(0xff << 8)
>  #define  RP_RX_HDR_LIMIT_PW		(0x0e << 8)
> @@ -327,6 +349,7 @@ struct tegra_pcie_soc {
>  	bool RAW_violation_fixup;
>  	bool program_deskew_time;
>  	bool updateFC_threshold;
> +	bool has_aspm_l1ss;
>  };
>  
>  static inline struct tegra_msi *to_tegra_msi(struct msi_controller *chip)
> @@ -2251,6 +2274,37 @@ static void tegra_pcie_apply_sw_fixup(struct tegra_pcie_port *port)
>  		value |= RP_VEND_XP_UPDATE_FC_THRESHOLD_T210;
>  		writel(value, port->base + RP_VEND_XP);
>  	}
> +
> +	if (soc->has_aspm_l1ss) {
> +		/* Set Common Mode Restore Time to 30us */
> +		value = readl(port->base + RP_L1_PM_SUBSTATES_CTL);
> +		value &= ~RP_L1_PM_SUBSTATES_CTL_CM_RTIME_MASK;
> +		value |= (0x1E << RP_L1_PM_SUBSTATES_CTL_CM_RTIME_SHIFT);
> +		writel(value, port->base + RP_L1_PM_SUBSTATES_CTL);
> +
> +		/* set T_Power_On to 70us */
> +		value = readl(port->base + RP_L1_PM_SUBSTATES_CTL);
> +		value &= ~(RP_L1_PM_SUBSTATES_CTL_T_PWRN_SCL_MASK |
> +			RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_MASK);
> +		value |= (1 << RP_L1_PM_SUBSTATES_CTL_T_PWRN_SCL_SHIFT) |
> +			(7 << RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_SHIFT);
> +		writel(value, port->base + RP_L1_PM_SUBSTATES_CTL);
> +
> +		/* Following is based on clk_m being 19.2 MHz */
> +		value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
> +		value &= ~RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK;
> +		value |= RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY;
> +		writel(value, port->base + RP_L1_PM_SUBSTATES_1_CTL);
> +
> +		value = readl(port->base + RP_L1_PM_SUBSTATES_2_CTL);
> +		value &= ~RP_L1_PM_SUBSTATES_2_CTL_T_L1_2_DLY_MASK;
> +		value |= RP_L1_PM_SUBSTATES_2_CTL_T_L1_2_DLY;
> +		value &= ~RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND_MASK;
> +		value |= RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND;
> +		value &= ~RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND_COMP_MASK;
> +		value |= RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND_COMP;
> +		writel(value, port->base + RP_L1_PM_SUBSTATES_2_CTL);
> +	}
>  }
>  /*
>   * FIXME: If there are no PCIe cards attached, then calling this function
> @@ -2391,6 +2445,7 @@ static const struct tegra_pcie_soc tegra20_pcie = {
>  	.RAW_violation_fixup = false,
>  	.program_deskew_time = false,
>  	.updateFC_threshold = false,
> +	.has_aspm_l1ss = false,
>  };
>  
>  static const struct tegra_pcie_soc tegra30_pcie = {
> @@ -2412,6 +2467,7 @@ static const struct tegra_pcie_soc tegra30_pcie = {
>  	.RAW_violation_fixup = false,
>  	.program_deskew_time = false,
>  	.updateFC_threshold = false,
> +	.has_aspm_l1ss = false,
>  };
>  
>  static const struct tegra_pcie_soc tegra124_pcie = {
> @@ -2432,6 +2488,7 @@ static const struct tegra_pcie_soc tegra124_pcie = {
>  	.RAW_violation_fixup = true,
>  	.program_deskew_time = false,
>  	.updateFC_threshold = false,
> +	.has_aspm_l1ss = false,
>  };
>  
>  static const struct tegra_pcie_soc tegra210_pcie = {
> @@ -2460,6 +2517,7 @@ static const struct tegra_pcie_soc tegra210_pcie = {
>  	.RAW_violation_fixup = false,
>  	.program_deskew_time = true,
>  	.updateFC_threshold = true,
> +	.has_aspm_l1ss = true,
>  };
>  
>  static const struct tegra_pcie_soc tegra186_pcie = {
> @@ -2481,6 +2539,7 @@ static const struct tegra_pcie_soc tegra186_pcie = {
>  	.RAW_violation_fixup = false,
>  	.program_deskew_time = false,
>  	.updateFC_threshold = false,
> +	.has_aspm_l1ss = true,
>  };
>  
>  static const struct of_device_id tegra_pcie_of_match[] = {
> -- 
> 2.7.4
> 

Re: [PATCH V3 2/3] PCI: tegra: fixups to avoid unnecessary wakeup from ASPM-L1.2

[Bjorn Helgaas]

s/PCI: tegra: fixups to avoid unnecessary .../
 /PCI: tegra: <Capitalized verb> .../

On Sun, Nov 12, 2017 at 06:47:53PM +0530, Vidya Sagar wrote:
> sets CLKREQ asserted delay to a higher value to avoid
> unnecessary wake up from L1.2_ENTRY state for Tegra210

"L1.2.Entry" to match the spec sec 5.5.3.1 (at least I assume that's
what this refers to).

> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> V2:
> * no change in this patch
> 
> V3:
> * no change in this patch
> 
>  drivers/pci/host/pci-tegra.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> index 6d68f49f152e..29ee4bb0b7c6 100644
> --- a/drivers/pci/host/pci-tegra.c
> +++ b/drivers/pci/host/pci-tegra.c
> @@ -204,6 +204,8 @@
>  #define RP_L1_PM_SUBSTATES_1_CTL			0xC04
>  #define RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK	0x1FFF
>  #define RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY		0x26
> +#define RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY_MASK		(0x1FF << 13)
> +#define RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY		(0x27 << 13)
>  
>  #define RP_L1_PM_SUBSTATES_2_CTL			0xC08
>  #define RP_L1_PM_SUBSTATES_2_CTL_T_L1_2_DLY_MASK	0x1FFF
> @@ -350,6 +352,7 @@ struct tegra_pcie_soc {
>  	bool program_deskew_time;
>  	bool updateFC_threshold;
>  	bool has_aspm_l1ss;
> +	bool l1ss_rp_wake_fixup;
>  };
>  
>  static inline struct tegra_msi *to_tegra_msi(struct msi_controller *chip)
> @@ -2290,6 +2293,16 @@ static void tegra_pcie_apply_sw_fixup(struct tegra_pcie_port *port)
>  			(7 << RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_SHIFT);
>  		writel(value, port->base + RP_L1_PM_SUBSTATES_CTL);
>  
> +		if (soc->l1ss_rp_wake_fixup) {
> +			/* Set CLKREQ asserted delay greater than Power_Off
> +			 * time (2us) to avoid RP wakeup in L1.2_ENTRY
> +			 */

Use standard multi-line comment formatting (and "L1.2.Entry" as above).

I assume "CLKREQ asserted delay" is a Tegra-internal thing, since it
doesn't obviously correspond to a timing parameter in the spec.

And I assume it doesn't depend on any of the OS-writable things in the
L1 PM Substates control registers, since this fixup isn't executed
during config writes to those registers?

Does this depend on any circuit details outside of Tegra, i.e., should
it be described via DT?

> +			value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
> +			value &= ~RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY_MASK;
> +			value |= RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY;
> +			writel(value, port->base + RP_L1_PM_SUBSTATES_1_CTL);
> +		}
> +
>  		/* Following is based on clk_m being 19.2 MHz */
>  		value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
>  		value &= ~RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK;
> @@ -2446,6 +2459,7 @@ static const struct tegra_pcie_soc tegra20_pcie = {
>  	.program_deskew_time = false,
>  	.updateFC_threshold = false,
>  	.has_aspm_l1ss = false,
> +	.l1ss_rp_wake_fixup = false,
>  };
>  
>  static const struct tegra_pcie_soc tegra30_pcie = {
> @@ -2468,6 +2482,7 @@ static const struct tegra_pcie_soc tegra30_pcie = {
>  	.program_deskew_time = false,
>  	.updateFC_threshold = false,
>  	.has_aspm_l1ss = false,
> +	.l1ss_rp_wake_fixup = false,
>  };
>  
>  static const struct tegra_pcie_soc tegra124_pcie = {
> @@ -2489,6 +2504,7 @@ static const struct tegra_pcie_soc tegra124_pcie = {
>  	.program_deskew_time = false,
>  	.updateFC_threshold = false,
>  	.has_aspm_l1ss = false,
> +	.l1ss_rp_wake_fixup = false,
>  };
>  
>  static const struct tegra_pcie_soc tegra210_pcie = {
> @@ -2518,6 +2534,7 @@ static const struct tegra_pcie_soc tegra210_pcie = {
>  	.program_deskew_time = true,
>  	.updateFC_threshold = true,
>  	.has_aspm_l1ss = true,
> +	.l1ss_rp_wake_fixup = true,
>  };
>  
>  static const struct tegra_pcie_soc tegra186_pcie = {
> @@ -2540,6 +2557,7 @@ static const struct tegra_pcie_soc tegra186_pcie = {
>  	.program_deskew_time = false,
>  	.updateFC_threshold = false,
>  	.has_aspm_l1ss = true,
> +	.l1ss_rp_wake_fixup = false,
>  };
>  
>  static const struct of_device_id tegra_pcie_of_match[] = {
> -- 
> 2.7.4
> 

Re: [PATCH V3 3/3] PCI: tegra: Enable ASPM-L1 capability advertisement

[Bjorn Helgaas]

s/Enable ASPM-L1 capability advertisement/
 /Advertise ASPM L1 PM Substates support/

On Sun, Nov 12, 2017 at 06:47:54PM +0530, Vidya Sagar wrote:
> Enables advertisement of ASPM-L1 support in capability
> registers of applicable Tegra chips

Actually, I'm a little confused about whether this has to do with ASPM
L1 support (which would be advertised in the Link Capabilities
register) or the ASPM L1 Substates support (which would be advertised
in the L1 PM Substates Capabilities register)?

> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> V2:
> * no change in this patch
> 
> V3:
> * no change in this patch
> 
>  drivers/pci/host/pci-tegra.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> index 29ee4bb0b7c6..fb61202ee60f 100644
> --- a/drivers/pci/host/pci-tegra.c
> +++ b/drivers/pci/host/pci-tegra.c
> @@ -250,6 +250,9 @@
>  #define  RP_VEND_XP_UPDATE_FC_THRESHOLD_MASK	(0xff << 18)
>  #define  RP_VEND_XP_UPDATE_FC_THRESHOLD_T210	(0x60 << 18)
>  
> +#define RP_VEND_XP1	0xf04
> +#define RP_VEND_XP1_LINK_PVT_CTL_L1_ASPM_SUPPORT	BIT(21)
> +
>  #define RP_VEND_CTL0	0xf44
>  #define  RP_VEND_CTL0_DSK_RST_PULSE_WIDTH_MASK	(0xf << 12)
>  #define  RP_VEND_CTL0_DSK_RST_PULSE_WIDTH	(0x9 << 12)
> @@ -351,6 +354,7 @@ struct tegra_pcie_soc {
>  	bool RAW_violation_fixup;
>  	bool program_deskew_time;
>  	bool updateFC_threshold;
> +	bool has_aspm_l1;
>  	bool has_aspm_l1ss;
>  	bool l1ss_rp_wake_fixup;
>  };
> @@ -2214,6 +2218,13 @@ static void tegra_pcie_enable_rp_features(struct tegra_pcie_port *port)
>  	value = readl(port->base + RP_VEND_CTL1);
>  	value |= RP_VEND_CTL1_ERPT;
>  	writel(value, port->base + RP_VEND_CTL1);
> +
> +	if (port->pcie->soc->has_aspm_l1) {
> +		/* Advertise ASPM-L1 state capability*/
> +		value = readl(port->base + RP_VEND_XP1);
> +		value |= RP_VEND_XP1_LINK_PVT_CTL_L1_ASPM_SUPPORT;
> +		writel(value, port->base + RP_VEND_XP1);
> +	}
>  }
>  
>  static void tegra_pcie_apply_sw_fixup(struct tegra_pcie_port *port)
> @@ -2458,6 +2469,7 @@ static const struct tegra_pcie_soc tegra20_pcie = {
>  	.RAW_violation_fixup = false,
>  	.program_deskew_time = false,
>  	.updateFC_threshold = false,
> +	.has_aspm_l1 = false,
>  	.has_aspm_l1ss = false,
>  	.l1ss_rp_wake_fixup = false,
>  };
> @@ -2481,6 +2493,7 @@ static const struct tegra_pcie_soc tegra30_pcie = {
>  	.RAW_violation_fixup = false,
>  	.program_deskew_time = false,
>  	.updateFC_threshold = false,
> +	.has_aspm_l1 = true,
>  	.has_aspm_l1ss = false,
>  	.l1ss_rp_wake_fixup = false,
>  };
> @@ -2503,6 +2516,7 @@ static const struct tegra_pcie_soc tegra124_pcie = {
>  	.RAW_violation_fixup = true,
>  	.program_deskew_time = false,
>  	.updateFC_threshold = false,
> +	.has_aspm_l1 = true,
>  	.has_aspm_l1ss = false,
>  	.l1ss_rp_wake_fixup = false,
>  };
> @@ -2533,6 +2547,7 @@ static const struct tegra_pcie_soc tegra210_pcie = {
>  	.RAW_violation_fixup = false,
>  	.program_deskew_time = true,
>  	.updateFC_threshold = true,
> +	.has_aspm_l1 = true,
>  	.has_aspm_l1ss = true,
>  	.l1ss_rp_wake_fixup = true,
>  };
> @@ -2556,6 +2571,7 @@ static const struct tegra_pcie_soc tegra186_pcie = {
>  	.RAW_violation_fixup = false,
>  	.program_deskew_time = false,
>  	.updateFC_threshold = false,
> +	.has_aspm_l1 = true,
>  	.has_aspm_l1ss = true,
>  	.l1ss_rp_wake_fixup = false,
>  };
> -- 
> 2.7.4
> 

Re: [PATCH V3 2/3] PCI: tegra: fixups to avoid unnecessary wakeup from ASPM-L1.2

[Lorenzo Pieralisi]

On Mon, Nov 20, 2017 at 03:30:52PM -0600, Bjorn Helgaas wrote:

[...]

> >  static inline struct tegra_msi *to_tegra_msi(struct msi_controller *chip)
> > @@ -2290,6 +2293,16 @@ static void tegra_pcie_apply_sw_fixup(struct tegra_pcie_port *port)
> >  			(7 << RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_SHIFT);
> >  		writel(value, port->base + RP_L1_PM_SUBSTATES_CTL);
> >  
> > +		if (soc->l1ss_rp_wake_fixup) {
> > +			/* Set CLKREQ asserted delay greater than Power_Off
> > +			 * time (2us) to avoid RP wakeup in L1.2_ENTRY
> > +			 */
> 
> Use standard multi-line comment formatting (and "L1.2.Entry" as above).
> 
> I assume "CLKREQ asserted delay" is a Tegra-internal thing, since it
> doesn't obviously correspond to a timing parameter in the spec.
> 
> And I assume it doesn't depend on any of the OS-writable things in the
> L1 PM Substates control registers, since this fixup isn't executed
> during config writes to those registers?
> 
> Does this depend on any circuit details outside of Tegra, i.e., should
> it be described via DT?

Yes, I have noticed too that there are lots of hardcoded values in this
driver that may benefit from some Tegra DT bindings instead of
hardcoding everything in the kernel.

Thanks,
Lorenzo

> > +			value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
> > +			value &= ~RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY_MASK;
> > +			value |= RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY;
> > +			writel(value, port->base + RP_L1_PM_SUBSTATES_1_CTL);
> > +		}
> > +
> >  		/* Following is based on clk_m being 19.2 MHz */
> >  		value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
> >  		value &= ~RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK;
> > @@ -2446,6 +2459,7 @@ static const struct tegra_pcie_soc tegra20_pcie = {
> >  	.program_deskew_time = false,
> >  	.updateFC_threshold = false,
> >  	.has_aspm_l1ss = false,
> > +	.l1ss_rp_wake_fixup = false,
> >  };
> >  
> >  static const struct tegra_pcie_soc tegra30_pcie = {
> > @@ -2468,6 +2482,7 @@ static const struct tegra_pcie_soc tegra30_pcie = {
> >  	.program_deskew_time = false,
> >  	.updateFC_threshold = false,
> >  	.has_aspm_l1ss = false,
> > +	.l1ss_rp_wake_fixup = false,
> >  };
> >  
> >  static const struct tegra_pcie_soc tegra124_pcie = {
> > @@ -2489,6 +2504,7 @@ static const struct tegra_pcie_soc tegra124_pcie = {
> >  	.program_deskew_time = false,
> >  	.updateFC_threshold = false,
> >  	.has_aspm_l1ss = false,
> > +	.l1ss_rp_wake_fixup = false,
> >  };
> >  
> >  static const struct tegra_pcie_soc tegra210_pcie = {
> > @@ -2518,6 +2534,7 @@ static const struct tegra_pcie_soc tegra210_pcie = {
> >  	.program_deskew_time = true,
> >  	.updateFC_threshold = true,
> >  	.has_aspm_l1ss = true,
> > +	.l1ss_rp_wake_fixup = true,
> >  };
> >  
> >  static const struct tegra_pcie_soc tegra186_pcie = {
> > @@ -2540,6 +2557,7 @@ static const struct tegra_pcie_soc tegra186_pcie = {
> >  	.program_deskew_time = false,
> >  	.updateFC_threshold = false,
> >  	.has_aspm_l1ss = true,
> > +	.l1ss_rp_wake_fixup = false,
> >  };
> >  
> >  static const struct of_device_id tegra_pcie_of_match[] = {
> > -- 
> > 2.7.4
> > 

Re: [PATCH V3 0/3] Add ASPM-L1 Substates support for Tegra

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2508 bytes --]

On Sun, Nov 12, 2017 at 06:47:51PM +0530, Vidya Sagar wrote:
> Tegra chips T210 and T186 support ASPM-L1 Substates (i.e. L1.1 and L1.2)
> This patch series
> - applies fixups to reflect correct capability values for
>   T_cmrt (Common Mode Restore Time) and
>   T_pwr_on (Power On)
>   and adjusts counter values for 19.2 MHz of clk_m
> - applies fixup specific to T210 to avoid unnecessary wake ups from L1.2 state
> - enables advertisement of ASPM-L1 support in capability register
> 
> NOTE:- This version of patch series drops a patch (that was present till V2 series)
> that introduces an API for host controller drivers to specify LTR_L1.2_THRESHOLD
> value as more details and better understanding of spec is required to program
> LTR_L1.2_THRESHOLD value.
> It also reorders patches to appropriately have software fixups and advertisement
> of capabilities
> 
> PCIe - ASPM L1 Sub States spec
> https://pcisig.com/sites/default/files/specification_documents/ECN_L1_PM_Substates_with_CLKREQ_31_May_2013_Rev10a.pdf
> 
> Testing Done on T210 and T186
> - ASPM-L1:
>   Verified ASPM-L1 enablement by selecting PCIEASPM_POWERSAVE config
>   With the help of Tegra rootport's internal counter registers, confirmed
>   link entry in and out of ASPM-L1 state using USB3.0 add-on card, NVMe and NIC
>   cards
> - ASPM-L1 SubStates:
>   Verified ASPM-L1 Substates enablement by selecting PCIEASPM_POWER_SUPERSAVE config
>   Confirmed link's entry into L1SS using Westren Digital NVMe card (with Sandisk
>   Controller) using Tegra rootport's internal counter registers
> 
> Vidya Sagar (3):
>   PCI: tegra: Apply sw fixups to support ASPM-L1 Sub-States
>   PCI: tegra: fixups to avoid unnecessary wakeup from ASPM-L1.2
>   PCI: tegra: Enable ASPM-L1 capability advertisement
> 
>  drivers/pci/host/pci-tegra.c | 93 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 93 insertions(+)

Looks like these don't apply to an upstream kernel. Do you have any
other patches in your tree that would cause the context to be different?
I see a couple of fields in struct tegra_pcie_soc that aren't in the
driver upstream and I didn't find any patches on the list that add them.

Can you rebase this on top of a recent linux-next or -rc and resend?
Also, if the series depends on any other patches that have been sent to
the list but haven't been merged yet, please include a reference to the
patchwork or email thread for the dependencies.

Thanks,
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH V3 3/3] PCI: tegra: Enable ASPM-L1 capability advertisement

[Vidya Sagar]

On Tuesday 21 November 2017 03:07 AM, Bjorn Helgaas wrote:
> s/Enable ASPM-L1 capability advertisement/
>   /Advertise ASPM L1 PM Substates support/
This patch controls advertisement of ASPM-L1 capability (not ASPM-L1 
Substates)
>
> On Sun, Nov 12, 2017 at 06:47:54PM +0530, Vidya Sagar wrote:
>> Enables advertisement of ASPM-L1 support in capability
>> registers of applicable Tegra chips
> Actually, I'm a little confused about whether this has to do with ASPM
> L1 support (which would be advertised in the Link Capabilities
> register) or the ASPM L1 Substates support (which would be advertised
> in the L1 PM Substates Capabilities register)?
This is to do with ASPM L1 support (and not ASPM L1 Substates)
But, since ASPM-L1SS needs ASPM-L1 anyway, I think it is ok to enable
advertisement of ASPM-L1 in the last patch of the series.
>
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> ---
>> V2:
>> * no change in this patch
>>
>> V3:
>> * no change in this patch
>>
>>   drivers/pci/host/pci-tegra.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
>> index 29ee4bb0b7c6..fb61202ee60f 100644
>> --- a/drivers/pci/host/pci-tegra.c
>> +++ b/drivers/pci/host/pci-tegra.c
>> @@ -250,6 +250,9 @@
>>   #define  RP_VEND_XP_UPDATE_FC_THRESHOLD_MASK	(0xff << 18)
>>   #define  RP_VEND_XP_UPDATE_FC_THRESHOLD_T210	(0x60 << 18)
>>   
>> +#define RP_VEND_XP1	0xf04
>> +#define RP_VEND_XP1_LINK_PVT_CTL_L1_ASPM_SUPPORT	BIT(21)
>> +
>>   #define RP_VEND_CTL0	0xf44
>>   #define  RP_VEND_CTL0_DSK_RST_PULSE_WIDTH_MASK	(0xf << 12)
>>   #define  RP_VEND_CTL0_DSK_RST_PULSE_WIDTH	(0x9 << 12)
>> @@ -351,6 +354,7 @@ struct tegra_pcie_soc {
>>   	bool RAW_violation_fixup;
>>   	bool program_deskew_time;
>>   	bool updateFC_threshold;
>> +	bool has_aspm_l1;
>>   	bool has_aspm_l1ss;
>>   	bool l1ss_rp_wake_fixup;
>>   };
>> @@ -2214,6 +2218,13 @@ static void tegra_pcie_enable_rp_features(struct tegra_pcie_port *port)
>>   	value = readl(port->base + RP_VEND_CTL1);
>>   	value |= RP_VEND_CTL1_ERPT;
>>   	writel(value, port->base + RP_VEND_CTL1);
>> +
>> +	if (port->pcie->soc->has_aspm_l1) {
>> +		/* Advertise ASPM-L1 state capability*/
>> +		value = readl(port->base + RP_VEND_XP1);
>> +		value |= RP_VEND_XP1_LINK_PVT_CTL_L1_ASPM_SUPPORT;
>> +		writel(value, port->base + RP_VEND_XP1);
>> +	}
>>   }
>>   
>>   static void tegra_pcie_apply_sw_fixup(struct tegra_pcie_port *port)
>> @@ -2458,6 +2469,7 @@ static const struct tegra_pcie_soc tegra20_pcie = {
>>   	.RAW_violation_fixup = false,
>>   	.program_deskew_time = false,
>>   	.updateFC_threshold = false,
>> +	.has_aspm_l1 = false,
>>   	.has_aspm_l1ss = false,
>>   	.l1ss_rp_wake_fixup = false,
>>   };
>> @@ -2481,6 +2493,7 @@ static const struct tegra_pcie_soc tegra30_pcie = {
>>   	.RAW_violation_fixup = false,
>>   	.program_deskew_time = false,
>>   	.updateFC_threshold = false,
>> +	.has_aspm_l1 = true,
>>   	.has_aspm_l1ss = false,
>>   	.l1ss_rp_wake_fixup = false,
>>   };
>> @@ -2503,6 +2516,7 @@ static const struct tegra_pcie_soc tegra124_pcie = {
>>   	.RAW_violation_fixup = true,
>>   	.program_deskew_time = false,
>>   	.updateFC_threshold = false,
>> +	.has_aspm_l1 = true,
>>   	.has_aspm_l1ss = false,
>>   	.l1ss_rp_wake_fixup = false,
>>   };
>> @@ -2533,6 +2547,7 @@ static const struct tegra_pcie_soc tegra210_pcie = {
>>   	.RAW_violation_fixup = false,
>>   	.program_deskew_time = true,
>>   	.updateFC_threshold = true,
>> +	.has_aspm_l1 = true,
>>   	.has_aspm_l1ss = true,
>>   	.l1ss_rp_wake_fixup = true,
>>   };
>> @@ -2556,6 +2571,7 @@ static const struct tegra_pcie_soc tegra186_pcie = {
>>   	.RAW_violation_fixup = false,
>>   	.program_deskew_time = false,
>>   	.updateFC_threshold = false,
>> +	.has_aspm_l1 = true,
>>   	.has_aspm_l1ss = true,
>>   	.l1ss_rp_wake_fixup = false,
>>   };
>> -- 
>> 2.7.4
>>

Re: [PATCH V3 2/3] PCI: tegra: fixups to avoid unnecessary wakeup from ASPM-L1.2

[Vidya Sagar]

On Tuesday 21 November 2017 03:00 AM, Bjorn Helgaas wrote:
> s/PCI: tegra: fixups to avoid unnecessary .../
>   /PCI: tegra: <Capitalized verb> .../
I'll take care of this in next patch
> On Sun, Nov 12, 2017 at 06:47:53PM +0530, Vidya Sagar wrote:
>> sets CLKREQ asserted delay to a higher value to avoid
>> unnecessary wake up from L1.2_ENTRY state for Tegra210
> "L1.2.Entry" to match the spec sec 5.5.3.1 (at least I assume that's
> what this refers to).
I'll take care of this in next patch
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> ---
>> V2:
>> * no change in this patch
>>
>> V3:
>> * no change in this patch
>>
>>   drivers/pci/host/pci-tegra.c | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
>> index 6d68f49f152e..29ee4bb0b7c6 100644
>> --- a/drivers/pci/host/pci-tegra.c
>> +++ b/drivers/pci/host/pci-tegra.c
>> @@ -204,6 +204,8 @@
>>   #define RP_L1_PM_SUBSTATES_1_CTL			0xC04
>>   #define RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK	0x1FFF
>>   #define RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY		0x26
>> +#define RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY_MASK		(0x1FF << 13)
>> +#define RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY		(0x27 << 13)
>>   
>>   #define RP_L1_PM_SUBSTATES_2_CTL			0xC08
>>   #define RP_L1_PM_SUBSTATES_2_CTL_T_L1_2_DLY_MASK	0x1FFF
>> @@ -350,6 +352,7 @@ struct tegra_pcie_soc {
>>   	bool program_deskew_time;
>>   	bool updateFC_threshold;
>>   	bool has_aspm_l1ss;
>> +	bool l1ss_rp_wake_fixup;
>>   };
>>   
>>   static inline struct tegra_msi *to_tegra_msi(struct msi_controller *chip)
>> @@ -2290,6 +2293,16 @@ static void tegra_pcie_apply_sw_fixup(struct tegra_pcie_port *port)
>>   			(7 << RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_SHIFT);
>>   		writel(value, port->base + RP_L1_PM_SUBSTATES_CTL);
>>   
>> +		if (soc->l1ss_rp_wake_fixup) {
>> +			/* Set CLKREQ asserted delay greater than Power_Off
>> +			 * time (2us) to avoid RP wakeup in L1.2_ENTRY
>> +			 */
> Use standard multi-line comment formatting (and "L1.2.Entry" as above).
I'll take care of it in next patch
> I assume "CLKREQ asserted delay" is a Tegra-internal thing, since it
> doesn't obviously correspond to a timing parameter in the spec.
Yes. Its Tegra's internal thing
> And I assume it doesn't depend on any of the OS-writable things in the
> L1 PM Substates control registers, since this fixup isn't executed
> during config writes to those registers?
Yes. It doesn't depend on any OS-writable value in L1 PM Substates 
control registers.
Its a one time programmable value to be written before root port's LTSSM 
starts.
> Does this depend on any circuit details outside of Tegra, i.e., should
> it be described via DT?	
Nope. It doesn't depend on any external (to Tegra) circuitry.
>> +			value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
>> +			value &= ~RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY_MASK;
>> +			value |= RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY;
>> +			writel(value, port->base + RP_L1_PM_SUBSTATES_1_CTL);
>> +		}
>> +
>>   		/* Following is based on clk_m being 19.2 MHz */
>>   		value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
>>   		value &= ~RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK;
>> @@ -2446,6 +2459,7 @@ static const struct tegra_pcie_soc tegra20_pcie = {
>>   	.program_deskew_time = false,
>>   	.updateFC_threshold = false,
>>   	.has_aspm_l1ss = false,
>> +	.l1ss_rp_wake_fixup = false,
>>   };
>>   
>>   static const struct tegra_pcie_soc tegra30_pcie = {
>> @@ -2468,6 +2482,7 @@ static const struct tegra_pcie_soc tegra30_pcie = {
>>   	.program_deskew_time = false,
>>   	.updateFC_threshold = false,
>>   	.has_aspm_l1ss = false,
>> +	.l1ss_rp_wake_fixup = false,
>>   };
>>   
>>   static const struct tegra_pcie_soc tegra124_pcie = {
>> @@ -2489,6 +2504,7 @@ static const struct tegra_pcie_soc tegra124_pcie = {
>>   	.program_deskew_time = false,
>>   	.updateFC_threshold = false,
>>   	.has_aspm_l1ss = false,
>> +	.l1ss_rp_wake_fixup = false,
>>   };
>>   
>>   static const struct tegra_pcie_soc tegra210_pcie = {
>> @@ -2518,6 +2534,7 @@ static const struct tegra_pcie_soc tegra210_pcie = {
>>   	.program_deskew_time = true,
>>   	.updateFC_threshold = true,
>>   	.has_aspm_l1ss = true,
>> +	.l1ss_rp_wake_fixup = true,
>>   };
>>   
>>   static const struct tegra_pcie_soc tegra186_pcie = {
>> @@ -2540,6 +2557,7 @@ static const struct tegra_pcie_soc tegra186_pcie = {
>>   	.program_deskew_time = false,
>>   	.updateFC_threshold = false,
>>   	.has_aspm_l1ss = true,
>> +	.l1ss_rp_wake_fixup = false,
>>   };
>>   
>>   static const struct of_device_id tegra_pcie_of_match[] = {
>> -- 
>> 2.7.4
>>

Re: [PATCH V3 1/3] PCI: tegra: Apply sw fixups to support ASPM-L1 Sub-States

[Vidya Sagar]

On Tuesday 21 November 2017 02:51 AM, Bjorn Helgaas wrote:
> On Sun, Nov 12, 2017 at 06:47:52PM +0530, Vidya Sagar wrote:
>> Programs T_cmrt (Commmon Mode Restore Time) and T_pwr_on (Power On)
> Please use the exact names used in the spec.  In this case, I think
> that would be "Port Common_Mode_Restore_Time" and "Port T_POWER_ON"
> (from the L1 PM Substates Capabilities Register, sec 7.33.2).
> Searching the spec for "T_cmrt" isn't going to find anything useful.
I'll take care of this in next patch
> These look like things that maybe should be in the DT because they
> depend on external component values, so you might need different
> values depending on what board the Tegra is on?  Same for clk_m?
I'm afraid not. These are fixed for Tegra including clk_m.
>
>> values to get them reflected in ASPM-L1 Sub-States capability registers
>> Also adjusts internal counter values according to 19.2 MHz clk_m value
>>
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> ---
>> V2:
>> * no change in this patch
>>
>> V3:
>> * no change in this patch
>>
>>   drivers/pci/host/pci-tegra.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 59 insertions(+)
>>
>> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
>> index adae03d671ab..6d68f49f152e 100644
>> --- a/drivers/pci/host/pci-tegra.c
>> +++ b/drivers/pci/host/pci-tegra.c
>> @@ -40,6 +40,7 @@
>>   #include <linux/of_pci.h>
>>   #include <linux/of_platform.h>
>>   #include <linux/pci.h>
>> +#include <linux/pci-aspm.h>
>>   #include <linux/phy/phy.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/reset.h>
>> @@ -191,6 +192,27 @@
>>   #define RP_PRIV_XP_DL	0x494
>>   #define  RP_PRIV_XP_DL_GEN2_UPD_FC_TSHOLD	(0x1ff << 1)
>>   
>> +#define RP_L1_PM_SUBSTATES_CTL				0xC00
>> +#define RP_L1_PM_SUBSTATES_CTL_CM_RTIME_MASK		(0xFF << 8)
>> +#define RP_L1_PM_SUBSTATES_CTL_CM_RTIME_SHIFT		8
>> +#define RP_L1_PM_SUBSTATES_CTL_T_PWRN_SCL_MASK		(0x3 << 16)
>> +#define RP_L1_PM_SUBSTATES_CTL_T_PWRN_SCL_SHIFT		16
>> +#define RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_MASK		(0x1F << 19)
>> +#define RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_SHIFT		19
>> +#define RP_L1_PM_SUBSTATES_CTL_HIDE_CAP			(0x1 << 24)
>> +
>> +#define RP_L1_PM_SUBSTATES_1_CTL			0xC04
>> +#define RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK	0x1FFF
>> +#define RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY		0x26
>> +
>> +#define RP_L1_PM_SUBSTATES_2_CTL			0xC08
>> +#define RP_L1_PM_SUBSTATES_2_CTL_T_L1_2_DLY_MASK	0x1FFF
>> +#define RP_L1_PM_SUBSTATES_2_CTL_T_L1_2_DLY		0x4D
>> +#define RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND_MASK	(0xFF << 13)
>> +#define RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND		(0x13 << 13)
>> +#define RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND_COMP_MASK	(0xF << 21)
>> +#define RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND_COMP	(0x2 << 21)
>> +
>>   #define RP_RX_HDR_LIMIT	0xe00
>>   #define  RP_RX_HDR_LIMIT_PW_MASK	(0xff << 8)
>>   #define  RP_RX_HDR_LIMIT_PW		(0x0e << 8)
>> @@ -327,6 +349,7 @@ struct tegra_pcie_soc {
>>   	bool RAW_violation_fixup;
>>   	bool program_deskew_time;
>>   	bool updateFC_threshold;
>> +	bool has_aspm_l1ss;
>>   };
>>   
>>   static inline struct tegra_msi *to_tegra_msi(struct msi_controller *chip)
>> @@ -2251,6 +2274,37 @@ static void tegra_pcie_apply_sw_fixup(struct tegra_pcie_port *port)
>>   		value |= RP_VEND_XP_UPDATE_FC_THRESHOLD_T210;
>>   		writel(value, port->base + RP_VEND_XP);
>>   	}
>> +
>> +	if (soc->has_aspm_l1ss) {
>> +		/* Set Common Mode Restore Time to 30us */
>> +		value = readl(port->base + RP_L1_PM_SUBSTATES_CTL);
>> +		value &= ~RP_L1_PM_SUBSTATES_CTL_CM_RTIME_MASK;
>> +		value |= (0x1E << RP_L1_PM_SUBSTATES_CTL_CM_RTIME_SHIFT);
>> +		writel(value, port->base + RP_L1_PM_SUBSTATES_CTL);
>> +
>> +		/* set T_Power_On to 70us */
>> +		value = readl(port->base + RP_L1_PM_SUBSTATES_CTL);
>> +		value &= ~(RP_L1_PM_SUBSTATES_CTL_T_PWRN_SCL_MASK |
>> +			RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_MASK);
>> +		value |= (1 << RP_L1_PM_SUBSTATES_CTL_T_PWRN_SCL_SHIFT) |
>> +			(7 << RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_SHIFT);
>> +		writel(value, port->base + RP_L1_PM_SUBSTATES_CTL);
>> +
>> +		/* Following is based on clk_m being 19.2 MHz */
>> +		value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
>> +		value &= ~RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK;
>> +		value |= RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY;
>> +		writel(value, port->base + RP_L1_PM_SUBSTATES_1_CTL);
>> +
>> +		value = readl(port->base + RP_L1_PM_SUBSTATES_2_CTL);
>> +		value &= ~RP_L1_PM_SUBSTATES_2_CTL_T_L1_2_DLY_MASK;
>> +		value |= RP_L1_PM_SUBSTATES_2_CTL_T_L1_2_DLY;
>> +		value &= ~RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND_MASK;
>> +		value |= RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND;
>> +		value &= ~RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND_COMP_MASK;
>> +		value |= RP_L1_PM_SUBSTATES_2_CTL_MICROSECOND_COMP;
>> +		writel(value, port->base + RP_L1_PM_SUBSTATES_2_CTL);
>> +	}
>>   }
>>   /*
>>    * FIXME: If there are no PCIe cards attached, then calling this function
>> @@ -2391,6 +2445,7 @@ static const struct tegra_pcie_soc tegra20_pcie = {
>>   	.RAW_violation_fixup = false,
>>   	.program_deskew_time = false,
>>   	.updateFC_threshold = false,
>> +	.has_aspm_l1ss = false,
>>   };
>>   
>>   static const struct tegra_pcie_soc tegra30_pcie = {
>> @@ -2412,6 +2467,7 @@ static const struct tegra_pcie_soc tegra30_pcie = {
>>   	.RAW_violation_fixup = false,
>>   	.program_deskew_time = false,
>>   	.updateFC_threshold = false,
>> +	.has_aspm_l1ss = false,
>>   };
>>   
>>   static const struct tegra_pcie_soc tegra124_pcie = {
>> @@ -2432,6 +2488,7 @@ static const struct tegra_pcie_soc tegra124_pcie = {
>>   	.RAW_violation_fixup = true,
>>   	.program_deskew_time = false,
>>   	.updateFC_threshold = false,
>> +	.has_aspm_l1ss = false,
>>   };
>>   
>>   static const struct tegra_pcie_soc tegra210_pcie = {
>> @@ -2460,6 +2517,7 @@ static const struct tegra_pcie_soc tegra210_pcie = {
>>   	.RAW_violation_fixup = false,
>>   	.program_deskew_time = true,
>>   	.updateFC_threshold = true,
>> +	.has_aspm_l1ss = true,
>>   };
>>   
>>   static const struct tegra_pcie_soc tegra186_pcie = {
>> @@ -2481,6 +2539,7 @@ static const struct tegra_pcie_soc tegra186_pcie = {
>>   	.RAW_violation_fixup = false,
>>   	.program_deskew_time = false,
>>   	.updateFC_threshold = false,
>> +	.has_aspm_l1ss = true,
>>   };
>>   
>>   static const struct of_device_id tegra_pcie_of_match[] = {
>> -- 
>> 2.7.4
>>

Re: [PATCH V3 0/3] Add ASPM-L1 Substates support for Tegra

[Vidya Sagar]

On Thursday 14 December 2017 08:29 PM, Thierry Reding wrote:
> On Sun, Nov 12, 2017 at 06:47:51PM +0530, Vidya Sagar wrote:
>> Tegra chips T210 and T186 support ASPM-L1 Substates (i.e. L1.1 and L1.2)
>> This patch series
>> - applies fixups to reflect correct capability values for
>>    T_cmrt (Common Mode Restore Time) and
>>    T_pwr_on (Power On)
>>    and adjusts counter values for 19.2 MHz of clk_m
>> - applies fixup specific to T210 to avoid unnecessary wake ups from L1.2 state
>> - enables advertisement of ASPM-L1 support in capability register
>>
>> NOTE:- This version of patch series drops a patch (that was present till V2 series)
>> that introduces an API for host controller drivers to specify LTR_L1.2_THRESHOLD
>> value as more details and better understanding of spec is required to program
>> LTR_L1.2_THRESHOLD value.
>> It also reorders patches to appropriately have software fixups and advertisement
>> of capabilities
>>
>> PCIe - ASPM L1 Sub States spec
>> https://pcisig.com/sites/default/files/specification_documents/ECN_L1_PM_Substates_with_CLKREQ_31_May_2013_Rev10a.pdf
>>
>> Testing Done on T210 and T186
>> - ASPM-L1:
>>    Verified ASPM-L1 enablement by selecting PCIEASPM_POWERSAVE config
>>    With the help of Tegra rootport's internal counter registers, confirmed
>>    link entry in and out of ASPM-L1 state using USB3.0 add-on card, NVMe and NIC
>>    cards
>> - ASPM-L1 SubStates:
>>    Verified ASPM-L1 Substates enablement by selecting PCIEASPM_POWER_SUPERSAVE config
>>    Confirmed link's entry into L1SS using Westren Digital NVMe card (with Sandisk
>>    Controller) using Tegra rootport's internal counter registers
>>
>> Vidya Sagar (3):
>>    PCI: tegra: Apply sw fixups to support ASPM-L1 Sub-States
>>    PCI: tegra: fixups to avoid unnecessary wakeup from ASPM-L1.2
>>    PCI: tegra: Enable ASPM-L1 capability advertisement
>>
>>   drivers/pci/host/pci-tegra.c | 93 ++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 93 insertions(+)
> Looks like these don't apply to an upstream kernel. Do you have any
> other patches in your tree that would cause the context to be different?
> I see a couple of fields in struct tegra_pcie_soc that aren't in the
> driver upstream and I didn't find any patches on the list that add them.
>
> Can you rebase this on top of a recent linux-next or -rc and resend?
> Also, if the series depends on any other patches that have been sent to
> the list but haven't been merged yet, please include a reference to the
> patchwork or email thread for the dependencies.
>
> Thanks,
> Thierry
Yes. These patches depend on 
https://patchwork.ozlabs.org/project/linux-pci/list/?series=10924
series, but this series is being under reviewed getting modified 
frequently. I'll re-post my patches
once this series reaches a stable state.

Re: [PATCH V3 1/3] PCI: tegra: Apply sw fixups to support ASPM-L1 Sub-States

[Bjorn Helgaas]

On Thu, Dec 14, 2017 at 10:02:15PM +0530, Vidya Sagar wrote:
> On Tuesday 21 November 2017 02:51 AM, Bjorn Helgaas wrote:
> >On Sun, Nov 12, 2017 at 06:47:52PM +0530, Vidya Sagar wrote:
> >>Programs T_cmrt (Commmon Mode Restore Time) and T_pwr_on (Power On)
> >Please use the exact names used in the spec.  In this case, I think
> >that would be "Port Common_Mode_Restore_Time" and "Port T_POWER_ON"
> >(from the L1 PM Substates Capabilities Register, sec 7.33.2).
> >Searching the spec for "T_cmrt" isn't going to find anything useful.
> I'll take care of this in next patch
> >These look like things that maybe should be in the DT because they
> >depend on external component values, so you might need different
> >values depending on what board the Tegra is on?  Same for clk_m?
> I'm afraid not. These are fixed for Tegra including clk_m.

That's actually a *good* thing :)  If they're fixed and can be
embedded in the driver, that's better than having to extend the DT.

Bjorn

Re: [PATCH V3 3/3] PCI: tegra: Enable ASPM-L1 capability advertisement

[Bjorn Helgaas]

On Thu, Dec 14, 2017 at 10:01:57PM +0530, Vidya Sagar wrote:
> On Tuesday 21 November 2017 03:07 AM, Bjorn Helgaas wrote:
> >s/Enable ASPM-L1 capability advertisement/
> >  /Advertise ASPM L1 PM Substates support/
> This patch controls advertisement of ASPM-L1 capability (not ASPM-L1
> Substates)
> >
> >On Sun, Nov 12, 2017 at 06:47:54PM +0530, Vidya Sagar wrote:
> >>Enables advertisement of ASPM-L1 support in capability
> >>registers of applicable Tegra chips
> >Actually, I'm a little confused about whether this has to do with ASPM
> >L1 support (which would be advertised in the Link Capabilities
> >register) or the ASPM L1 Substates support (which would be advertised
> >in the L1 PM Substates Capabilities register)?
> This is to do with ASPM L1 support (and not ASPM L1 Substates)
> But, since ASPM-L1SS needs ASPM-L1 anyway, I think it is ok to enable
> advertisement of ASPM-L1 in the last patch of the series.

It makes this part of the patch look funny:

> >>@@ -2458,6 +2469,7 @@ static const struct tegra_pcie_soc tegra20_pcie = {
> >>  	.RAW_violation_fixup = false,
> >>  	.program_deskew_time = false,
> >>  	.updateFC_threshold = false,

> >>+	.has_aspm_l1 = false,
> >>  	.has_aspm_l1ss = false,
> >>  	.l1ss_rp_wake_fixup = false,
> >>  };

because in PCIe terms, you have to have ASPM before you can have L1SS.
But the code had .has_aspm_l1ss before you add .has_aspm_l1.

It would make more sense if you could:

  1) make ASPM work, *then*
  2) incrementally add ASPM L1SS

But maybe that's not practical, e.g., maybe you can't make the
hardware hide the L1SS capability the way you can make it hide ASPM.

It's not that big a deal to me either way.

Bjorn