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

[PATCH 1/2] 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.

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 93d59f15c589..099998f1923a 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -528,13 +528,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

[PATCH 2/2] PCI: rcar: Clean up the macros

[Marek Vasut]

This patch replaces the (1 << n) with BIT(n) and cleans up whitespace,
no functional change.

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: Reword the commit message
---
 drivers/pci/host/pcie-rcar.c | 52 ++++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index 099998f1923a..35815d516125 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -30,9 +30,9 @@
 
 #define PCIECAR			0x000010
 #define PCIECCTLR		0x000018
-#define  CONFIG_SEND_ENABLE	(1 << 31)
+#define  CONFIG_SEND_ENABLE	BIT(31)
 #define  TYPE0			(0 << 8)
-#define  TYPE1			(1 << 8)
+#define  TYPE1			BIT(8)
 #define PCIECDR			0x000020
 #define PCIEMSR			0x000028
 #define PCIEINTXR		0x000400
@@ -44,7 +44,7 @@
 #define PCIETSTR		0x02004
 #define  DATA_LINK_ACTIVE	1
 #define PCIEERRFR		0x02020
-#define  UNSUPPORTED_REQUEST	(1 << 4)
+#define  UNSUPPORTED_REQUEST	BIT(4)
 #define PCIEMSIFR		0x02044
 #define PCIEMSIALR		0x02048
 #define  MSIFE			1
@@ -57,17 +57,17 @@
 /* local address reg & mask */
 #define PCIELAR(x)		(0x02200 + ((x) * 0x20))
 #define PCIELAMR(x)		(0x02208 + ((x) * 0x20))
-#define  LAM_PREFETCH		(1 << 3)
-#define  LAM_64BIT		(1 << 2)
-#define  LAR_ENABLE		(1 << 1)
+#define  LAM_PREFETCH		BIT(3)
+#define  LAM_64BIT		BIT(2)
+#define  LAR_ENABLE		BIT(1)
 
 /* PCIe address reg & mask */
 #define PCIEPALR(x)		(0x03400 + ((x) * 0x20))
 #define PCIEPAUR(x)		(0x03404 + ((x) * 0x20))
 #define PCIEPAMR(x)		(0x03408 + ((x) * 0x20))
 #define PCIEPTCTLR(x)		(0x0340c + ((x) * 0x20))
-#define  PAR_ENABLE		(1 << 31)
-#define  IO_SPACE		(1 << 8)
+#define  PAR_ENABLE		BIT(31)
+#define  IO_SPACE		BIT(8)
 
 /* Configuration */
 #define PCICONF(x)		(0x010000 + ((x) * 0x4))
@@ -79,23 +79,23 @@
 #define IDSETR1			0x011004
 #define TLCTLR			0x011048
 #define MACSR			0x011054
-#define  SPCHGFIN		(1 << 4)
-#define  SPCHGFAIL		(1 << 6)
-#define  SPCHGSUC		(1 << 7)
+#define  SPCHGFIN		BIT(4)
+#define  SPCHGFAIL		BIT(6)
+#define  SPCHGSUC		BIT(7)
 #define  LINK_SPEED		(0xf << 16)
 #define  LINK_SPEED_2_5GTS	(1 << 16)
 #define  LINK_SPEED_5_0GTS	(2 << 16)
 #define MACCTLR			0x011058
-#define  SPEED_CHANGE		(1 << 24)
-#define  SCRAMBLE_DISABLE	(1 << 27)
+#define  SPEED_CHANGE		BIT(24)
+#define  SCRAMBLE_DISABLE	BIT(27)
 #define MACS2R			0x011078
 #define MACCGSPSETR		0x011084
-#define  SPCNGRSN		(1 << 31)
+#define  SPCNGRSN		BIT(31)
 
 /* R-Car H1 PHY */
 #define H1_PCIEPHYADRR		0x04000c
-#define  WRITE_CMD		(1 << 16)
-#define  PHY_ACK		(1 << 24)
+#define  WRITE_CMD		BIT(16)
+#define  PHY_ACK		BIT(24)
 #define  RATE_POS		12
 #define  LANE_POS		8
 #define  ADR_POS		0
