| Message ID | 20180521210522.29346-1-marek.vasut+renesas@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: rcar: Poll more often in rcar_pcie_wait_for_dl() | expand |
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
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.
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
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
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;
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(-)