* [RFC PATCH 1/1] pci-imx6: add speed change timeout message
@ 2015-05-21 18:35 Troy Kisky
2015-05-21 21:19 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Troy Kisky @ 2015-05-21 18:35 UTC (permalink / raw)
To: linux-arm-kernel
Currently, the timeout is never detected as count
has a value of -1 if a timeout happens, but the code is checking
for 0.
Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
This patch breaks pcie for imx6sx as my board always times out.
So, if someone could check this on an imx6sx I'd appreciate it.
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] 8+ messages in thread* [RFC PATCH 1/1] pci-imx6: add speed change timeout message
2015-05-21 18:35 [RFC PATCH 1/1] pci-imx6: add speed change timeout message Troy Kisky
@ 2015-05-21 21:19 ` Marek Vasut
2015-05-21 23:17 ` Troy Kisky
0 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2015-05-21 21:19 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday, May 21, 2015 at 08:35:45 PM, 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.
>
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>
> ---
>
> This patch breaks pcie for imx6sx as my board always times out.
> So, if someone could check this on an imx6sx I'd appreciate it.
>
> 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--) {
Uh, wouldn't "while (--count)" fix this as well, with a smaller patch?
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread* [RFC PATCH 1/1] pci-imx6: add speed change timeout message
2015-05-21 21:19 ` Marek Vasut
@ 2015-05-21 23:17 ` Troy Kisky
2015-05-21 23:31 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Troy Kisky @ 2015-05-21 23:17 UTC (permalink / raw)
To: linux-arm-kernel
On 5/21/2015 2:19 PM, Marek Vasut wrote:
> On Thursday, May 21, 2015 at 08:35:45 PM, 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.
>>
>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>>
>> ---
>>
>> This patch breaks pcie for imx6sx as my board always times out.
>> So, if someone could check this on an imx6sx I'd appreciate it.
>>
>> 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--) {
>
> Uh, wouldn't "while (--count)" fix this as well, with a smaller patch?
>
> Best regards,
> Marek Vasut
>
Yes, but you'd have an unnecessary usleep_range (no check for finished after it) if a
timeout happens.
Troy
^ permalink raw reply [flat|nested] 8+ messages in thread* [RFC PATCH 1/1] pci-imx6: add speed change timeout message
2015-05-21 23:17 ` Troy Kisky
@ 2015-05-21 23:31 ` Marek Vasut
2015-05-22 17:43 ` Troy Kisky
0 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2015-05-21 23:31 UTC (permalink / raw)
To: linux-arm-kernel
On Friday, May 22, 2015 at 01:17:41 AM, Troy Kisky wrote:
> On 5/21/2015 2:19 PM, Marek Vasut wrote:
> > On Thursday, May 21, 2015 at 08:35:45 PM, 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.
> >>
> >> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> >>
> >> ---
> >>
> >> This patch breaks pcie for imx6sx as my board always times out.
> >> So, if someone could check this on an imx6sx I'd appreciate it.
> >>
> >> 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--) {
> >
> > Uh, wouldn't "while (--count)" fix this as well, with a smaller patch?
> >
> > Best regards,
> > Marek Vasut
>
> Yes, but you'd have an unnecessary usleep_range (no check for finished
> after it) if a timeout happens.
Oki, then I'll wait for Fabio to ACK this and that'd be it :)
Thanks!
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread* [RFC PATCH 1/1] pci-imx6: add speed change timeout message
2015-05-21 23:31 ` Marek Vasut
@ 2015-05-22 17:43 ` Troy Kisky
2015-05-22 18:02 ` Fabio Estevam
0 siblings, 1 reply; 8+ messages in thread
From: Troy Kisky @ 2015-05-22 17:43 UTC (permalink / raw)
To: linux-arm-kernel
On 5/21/2015 4:31 PM, Marek Vasut wrote:
> On Friday, May 22, 2015 at 01:17:41 AM, Troy Kisky wrote:
>> On 5/21/2015 2:19 PM, Marek Vasut wrote:
>>> On Thursday, May 21, 2015 at 08:35:45 PM, 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.
>>>>
>>>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>>>>
>>>> ---
>>>>
>>>> This patch breaks pcie for imx6sx as my board always times out.
>>>> So, if someone could check this on an imx6sx I'd appreciate it.
>>>>
>>>> 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--) {
>>>
>>> Uh, wouldn't "while (--count)" fix this as well, with a smaller patch?
>>>
>>> Best regards,
>>> Marek Vasut
>>
>> Yes, but you'd have an unnecessary usleep_range (no check for finished
>> after it) if a timeout happens.
>
> Oki, then I'll wait for Fabio to ACK this and that'd be it :)
>
> Thanks!
>
> Best regards,
> Marek Vasut
>
This patch breaks my imx6sx board, so it shouldn't be applied until there is
an explanation and work around of some sort.
Thanks
Troy
^ permalink raw reply [flat|nested] 8+ messages in thread* [RFC PATCH 1/1] pci-imx6: add speed change timeout message
2015-05-22 17:43 ` Troy Kisky
@ 2015-05-22 18:02 ` Fabio Estevam
2015-05-22 19:52 ` Troy Kisky
0 siblings, 1 reply; 8+ messages in thread
From: Fabio Estevam @ 2015-05-22 18:02 UTC (permalink / raw)
To: linux-arm-kernel
Hi Troy,
On Fri, May 22, 2015 at 2:43 PM, Troy Kisky
<troy.kisky@boundarydevices.com> wrote:
> This patch breaks my imx6sx board, so it shouldn't be applied until there is
> an explanation and work around of some sort.
Do you have mx6sx PCI working with mainline or only with FSL 3.14 kernel?
Regards,
Fabio Estevam
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH 1/1] pci-imx6: add speed change timeout message
2015-05-22 18:02 ` Fabio Estevam
@ 2015-05-22 19:52 ` Troy Kisky
2015-05-22 20:18 ` Fabio Estevam
0 siblings, 1 reply; 8+ messages in thread
From: Troy Kisky @ 2015-05-22 19:52 UTC (permalink / raw)
To: linux-arm-kernel
On 5/22/2015 11:02 AM, Fabio Estevam wrote:
> Hi Troy,
>
> On Fri, May 22, 2015 at 2:43 PM, Troy Kisky
> <troy.kisky@boundarydevices.com> wrote:
>
>> This patch breaks my imx6sx board, so it shouldn't be applied until there is
>> an explanation and work around of some sort.
>
> Do you have mx6sx PCI working with mainline or only with FSL 3.14 kernel?
>
> Regards,
>
> Fabio Estevam
>
Your right, mainline does not support pcie for imx6sx yet, so the patch will not cause
a regression. I saw pcie in imx6sx.dtsi and assumed it did. I guess whoever adds imx6sx
support can figure it out.
Thanks
Troy
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH 1/1] pci-imx6: add speed change timeout message
2015-05-22 19:52 ` Troy Kisky
@ 2015-05-22 20:18 ` Fabio Estevam
0 siblings, 0 replies; 8+ messages in thread
From: Fabio Estevam @ 2015-05-22 20:18 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, May 22, 2015 at 4:52 PM, Troy Kisky
<troy.kisky@boundarydevices.com> wrote:
> Your right, mainline does not support pcie for imx6sx yet, so the patch will not cause
> a regression. I saw pcie in imx6sx.dtsi and assumed it did. I guess whoever adds imx6sx
> support can figure it out.
Yes, Richard Zhu has made some attemps to add mx6sx PCI support, but
the support is not in place yet.
Maybe you can send your patch without RFC to the linux-pci list as well.
Please add Tim Harvey on Cc as he ran some extensive PCI tests and it
would be good to have him look at your patch as well.
Regards,
Fabio Estevam
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-05-22 20:18 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-21 18:35 [RFC PATCH 1/1] pci-imx6: add speed change timeout message Troy Kisky
2015-05-21 21:19 ` Marek Vasut
2015-05-21 23:17 ` Troy Kisky
2015-05-21 23:31 ` Marek Vasut
2015-05-22 17:43 ` Troy Kisky
2015-05-22 18:02 ` Fabio Estevam
2015-05-22 19:52 ` Troy Kisky
2015-05-22 20:18 ` 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).