@@ -107,19 +107,19 @@
 #define GEN2_PCIEPHYDATA	0x784
 #define GEN2_PCIEPHYCTRL	0x78c
 
-#define INT_PCI_MSI_NR	32
+#define INT_PCI_MSI_NR		32
 
-#define RCONF(x)	(PCICONF(0)+(x))
-#define RPMCAP(x)	(PMCAP(0)+(x))
-#define REXPCAP(x)	(EXPCAP(0)+(x))
-#define RVCCAP(x)	(VCCAP(0)+(x))
+#define RCONF(x)		(PCICONF(0) + (x))
+#define RPMCAP(x)		(PMCAP(0) + (x))
+#define REXPCAP(x)		(EXPCAP(0) + (x))
+#define RVCCAP(x)		(VCCAP(0) + (x))
 
-#define  PCIE_CONF_BUS(b)	(((b) & 0xff) << 24)
-#define  PCIE_CONF_DEV(d)	(((d) & 0x1f) << 19)
-#define  PCIE_CONF_FUNC(f)	(((f) & 0x7) << 16)
+#define PCIE_CONF_BUS(b)	(((b) & 0xff) << 24)
+#define PCIE_CONF_DEV(d)	(((d) & 0x1f) << 19)
+#define PCIE_CONF_FUNC(f)	(((f) & 0x7) << 16)
 
-#define RCAR_PCI_MAX_RESOURCES 4
-#define MAX_NR_INBOUND_MAPS 6
+#define RCAR_PCI_MAX_RESOURCES	4
+#define MAX_NR_INBOUND_MAPS	6
 
 struct rcar_msi {
 	DECLARE_BITMAP(used, INT_PCI_MSI_NR);
-- 
2.16.2

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

[Simon Horman]

On Sun, Mar 18, 2018 at 11:52:52AM +0100, 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.
> 
> 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

Unless my eyes deceive me this seems to be quite a lot (100x) more often,
but so be it.

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>


> ---
>  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 93d59f15c589..099998f1923a 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -528,13 +528,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 2/2] PCI: rcar: Clean up the macros

[Simon Horman]

On Sun, Mar 18, 2018 at 11:52:53AM +0100, Marek Vasut wrote:
> This patch replaces the (1 << n) with BIT(n) and cleans up whitespace,
> no functional change.
> 
> 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

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

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

[Marek Vasut]

On 03/19/2018 09:38 AM, Simon Horman wrote:
> On Sun, Mar 18, 2018 at 11:52:52AM +0100, 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.
>>
>> 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
> 
> Unless my eyes deceive me this seems to be quite a lot (100x) more often,
> but so be it.

It's just a higher frequency to avoid slowdown when bringing the link up.

> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> 
> 
>> ---
>>  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 93d59f15c589..099998f1923a 100644
>> --- a/drivers/pci/host/pcie-rcar.c
>> +++ b/drivers/pci/host/pcie-rcar.c
>> @@ -528,13 +528,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
>>


-- 
Best regards,
Marek Vasut

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

[Geert Uytterhoeven]

Hi Marek,

On Mon, Mar 19, 2018 at 10:53 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> On 03/19/2018 09:38 AM, Simon Horman wrote:
>> On Sun, Mar 18, 2018 at 11:52:52AM +0100, 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.
>>>
>>> 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
>>
>> Unless my eyes deceive me this seems to be quite a lot (100x) more often,
>> but so be it.
>
> It's just a higher frequency to avoid slowdown when bringing the link up.

No it isn't: you replaced a sleep by a delay, thus making it blocking.
So this can spin for up to 50 ms (+ overhead)?

>>> --- a/drivers/pci/host/pcie-rcar.c
>>> +++ b/drivers/pci/host/pcie-rcar.c
>>> @@ -528,13 +528,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;

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 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl()

[Marek Vasut]

