* [PATCH 1/1] pci-imx6: add speed change timeout message
@ 2015-06-05 22:37 Troy Kisky
2015-06-06 0:03 ` Marek Vasut
2015-06-12 20:20 ` Bjorn Helgaas
0 siblings, 2 replies; 5+ messages in thread
From: Troy Kisky @ 2015-06-05 22:37 UTC (permalink / raw)
To: bhelgaas; +Cc: festevam, marex, linux-pci, r65037, l.stach, tharvey, Troy Kisky
Currently, the timeout is never detected as count
has a value of -1 if a timeout happens, but the code is checking
for 0. Also, this patch removes the unneeded final wait
if a timeout occurs.
Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
drivers/pci/host/pci-imx6.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index fdb9536..51be92c 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -398,20 +398,22 @@ static int imx6_pcie_start_link(struct pcie_port *pp)
writel(tmp, pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
count = 200;
- while (count--) {
+ while (1) {
tmp = readl(pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
/* Test if the speed change finished. */
- if (!(tmp & PORT_LOGIC_SPEED_CHANGE))
+ if (!(tmp & PORT_LOGIC_SPEED_CHANGE)) {
+ /* Make sure link training is finished as well! */
+ ret = imx6_pcie_wait_for_link(pp);
+ break;
+ }
+ if (count-- == 0) {
+ dev_err(pp->dev, "Speed change timeout\n");
+ ret = -EINVAL;
break;
+ }
usleep_range(100, 1000);
}
- /* Make sure link training is finished as well! */
- if (count)
- ret = imx6_pcie_wait_for_link(pp);
- else
- ret = -EINVAL;
-
if (ret) {
dev_err(pp->dev, "Failed to bring link up!\n");
} else {
--
1.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] pci-imx6: add speed change timeout message
2015-06-05 22:37 [PATCH 1/1] pci-imx6: add speed change timeout message Troy Kisky
@ 2015-06-06 0:03 ` Marek Vasut
2015-06-12 20:20 ` Bjorn Helgaas
1 sibling, 0 replies; 5+ messages in thread
From: Marek Vasut @ 2015-06-06 0:03 UTC (permalink / raw)
To: Troy Kisky; +Cc: bhelgaas, festevam, linux-pci, r65037, l.stach, tharvey
On Saturday, June 06, 2015 at 12:37:44 AM, Troy Kisky wrote:
> Currently, the timeout is never detected as count
> has a value of -1 if a timeout happens, but the code is checking
> for 0. Also, this patch removes the unneeded final wait
> if a timeout occurs.
>
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
Acked-by: Marek Vasut <marex@denx.de>
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] pci-imx6: add speed change timeout message
2015-06-05 22:37 [PATCH 1/1] pci-imx6: add speed change timeout message Troy Kisky
2015-06-06 0:03 ` Marek Vasut
@ 2015-06-12 20:20 ` Bjorn Helgaas
2015-06-12 22:25 ` Troy Kisky
1 sibling, 1 reply; 5+ messages in thread
From: Bjorn Helgaas @ 2015-06-12 20:20 UTC (permalink / raw)
To: Troy Kisky; +Cc: festevam, marex, linux-pci, r65037, l.stach, tharvey
Hi Troy,
On Fri, Jun 05, 2015 at 03:37:44PM -0700, Troy Kisky wrote:
> Currently, the timeout is never detected as count
> has a value of -1 if a timeout happens, but the code is checking
> for 0. Also, this patch removes the unneeded final wait
> if a timeout occurs.
>
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---
> drivers/pci/host/pci-imx6.c | 18 ++++++++++--------
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index fdb9536..51be92c 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -398,20 +398,22 @@ static int imx6_pcie_start_link(struct pcie_port *pp)
> writel(tmp, pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
>
> count = 200;
> - while (count--) {
> + while (1) {
> tmp = readl(pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
> /* Test if the speed change finished. */
> - if (!(tmp & PORT_LOGIC_SPEED_CHANGE))
> + if (!(tmp & PORT_LOGIC_SPEED_CHANGE)) {
> + /* Make sure link training is finished as well! */
> + ret = imx6_pcie_wait_for_link(pp);
> + break;
> + }
> + if (count-- == 0) {
> + dev_err(pp->dev, "Speed change timeout\n");
> + ret = -EINVAL;
> break;
> + }
> usleep_range(100, 1000);
> }
>
> - /* Make sure link training is finished as well! */
> - if (count)
> - ret = imx6_pcie_wait_for_link(pp);
> - else
> - ret = -EINVAL;
> -
> if (ret) {
> dev_err(pp->dev, "Failed to bring link up!\n");
> } else {
This looks fine functionally, but I propose the following slight
rearrangement because I have another patch that makes all the
"wait_for_link" timeout loops look similar, and this loop might as
well be similar, too.
Unrelated to your patch, but noticed while doing this: what's the magic
constant 0x80 here?
+ tmp = readl(pp->dbi_base + 0x80);
Is that correct? Can we add a symbolic name for it?
Bjorn
commit 24533d23e8fa
Author: Troy Kisky <troy.kisky@boundarydevices.com>
Date: Fri Jun 12 14:30:16 2015 -0500
PCI: imx6: Add speed change timeout message
Currently, the timeout is never detected as count has a value of -1 if a
timeout happens, but the code is checking for 0. Also, this patch removes
the unneeded final wait if a timeout occurs.
[bhelgaas: reworked starting from http://lkml.kernel.org/r/1433543864-7252-1-git-send-email-troy.kisky@boundarydevices.com]
Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index fdb9536..2bbd38d 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -352,6 +352,23 @@ static int imx6_pcie_wait_for_link(struct pcie_port *pp)
return 0;
}
+static int imx6_pcie_wait_for_speed_change(struct pcie_port *pp)
+{
+ uint32_t tmp;
+ unsigned int retries;
+
+ for (retries = 0; retries < 200; retries++)
+ tmp = readl(pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
+ /* Test if the speed change finished. */
+ if (!(tmp & PORT_LOGIC_SPEED_CHANGE))
+ return 0;
+ usleep_range(100, 1000);
+ }
+
+ dev_err(pp->dev, "Speed change timeout\n");
+ return -EINVAL;
+}
+
static irqreturn_t imx6_pcie_msi_handler(int irq, void *arg)
{
struct pcie_port *pp = arg;
@@ -363,7 +380,7 @@ static int imx6_pcie_start_link(struct pcie_port *pp)
{
struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
uint32_t tmp;
- int ret, count;
+ int ret;
/*
* Force Gen1 operation when starting the link. In case the link is
@@ -397,29 +414,22 @@ static int imx6_pcie_start_link(struct pcie_port *pp)
tmp |= PORT_LOGIC_SPEED_CHANGE;
writel(tmp, pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
- count = 200;
- while (count--) {
- tmp = readl(pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
- /* Test if the speed change finished. */
- if (!(tmp & PORT_LOGIC_SPEED_CHANGE))
- break;
- usleep_range(100, 1000);
+ ret = imx6_pcie_wait_for_speed_change(pp);
+ if (ret) {
+ dev_err(pp->dev, "Failed to bring link up!\n");
+ return ret;
}
/* Make sure link training is finished as well! */
- if (count)
- ret = imx6_pcie_wait_for_link(pp);
- else
- ret = -EINVAL;
-
+ ret = imx6_pcie_wait_for_link(pp);
if (ret) {
dev_err(pp->dev, "Failed to bring link up!\n");
- } else {
- tmp = readl(pp->dbi_base + 0x80);
- dev_dbg(pp->dev, "Link up, Gen=%i\n", (tmp >> 16) & 0xf);
+ return ret;
}
- return ret;
+ tmp = readl(pp->dbi_base + 0x80);
+ dev_dbg(pp->dev, "Link up, Gen=%i\n", (tmp >> 16) & 0xf);
+ return 0;
}
static void imx6_pcie_host_init(struct pcie_port *pp)
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] pci-imx6: add speed change timeout message
2015-06-12 20:20 ` Bjorn Helgaas
@ 2015-06-12 22:25 ` Troy Kisky
2015-06-12 23:39 ` Bjorn Helgaas
0 siblings, 1 reply; 5+ messages in thread
From: Troy Kisky @ 2015-06-12 22:25 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: festevam, marex, linux-pci, r65037, l.stach, tharvey
On 6/12/2015 1:20 PM, Bjorn Helgaas wrote:
> Hi Troy,
>
> On Fri, Jun 05, 2015 at 03:37:44PM -0700, Troy Kisky wrote:
>> Currently, the timeout is never detected as count
>> has a value of -1 if a timeout happens, but the code is checking
>> for 0. Also, this patch removes the unneeded final wait
>> if a timeout occurs.
>>
>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>> ---
>> drivers/pci/host/pci-imx6.c | 18 ++++++++++--------
>> 1 file changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
>> index fdb9536..51be92c 100644
>> --- a/drivers/pci/host/pci-imx6.c
>> +++ b/drivers/pci/host/pci-imx6.c
>> @@ -398,20 +398,22 @@ static int imx6_pcie_start_link(struct pcie_port *pp)
>> writel(tmp, pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
>>
>> count = 200;
>> - while (count--) {
>> + while (1) {
>> tmp = readl(pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
>> /* Test if the speed change finished. */
>> - if (!(tmp & PORT_LOGIC_SPEED_CHANGE))
>> + if (!(tmp & PORT_LOGIC_SPEED_CHANGE)) {
>> + /* Make sure link training is finished as well! */
>> + ret = imx6_pcie_wait_for_link(pp);
>> + break;
>> + }
>> + if (count-- == 0) {
>> + dev_err(pp->dev, "Speed change timeout\n");
>> + ret = -EINVAL;
>> break;
>> + }
>> usleep_range(100, 1000);
>> }
>>
>> - /* Make sure link training is finished as well! */
>> - if (count)
>> - ret = imx6_pcie_wait_for_link(pp);
>> - else
>> - ret = -EINVAL;
>> -
>> if (ret) {
>> dev_err(pp->dev, "Failed to bring link up!\n");
>> } else {
>
> This looks fine functionally, but I propose the following slight
> rearrangement because I have another patch that makes all the
> "wait_for_link" timeout loops look similar, and this loop might as
> well be similar, too.
>
> Unrelated to your patch, but noticed while doing this: what's the magic
> constant 0x80 here?
>
> + tmp = readl(pp->dbi_base + 0x80);
>
> Is that correct? Can we add a symbolic name for it?
>
> Bjorn
>
The name in the manual for +x80 is
PCIE_RC_LCSR - Link Control and Status Register
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] pci-imx6: add speed change timeout message
2015-06-12 22:25 ` Troy Kisky
@ 2015-06-12 23:39 ` Bjorn Helgaas
0 siblings, 0 replies; 5+ messages in thread
From: Bjorn Helgaas @ 2015-06-12 23:39 UTC (permalink / raw)
To: Troy Kisky; +Cc: festevam, marex, linux-pci, r65037, l.stach, tharvey
On Fri, Jun 12, 2015 at 03:25:57PM -0700, Troy Kisky wrote:
> On 6/12/2015 1:20 PM, Bjorn Helgaas wrote:
> > Unrelated to your patch, but noticed while doing this: what's the magic
> > constant 0x80 here?
> >
> > + tmp = readl(pp->dbi_base + 0x80);
> >
> > Is that correct? Can we add a symbolic name for it?
> >
> > Bjorn
> >
>
> The name in the manual for +x80 is
> PCIE_RC_LCSR - Link Control and Status Register
commit bc82467358d3793e1291e38cd01883e74e872eeb
Author: Bjorn Helgaas <bhelgaas@google.com>
Date: Fri Jun 12 17:27:43 2015 -0500
PCI: imx6: Add #define PCIE_RC_LCSR
Define PCIE_RC_LCSR and use it instead of the bare offset "0x80."
No functional change.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index a032c01..57db1c1 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -47,6 +47,8 @@ struct imx6_pcie {
#define PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN2 0x2
#define PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK 0xf
+#define PCIE_RC_LCSR 0x80
+
/* PCIe Port Logic registers (memory-mapped) */
#define PL_OFFSET 0x700
#define PCIE_PL_PFLR (PL_OFFSET + 0x08)
@@ -427,7 +429,7 @@ static int imx6_pcie_start_link(struct pcie_port *pp)
return ret;
}
- tmp = readl(pp->dbi_base + 0x80);
+ tmp = readl(pp->dbi_base + PCIE_RC_LCSR);
dev_dbg(pp->dev, "Link up, Gen=%i\n", (tmp >> 16) & 0xf);
return 0;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-06-12 23:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-05 22:37 [PATCH 1/1] pci-imx6: add speed change timeout message Troy Kisky
2015-06-06 0:03 ` Marek Vasut
2015-06-12 20:20 ` Bjorn Helgaas
2015-06-12 22:25 ` Troy Kisky
2015-06-12 23:39 ` Bjorn Helgaas
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).