[patch] mmc: fix init f_min

[patch] mmc: fix init f_min

[zhangfei gao]

[-- Attachment #1: Type: text/plain, Size: 947 bytes --]

>From fa357144df43e6c1a9e5e36af497c53437cd41e1 Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zhangfei.gao@marvell.com>
Date: Mon, 20 Sep 2010 15:12:26 -0400
Subject: [PATCH 3/4] mmc: fix init f_min

Change-Id: I7fab1a1a162d9f26fc92c497078f26912c04a3fc

Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
---
 drivers/mmc/core/core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 5db49b1..9114c87 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -910,9 +910,9 @@ static void mmc_power_up(struct mmc_host *host)
 	if (host->f_min > 400000) {
 		pr_warning("%s: Minimum clock frequency too high for "
 				"identification mode\n", mmc_hostname(host));
-		host->ios.clock = host->f_min;
-	} else
 		host->ios.clock = 400000;
+	} else
+		host->ios.clock = host->f_min;

 	host->ios.power_mode = MMC_POWER_ON;
 	mmc_set_ios(host);
-- 
1.7.0.4

[-- Attachment #2: 0003-mmc-fix-init-f_min.patch --]
[-- Type: application/octet-stream, Size: 948 bytes --]

From fa357144df43e6c1a9e5e36af497c53437cd41e1 Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zhangfei.gao@marvell.com>
Date: Mon, 20 Sep 2010 15:12:26 -0400
Subject: [PATCH 3/4] mmc: fix init f_min

Change-Id: I7fab1a1a162d9f26fc92c497078f26912c04a3fc

Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
---
 drivers/mmc/core/core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 5db49b1..9114c87 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -910,9 +910,9 @@ static void mmc_power_up(struct mmc_host *host)
 	if (host->f_min > 400000) {
 		pr_warning("%s: Minimum clock frequency too high for "
 				"identification mode\n", mmc_hostname(host));
-		host->ios.clock = host->f_min;
-	} else
 		host->ios.clock = 400000;
+	} else
+		host->ios.clock = host->f_min;
 
 	host->ios.power_mode = MMC_POWER_ON;
 	mmc_set_ios(host);
-- 
1.7.0.4

Re: [patch] mmc: fix init f_min

[David Vrabel]

zhangfei gao wrote:
> From fa357144df43e6c1a9e5e36af497c53437cd41e1 Mon Sep 17 00:00:00 2001
> From: Zhangfei Gao <zhangfei.gao@marvell.com>
> Date: Mon, 20 Sep 2010 15:12:26 -0400
> Subject: [PATCH 3/4] mmc: fix init f_min
> 
> Change-Id: I7fab1a1a162d9f26fc92c497078f26912c04a3fc

What's this line in the description for?

> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
> ---
>  drivers/mmc/core/core.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 5db49b1..9114c87 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -910,9 +910,9 @@ static void mmc_power_up(struct mmc_host *host)
>  	if (host->f_min > 400000) {
>  		pr_warning("%s: Minimum clock frequency too high for "
>  				"identification mode\n", mmc_hostname(host));
> -		host->ios.clock = host->f_min;
> -	} else
>  		host->ios.clock = 400000;
> +	} else
> +		host->ios.clock = host->f_min;

NAK.

