All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [patch] sdhci: correct f_min in sd 3.0
@ 2010-09-28  4:05 Philip Rakity
  2010-09-28  4:58 ` zhangfei gao
  2010-09-28  7:39 ` [RFC] remove quirk for broken clock - redundant since ops->max_clock defined Philip Rakity
  0 siblings, 2 replies; 4+ messages in thread
From: Philip Rakity @ 2010-09-28  4:05 UTC (permalink / raw)
  To: linux-mmc@vger.kernel.org; +Cc: Zhangfei Gao


The code snippet
> +			for (div = 2; div < SDHCI_MAX_DIV_SPEC_300; div += 2) {
>  				if ((host->max_clk / div) <= clock)
>  					break;

is not correct.  In sdio 3.0 the value is not a power of 2 for the divider but the value to be divided down-- all the bits count
2) 10-bit Divided Clock
Host Controller Version 3.00 supports this mandatory mode instead of the
8-bit Divided Clock Mode.  The length of the divider is extended to 10 bits and all
divider values shall be supported.
3FFh    1/2046  Divided Clock

Philip

Hi Zhangfei,

On Sat, Sep 25, 2010 at 12:23:33AM -0400, zhangfei gao wrote:
> From: Zhangfei Gao <zhangfei.gao@marvell.com>
> Date: Mon, 20 Sep 2010 15:15:18 -0400
> Subject: [PATCH] sdhci: correct f_min in sd 3.0
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
> ---
>  drivers/mmc/host/sdhci.c |    8 +++++---
>  drivers/mmc/host/sdhci.h |    3 +++
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 5928b0a..829e78a 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1007,14 +1007,14 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>  		if (host->max_clk <= clock)
>  			div = 1;
>  		else {
> -			for (div = 2; div < 2046; div += 2) {
> +			for (div = 2; div < SDHCI_MAX_DIV_SPEC_300; div += 2) {
>  				if ((host->max_clk / div) <= clock)
>  					break;
>  			}
>  		}
>  	} else {
>  		/* Version 2.00 divisors must be a power of 2. */
> -		for (div = 1; div < 256; div *= 2) {
> +		for (div = 1; div < SDHCI_MAX_DIV_SPEC_200; div *= 2) {
>  			if ((host->max_clk / div) <= clock)
>  				break;
>  		}
> @@ -1836,8 +1836,10 @@ int sdhci_add_host(struct sdhci_host *host)
>  	mmc->ops = &sdhci_ops;
>  	if (host->ops->get_min_clock)
>  		mmc->f_min = host->ops->get_min_clock(host);
> +	else if (host->version >= SDHCI_SPEC_300)
> +		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_300;
>  	else
> -		mmc->f_min = host->max_clk / 256;
> +		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_200;
>  	mmc->f_max = host->max_clk;
>  	mmc->caps |= MMC_CAP_SDIO_IRQ;
>  
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index ae28a31..d35fb3d 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -185,6 +185,9 @@
>  #define   SDHCI_SPEC_200	1
>  #define   SDHCI_SPEC_300	2
>  
> +#define	SDHCI_MAX_DIV_SPEC_200	256
> +#define	SDHCI_MAX_DIV_SPEC_300	2046
> +
>  struct sdhci_ops;
>  
>  struct sdhci_host {

Thanks very much, I've applied this to mmc-next now.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

* Re: [patch] sdhci: correct f_min in sd 3.0
  2010-09-28  4:05 [patch] sdhci: correct f_min in sd 3.0 Philip Rakity
@ 2010-09-28  4:58 ` zhangfei gao
  2010-09-28  7:39 ` [RFC] remove quirk for broken clock - redundant since ops->max_clock defined Philip Rakity
  1 sibling, 0 replies; 4+ messages in thread
From: zhangfei gao @ 2010-09-28  4:58 UTC (permalink / raw)
  To: Philip Rakity; +Cc: linux-mmc@vger.kernel.org, Zhangfei Gao

On Tue, Sep 28, 2010 at 12:05 AM, Philip Rakity <prakity@marvell.com> wrote:
>
> The code snippet
>> +                     for (div = 2; div < SDHCI_MAX_DIV_SPEC_300; div += 2) {
>>                               if ((host->max_clk / div) <= clock)
>>                                       break;
>
> is not correct.  In sdio 3.0 the value is not a power of 2 for the divider but the value to be divided down-- all the bits count

That's the reason using div += 2 instead of div *= 2

> 2) 10-bit Divided Clock
> Host Controller Version 3.00 supports this mandatory mode instead of the
> 8-bit Divided Clock Mode.  The length of the divider is extended to 10 bits and all
> divider values shall be supported.
> 3FFh    1/2046  Divided Clock
>
> Philip
>
> Hi Zhangfei,
>
> On Sat, Sep 25, 2010 at 12:23:33AM -0400, zhangfei gao wrote:
>> From: Zhangfei Gao <zhangfei.gao@marvell.com>
>> Date: Mon, 20 Sep 2010 15:15:18 -0400
>> Subject: [PATCH] sdhci: correct f_min in sd 3.0
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
>> ---
>>  drivers/mmc/host/sdhci.c |    8 +++++---
>>  drivers/mmc/host/sdhci.h |    3 +++
>>  2 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 5928b0a..829e78a 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1007,14 +1007,14 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>               if (host->max_clk <= clock)
>>                       div = 1;
>>               else {
>> -                     for (div = 2; div < 2046; div += 2) {
>> +                     for (div = 2; div < SDHCI_MAX_DIV_SPEC_300; div += 2) {
>>                               if ((host->max_clk / div) <= clock)
>>                                       break;
>>                       }
>>               }
>>       } else {
>>               /* Version 2.00 divisors must be a power of 2. */
>> -             for (div = 1; div < 256; div *= 2) {
>> +             for (div = 1; div < SDHCI_MAX_DIV_SPEC_200; div *= 2) {
>>                       if ((host->max_clk / div) <= clock)
>>                               break;
>>               }
>> @@ -1836,8 +1836,10 @@ int sdhci_add_host(struct sdhci_host *host)
>>       mmc->ops = &sdhci_ops;
>>       if (host->ops->get_min_clock)
>>               mmc->f_min = host->ops->get_min_clock(host);
>> +     else if (host->version >= SDHCI_SPEC_300)
>> +             mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_300;
>>       else
>> -             mmc->f_min = host->max_clk / 256;
>> +             mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_200;
>>       mmc->f_max = host->max_clk;
>>       mmc->caps |= MMC_CAP_SDIO_IRQ;
>>
>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>> index ae28a31..d35fb3d 100644
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -185,6 +185,9 @@
>>  #define   SDHCI_SPEC_200     1
>>  #define   SDHCI_SPEC_300     2
>>
>> +#define      SDHCI_MAX_DIV_SPEC_200  256
>> +#define      SDHCI_MAX_DIV_SPEC_300  2046
>> +
>>  struct sdhci_ops;
>>
>>  struct sdhci_host {
>
> Thanks very much, I've applied this to mmc-next now.
>
> - Chris.
> --
> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
> One Laptop Per Child
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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

* [RFC] remove quirk for broken clock - redundant since ops->max_clock defined
  2010-09-28  4:05 [patch] sdhci: correct f_min in sd 3.0 Philip Rakity
  2010-09-28  4:58 ` zhangfei gao
@ 2010-09-28  7:39 ` Philip Rakity
  2010-09-28 11:11   ` Anton Vorontsov
  1 sibling, 1 reply; 4+ messages in thread
From: Philip Rakity @ 2010-09-28  7:39 UTC (permalink / raw)
  To: linux-mmc@vger.kernel.org; +Cc: Anton Vorontsov


>From 2b436be109ad004e94a2b63f71f6bb8e00f9d3da Mon Sep 17 00:00:00 2001
From: Philip Rakity <prakity@marvell.com>
Date: Tue, 28 Sep 2010 00:34:17 -0700
Subject: [RFC] remove quirk for broken clock - redundant since ops->max_clock defined

Are we okay moving in this direction?  Patch is NOT tested since RFC -- does compile.

If we are moving away from using both quirks and host->ops to define
platform specific actions -- just use the host->ops operation to
get max_clock.

Signed-off-by: Philip Rakity <prakity@marvell.com>
---
 drivers/mmc/host/sdhci-cns3xxx.c |    1 -
 drivers/mmc/host/sdhci.c         |   24 +++++++++++++-----------
 drivers/mmc/host/sdhci.h         |    2 --
 3 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/host/sdhci-cns3xxx.c b/drivers/mmc/host/sdhci-cns3xxx.c
index b7050b3..27c896d 100644
--- a/drivers/mmc/host/sdhci-cns3xxx.c
+++ b/drivers/mmc/host/sdhci-cns3xxx.c
@@ -91,7 +91,6 @@ struct sdhci_pltfm_data sdhci_cns3xxx_pdata = {
 	.quirks = SDHCI_QUIRK_BROKEN_DMA |
 		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
 		  SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
-		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
 		  SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
 		  SDHCI_QUIRK_NONSTANDARD_CLOCK,
 };
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 451fad3..64af676 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1799,18 +1799,20 @@ int sdhci_add_host(struct sdhci_host *host)
 		mmc_dev(host->mmc)->dma_mask = &host->dma_mask;
 	}
 
-	host->max_clk =
-		(caps & SDHCI_CLOCK_BASE_MASK) >> SDHCI_CLOCK_BASE_SHIFT;
-	host->max_clk *= 1000000;
-	if (host->max_clk == 0 || host->quirks &
-			SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN) {
-		if (!host->ops->get_max_clock) {
-			printk(KERN_ERR
-			       "%s: Hardware doesn't specify base clock "
-			       "frequency.\n", mmc_hostname(mmc));
-			return -ENODEV;
-		}
+	if (host->ops->get_max_clock) {
 		host->max_clk = host->ops->get_max_clock(host);
+	} else {
+		host->max_clk =
+			(caps & SDHCI_CLOCK_BASE_MASK)
+				>> SDHCI_CLOCK_BASE_SHIFT;
+		host->max_clk *= 1000000;
+	}
+
+	if (host->max_clk == 0 ) {
+		printk(KERN_ERR
+		       "%s: Hardware doesn't specify base clock "
+		       "frequency.\n", mmc_hostname(mmc));
+		return -ENODEV;
 	}
 
 	host->timeout_clk =
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index f8c18cd..7c15bc5 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -237,8 +237,6 @@ struct sdhci_host {
 #define SDHCI_QUIRK_DELAY_AFTER_POWER			(1<<23)
 /* Controller uses SDCLK instead of TMCLK for data timeouts */
 #define SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK		(1<<24)
-/* Controller reports wrong base clock capability */
-#define SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN		(1<<25)
 /* Controller cannot support End Attribute in NOP ADMA descriptor */
 #define SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC		(1<<26)
 /* Controller is missing device caps. Use caps provided by host */
-- 
1.6.0.4


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

* Re: [RFC] remove quirk for broken clock - redundant since ops->max_clock defined
  2010-09-28  7:39 ` [RFC] remove quirk for broken clock - redundant since ops->max_clock defined Philip Rakity
@ 2010-09-28 11:11   ` Anton Vorontsov
  0 siblings, 0 replies; 4+ messages in thread
From: Anton Vorontsov @ 2010-09-28 11:11 UTC (permalink / raw)
  To: Philip Rakity; +Cc: linux-mmc@vger.kernel.org

On Tue, Sep 28, 2010 at 12:39:47AM -0700, Philip Rakity wrote:
> 
> >From 2b436be109ad004e94a2b63f71f6bb8e00f9d3da Mon Sep 17 00:00:00 2001
> From: Philip Rakity <prakity@marvell.com>
> Date: Tue, 28 Sep 2010 00:34:17 -0700
> Subject: [RFC] remove quirk for broken clock - redundant since ops->max_clock defined
> 
> Are we okay moving in this direction?  Patch is NOT tested since RFC -- does compile.
> 
> If we are moving away from using both quirks and host->ops to define
> platform specific actions -- just use the host->ops operation to
> get max_clock.

Again, I can only cite Pierre Ossman:

  On Sun, Feb 08, 2009 at 10:04:40PM +0100, Pierre Ossman wrote:
  | As I told Ben, I prefer if we stick to the standard as much as
  | possible. So no external info unless the register is set to zero.

Your patch breaks that rule. Other than that, technically it
looks OK.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

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

end of thread, other threads:[~2010-09-28 11:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-28  4:05 [patch] sdhci: correct f_min in sd 3.0 Philip Rakity
2010-09-28  4:58 ` zhangfei gao
2010-09-28  7:39 ` [RFC] remove quirk for broken clock - redundant since ops->max_clock defined Philip Rakity
2010-09-28 11:11   ` Anton Vorontsov

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.