Re: [PATCH RFT] mmc: core: use usleep_range rather than HZ magic in mmc_delay()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 2017/11/16 15:47, Ulf Hansson wrote:
On 14 November 2017 at 23:55, Wolfram Sang
<wsa+renesas@xxxxxxxxxxxxxxxxxxxx> wrote:
Documentation/timers/timers-howto.txt recommends to use usleep_range for
delays 1-20ms. Let's adhere to it. No need for messing with HZ and still
do busy looping these days.


Sorry for chime in this topic but I think for some cases, for instance,
mmc_host_set_uhs_voltage calls mmc_delay and it should guarantee the
delay is accurate, especially for the comment"Keep clock gated for at
least 10 ms, though spec only says 5 ms"

But the usleep_range won't guarantee that, see

https://patchwork.kernel.org/patch/9369963/

It wasn't fixed at the end, so you should carefully modify
this fundamentally.

Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
---

Here is a more detailed test page for this describing my tests:

https://elinux.org/Tests:mmc-delay-refactor

I did mainly test the insert/eject cycle because powering up the cards
seemed to trigger most delays. Transferring data did not cause any calls
to mmc_delay() for me. Please let me know if someone knows a test pattern
which should be included before applying this change.

Works fine for me on Lager (R-Car H2) and Salvator-X (R-Car M3-W). Testing on
other platforms very welcome. This should be independent of the IP core.

  drivers/mmc/core/core.h | 8 +++-----
  1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 71e6c6d7ceb70d..b2877e2d740fa5 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -62,12 +62,10 @@ void mmc_set_initial_state(struct mmc_host *host);

  static inline void mmc_delay(unsigned int ms)
  {
-       if (ms < 1000 / HZ) {
-               cond_resched();
-               mdelay(ms);
-       } else {
+       if (ms <= 20)
+               usleep_range(ms * 1000, ms * 1250);

This means we get usleep_range(20000, 25000) for the worst case.

Perhaps we want "ms <= 16" instead, thus getting usleep
usleep_range(16000, 20000) for the worst case? No?

+       else
                 msleep(ms);
-       }
  }

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux