All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mmc: sdhci: use udelay instead of mdelay
@ 2016-05-30  7:55 Baranowska, BeataX
  2016-05-30  8:00 ` Arnd Bergmann
  0 siblings, 1 reply; 6+ messages in thread
From: Baranowska, BeataX @ 2016-05-30  7:55 UTC (permalink / raw)
  To: Hunter, Adrian, Ulf Hansson, linux-mmc, linux-kernel
  Cc: Dong, Chuanxiao, Jarosz, SebastianX

From: Chuanxiao Dong <chuanxiao.dong@intel.com>

This patch will use udelay instead of mdelay when waiting for
SDHCI hardware to be stable. udelay can help to reduce the waiting
time when is in critical region which is protected by spinlock.

With this patch, __sdhci_set_ios only take a few microseconds to be
done.

Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
---
drivers/mmc/host/sdhci.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index e010ea4eb6f5..56d2c7567d97 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -173,8 +173,8 @@ void sdhci_reset(struct sdhci_host *host, u8 mask)
                        sdhci_runtime_pm_bus_off(host);
        }

-       /* Wait max 100 ms */
-       timeout = 100;
+       /* Wait max 10000 ms */
+       timeout = 10000;

        /* hw clears the bit when it's done */
        while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
@@ -185,7 +185,7 @@ void sdhci_reset(struct sdhci_host *host, u8 mask)
                        return;
                }
                timeout--;
-               mdelay(1);
+               udelay(10);
        }
 }
 EXPORT_SYMBOL_GPL(sdhci_reset);
@@ -985,8 +985,8 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
        /* Initially, a command has no error */
        cmd->error = 0;

-       /* Wait max 10 ms */
-       timeout = 10;
+       /* Wait max 1000 ms */
+       timeout = 1000;

        mask = SDHCI_CMD_INHIBIT;
        if ((cmd->data != NULL) || (cmd->flags & MMC_RSP_BUSY))
@@ -1007,7 +1007,7 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
                        return;
                }
                timeout--;
-               mdelay(1);
+               udelay(10);
        }

        timeout = jiffies;
@@ -1240,8 +1240,8 @@ void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
        clk |= SDHCI_CLOCK_INT_EN;
        sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);

-       /* Wait max 20 ms */
-       timeout = 20;
+       /* Wait max 2000 ms */
+       timeout = 2000;
        while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
                & SDHCI_CLOCK_INT_STABLE)) {
                if (timeout == 0) {
@@ -1251,7 +1251,7 @@ void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
                        return;
                }
                timeout--;
-               mdelay(1);
+               udelay(10);
        }

        clk |= SDHCI_CLOCK_CARD_EN;
--


Regards,
Beata Baranowska

Intel Technology Poland sp. z o.o.
ul. Słowackiego 173 | 80-298 Gdańsk 
Sąd Rejonowy Gdańsk Północ 
VII Wydział Gospodarczy Krajowego Rejestru Sądowego - KRS 101882 
NIP 957-07-52-316 | Kapitał zakładowy 200.000 PLN

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] mmc: sdhci: use udelay instead of mdelay
  2016-05-30  7:55 [PATCH] mmc: sdhci: use udelay instead of mdelay Baranowska, BeataX
@ 2016-05-30  8:00 ` Arnd Bergmann
  2016-05-31  8:53   ` Baranowska, BeataX
  0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2016-05-30  8:00 UTC (permalink / raw)
  To: Baranowska, BeataX
  Cc: Hunter, Adrian, Ulf Hansson, linux-mmc, linux-kernel, Dong,
	Chuanxiao, Jarosz, SebastianX

On Monday, May 30, 2016 7:55:55 AM CEST Baranowska, BeataX wrote:
> From: Chuanxiao Dong <chuanxiao.dong@intel.com>
> 
> This patch will use udelay instead of mdelay when waiting for
> SDHCI hardware to be stable. udelay can help to reduce the waiting
> time when is in critical region which is protected by spinlock.
> 
> With this patch, __sdhci_set_ios only take a few microseconds to be
> done.
> 
> Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> ---
> drivers/mmc/host/sdhci.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index e010ea4eb6f5..56d2c7567d97 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -173,8 +173,8 @@ void sdhci_reset(struct sdhci_host *host, u8 mask)
>                         sdhci_runtime_pm_bus_off(host);
>         }
> 
> -       /* Wait max 100 ms */
> -       timeout = 100;
> +       /* Wait max 10000 ms */
> +       timeout = 10000;
> 
>         /* hw clears the bit when it's done */
>         while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
> @@ -185,7 +185,7 @@ void sdhci_reset(struct sdhci_host *host, u8 mask)
>                         return;
>                 }
>                 timeout--;
> -               mdelay(1);
> +               udelay(10);
>         }
>  }
>  EXPORT_SYMBOL_GPL(sdhci_reset);

This can significantly increase the timeout length. I think you should
instead use time_before() to see how many jiffies have passed since
the start.

However, the real question is why the reset function gets called under
a spinlock in the first place. Can you try to rearrange the code so
it doesn't need the lock at all and you can just use msleep() instead?

	Arnd

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] mmc: sdhci: use udelay instead of mdelay
  2016-05-30  8:00 ` Arnd Bergmann