On 03/19/2018 11:53 AM, Geert Uytterhoeven wrote:
> Hi Marek,
> 
> On Mon, Mar 19, 2018 at 10:53 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> On 03/19/2018 09:38 AM, Simon Horman wrote:
>>> On Sun, Mar 18, 2018 at 11:52:52AM +0100, 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.
>>>>
>>>> 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
>>>
>>> Unless my eyes deceive me this seems to be quite a lot (100x) more often,
>>> but so be it.
>>
>> It's just a higher frequency to avoid slowdown when bringing the link up.
> 
> No it isn't: you replaced a sleep by a delay, thus making it blocking.

For much shorter period of time.

> So this can spin for up to 50 ms (+ overhead)?

That's what it did before too , it used msleep and now it uses udelay.

>>>> --- a/drivers/pci/host/pcie-rcar.c
>>>> +++ b/drivers/pci/host/pcie-rcar.c
>>>> @@ -528,13 +528,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;
> 
> 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
> 


-- 
Best regards,
Marek Vasut

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

[Vladimir Zapolskiy]

On 03/19/2018 12:56 PM, Marek Vasut wrote:
> On 03/19/2018 11:53 AM, Geert Uytterhoeven wrote:
>> Hi Marek,
>>
>> On Mon, Mar 19, 2018 at 10:53 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>> On 03/19/2018 09:38 AM, Simon Horman wrote:
>>>> On Sun, Mar 18, 2018 at 11:52:52AM +0100, 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.
>>>>>
>>>>> 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
>>>>
>>>> Unless my eyes deceive me this seems to be quite a lot (100x) more often,
>>>> but so be it.
>>>
>>> It's just a higher frequency to avoid slowdown when bringing the link up.
>>
>> No it isn't: you replaced a sleep by a delay, thus making it blocking.
> 
> For much shorter period of time.
> 
>> So this can spin for up to 50 ms (+ overhead)?
> 
> That's what it did before too , it used msleep and now it uses udelay.
> 

msleep() does not spin, it reschedules the process.

Instead to find a balance you may want to play with usleep_range().

--
With best wishes,
Vladimir

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

[Marek Vasut]

On 03/19/2018 02:43 PM, Vladimir Zapolskiy wrote:
> On 03/19/2018 12:56 PM, Marek Vasut wrote:
>> On 03/19/2018 11:53 AM, Geert Uytterhoeven wrote:
>>> Hi Marek,
>>>
>>> On Mon, Mar 19, 2018 at 10:53 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>>> On 03/19/2018 09:38 AM, Simon Horman wrote:
>>>>> On Sun, Mar 18, 2018 at 11:52:52AM +0100, 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.
>>>>>>
>>>>>> 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
>>>>>
>>>>> Unless my eyes deceive me this seems to be quite a lot (100x) more often,
>>>>> but so be it.
>>>>
>>>> It's just a higher frequency to avoid slowdown when bringing the link up.
>>>
>>> No it isn't: you replaced a sleep by a delay, thus making it blocking.
>>
>> For much shorter period of time.
>>
>>> So this can spin for up to 50 ms (+ overhead)?
>>
>> That's what it did before too , it used msleep and now it uses udelay.
>>
> 
> msleep() does not spin, it reschedules the process.
> 
> Instead to find a balance you may want to play with usleep_range().

Which does not work in atomic context, which will be needed when using
this function in suspend/resume later on.

-- 
Best regards,
Marek Vasut

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

[Vladimir Zapolskiy]

