[PATCH] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl()

[PATCH] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl()

[Marek Vasut]

The data link active signal usually takes ~20 uSec to be asserted,
poll the bit more often to avoid useless delays in this function.
Use udelay() instead of usleep() for such a small delay as suggested
by the timer documentation and because this will be used in atomic
content later on when the suspend/resume patches land.

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Phil Edworthy <phil.edworthy@renesas.com>
Cc: Simon Horman <horms+renesas@verge.net.au>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: linux-renesas-soc@vger.kernel.org
---
 drivers/pci/host/pcie-rcar.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index 5c365f743df5..65ebe7aa3488 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -529,13 +529,13 @@ static void phy_write_reg(struct rcar_pcie *pcie,
 
 static int rcar_pcie_wait_for_dl(struct rcar_pcie *pcie)
 {
-	unsigned int timeout = 10;
+	unsigned int timeout = 10000;
 
 	while (timeout--) {
 		if ((rcar_pci_read_reg(pcie, PCIETSTR) & DATA_LINK_ACTIVE))
 			return 0;
 
-		msleep(5);
+		udelay(5);
 	}
 
 	return -ETIMEDOUT;
-- 
2.16.2

Re: [PATCH] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl()

[Sergei Shtylyov]

On 05/21/2018 08:02 PM, Marek Vasut wrote:

> The data link active signal usually takes ~20 uSec to be asserted,
> poll the bit more often to avoid useless delays in this function.
> Use udelay() instead of usleep() for such a small delay as suggested
> by the timer documentation and because this will be used in atomic
> content later on when the suspend/resume patches land.
  ^^^^^^^ context :-)

> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Phil Edworthy <phil.edworthy@renesas.com>
> Cc: Simon Horman <horms+renesas@verge.net.au>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: linux-renesas-soc@vger.kernel.org
[...]

MBR, Sergei

[PATCH] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl()

[Marek Vasut]

The data link active signal usually takes ~20 uSec to be asserted,
poll the bit more often to avoid useless delays in this function.
Use udelay() instead of usleep() for such a small delay as suggested
by the timer documentation and because this will be used in atomic
context later on when the suspend/resume patches land.

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Phil Edworthy <phil.edworthy@renesas.com>
Cc: Simon Horman <horms+renesas@verge.net.au>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: linux-renesas-soc@vger.kernel.org
---
V2: s/content/context in commit message
---
 drivers/pci/host/pcie-rcar.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index 5c365f743df5..65ebe7aa3488 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -529,13 +529,13 @@ static void phy_write_reg(struct rcar_pcie *pcie,
 
 static int rcar_pcie_wait_for_dl(struct rcar_pcie *pcie)
 {
-	unsigned int timeout = 10;
+	unsigned int timeout = 10000;
 
 	while (timeout--) {
 		if ((rcar_pci_read_reg(pcie, PCIETSTR) & DATA_LINK_ACTIVE))
 			return 0;
 
-		msleep(5);
+		udelay(5);
 	}
 
 	return -ETIMEDOUT;
-- 
2.16.2

Re: [PATCH] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl()

[Geert Uytterhoeven]

Hi Marek,

On Mon, May 21, 2018 at 11:05 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> The data link active signal usually takes ~20 uSec to be asserted,
> poll the bit more often to avoid useless delays in this function.
> Use udelay() instead of usleep() for such a small delay as suggested
> by the timer documentation and because this will be used in atomic
> context later on when the suspend/resume patches land.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Thanks for your patch!

> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -529,13 +529,13 @@ static void phy_write_reg(struct rcar_pcie *pcie,
>
>  static int rcar_pcie_wait_for_dl(struct rcar_pcie *pcie)
>  {
> -       unsigned int timeout = 10;
> +       unsigned int timeout = 10000;
>
>         while (timeout--) {
>                 if ((rcar_pci_read_reg(pcie, PCIETSTR) & DATA_LINK_ACTIVE))
>                         return 0;
>
> -               msleep(5);
> +               udelay(5);

+ cpu_relax()?

>         }

if this ever happens, it will have blocked for more than 50 ms...

>         return -ETIMEDOUT;

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl()

[Marek Vasut]

On 05/22/2018 11:42 AM, Geert Uytterhoeven wrote:
> Hi Marek,

Hi,

> On Mon, May 21, 2018 at 11:05 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> The data link active signal usually takes ~20 uSec to be asserted,
>> poll the bit more often to avoid useless delays in this function.
>> Use udelay() instead of usleep() for such a small delay as suggested
>> by the timer documentation and because this will be used in atomic
>> context later on when the suspend/resume patches land.
>>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> 
> Thanks for your patch!
> 
>> --- a/drivers/pci/host/pcie-rcar.c
>> +++ b/drivers/pci/host/pcie-rcar.c
>> @@ -529,13 +529,13 @@ static void phy_write_reg(struct rcar_pcie *pcie,
>>
>>  static int rcar_pcie_wait_for_dl(struct rcar_pcie *pcie)
>>  {
>> -       unsigned int timeout = 10;
>> +       unsigned int timeout = 10000;
>>
>>         while (timeout--) {
>>                 if ((rcar_pci_read_reg(pcie, PCIETSTR) & DATA_LINK_ACTIVE))
>>                         return 0;
>>
>> -               msleep(5);
>> +               udelay(5);
> 
> + cpu_relax()?

Is it safe to use in atomic context ? Because of that suspend/resume thing.

>>         }
> 
> if this ever happens, it will have blocked for more than 50 ms...

Well yes, so did the previous thing.

-- 
Best regards,
Marek Vasut

Re: [PATCH] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl()

[Geert Uytterhoeven]

Hi Marek,

On Tue, May 22, 2018 at 11:48 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> On 05/22/2018 11:42 AM, Geert Uytterhoeven wrote:
>> On Mon, May 21, 2018 at 11:05 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>> The data link active signal usually takes ~20 uSec to be asserted,
>>> poll the bit more often to avoid useless delays in this function.
>>> Use udelay() instead of usleep() for such a small delay as suggested
>>> by the timer documentation and because this will be used in atomic
>>> context later on when the suspend/resume patches land.
>>>
>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>>
>> Thanks for your patch!
>>
>>> --- a/drivers/pci/host/pcie-rcar.c
>>> +++ b/drivers/pci/host/pcie-rcar.c
>>> @@ -529,13 +529,13 @@ static void phy_write_reg(struct rcar_pcie *pcie,
>>>
>>>  static int rcar_pcie_wait_for_dl(struct rcar_pcie *pcie)
>>>  {
>>> -       unsigned int timeout = 10;
>>> +       unsigned int timeout = 10000;
>>>
>>>         while (timeout--) {
>>>                 if ((rcar_pci_read_reg(pcie, PCIETSTR) & DATA_LINK_ACTIVE))
>>>                         return 0;
>>>
>>> -               msleep(5);
>>> +               udelay(5);
>>
>> + cpu_relax()?
>
> Is it safe to use in atomic context ? Because of that suspend/resume thing.

Yes.

>>>         }
>>
>> if this ever happens, it will have blocked for more than 50 ms...
>
> Well yes, so did the previous thing.

No, the previous thing slept. Big difference.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl()

[Marek Vasut]

On 05/22/2018 12:33 PM, Geert Uytterhoeven wrote:
> Hi Marek,
> 
> On Tue, May 22, 2018 at 11:48 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> On 05/22/2018 11:42 AM, Geert Uytterhoeven wrote:
>>> On Mon, May 21, 2018 at 11:05 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>>> The data link active signal usually takes ~20 uSec to be asserted,
>>>> poll the bit more often to avoid useless delays in this function.
>>>> Use udelay() instead of usleep() for such a small delay as suggested
>>>> by the timer documentation and because this will be used in atomic
>>>> context later on when the suspend/resume patches land.
>>>>
>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>
>>> Thanks for your patch!
>>>
>>>> --- a/drivers/pci/host/pcie-rcar.c
>>>> +++ b/drivers/pci/host/pcie-rcar.c
>>>> @@ -529,13 +529,13 @@ static void phy_write_reg(struct rcar_pcie *pcie,
>>>>
>>>>  static int rcar_pcie_wait_for_dl(struct rcar_pcie *pcie)
>>>>  {
>>>> -       unsigned int timeout = 10;
>>>> +       unsigned int timeout = 10000;
>>>>
>>>>         while (timeout--) {
>>>>                 if ((rcar_pci_read_reg(pcie, PCIETSTR) & DATA_LINK_ACTIVE))
>>>>                         return 0;
>>>>
>>>> -               msleep(5);
>>>> +               udelay(5);
>>>
>>> + cpu_relax()?
>>
>> Is it safe to use in atomic context ? Because of that suspend/resume thing.
> 
> Yes.

OK, added.

>>>>         }
>>>
>>> if this ever happens, it will have blocked for more than 50 ms...
>>
>> Well yes, so did the previous thing.
> 
> No, the previous thing slept. Big difference.

True

-- 
Best regards,
Marek Vasut