* [PATCH v2] PCI: tegra: Enable root port specific features
@ 2014-10-20 11:04 Vidya Sagar
2014-10-20 16:52 ` Stephen Warren
2014-10-28 14:51 ` Thierry Reding
0 siblings, 2 replies; 4+ messages in thread
From: Vidya Sagar @ 2014-10-20 11:04 UTC (permalink / raw)
To: thierry.reding, bhelgaas, swarren
Cc: kthota, linux-tegra, linux-pci, linux-kernel, Vidya Sagar
Enables root port to advertise its ASPM-L1 capability
resulting in possible link entry to L1 when an ASPM-L1 capable
device is connected
Enables per-controller & per-TMS clock clamping by default
Enabling above features result in more power saving
It also avoids PM message truncation by waiting for DLLP to finish
before entering into L1 or L2
Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
v2:
Removed rp_read() & rp_write() as they seem to be redundant
Moved port disable code under error condition i.e. it the link
is down, corresponding port will be disabled
drivers/pci/host/pci-tegra.c | 50 +++++++++++++++++++++++++++++++++++++++-----
1 file changed, 45 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 3d43874..7f32b07 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -237,6 +237,18 @@
(0xf << PADS_REFCLK_CFG_DRVI_SHIFT) \
)
+#define NV_PCIE2_RP_VEND_XP1 0x00000F04
+#define NV_PCIE2_RP_VEND_XP_LINK_PVT_CTL_L1_ASPM_SUPPORT (1 << 21)
+
+#define NV_PCIE2_RP_VEND_XP_BIST 0x00000F4C
+#define PCIE2_RP_VEND_XP_BIST_GOTO_L1_L2_AFTER_DLLP_DONE (1 << 28)
+
+#define NV_PCIE2_RP_PRIV_MISC 0x00000FE0
+#define PCIE2_RP_PRIV_MISC_CTLR_CLK_CLAMP_THRESHOLD (0xF << 16)
+#define PCIE2_RP_PRIV_MISC_CTLR_CLK_CLAMP_ENABLE (1 << 23)
+#define PCIE2_RP_PRIV_MISC_TMS_CLK_CLAMP_THRESHOLD (0xF << 24)
+#define PCIE2_RP_PRIV_MISC_TMS_CLK_CLAMP_ENABLE (1 << 31)
+
struct tegra_msi {
struct msi_chip chip;
DECLARE_BITMAP(used, INT_PCI_MSI_NR);
@@ -1859,6 +1871,32 @@ retry:
return false;
}
+/* Enable various features of root port */
+static void tegra_pcie_enable_rp_features(struct tegra_pcie_port *port)
+{
+ unsigned int data;
+
+ /* Power mangagement settings */
+ /* Enable clock clamping by default */
+ data = readl(port->base + NV_PCIE2_RP_PRIV_MISC);
+ data |= PCIE2_RP_PRIV_MISC_CTLR_CLK_CLAMP_THRESHOLD |
+ PCIE2_RP_PRIV_MISC_CTLR_CLK_CLAMP_ENABLE |
+ PCIE2_RP_PRIV_MISC_TMS_CLK_CLAMP_THRESHOLD |
+ PCIE2_RP_PRIV_MISC_TMS_CLK_CLAMP_ENABLE;
+ writel(data, port->base + NV_PCIE2_RP_PRIV_MISC);
+
+ /* Enable rootport to advertise its ASPM - L1 capability */
+ data = readl(port->base + NV_PCIE2_RP_VEND_XP1);
+ data |= NV_PCIE2_RP_VEND_XP_LINK_PVT_CTL_L1_ASPM_SUPPORT;
+ writel(data, port->base + NV_PCIE2_RP_VEND_XP1);
+
+ /* LTSSM : wait for DLLP to finish before entering L1 or L2/L3 */
+ /* to avoid truncating PM messages resulting in receiver errors */
+ data = readl(port->base + NV_PCIE2_RP_VEND_XP_BIST);
+ data |= PCIE2_RP_VEND_XP_BIST_GOTO_L1_L2_AFTER_DLLP_DONE;
+ writel(data, port->base + NV_PCIE2_RP_VEND_XP_BIST);
+}
+
static int tegra_pcie_enable(struct tegra_pcie *pcie)
{
struct tegra_pcie_port *port, *tmp;
@@ -1870,13 +1908,15 @@ static int tegra_pcie_enable(struct tegra_pcie *pcie)
tegra_pcie_port_enable(port);
- if (tegra_pcie_port_check_link(port))
+ if (!tegra_pcie_port_check_link(port)) {
+ dev_info(pcie->dev, "link %u down, ignoring\n",
+ port->index);
+ tegra_pcie_port_disable(port);
+ tegra_pcie_port_free(port);
continue;
+ }
- dev_info(pcie->dev, "link %u down, ignoring\n", port->index);
-
- tegra_pcie_port_disable(port);
- tegra_pcie_port_free(port);
+ tegra_pcie_enable_rp_features(port);
}
memset(&hw, 0, sizeof(hw));
--
1.8.1.5
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] PCI: tegra: Enable root port specific features
2014-10-20 11:04 [PATCH v2] PCI: tegra: Enable root port specific features Vidya Sagar
@ 2014-10-20 16:52 ` Stephen Warren
2014-10-28 14:51 ` Thierry Reding
1 sibling, 0 replies; 4+ messages in thread
From: Stephen Warren @ 2014-10-20 16:52 UTC (permalink / raw)
To: Vidya Sagar, thierry.reding, bhelgaas
Cc: kthota, linux-tegra, linux-pci, linux-kernel
On 10/20/2014 05:04 AM, Vidya Sagar wrote:
> Enables root port to advertise its ASPM-L1 capability
> resulting in possible link entry to L1 when an ASPM-L1 capable
> device is connected
> Enables per-controller & per-TMS clock clamping by default
> Enabling above features result in more power saving
>
> It also avoids PM message truncation by waiting for DLLP to finish
> before entering into L1 or L2
The updated tegra_pcie_enable() looks better in this patch, so that part,
Acked-by: Stephen Warren <swarren@nvidia.com>
I'll leave the review of the additional #defines and
tegra_pcie_enable_rp_features() to Thierry.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] PCI: tegra: Enable root port specific features
2014-10-20 11:04 [PATCH v2] PCI: tegra: Enable root port specific features Vidya Sagar
2014-10-20 16:52 ` Stephen Warren
@ 2014-10-28 14:51 ` Thierry Reding
1 sibling, 0 replies; 4+ messages in thread
From: Thierry Reding @ 2014-10-28 14:51 UTC (permalink / raw)
To: Vidya Sagar
Cc: bhelgaas, swarren, kthota, linux-tegra, linux-pci, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 4537 bytes --]
On Mon, Oct 20, 2014 at 04:34:51PM +0530, Vidya Sagar wrote:
> Enables root port to advertise its ASPM-L1 capability
> resulting in possible link entry to L1 when an ASPM-L1 capable
> device is connected
> Enables per-controller & per-TMS clock clamping by default
> Enabling above features result in more power saving
>
> It also avoids PM message truncation by waiting for DLLP to finish
> before entering into L1 or L2
>
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> v2:
> Removed rp_read() & rp_write() as they seem to be redundant
> Moved port disable code under error condition i.e. it the link
> is down, corresponding port will be disabled
>
> drivers/pci/host/pci-tegra.c | 50 +++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 45 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> index 3d43874..7f32b07 100644
> --- a/drivers/pci/host/pci-tegra.c
> +++ b/drivers/pci/host/pci-tegra.c
> @@ -237,6 +237,18 @@
> (0xf << PADS_REFCLK_CFG_DRVI_SHIFT) \
> )
>
> +#define NV_PCIE2_RP_VEND_XP1 0x00000F04
> +#define NV_PCIE2_RP_VEND_XP_LINK_PVT_CTL_L1_ASPM_SUPPORT (1 << 21)
> +
> +#define NV_PCIE2_RP_VEND_XP_BIST 0x00000F4C
> +#define PCIE2_RP_VEND_XP_BIST_GOTO_L1_L2_AFTER_DLLP_DONE (1 << 28)
> +
> +#define NV_PCIE2_RP_PRIV_MISC 0x00000FE0
> +#define PCIE2_RP_PRIV_MISC_CTLR_CLK_CLAMP_THRESHOLD (0xF << 16)
> +#define PCIE2_RP_PRIV_MISC_CTLR_CLK_CLAMP_ENABLE (1 << 23)
> +#define PCIE2_RP_PRIV_MISC_TMS_CLK_CLAMP_THRESHOLD (0xF << 24)
> +#define PCIE2_RP_PRIV_MISC_TMS_CLK_CLAMP_ENABLE (1 << 31)
These registers should move to the section where other RP_* registers
are already defined. Also, please adjust the name to match what's
already there.
In particular you also redefine RP_PRIV_MISC here.
> +/* Enable various features of root port */
> +static void tegra_pcie_enable_rp_features(struct tegra_pcie_port *port)
> +{
> + unsigned int data;
u32 please. And maybe rename it to something like "value" to be more
consistent with other parts of the driver.
> +
> + /* Power mangagement settings */
> + /* Enable clock clamping by default */
> + data = readl(port->base + NV_PCIE2_RP_PRIV_MISC);
> + data |= PCIE2_RP_PRIV_MISC_CTLR_CLK_CLAMP_THRESHOLD |
> + PCIE2_RP_PRIV_MISC_CTLR_CLK_CLAMP_ENABLE |
> + PCIE2_RP_PRIV_MISC_TMS_CLK_CLAMP_THRESHOLD |
> + PCIE2_RP_PRIV_MISC_TMS_CLK_CLAMP_ENABLE;
> + writel(data, port->base + NV_PCIE2_RP_PRIV_MISC);
> +
> + /* Enable rootport to advertise its ASPM - L1 capability */
> + data = readl(port->base + NV_PCIE2_RP_VEND_XP1);
> + data |= NV_PCIE2_RP_VEND_XP_LINK_PVT_CTL_L1_ASPM_SUPPORT;
> + writel(data, port->base + NV_PCIE2_RP_VEND_XP1);
> +
> + /* LTSSM : wait for DLLP to finish before entering L1 or L2/L3 */
> + /* to avoid truncating PM messages resulting in receiver errors */
See Documentation/CodingStyle for the proper format of multi-line
comments.
> + data = readl(port->base + NV_PCIE2_RP_VEND_XP_BIST);
> + data |= PCIE2_RP_VEND_XP_BIST_GOTO_L1_L2_AFTER_DLLP_DONE;
> + writel(data, port->base + NV_PCIE2_RP_VEND_XP_BIST);
> +}
I think these features could all be implemented in separate functions,
which would allow you to give them meaningful names and make these odd
comments obsolete. Or if you want to be more verbose about explaining
the features, having separate functions for them will give you a good
place for such comments.
> +
> static int tegra_pcie_enable(struct tegra_pcie *pcie)
> {
> struct tegra_pcie_port *port, *tmp;
> @@ -1870,13 +1908,15 @@ static int tegra_pcie_enable(struct tegra_pcie *pcie)
>
> tegra_pcie_port_enable(port);
>
> - if (tegra_pcie_port_check_link(port))
> + if (!tegra_pcie_port_check_link(port)) {
> + dev_info(pcie->dev, "link %u down, ignoring\n",
> + port->index);
This isn't properly aligned.
> + tegra_pcie_port_disable(port);
> + tegra_pcie_port_free(port);
> continue;
> + }
>
> - dev_info(pcie->dev, "link %u down, ignoring\n", port->index);
> -
> - tegra_pcie_port_disable(port);
> - tegra_pcie_port_free(port);
> + tegra_pcie_enable_rp_features(port);
> }
I think the new function is oddly named. Perhaps something like
tegra_pcie_enable_pm() or tegra_pcie_port_init() would work better. The
latter would for example give us a good point to do other work that's
not "enabling a feature" if we need that kind of thing in the future.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] PCI: tegra: Enable root port specific features
@ 2014-10-10 5:15 Vidya Sagar
0 siblings, 0 replies; 4+ messages in thread
From: Vidya Sagar @ 2014-10-10 5:15 UTC (permalink / raw)
To: thierry.reding, bhelgaas, swarren
Cc: kthota, linux-tegra, linux-pci, linux-kernel, Vidya Sagar
Enables root port to advertise its ASPM-L1 capability
resulting in possible link entry to L1 when an ASPM-L1 capable
device is connected
Enables per-controller & per-TMS clock clamping by default
Enabling above features result in more power saving
It also avoids PM message truncation by waiting for DLLP to finish
before entering into L1 or L2
Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
v2:
Removed rp_read() & rp_write() as they seem to be redundant
Moved port disable code under error condition i.e. it the link
is down, corresponding port will be disabled
drivers/pci/host/pci-tegra.c | 50 +++++++++++++++++++++++++++++++++++++++-----
1 file changed, 45 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 3d43874..7f32b07 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -237,6 +237,18 @@
(0xf << PADS_REFCLK_CFG_DRVI_SHIFT) \
)
+#define NV_PCIE2_RP_VEND_XP1 0x00000F04
+#define NV_PCIE2_RP_VEND_XP_LINK_PVT_CTL_L1_ASPM_SUPPORT (1 << 21)
+
+#define NV_PCIE2_RP_VEND_XP_BIST 0x00000F4C
+#define PCIE2_RP_VEND_XP_BIST_GOTO_L1_L2_AFTER_DLLP_DONE (1 << 28)
+
+#define NV_PCIE2_RP_PRIV_MISC 0x00000FE0
+#define PCIE2_RP_PRIV_MISC_CTLR_CLK_CLAMP_THRESHOLD (0xF << 16)
+#define PCIE2_RP_PRIV_MISC_CTLR_CLK_CLAMP_ENABLE (1 << 23)
+#define PCIE2_RP_PRIV_MISC_TMS_CLK_CLAMP_THRESHOLD (0xF << 24)
+#define PCIE2_RP_PRIV_MISC_TMS_CLK_CLAMP_ENABLE (1 << 31)
+
struct tegra_msi {
struct msi_chip chip;
DECLARE_BITMAP(used, INT_PCI_MSI_NR);
@@ -1859,6 +1871,32 @@ retry:
return false;
}
+/* Enable various features of root port */
+static void tegra_pcie_enable_rp_features(struct tegra_pcie_port *port)
+{
+ unsigned int data;
+
+ /* Power mangagement settings */
+ /* Enable clock clamping by default */
+ data = readl(port->base + NV_PCIE2_RP_PRIV_MISC);
+ data |= PCIE2_RP_PRIV_MISC_CTLR_CLK_CLAMP_THRESHOLD |
+ PCIE2_RP_PRIV_MISC_CTLR_CLK_CLAMP_ENABLE |
+ PCIE2_RP_PRIV_MISC_TMS_CLK_CLAMP_THRESHOLD |
+ PCIE2_RP_PRIV_MISC_TMS_CLK_CLAMP_ENABLE;
+ writel(data, port->base + NV_PCIE2_RP_PRIV_MISC);
+
+ /* Enable rootport to advertise its ASPM - L1 capability */
+ data = readl(port->base + NV_PCIE2_RP_VEND_XP1);
+ data |= NV_PCIE2_RP_VEND_XP_LINK_PVT_CTL_L1_ASPM_SUPPORT;
+ writel(data, port->base + NV_PCIE2_RP_VEND_XP1);
+
+ /* LTSSM : wait for DLLP to finish before entering L1 or L2/L3 */
+ /* to avoid truncating PM messages resulting in receiver errors */
+ data = readl(port->base + NV_PCIE2_RP_VEND_XP_BIST);
+ data |= PCIE2_RP_VEND_XP_BIST_GOTO_L1_L2_AFTER_DLLP_DONE;
+ writel(data, port->base + NV_PCIE2_RP_VEND_XP_BIST);
+}
+
static int tegra_pcie_enable(struct tegra_pcie *pcie)
{
struct tegra_pcie_port *port, *tmp;
@@ -1870,13 +1908,15 @@ static int tegra_pcie_enable(struct tegra_pcie *pcie)
tegra_pcie_port_enable(port);
- if (tegra_pcie_port_check_link(port))
+ if (!tegra_pcie_port_check_link(port)) {
+ dev_info(pcie->dev, "link %u down, ignoring\n",
+ port->index);
+ tegra_pcie_port_disable(port);
+ tegra_pcie_port_free(port);
continue;
+ }
- dev_info(pcie->dev, "link %u down, ignoring\n", port->index);
-
- tegra_pcie_port_disable(port);
- tegra_pcie_port_free(port);
+ tegra_pcie_enable_rp_features(port);
}
memset(&hw, 0, sizeof(hw));
--
1.8.1.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-10-28 14:51 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-20 11:04 [PATCH v2] PCI: tegra: Enable root port specific features Vidya Sagar
2014-10-20 16:52 ` Stephen Warren
2014-10-28 14:51 ` Thierry Reding
-- strict thread matches above, loose matches on Subject: below --
2014-10-10 5:15 Vidya Sagar
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).