[PATCH] mmc: change initial clock setting

[PATCH] mmc: change initial clock setting

[Jaehoon Chung]

This patch is bug fixed..
I think that host->ios.clock(initial clock) will be set lower than 400KHz.
But in this code, if host->f_min is higher than 400KHz, print warning message and set higher than 400KHz.
And We use SDHCI_QUIRK_NONSTANDARD_CLOCK..so using get_min_clock function. 
but didn't effect nothing through used get_min_clock function.


 Signed-off-by: Jaehoon Chung <jh80.chung@samsung.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 569e94d..9cd5767 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.6.0.4 

Re: [PATCH] mmc: change initial clock setting

[Matt Fleming]

On Fri, 18 Jun 2010 13:05:44 +0900, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> This patch is bug fixed..
> I think that host->ios.clock(initial clock) will be set lower than 400KHz.
> But in this code, if host->f_min is higher than 400KHz, print warning message and set higher than 400KHz.
> And We use SDHCI_QUIRK_NONSTANDARD_CLOCK..so using get_min_clock function. 
> but didn't effect nothing through used get_min_clock function.

I'm a little confused. What problem are you fixing with this patch? What
issue are you seeing? SDHCI_QUIRK_NONSTANDARD_CLOCK is only for sdhci.
If this is the old "my card won't initialise at 400kHz" chestnut I don't
think a solution has ever been agreed upon. The problem was that while
some cards won't initialise at 400kHz, some controllers will try to
initialise at too low a frequency which causes other problems.

Sascha Hauer explains the reason for setting ios.clock to 400kHz here,

       http://lkml.org/lkml/2010/1/5/120

So I think your patch will cause problems.

Which driver are you using? The driver should have set the f_min value
to be <= 400kHz. Maybe it is a good idea to set the value of ios.clock
to 400kHz if the driver has set a bogus f_min value, we will at least
have a fighting chance of initialising the card (though I would say that
the driver was broken, but at least core.c will print a warning).

Re: [PATCH] mmc: change initial clock setting

[Jaehoon Chung]

On 2010-06-18 오후 9:42, Matt Fleming wrote:
> On Fri, 18 Jun 2010 13:05:44 +0900, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> This patch is bug fixed..
>> I think that host->ios.clock(initial clock) will be set lower than 400KHz.
>> But in this code, if host->f_min is higher than 400KHz, print warning message and set higher than 400KHz.
>> And We use SDHCI_QUIRK_NONSTANDARD_CLOCK..so using get_min_clock function. 
>> but didn't effect nothing through used get_min_clock function.
> 
> I'm a little confused. What problem are you fixing with this patch? What
> issue are you seeing? SDHCI_QUIRK_NONSTANDARD_CLOCK is only for sdhci.
> If this is the old "my card won't initialise at 400kHz" chestnut I don't
> think a solution has ever been agreed upon. The problem was that while
> some cards won't initialise at 400kHz, some controllers will try to
> initialise at too low a frequency which causes other problems.
> 
> Sascha Hauer explains the reason for setting ios.clock to 400kHz here,
> 
>        http://lkml.org/lkml/2010/1/5/120
> 
> So I think your patch will cause problems.
> 
> Which driver are you using? The driver should have set the f_min value
> to be <= 400kHz. Maybe it is a good idea to set the value of ios.clock
> to 400kHz if the driver has set a bogus f_min value, we will at least
> have a fighting chance of initialising the card (though I would say that
> the driver was broken, but at least core.c will print a warning).
> 

when ios.clock is higher than 400KHz, why not change 400KHz?(if 400KHz seemed like good value)
if clock value is higher than 400KHz, i think card can't initialize...maybe

when sdhci controller set f_min value(ie, 300KHz), clock changed 400KHz in core.c.
just this code purpose to print warning message?

Re: [PATCH] mmc: change initial clock setting

[Matt Fleming]

On Mon, 21 Jun 2010 13:29:21 +0900, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> On 2010-06-18 오후 9:42, Matt Fleming wrote:
> > On Fri, 18 Jun 2010 13:05:44 +0900, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> >> This patch is bug fixed..
> >> I think that host->ios.clock(initial clock) will be set lower than 400KHz.
> >> But in this code, if host->f_min is higher than 400KHz, print warning message and set higher than 400KHz.
> >> And We use SDHCI_QUIRK_NONSTANDARD_CLOCK..so using get_min_clock function. 
> >> but didn't effect nothing through used get_min_clock function.
> > 
> > I'm a little confused. What problem are you fixing with this patch? What
> > issue are you seeing? SDHCI_QUIRK_NONSTANDARD_CLOCK is only for sdhci.
> > If this is the old "my card won't initialise at 400kHz" chestnut I don't
> > think a solution has ever been agreed upon. The problem was that while
> > some cards won't initialise at 400kHz, some controllers will try to
> > initialise at too low a frequency which causes other problems.
> > 
> > Sascha Hauer explains the reason for setting ios.clock to 400kHz here,
> > 
> >        http://lkml.org/lkml/2010/1/5/120
> > 
> > So I think your patch will cause problems.
> > 
> > Which driver are you using? The driver should have set the f_min value
> > to be <= 400kHz. Maybe it is a good idea to set the value of ios.clock
> > to 400kHz if the driver has set a bogus f_min value, we will at least
> > have a fighting chance of initialising the card (though I would say that
> > the driver was broken, but at least core.c will print a warning).
> > 
> 
> when ios.clock is higher than 400KHz, why not change 400KHz?(if 400KHz seemed like good value)
> if clock value is higher than 400KHz, i think card can't initialize...maybe

I think this part of your patch is OK.

> when sdhci controller set f_min value(ie, 300KHz), clock changed 400KHz in core.c.
> just this code purpose to print warning message?

Well, the purpose of the code is to set the clock to 400kHz if the value
of f_min is too low, as having a low value can cause some controllers to
break.