[v3,1/2] mmc: Move mmc_delay() to include/linux/mmc/core.h

Move mmc_delay() from drivers/mmc/core/core.h to
include/linux/mmc/core.h. So when other functions
call it with include syntax using <linux/mmc/core.h>
of absolute path rather than "../core/core.h" of
relative path.

Signed-off-by: Chunhe Lan <Chunhe.Lan@freescale.com>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
Cc: Chris Ball <cjb@laptop.org>
---
 drivers/mmc/core/core.h  |   12 ------------
 include/linux/mmc/core.h |   11 +++++++++++
 2 files changed, 11 insertions(+), 12 deletions(-)

On Friday 10 August 2012, Chunhe Lan wrote:

> +static inline void mmc_delay(unsigned int ms)
> +{
> +	if (ms < 1000 / HZ) {
> +		cond_resched();
> +		mdelay(ms);
> +	} else {
> +		msleep(ms);
> +	}
> +}

I would actually question the point in this function to start with: The
decision whether to call mdelay() or msleep() should only be based on
whether you are allowed to sleep in the caller context. The idea of


	cond_resched();
	mdelay(ms);

sets off alarm bells, and I would always replace that with msleep().

	Arnd

On Friday 21 September 2012, Chunhe Lan wrote:
> On 08/10/2012 09:27 AM, Arnd Bergmann wrote:
> > On Friday 10 August 2012, Chunhe Lan wrote:
> >
> >       cond_resched();
> >       mdelay(ms);
> >
> > sets off alarm bells, and I would always replace that with msleep().
>      I think that it does not replace with msleep().
>      When the time of sleep is very short, program should not been scheduled
>      in the context. Because it expends the more time.
> 

A time measured in miliseconds is never "very short" for the scheduler,
a lot of things can happen during that time span. The code I quoted
also does not care too much about accuracy, otherwise it would adapt
the time in the mdelay based on whether the cond_resched() actually
schedules to another thread.

	Arnd

On 08/10/2012 09:27 AM, Arnd Bergmann wrote:
> On Friday 10 August 2012, Chunhe Lan wrote:
>
>> +static inline void mmc_delay(unsigned int ms)
>> +{
>> +	if (ms < 1000 / HZ) {
>> +		cond_resched();
>> +		mdelay(ms);
>> +	} else {
>> +		msleep(ms);
>> +	}
>> +}
> I would actually question the point in this function to start with: The
> decision whether to call mdelay() or msleep() should only be based on
> whether you are allowed to sleep in the caller context. The idea of
>
>
> 	cond_resched();
> 	mdelay(ms);
>
> sets off alarm bells, and I would always replace that with msleep().
     I think that it does not replace with msleep().
     When the time of sleep is very short, program should not been scheduled
     in the context. Because it expends the more time.

     Thanks,
     Chunhe
>
> 	Arnd
>

On Monday 24 September 2012, Chunhe Lan wrote:
>     OK. As you have mentioned, it would been modified to such:
> 
> static inline void mmc_delay(unsigned int ms)
> {
>          if (ms < 1000 / HZ) {
>                  cond_resched();
>                  msleep(ms);
>          } else {
>                  msleep(ms);
>          }
> }

This version would be rather broken, because it compares times
in two different units (ms and jiffies), and because it
does a cond_resched() directly before an msleep: both of which
end up calling schedule() and being away for some time,
cond_resched() for an unknown time, and msleep for a minimum
time on top of that.

> OR such:
> 
> static inline void mmc_delay(unsigned int ms)
> {
>           msleep(ms);
> }

That would be my preferred choice, unless someone has specific issues with this.

> OR other code?

Well, in principle, you could implement something like

static inline void mmc_delay(unsigned int ms)
{
	ktime_t end = ktime_add_us(ktime_get(), ms * 1000);

	while (1) {
		s64 remaining;

		cond_resched();

		remaining = ktime_to_us(ktime_sub(end, ktime_get()));

		if (remaining < 0)
			break;

		udelay(min_t(u32, remaining, 100));
	}
}

	Arnd

On Mon, Sep 24, 2012 at 8:17 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>
>> static inline void mmc_delay(unsigned int ms)
>> {
>>           msleep(ms);
>> }
>
> That would be my preferred choice, unless someone has specific issues with this.

If we're going to do that, then just get rid of mmc_delay and replace
all calls to it with msleep().  Why bother with the inline function?
There's nothing really MMC-specific about it.

On 09/21/2012 08:33 AM, Arnd Bergmann wrote:
> On Friday 21 September 2012, Chunhe Lan wrote:
>> On 08/10/2012 09:27 AM, Arnd Bergmann wrote:
>>> On Friday 10 August 2012, Chunhe Lan wrote:
>>>
>>>        cond_resched();
>>>        mdelay(ms);
>>>
>>> sets off alarm bells, and I would always replace that with msleep().
>>       I think that it does not replace with msleep().
>>       When the time of sleep is very short, program should not been scheduled
>>       in the context. Because it expends the more time.
>>
> A time measured in miliseconds is never "very short" for the scheduler,
> a lot of things can happen during that time span. The code I quoted
> also does not care too much about accuracy, otherwise it would adapt
> the time in the mdelay based on whether the cond_resched() actually
> schedules to another thread.
    OK. As you have mentioned, it would been modified to such:

static inline void mmc_delay(unsigned int ms)
{
         if (ms < 1000 / HZ) {
                 cond_resched();
                 msleep(ms);
         } else {
                 msleep(ms);
         }
}

OR such:

static inline void mmc_delay(unsigned int ms)
{
                 msleep(ms);
}

OR other code?

Thanks,
Chunhe
>
> 	Arnd
>