@ 2016-05-31  8:53   ` Baranowska, BeataX
  2016-05-31  9:16     ` Arnd Bergmann
  0 siblings, 1 reply; 6+ messages in thread
From: Baranowska, BeataX @ 2016-05-31  8:53 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Hunter, Adrian, Ulf Hansson, linux-mmc, linux-kernel, Dong,
	Chuanxiao, Jarosz, SebastianX

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Monday, May 30, 2016 10:01 AM
> To: Baranowska, BeataX <beatax.baranowska@intel.com>
> Cc: Hunter, Adrian <adrian.hunter@intel.com>; Ulf Hansson
> <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; linux-
> kernel@vger.kernel.org; Dong, Chuanxiao <chuanxiao.dong@intel.com>;
> Jarosz, SebastianX <sebastianx.jarosz@intel.com>
> Subject: Re: [PATCH] mmc: sdhci: use udelay instead of mdelay
> 
> On Monday, May 30, 2016 7:55:55 AM CEST Baranowska, BeataX wrote:
> > From: Chuanxiao Dong <chuanxiao.dong@intel.com>
> >
> > This patch will use udelay instead of mdelay when waiting for SDHCI
> > hardware to be stable. udelay can help to reduce the waiting time when
> > is in critical region which is protected by spinlock.
> >
> > With this patch, __sdhci_set_ios only take a few microseconds to be
> > done.
> >
> > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> > ---
> > drivers/mmc/host/sdhci.c | 18 +++++++++---------
> >  1 file changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index
> > e010ea4eb6f5..56d2c7567d97 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -173,8 +173,8 @@ void sdhci_reset(struct sdhci_host *host, u8 mask)
> >                         sdhci_runtime_pm_bus_off(host);
> >         }
> >
> > -       /* Wait max 100 ms */
> > -       timeout = 100;
> > +       /* Wait max 10000 ms */
> > +       timeout = 10000;
> >
> >         /* hw clears the bit when it's done */
> >         while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) { @@
> > -185,7 +185,7 @@ void sdhci_reset(struct sdhci_host *host, u8 mask)
> >                         return;
> >                 }
> >                 timeout--;
> > -               mdelay(1);
> > +               udelay(10);
> >         }
> >  }
> >  EXPORT_SYMBOL_GPL(sdhci_reset);
> 
> This can significantly increase the timeout length. I think you should instead
> use time_before() to see how many jiffies have passed since the start.
> 
> However, the real question is why the reset function gets called under a
> spinlock in the first place. Can you try to rearrange the code so it doesn't
> need the lock at all and you can just use msleep() instead?
> 
> 	Arnd

Thank you for your quick reply.
Could you please clarify what do you mean is called under a spinlock? Any is not used here?