On 03/19/2018 03:48 PM, Marek Vasut wrote:
> On 03/19/2018 02:43 PM, Vladimir Zapolskiy wrote:
>> On 03/19/2018 12:56 PM, Marek Vasut wrote:
>>> On 03/19/2018 11:53 AM, Geert Uytterhoeven wrote:
>>>> Hi Marek,
>>>>
>>>> On Mon, Mar 19, 2018 at 10:53 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>> On 03/19/2018 09:38 AM, Simon Horman wrote:
>>>>>> On Sun, Mar 18, 2018 at 11:52:52AM +0100, 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.
>>>>>>>
>>>>>>> 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
>>>>>>
>>>>>> Unless my eyes deceive me this seems to be quite a lot (100x) more often,
>>>>>> but so be it.
>>>>>
>>>>> It's just a higher frequency to avoid slowdown when bringing the link up.
>>>>
>>>> No it isn't: you replaced a sleep by a delay, thus making it blocking.
>>>
>>> For much shorter period of time.
>>>
>>>> So this can spin for up to 50 ms (+ overhead)?
>>>
>>> That's what it did before too , it used msleep and now it uses udelay.
>>>
>>
>> msleep() does not spin, it reschedules the process.
>>
>> Instead to find a balance you may want to play with usleep_range().
> 
> Which does not work in atomic context, which will be needed when using
> this function in suspend/resume later on.
> 

IIRC suspend/resume are never executed in atomic context, and runtime
suspend/resume are invoked in atomic context only if pm_runtime_irq_safe()
is called (just a few drivers in vanilla uses it at the moment).

Nevertheless, the commit message does not say that the change is needed
for future suspend/resume add-on.

--
With best wishes,
Vladimir

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

[Marek Vasut]

On 03/19/2018 02:56 PM, Vladimir Zapolskiy wrote:
> On 03/19/2018 03:48 PM, Marek Vasut wrote:
>> On 03/19/2018 02:43 PM, Vladimir Zapolskiy wrote:
>>> On 03/19/2018 12:56 PM, Marek Vasut wrote:
>>>> On 03/19/2018 11:53 AM, Geert Uytterhoeven wrote:
>>>>> Hi Marek,
>>>>>
>>>>> On Mon, Mar 19, 2018 at 10:53 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>>> On 03/19/2018 09:38 AM, Simon Horman wrote:
>>>>>>> On Sun, Mar 18, 2018 at 11:52:52AM +0100, 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.
>>>>>>>>
>>>>>>>> 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
>>>>>>>
>>>>>>> Unless my eyes deceive me this seems to be quite a lot (100x) more often,
>>>>>>> but so be it.
>>>>>>
>>>>>> It's just a higher frequency to avoid slowdown when bringing the link up.
>>>>>
>>>>> No it isn't: you replaced a sleep by a delay, thus making it blocking.
>>>>
>>>> For much shorter period of time.
>>>>
>>>>> So this can spin for up to 50 ms (+ overhead)?
>>>>
>>>> That's what it did before too , it used msleep and now it uses udelay.
>>>>
>>>
>>> msleep() does not spin, it reschedules the process.
>>>
>>> Instead to find a balance you may want to play with usleep_range().
>>
>> Which does not work in atomic context, which will be needed when using
>> this function in suspend/resume later on.
>>
> 
> IIRC suspend/resume are never executed in atomic context, and runtime
> suspend/resume are invoked in atomic context only if pm_runtime_irq_safe()
> is called (just a few drivers in vanilla uses it at the moment).
> 
> Nevertheless, the commit message does not say that the change is needed
> for future suspend/resume add-on.

Well, then drop this patch for now, I'll resubmit it later with the
suspend/resume series. I presume 2/2 can go in though, so I don't have
to resubmit it over and over again.

-- 
Best regards,
Marek Vasut

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

[Lorenzo Pieralisi]

Hi Marek,

On Sun, Mar 18, 2018 at 11:52:52AM +0100, 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.
> 
> 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(-)

I assume this patch is still a valid change, I noticed you split this
series but I think this patch was not updated so I should take it as-is.

Please let me know.

Thanks,
Lorenzo

> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index 93d59f15c589..099998f1923a 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -528,13 +528,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 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl()

[Marek Vasut]

On 04/25/2018 04:13 PM, Lorenzo Pieralisi wrote:
> Hi Marek,
> 
> On Sun, Mar 18, 2018 at 11:52:52AM +0100, 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.
>>
>> 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(-)
> 
> I assume this patch is still a valid change, I noticed you split this
> series but I think this patch was not updated so I should take it as-is.
> 
> Please let me know.

The discussion from a month ago would indicate that I need to resubmit it.

-- 
Best regards,
Marek Vasut