The code is already correctly requesting 400 kHz (unless the controller
can't go that slow).

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: [patch] mmc: fix init f_min

[zhangfei gao]

On Mon, Sep 20, 2010 at 10:34 PM, David Vrabel <david.vrabel@csr.com> wrote:
> zhangfei gao wrote:
>> From fa357144df43e6c1a9e5e36af497c53437cd41e1 Mon Sep 17 00:00:00 2001
>> From: Zhangfei Gao <zhangfei.gao@marvell.com>
>> Date: Mon, 20 Sep 2010 15:12:26 -0400
>> Subject: [PATCH 3/4] mmc: fix init f_min
>>
>> Change-Id: I7fab1a1a162d9f26fc92c497078f26912c04a3fc
>
> What's this line in the description for?
>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
>> ---
>>  drivers/mmc/core/core.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 5db49b1..9114c87 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -910,9 +910,9 @@ static void mmc_power_up(struct mmc_host *host)
>>       if (host->f_min > 400000) {
>>               pr_warning("%s: Minimum clock frequency too high for "
>>                               "identification mode\n", mmc_hostname(host));
>> -             host->ios.clock = host->f_min;
>> -     } else
>>               host->ios.clock = 400000;
>> +     } else
>> +             host->ios.clock = host->f_min;
>
> NAK.
>
> The code is already correctly requesting 400 kHz (unless the controller
> can't go that slow).
>

Maybe i misunderstood.
Is it mean using f_min when f_min > 400000, for example, when f_min =
2000000, use f_min?
But using 400000 when f_min is smaller than 400000, then how about the
card not support 400000 at init.

> David
> --
> David Vrabel, Senior Software Engineer, Drivers
> CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
> Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/
>
>
> Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
>

Re: [patch] mmc: fix init f_min

[zhangfei gao]

On Mon, Sep 20, 2010 at 10:34 AM, David Vrabel <david.vrabel@csr.com> wrote:
> zhangfei gao wrote:
>> From fa357144df43e6c1a9e5e36af497c53437cd41e1 Mon Sep 17 00:00:00 2001
>> From: Zhangfei Gao <zhangfei.gao@marvell.com>
>> Date: Mon, 20 Sep 2010 15:12:26 -0400
>> Subject: [PATCH 3/4] mmc: fix init f_min
>>
>> Change-Id: I7fab1a1a162d9f26fc92c497078f26912c04a3fc
>
> What's this line in the description for?
>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
>> ---
>>  drivers/mmc/core/core.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 5db49b1..9114c87 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -910,9 +910,9 @@ static void mmc_power_up(struct mmc_host *host)
>>       if (host->f_min > 400000) {
>>               pr_warning("%s: Minimum clock frequency too high for "
>>                               "identification mode\n", mmc_hostname(host));
>> -             host->ios.clock = host->f_min;
>> -     } else
>>               host->ios.clock = 400000;
>> +     } else
>> +             host->ios.clock = host->f_min;
>
> NAK.
>
> The code is already correctly requesting 400 kHz (unless the controller
> can't go that slow).

Original code
if (host->f_min > 400000) {
                pr_warning("%s: Minimum clock frequency too high for "
                                "identification mode\n", mmc_hostname(host));
                host->ios.clock = host->f_min;
        } else
                host->ios.clock = 400000;

With this code, the init clock is would be at lease 400 kHz, and no
matter how bigger the host->f_min it is, is this really correct?

>
> David
> --
> David Vrabel, Senior Software Engineer, Drivers
> CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
> Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/
>
>
> Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
>

Re: [patch] mmc: fix init f_min

[David Vrabel]

zhangfei gao wrote:
> On Mon, Sep 20, 2010 at 10:34 AM, David Vrabel <david.vrabel@csr.com> wrote:
>> zhangfei gao wrote:
>>> index 5db49b1..9114c87 100644
>>> --- a/drivers/mmc/core/core.c
>>> +++ b/drivers/mmc/core/core.c
>>> @@ -910,9 +910,9 @@ static void mmc_power_up(struct mmc_host *host)
>>>       if (host->f_min > 400000) {
>>>               pr_warning("%s: Minimum clock frequency too high for "
>>>                               "identification mode\n", mmc_hostname(host));
>>> -             host->ios.clock = host->f_min;
>>> -     } else
>>>               host->ios.clock = 400000;
>>> +     } else
>>> +             host->ios.clock = host->f_min;
>> NAK.
>>
>> The code is already correctly requesting 400 kHz (unless the controller
>> can't go that slow).
> 
> Original code
> if (host->f_min > 400000) {
>                 pr_warning("%s: Minimum clock frequency too high for "
>                                 "identification mode\n", mmc_hostname(host));
>                 host->ios.clock = host->f_min;
>         } else
>                 host->ios.clock = 400000;
> 
> With this code, the init clock is would be at lease 400 kHz, and no
> matter how bigger the host->f_min it is, is this really correct?

It's not clear what you're saying or why you think the original code is
wrong.  Consider a host controller that reports a minimum frequency of 1
Hz.  Obviously we wouldn't want to do the card identification at this
clock rate, yes?

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: [patch] mmc: fix init f_min

[zhangfei gao]

On Mon, Sep 27, 2010 at 5:33 PM, David Vrabel <david.vrabel@csr.com> wrote:
> zhangfei gao wrote:
>> On Mon, Sep 20, 2010 at 10:34 AM, David Vrabel <david.vrabel@csr.com> wrote:
>>> zhangfei gao wrote:
>>>> index 5db49b1..9114c87 100644
>>>> --- a/drivers/mmc/core/core.c
>>>> +++ b/drivers/mmc/core/core.c
>>>> @@ -910,9 +910,9 @@ static void mmc_power_up(struct mmc_host *host)
>>>>       if (host->f_min > 400000) {
>>>>               pr_warning("%s: Minimum clock frequency too high for "
>>>>                               "identification mode\n", mmc_hostname(host));
>>>> -             host->ios.clock = host->f_min;
>>>> -     } else
>>>>               host->ios.clock = 400000;
>>>> +     } else
>>>> +             host->ios.clock = host->f_min;
>>> NAK.
>>>
>>> The code is already correctly requesting 400 kHz (unless the controller
>>> can't go that slow).
>>
>> Original code
>> if (host->f_min > 400000) {
>>                 pr_warning("%s: Minimum clock frequency too high for "
>>                                 "identification mode\n", mmc_hostname(host));
>>                 host->ios.clock = host->f_min;
>>         } else
>>                 host->ios.clock = 400000;
>>
>> With this code, the init clock is would be at lease 400 kHz, and no
>> matter how bigger the host->f_min it is, is this really correct?
>
> It's not clear what you're saying or why you think the original code is
> wrong.  Consider a host controller that reports a minimum frequency of 1
> Hz.  Obviously we wouldn't want to do the card identification at this
> clock rate, yes?

Thanks for your explanation.

The issue you met is f_min is too slow.
The issue we met is f_min is too high which also cause init fail,
But it is OK, we could prevent in other place, duringinit set
mmc->f_min = host->max_clk/256 in sdc 2.0 and mmc->f_min =
host->max_clk/2046 in sdc 3.0

Just curious how to get 400000, which is slowest speed for
initializaion, is this OK for all card?

>
> David
> --
> David Vrabel, Senior Software Engineer, Drivers
> CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
> Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/
>
>
> Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
>

Re: [patch] mmc: fix init f_min

[Hein_Tibosch]

 On 28-9-2010 11:34, zhangfei gao wrote:
>
> Just curious how to get 400000, which is slowest speed for
> initializaion, is this OK for all card?
>
Hi Zhangfei,

It looks as if you missed another thread about f_min recently?

See: https://patchwork.kernel.org/patch/177932/

This was in response to complaints about 400 Khz being too high for some ... platforms

But note that 400 Khz is the highest possible speed to initialize an SD card,

Hein Tibosch