Beata Baranowska

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mmc: sdhci: use udelay instead of mdelay
  2016-05-31  8:53   ` Baranowska, BeataX
@ 2016-05-31  9:16     ` Arnd Bergmann
  2016-05-31  9:38       ` Baranowska, BeataX
  0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2016-05-31  9:16 UTC (permalink / raw)
  To: Baranowska, BeataX
  Cc: Hunter, Adrian, Ulf Hansson, linux-mmc, linux-kernel, Dong,
	Chuanxiao, Jarosz, SebastianX

On Tuesday, May 31, 2016 8:53:18 AM CEST Baranowska, BeataX wrote:
> > 
> > On Monday, May 30, 2016 7:55:55 AM CEST Baranowska, BeataX wrote:
> > > From: Chuanxiao Dong <chuanxiao.dong@intel.com>
> > >
> > > This patch will use udelay instead of mdelay when waiting for SDHCI
> > > hardware to be stable. udelay can help to reduce the waiting time when
> > > is in critical region which is protected by spinlock.
> > >
> > > With this patch, __sdhci_set_ios only take a few microseconds to be
> > > done.
> > >
> > > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> > > ---
> > > drivers/mmc/host/sdhci.c | 18 +++++++++---------
> > >  1 file changed, 9 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index
> > > e010ea4eb6f5..56d2c7567d97 100644
> > > --- a/drivers/mmc/host/sdhci.c
> > > +++ b/drivers/mmc/host/sdhci.c
> > > @@ -173,8 +173,8 @@ void sdhci_reset(struct sdhci_host *host, u8 mask)
> > >                         sdhci_runtime_pm_bus_off(host);
> > >         }
> > >
> > > -       /* Wait max 100 ms */
> > > -       timeout = 100;
> > > +       /* Wait max 10000 ms */
> > > +       timeout = 10000;
> > >
> > >         /* hw clears the bit when it's done */
> > >         while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) { @@
> > > -185,7 +185,7 @@ void sdhci_reset(struct sdhci_host *host, u8 mask)
> > >                         return;
> > >                 }
> > >                 timeout--;
> > > -               mdelay(1);
> > > +               udelay(10);
> > >         }
> > >  }
> > >  EXPORT_SYMBOL_GPL(sdhci_reset);
> > 
> > This can significantly increase the timeout length. I think you should instead
> > use time_before() to see how many jiffies have passed since the start.
> > 
> > However, the real question is why the reset function gets called under a
> > spinlock in the first place. Can you try to rearrange the code so it doesn't
> > need the lock at all and you can just use msleep() instead?
> > 
> >       Arnd
> 
> Thank you for your quick reply.
> Could you please clarify what do you mean is called under a spinlock?
> Any is not used here?

You write that the function is called in a critical region protected
by the spinlock, so I was wondering if that is actually necessary.

Usually a device reset should be done in normal process context without
any spinlocks so you can call normal sleeping functions.

	Arnd

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] mmc: sdhci: use udelay instead of mdelay
  2016-05-31  9:16     ` Arnd Bergmann
