* [PATCH 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() @ 2018-03-18 10:52 Marek Vasut 2018-03-18 10:52 ` [PATCH 2/2] PCI: rcar: Clean up the macros Marek Vasut ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Marek Vasut @ 2018-03-18 10:52 UTC (permalink / raw) To: linux-pci Cc: Marek Vasut, Geert Uytterhoeven, Phil Edworthy, Simon Horman, Wolfram Sang, linux-renesas-soc 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] PCI: rcar: Clean up the macros 2018-03-18 10:52 [PATCH 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() Marek Vasut @ 2018-03-18 10:52 ` Marek Vasut 2018-03-19 8:40 ` Simon Horman 2018-03-19 8:38 ` [PATCH 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() Simon Horman 2018-04-25 14:13 ` Lorenzo Pieralisi 2 siblings, 1 reply; 13+ messages in thread From: Marek Vasut @ 2018-03-18 10:52 UTC (permalink / raw) To: linux-pci Cc: Marek Vasut, Geert Uytterhoeven, Phil Edworthy, Simon Horman, Wolfram Sang, linux-renesas-soc 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] PCI: rcar: Clean up the macros 2018-03-18 10:52 ` [PATCH 2/2] PCI: rcar: Clean up the macros Marek Vasut @ 2018-03-19 8:40 ` Simon Horman 0 siblings, 0 replies; 13+ messages in thread From: Simon Horman @ 2018-03-19 8:40 UTC (permalink / raw) To: Marek Vasut Cc: linux-pci, Marek Vasut, Geert Uytterhoeven, Phil Edworthy, Wolfram Sang, linux-renesas-soc 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> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() 2018-03-18 10:52 [PATCH 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() Marek Vasut 2018-03-18 10:52 ` [PATCH 2/2] PCI: rcar: Clean up the macros Marek Vasut @ 2018-03-19 8:38 ` Simon Horman 2018-03-19 9:53 ` Marek Vasut 2018-04-25 14:13 ` Lorenzo Pieralisi 2 siblings, 1 reply; 13+ messages in thread From: Simon Horman @ 2018-03-19 8:38 UTC (permalink / raw) To: Marek Vasut Cc: linux-pci, Marek Vasut, Geert Uytterhoeven, Phil Edworthy, Wolfram Sang, linux-renesas-soc 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 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() 2018-03-19 8:38 ` [PATCH 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() Simon Horman @ 2018-03-19 9:53 ` Marek Vasut 2018-03-19 10:53 ` Geert Uytterhoeven 0 siblings, 1 reply; 13+ messages in thread From: Marek Vasut @ 2018-03-19 9:53 UTC (permalink / raw) To: Simon Horman Cc: linux-pci, Marek Vasut, Geert Uytterhoeven, Phil Edworthy, Wolfram Sang, linux-renesas-soc 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() 2018-03-19 9:53 ` Marek Vasut @ 2018-03-19 10:53 ` Geert Uytterhoeven 2018-03-19 10:56 ` Marek Vasut 0 siblings, 1 reply; 13+ messages in thread From: Geert Uytterhoeven @ 2018-03-19 10:53 UTC (permalink / raw) To: Marek Vasut Cc: Simon Horman, linux-pci, Marek Vasut, Geert Uytterhoeven, Phil Edworthy, Wolfram Sang, Linux-Renesas 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() 2018-03-19 10:53 ` Geert Uytterhoeven @ 2018-03-19 10:56 ` Marek Vasut 2018-03-19 13:43 ` Vladimir Zapolskiy 0 siblings, 1 reply; 13+ messages in thread From: Marek Vasut @ 2018-03-19 10:56 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Simon Horman, linux-pci, Marek Vasut, Geert Uytterhoeven, Phil Edworthy, Wolfram Sang, Linux-Renesas 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() 2018-03-19 10:56 ` Marek Vasut @ 2018-03-19 13:43 ` Vladimir Zapolskiy 2018-03-19 13:48 ` Marek Vasut 0 siblings, 1 reply; 13+ messages in thread From: Vladimir Zapolskiy @ 2018-03-19 13:43 UTC (permalink / raw) To: Marek Vasut, Geert Uytterhoeven Cc: Simon Horman, linux-pci, Marek Vasut, Geert Uytterhoeven, Phil Edworthy, Wolfram Sang, Linux-Renesas 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() 2018-03-19 13:43 ` Vladimir Zapolskiy @ 2018-03-19 13:48 ` Marek Vasut 2018-03-19 13:56 ` Vladimir Zapolskiy 0 siblings, 1 reply; 13+ messages in thread From: Marek Vasut @ 2018-03-19 13:48 UTC (permalink / raw) To: Vladimir Zapolskiy, Geert Uytterhoeven Cc: Simon Horman, linux-pci, Marek Vasut, Geert Uytterhoeven, Phil Edworthy, Wolfram Sang, Linux-Renesas 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() 2018-03-19 13:48 ` Marek Vasut @ 2018-03-19 13:56 ` Vladimir Zapolskiy 2018-03-19 14:04 ` Marek Vasut 0 siblings, 1 reply; 13+ messages in thread From: Vladimir Zapolskiy @ 2018-03-19 13:56 UTC (permalink / raw) To: Marek Vasut, Geert Uytterhoeven Cc: Simon Horman, linux-pci, Marek Vasut, Geert Uytterhoeven, Phil Edworthy, Wolfram Sang, Linux-Renesas 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() 2018-03-19 13:56 ` Vladimir Zapolskiy @ 2018-03-19 14:04 ` Marek Vasut 0 siblings, 0 replies; 13+ messages in thread From: Marek Vasut @ 2018-03-19 14:04 UTC (permalink / raw) To: Vladimir Zapolskiy, Geert Uytterhoeven Cc: Simon Horman, linux-pci, Marek Vasut, Geert Uytterhoeven, Phil Edworthy, Wolfram Sang, Linux-Renesas 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() 2018-03-18 10:52 [PATCH 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() Marek Vasut 2018-03-18 10:52 ` [PATCH 2/2] PCI: rcar: Clean up the macros Marek Vasut 2018-03-19 8:38 ` [PATCH 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() Simon Horman @ 2018-04-25 14:13 ` Lorenzo Pieralisi 2018-04-25 15:49 ` Marek Vasut 2 siblings, 1 reply; 13+ messages in thread From: Lorenzo Pieralisi @ 2018-04-25 14:13 UTC (permalink / raw) To: Marek Vasut Cc: linux-pci, Marek Vasut, Geert Uytterhoeven, Phil Edworthy, Simon Horman, Wolfram Sang, linux-renesas-soc 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 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() 2018-04-25 14:13 ` Lorenzo Pieralisi @ 2018-04-25 15:49 ` Marek Vasut 0 siblings, 0 replies; 13+ messages in thread From: Marek Vasut @ 2018-04-25 15:49 UTC (permalink / raw) To: Lorenzo Pieralisi Cc: linux-pci, Marek Vasut, Geert Uytterhoeven, Phil Edworthy, Simon Horman, Wolfram Sang, linux-renesas-soc 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-04-25 15:49 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-18 10:52 [PATCH 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() Marek Vasut 2018-03-18 10:52 ` [PATCH 2/2] PCI: rcar: Clean up the macros Marek Vasut 2018-03-19 8:40 ` Simon Horman 2018-03-19 8:38 ` [PATCH 1/2] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() Simon Horman 2018-03-19 9:53 ` Marek Vasut 2018-03-19 10:53 ` Geert Uytterhoeven 2018-03-19 10:56 ` Marek Vasut 2018-03-19 13:43 ` Vladimir Zapolskiy 2018-03-19 13:48 ` Marek Vasut 2018-03-19 13:56 ` Vladimir Zapolskiy 2018-03-19 14:04 ` Marek Vasut 2018-04-25 14:13 ` Lorenzo Pieralisi 2018-04-25 15:49 ` Marek Vasut
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.