@ 2016-05-31  9:38       ` Baranowska, BeataX
  0 siblings, 0 replies; 6+ messages in thread
From: Baranowska, BeataX @ 2016-05-31  9:38 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Hunter, Adrian, Ulf Hansson, linux-mmc, linux-kernel, Dong,
	Chuanxiao, Jarosz, SebastianX



> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Tuesday, May 31, 2016 11:17 AM
> To: Baranowska, BeataX <beatax.baranowska@intel.com>
> Cc: Hunter, Adrian <adrian.hunter@intel.com>; Ulf Hansson
> <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; linux-
> kernel@vger.kernel.org; Dong, Chuanxiao <chuanxiao.dong@intel.com>;
> Jarosz, SebastianX <sebastianx.jarosz@intel.com>
> Subject: Re: [PATCH] mmc: sdhci: use udelay instead of mdelay
> 
> On Tuesday, May 31, 2016 8:53:18 AM CEST Baranowska, BeataX wrote:
> > >
> > > On Monday, May 30, 2016 7:55:55 AM CEST Baranowska, BeataX wrote:
> > > > From: Chuanxiao Dong <chuanxiao.dong@intel.com>
> > > >
> > > > This patch will use udelay instead of mdelay when waiting for
> > > > SDHCI hardware to be stable. udelay can help to reduce the waiting
> > > > time when is in critical region which is protected by spinlock.
> > > >
> > > > With this patch, __sdhci_set_ios only take a few microseconds to
> > > > be done.
> > > >
> > > > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> > > > ---
> > > > drivers/mmc/host/sdhci.c | 18 +++++++++---------
> > > >  1 file changed, 9 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > > > index
> > > > e010ea4eb6f5..56d2c7567d97 100644
> > > > --- a/drivers/mmc/host/sdhci.c
> > > > +++ b/drivers/mmc/host/sdhci.c
> > > > @@ -173,8 +173,8 @@ void sdhci_reset(struct sdhci_host *host, u8
> mask)
> > > >                         sdhci_runtime_pm_bus_off(host);
> > > >         }
> > > >
> > > > -       /* Wait max 100 ms */
> > > > -       timeout = 100;
> > > > +       /* Wait max 10000 ms */
> > > > +       timeout = 10000;
> > > >
> > > >         /* hw clears the bit when it's done */
> > > >         while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
> > > > @@
> > > > -185,7 +185,7 @@ void sdhci_reset(struct sdhci_host *host, u8 mask)
> > > >                         return;
> > > >                 }
> > > >                 timeout--;
> > > > -               mdelay(1);
> > > > +               udelay(10);
> > > >         }
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(sdhci_reset);
> > >
> > > This can significantly increase the timeout length. I think you
> > > should instead use time_before() to see how many jiffies have passed
> since the start.
> > >
> > > However, the real question is why the reset function gets called
> > > under a spinlock in the first place. Can you try to rearrange the
> > > code so it doesn't need the lock at all and you can just use msleep()
> instead?
> > >
> > >       Arnd
> >
> > Thank you for your quick reply.
> > Could you please clarify what do you mean is called under a spinlock?
> > Any is not used here?
> 
> You write that the function is called in a critical region protected by the
> spinlock, so I was wondering if that is actually necessary.
> 
> Usually a device reset should be done in normal process context without any
> spinlocks so you can call normal sleeping functions.
> 
> 	Arnd

Ok I understand now. Thank you.

Beata Baranowska

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] mmc: sdhci: use udelay instead of mdelay
@ 2014-08-12  8:19 Yunpeng Gao
  0 siblings, 0 replies; 6+ messages in thread
From: Yunpeng Gao @ 2014-08-12  8:19 UTC (permalink / raw)
  To: linux-mmc; +Cc: Chuanxiao Dong, Yunpeng Gao

From: Chuanxiao Dong <chuanxiao.dong@intel.com>

This patch uses udelay instead of mdelay when waiting
for SDHC hardware to be stable. udelay can help to
reduce the waiting time when is in critical region
which is protected by spinlock.

Signed-off-by: Yunpeng Gao <yunpeng.gao@intel.com>
Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
---
 drivers/mmc/host/sdhci.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 47055f3..74d8c42 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -179,7 +179,7 @@ void sdhci_reset(struct sdhci_host *host, u8 mask)
 	}
 
 	/* Wait max 100 ms */
-	timeout = 100;
+	timeout = 10000;
 
 	/* hw clears the bit when it's done */
 	while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
@@ -190,7 +190,7 @@ void sdhci_reset(struct sdhci_host *host, u8 mask)
 			return;
 		}
 		timeout--;
-		mdelay(1);
+		udelay(10);
 	}
 }
 EXPORT_SYMBOL_GPL(sdhci_reset);
@@ -975,7 +975,7 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 	WARN_ON(host->cmd);
 
 	/* Wait max 10 ms */
-	timeout = 10;
+	timeout = 1000;
 
 	mask = SDHCI_CMD_INHIBIT;
 	if ((cmd->data != NULL) || (cmd->flags & MMC_RSP_BUSY))
@@ -996,7 +996,7 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 			return;
 		}
 		timeout--;
-		mdelay(1);
+		udelay(10);
 	}
 
 	timeout = jiffies;
@@ -1202,7 +1202,7 @@ clock_set:
 	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
 
 	/* Wait max 20 ms */
-	timeout = 20;
+	timeout = 2000;
 	while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
 		& SDHCI_CLOCK_INT_STABLE)) {
 		if (timeout == 0) {
@@ -1212,7 +1212,7 @@ clock_set:
 			return;
 		}
 		timeout--;
-		mdelay(1);
+		udelay(10);
 	}
 
 	clk |= SDHCI_CLOCK_CARD_EN;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-05-31  9:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-30  7:55 [PATCH] mmc: sdhci: use udelay instead of mdelay Baranowska, BeataX
2016-05-30  8:00 ` Arnd Bergmann
2016-05-31  8:53   ` Baranowska, BeataX
2016-05-31  9:16     ` Arnd Bergmann
2016-05-31  9:38       ` Baranowska, BeataX
  -- strict thread matches above, loose matches on Subject: below --
2014-08-12  8:19 Yunpeng Gao

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.