[patch 1/1]sdhci: sdhc spec 3.0 add some modification

[patch 1/1]sdhci: sdhc spec 3.0 add some modification

[zhangfei gao]

sdhci spec 3.0 has some difference, and here is patch to add support.

>From 25eeab5e3b58128600968e5600aeaf9390c067d8 Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zgao6@marvell.com>
Date: Fri, 6 Aug 2010 05:47:59 +0800
Subject: [PATCH] sdhci: aligh with sdhc spec 3.00

	1. support 8 bit data transfer width
	2. support 10-bit Divided Clock Mode

Signed-off-by: Zhangfei Gao <zgao6@marvell.com>
---
 drivers/mmc/host/sdhci.c |   15 +++++++++++----
 drivers/mmc/host/sdhci.h |    5 +++++
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c6d1bd8..89b323e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1002,7 +1002,8 @@ static void sdhci_set_clock(struct sdhci_host
*host, unsigned int clock)
 	}
 	div >>= 1;

-	clk = div << SDHCI_DIVIDER_SHIFT;
+	clk = (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
+	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIVIDER_SHIFT) <<
SDHCI_DIVIDER_SHIFT_HI;
 	clk |= SDHCI_CLOCK_INT_EN;
 	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);

@@ -1159,10 +1160,16 @@ static void sdhci_set_ios(struct mmc_host
*mmc, struct mmc_ios *ios)

 	ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);

-	if (ios->bus_width == MMC_BUS_WIDTH_4)
+	if (ios->bus_width == MMC_BUS_WIDTH_8) {
+		ctrl |= SDHCI_CTRL_8BITBUS;
+		ctrl &= ~SDHCI_CTRL_4BITBUS;
+	} else if (ios->bus_width == MMC_BUS_WIDTH_4) {
 		ctrl |= SDHCI_CTRL_4BITBUS;
-	else
+		ctrl &= ~SDHCI_CTRL_8BITBUS;
+	} else {
 		ctrl &= ~SDHCI_CTRL_4BITBUS;
+		ctrl &= ~SDHCI_CTRL_4BITBUS;
+	}

 	if (ios->timing == MMC_TIMING_SD_HS)
 		ctrl |= SDHCI_CTRL_HISPD;
@@ -1681,7 +1688,7 @@ int sdhci_add_host(struct sdhci_host *host)
 	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
 	host->version = (host->version & SDHCI_SPEC_VER_MASK)
 				>> SDHCI_SPEC_VER_SHIFT;
-	if (host->version > SDHCI_SPEC_200) {
+	if (host->version > SDHCI_SPEC_300) {
 		printk(KERN_ERR "%s: Unknown controller version (%d). "
 			"You may experience problems.\n", mmc_hostname(mmc),
 			host->version);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index c846813..2cb14eb 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -72,6 +72,7 @@
 #define   SDHCI_CTRL_ADMA1	0x08
 #define   SDHCI_CTRL_ADMA32	0x10
 #define   SDHCI_CTRL_ADMA64	0x18
+#define  SDHCI_CTRL_8BITBUS	0x20

 #define SDHCI_POWER_CONTROL	0x29
 #define  SDHCI_POWER_ON		0x01
@@ -85,6 +86,9 @@

 #define SDHCI_CLOCK_CONTROL	0x2C
 #define  SDHCI_DIVIDER_SHIFT	8
+#define  SDHCI_DIVIDER_SHIFT_HI	6
+#define  SDHCI_DIV_MASK	0xFF
+#define  SDHCI_DIV_HI_MASK	0x300
 #define  SDHCI_CLOCK_CARD_EN	0x0004
 #define  SDHCI_CLOCK_INT_STABLE	0x0002
 #define  SDHCI_CLOCK_INT_EN	0x0001
@@ -177,6 +181,7 @@
 #define  SDHCI_SPEC_VER_SHIFT	0
 #define   SDHCI_SPEC_100	0
 #define   SDHCI_SPEC_200	1
+#define   SDHCI_SPEC_300	2

 struct sdhci_ops;

-- 
1.6.0.4

Re: [patch 1/1]sdhci: sdhc spec 3.0 add some modification

[Kyungmin Park]

On Thu, Aug 5, 2010 at 3:08 PM, zhangfei gao <zhangfei.gao@gmail.com> wrote:
> sdhci spec 3.0 has some difference, and here is patch to add support.
>
> From 25eeab5e3b58128600968e5600aeaf9390c067d8 Mon Sep 17 00:00:00 2001
> From: Zhangfei Gao <zgao6@marvell.com>
> Date: Fri, 6 Aug 2010 05:47:59 +0800
> Subject: [PATCH] sdhci: aligh with sdhc spec 3.00
>
>        1. support 8 bit data transfer width
8 bit buswidth patch is already merged to mm tree.

http://marc.info/?l=linux-mmc&m=127783663526810&w=3

Thank you,
Kyungmin Park


>        2. support 10-bit Divided Clock Mode
>
> Signed-off-by: Zhangfei Gao <zgao6@marvell.com>
> ---
>  drivers/mmc/host/sdhci.c |   15 +++++++++++----
>  drivers/mmc/host/sdhci.h |    5 +++++
>  2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index c6d1bd8..89b323e 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1002,7 +1002,8 @@ static void sdhci_set_clock(struct sdhci_host
> *host, unsigned int clock)
>        }
>        div >>= 1;
>
> -       clk = div << SDHCI_DIVIDER_SHIFT;
> +       clk = (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
> +       clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIVIDER_SHIFT) <<
> SDHCI_DIVIDER_SHIFT_HI;
>        clk |= SDHCI_CLOCK_INT_EN;
>        sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>
> @@ -1159,10 +1160,16 @@ static void sdhci_set_ios(struct mmc_host
> *mmc, struct mmc_ios *ios)
>
>        ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
>
> -       if (ios->bus_width == MMC_BUS_WIDTH_4)
> +       if (ios->bus_width == MMC_BUS_WIDTH_8) {
> +               ctrl |= SDHCI_CTRL_8BITBUS;
> +               ctrl &= ~SDHCI_CTRL_4BITBUS;
> +       } else if (ios->bus_width == MMC_BUS_WIDTH_4) {
>                ctrl |= SDHCI_CTRL_4BITBUS;
> -       else
> +               ctrl &= ~SDHCI_CTRL_8BITBUS;
> +       } else {
>                ctrl &= ~SDHCI_CTRL_4BITBUS;
> +               ctrl &= ~SDHCI_CTRL_4BITBUS;
> +       }
>
>        if (ios->timing == MMC_TIMING_SD_HS)
>                ctrl |= SDHCI_CTRL_HISPD;
> @@ -1681,7 +1688,7 @@ int sdhci_add_host(struct sdhci_host *host)
>        host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
>        host->version = (host->version & SDHCI_SPEC_VER_MASK)
>                                >> SDHCI_SPEC_VER_SHIFT;
> -       if (host->version > SDHCI_SPEC_200) {
> +       if (host->version > SDHCI_SPEC_300) {
>                printk(KERN_ERR "%s: Unknown controller version (%d). "
>                        "You may experience problems.\n", mmc_hostname(mmc),
>                        host->version);
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index c846813..2cb14eb 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -72,6 +72,7 @@
>  #define   SDHCI_CTRL_ADMA1     0x08
>  #define   SDHCI_CTRL_ADMA32    0x10
>  #define   SDHCI_CTRL_ADMA64    0x18
> +#define  SDHCI_CTRL_8BITBUS    0x20
>
>  #define SDHCI_POWER_CONTROL    0x29
>  #define  SDHCI_POWER_ON                0x01
> @@ -85,6 +86,9 @@
>
>  #define SDHCI_CLOCK_CONTROL    0x2C
>  #define  SDHCI_DIVIDER_SHIFT   8
> +#define  SDHCI_DIVIDER_SHIFT_HI        6
> +#define  SDHCI_DIV_MASK        0xFF
> +#define  SDHCI_DIV_HI_MASK     0x300
>  #define  SDHCI_CLOCK_CARD_EN   0x0004
>  #define  SDHCI_CLOCK_INT_STABLE        0x0002
>  #define  SDHCI_CLOCK_INT_EN    0x0001
> @@ -177,6 +181,7 @@
>  #define  SDHCI_SPEC_VER_SHIFT  0
>  #define   SDHCI_SPEC_100       0
>  #define   SDHCI_SPEC_200       1
> +#define   SDHCI_SPEC_300       2
>
>  struct sdhci_ops;
>
> --
> 1.6.0.4
> --
> 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
>

Re: [patch 1/1]sdhci: sdhc spec 3.0 add some modification

[zhangfei gao]

>>
>>        1. support 8 bit data transfer width
> 8 bit buswidth patch is already merged to mm tree.
>
> http://marc.info/?l=linux-mmc&m=127783663526810&w=3
>
> Thank you,
> Kyungmin Park
>

Thanks Kyungmin, updated without 8 bit support.

From 2b717444443acdac90c5f31522a1515a2000749a Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zgao6@marvell.com>
Date: Fri, 6 Aug 2010 07:10:01 +0800
Subject: [PATCH] sdhc: support 10-bit divided clock Mode


Signed-off-by: Zhangfei Gao <zgao6@marvell.com>
---
 drivers/mmc/host/sdhci.c |    5 +++--
 drivers/mmc/host/sdhci.h |    4 ++++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c6d1bd8..212dc9e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1002,7 +1002,8 @@ static void sdhci_set_clock(struct sdhci_host
*host, unsigned int clock)
 	}
 	div >>= 1;

-	clk = div << SDHCI_DIVIDER_SHIFT;
+	clk = (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
+	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIVIDER_SHIFT) <<
SDHCI_DIVIDER_SHIFT_HI;
 	clk |= SDHCI_CLOCK_INT_EN;
 	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);

@@ -1681,7 +1682,7 @@ int sdhci_add_host(struct sdhci_host *host)
 	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
 	host->version = (host->version & SDHCI_SPEC_VER_MASK)
 				>> SDHCI_SPEC_VER_SHIFT;
-	if (host->version > SDHCI_SPEC_200) {
+	if (host->version > SDHCI_SPEC_300) {
 		printk(KERN_ERR "%s: Unknown controller version (%d). "
 			"You may experience problems.\n", mmc_hostname(mmc),
 			host->version);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index c846813..7c09b4f 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -85,6 +85,9 @@

 #define SDHCI_CLOCK_CONTROL	0x2C
 #define  SDHCI_DIVIDER_SHIFT	8
+#define  SDHCI_DIVIDER_SHIFT_HI	6
+#define  SDHCI_DIV_MASK	0xFF
+#define  SDHCI_DIV_HI_MASK	0x300
 #define  SDHCI_CLOCK_CARD_EN	0x0004
 #define  SDHCI_CLOCK_INT_STABLE	0x0002
 #define  SDHCI_CLOCK_INT_EN	0x0001
@@ -177,6 +180,7 @@
 #define  SDHCI_SPEC_VER_SHIFT	0
 #define   SDHCI_SPEC_100	0
 #define   SDHCI_SPEC_200	1
+#define   SDHCI_SPEC_300	2

 struct sdhci_ops;

-- 
1.6.0.4

Re: [patch 1/1]sdhci: sdhc spec 3.0 add some modification

[Matt Fleming]

On Thu, Aug 05, 2010 at 03:09:14PM +0800, zhangfei gao wrote:
> >>
> >>        1. support 8 bit data transfer width
> > 8 bit buswidth patch is already merged to mm tree.
> >
> > http://marc.info/?l=linux-mmc&m=127783663526810&w=3
> >
> > Thank you,
> > Kyungmin Park
> >
> 
> Thanks Kyungmin, updated without 8 bit support.
> 
> From 2b717444443acdac90c5f31522a1515a2000749a Mon Sep 17 00:00:00 2001
> From: Zhangfei Gao <zgao6@marvell.com>
> Date: Fri, 6 Aug 2010 07:10:01 +0800
> Subject: [PATCH] sdhc: support 10-bit divided clock Mode
> 
> 
> Signed-off-by: Zhangfei Gao <zgao6@marvell.com>
> ---
>  drivers/mmc/host/sdhci.c |    5 +++--
>  drivers/mmc/host/sdhci.h |    4 ++++
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index c6d1bd8..212dc9e 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1002,7 +1002,8 @@ static void sdhci_set_clock(struct sdhci_host
> *host, unsigned int clock)
>  	}
>  	div >>= 1;
> 
> -	clk = div << SDHCI_DIVIDER_SHIFT;
> +	clk = (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
> +	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIVIDER_SHIFT) <<
> SDHCI_DIVIDER_SHIFT_HI;
>  	clk |= SDHCI_CLOCK_INT_EN;
>  	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> 

This patch is missing a change to the way we calculate 'div'. While your
patch is correct, I think it would be better to fit some more things
into it.

This is the current code for calculating the clock divider,


	for (div = 1;div < 256;div *= 2) {
		if ((host->max_clk / div) <= clock)
			break;
	}
	div >>= 1;

	clk = div << SDHCI_DIVIDER_SHIFT;


even with your patch applied 'div' will never be greater 256 and we
won't be executing your changes. So I think this patch also needs to
bump the upper limit of 'div' to 2046.

We should probably introduce some safety at this point and check that
the divider value is legal for the version of the host controller, to
stop people shooting themselves in the foot if they mess up their clock
settings. I was thinking of something like,

	if (host->version >= SDHCI_SPEC_300)
		max_div = 2046;
	else
		max_div = 256;

	for (div = 1;div < max_div;div *= 2) {
		if ((host->max_clk / div) <= clock)
			break;
	}

What do you think?

Re: [patch 1/1]sdhci: sdhc spec 3.0 add some modification

[zhangfei gao]

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

On Mon, Aug 9, 2010 at 6:10 PM, Matt Fleming <matt@console-pimps.org> wrote:
> On Thu, Aug 05, 2010 at 03:09:14PM +0800, zhangfei gao wrote:
>> >>
>> >>        1. support 8 bit data transfer width
>> > 8 bit buswidth patch is already merged to mm tree.
>> >
>> > http://marc.info/?l=linux-mmc&m=127783663526810&w=3
>> >
>> > Thank you,
>> > Kyungmin Park
>> >
>>
>> Thanks Kyungmin, updated without 8 bit support.
>>
>> From 2b717444443acdac90c5f31522a1515a2000749a Mon Sep 17 00:00:00 2001
>> From: Zhangfei Gao <zgao6@marvell.com>
>> Date: Fri, 6 Aug 2010 07:10:01 +0800
>> Subject: [PATCH] sdhc: support 10-bit divided clock Mode
>>
>>
>> Signed-off-by: Zhangfei Gao <zgao6@marvell.com>
>> ---
>>  drivers/mmc/host/sdhci.c |    5 +++--
>>  drivers/mmc/host/sdhci.h |    4 ++++
>>  2 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index c6d1bd8..212dc9e 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1002,7 +1002,8 @@ static void sdhci_set_clock(struct sdhci_host
>> *host, unsigned int clock)
>>       }
>>       div >>= 1;
>>
>> -     clk = div << SDHCI_DIVIDER_SHIFT;
>> +     clk = (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
>> +     clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIVIDER_SHIFT) <<
>> SDHCI_DIVIDER_SHIFT_HI;
>>       clk |= SDHCI_CLOCK_INT_EN;
>>       sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>>
>
> This patch is missing a change to the way we calculate 'div'. While your
> patch is correct, I think it would be better to fit some more things
> into it.
>
> This is the current code for calculating the clock divider,
>
>
>        for (div = 1;div < 256;div *= 2) {
>                if ((host->max_clk / div) <= clock)
>                        break;
>        }
>        div >>= 1;
>
>        clk = div << SDHCI_DIVIDER_SHIFT;
>
>
> even with your patch applied 'div' will never be greater 256 and we
> won't be executing your changes. So I think this patch also needs to
> bump the upper limit of 'div' to 2046.
>
> We should probably introduce some safety at this point and check that
> the divider value is legal for the version of the host controller, to
> stop people shooting themselves in the foot if they mess up their clock
> settings. I was thinking of something like,
>
>        if (host->version >= SDHCI_SPEC_300)
>                max_div = 2046;
>        else
>                max_div = 256;
>
>        for (div = 1;div < max_div;div *= 2) {
>                if ((host->max_clk / div) <= clock)
>                        break;
>        }
>
> What do you think?
>

Thanks for your careful review, in our platform, we use max/min for
the max_div, so miss this modification :(
Update the patch, help review again.

>From 4b0a4d60fb39437a7fde7e52fb769fa0110c052f Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zgao6@marvell.com>
Date: Fri, 6 Aug 2010 07:10:01 +0800
Subject: [PATCH] sdhc: support 10-bit divided clock Mode

Signed-off-by: Zhangfei Gao <zgao6@marvell.com>
Reviewed-by: Matt Fleming <matt@console-pimps.org>
---
 drivers/mmc/host/sdhci.c |   14 ++++++++++----
 drivers/mmc/host/sdhci.h |    4 ++++
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c6d1bd8..c4b3d6f 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -978,7 +978,7 @@ static void sdhci_finish_command(struct sdhci_host *host)

 static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 {
-	int div;
+	int div, max_div;
 	u16 clk;
 	unsigned long timeout;

@@ -996,13 +996,19 @@ static void sdhci_set_clock(struct sdhci_host
*host, unsigned int clock)
 	if (clock == 0)
 		goto out;

-	for (div = 1;div < 256;div *= 2) {
+	if (host->version >= SDHCI_SPEC_300)
+		max_div = 2046;
+	else
+		max_div = 256;
+
+	for (div = 1;div < max_div;div *= 2) {
 		if ((host->max_clk / div) <= clock)
 			break;
 	}
 	div >>= 1;

-	clk = div << SDHCI_DIVIDER_SHIFT;
+	clk = (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
+	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIVIDER_SHIFT) <<
SDHCI_DIVIDER_SHIFT_HI;
 	clk |= SDHCI_CLOCK_INT_EN;
 	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);

@@ -1681,7 +1687,7 @@ int sdhci_add_host(struct sdhci_host *host)
 	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
 	host->version = (host->version & SDHCI_SPEC_VER_MASK)
 				>> SDHCI_SPEC_VER_SHIFT;
-	if (host->version > SDHCI_SPEC_200) {
+	if (host->version > SDHCI_SPEC_300) {
 		printk(KERN_ERR "%s: Unknown controller version (%d). "
 			"You may experience problems.\n", mmc_hostname(mmc),
 			host->version);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index c846813..7c09b4f 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -85,6 +85,9 @@

 #define SDHCI_CLOCK_CONTROL	0x2C
 #define  SDHCI_DIVIDER_SHIFT	8
+#define  SDHCI_DIVIDER_SHIFT_HI	6
+#define  SDHCI_DIV_MASK	0xFF
+#define  SDHCI_DIV_HI_MASK	0x300
 #define  SDHCI_CLOCK_CARD_EN	0x0004
 #define  SDHCI_CLOCK_INT_STABLE	0x0002
 #define  SDHCI_CLOCK_INT_EN	0x0001
@@ -177,6 +180,7 @@
 #define  SDHCI_SPEC_VER_SHIFT	0
 #define   SDHCI_SPEC_100	0
 #define   SDHCI_SPEC_200	1
+#define   SDHCI_SPEC_300	2

 struct sdhci_ops;

-- 
1.6.0.4

[-- Attachment #2: 0001-sdhc-support-10-bit-divided-clock-Mode.patch --]
[-- Type: application/octet-stream, Size: 2499 bytes --]

From 4b0a4d60fb39437a7fde7e52fb769fa0110c052f Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zgao6@marvell.com>
Date: Fri, 6 Aug 2010 07:10:01 +0800
Subject: [PATCH] sdhc: support 10-bit divided clock Mode

Signed-off-by: Zhangfei Gao <zgao6@marvell.com>
Reviewed-by: Matt Fleming <matt@console-pimps.org>
---
 drivers/mmc/host/sdhci.c |   14 ++++++++++----
 drivers/mmc/host/sdhci.h |    4 ++++
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c6d1bd8..c4b3d6f 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -978,7 +978,7 @@ static void sdhci_finish_command(struct sdhci_host *host)
 
 static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 {
-	int div;
+	int div, max_div;
 	u16 clk;
 	unsigned long timeout;
 
@@ -996,13 +996,19 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 	if (clock == 0)
 		goto out;
 
-	for (div = 1;div < 256;div *= 2) {
+	if (host->version >= SDHCI_SPEC_300)
+		max_div = 2046;
+	else
+		max_div = 256;
+
+	for (div = 1;div < max_div;div *= 2) {
 		if ((host->max_clk / div) <= clock)
 			break;
 	}
 	div >>= 1;
 
-	clk = div << SDHCI_DIVIDER_SHIFT;
+	clk = (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
+	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIVIDER_SHIFT) << SDHCI_DIVIDER_SHIFT_HI;
 	clk |= SDHCI_CLOCK_INT_EN;
 	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
 
@@ -1681,7 +1687,7 @@ int sdhci_add_host(struct sdhci_host *host)
 	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
 	host->version = (host->version & SDHCI_SPEC_VER_MASK)
 				>> SDHCI_SPEC_VER_SHIFT;
-	if (host->version > SDHCI_SPEC_200) {
+	if (host->version > SDHCI_SPEC_300) {
 		printk(KERN_ERR "%s: Unknown controller version (%d). "
 			"You may experience problems.\n", mmc_hostname(mmc),
 			host->version);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index c846813..7c09b4f 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -85,6 +85,9 @@
 
 #define SDHCI_CLOCK_CONTROL	0x2C
 #define  SDHCI_DIVIDER_SHIFT	8
+#define  SDHCI_DIVIDER_SHIFT_HI	6
+#define  SDHCI_DIV_MASK	0xFF
+#define  SDHCI_DIV_HI_MASK	0x300
 #define  SDHCI_CLOCK_CARD_EN	0x0004
 #define  SDHCI_CLOCK_INT_STABLE	0x0002
 #define  SDHCI_CLOCK_INT_EN	0x0001
@@ -177,6 +180,7 @@
 #define  SDHCI_SPEC_VER_SHIFT	0
 #define   SDHCI_SPEC_100	0
 #define   SDHCI_SPEC_200	1
+#define   SDHCI_SPEC_300	2
 
 struct sdhci_ops;
 
-- 
1.6.0.4

Re: [patch 1/1]sdhci: sdhc spec 3.0 add some modification

[Matt Fleming]

On Mon, Aug 09, 2010 at 08:33:16PM +0800, zhangfei gao wrote:
> 
> Thanks for your careful review, in our platform, we use max/min for
> the max_div, so miss this modification :(
> Update the patch, help review again.

What do you mean? Where does max/min come from? It would definitely
be best if everybody uses the same code to calculate max_div. Have
you tested the max_div changes? Are you now using this patch on
your platform or are you still maintaining a patch to do it a
different way?

If you've got a separate patch and it does this things better
than the way we currently do them, get it upstream! :-)

Re: [patch 1/1]sdhci: sdhc spec 3.0 add some modification

[zhangfei gao]

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

On Wed, Aug 11, 2010 at 12:32 AM, Matt Fleming <matt@console-pimps.org> wrote:
> On Mon, Aug 09, 2010 at 08:33:16PM +0800, zhangfei gao wrote:
>>
>> Thanks for your careful review, in our platform, we use max/min for
>> the max_div, so miss this modification :(
>> Update the patch, help review again.
>
> What do you mean? Where does max/min come from? It would definitely
> be best if everybody uses the same code to calculate max_div. Have
> you tested the max_div changes? Are you now using this patch on
> your platform or are you still maintaining a patch to do it a
> different way?
>
> If you've got a separate patch and it does this things better
> than the way we currently do them, get it upstream! :-)
>

Hi, Matt

Thanks for your reminder, totally agree with you.
We also use this patch in our platform, and find another issue, div
should +=2 instead of *=2.
-	for (div = 1;div < 256;div *= 2) {
+	for (div = 1;div < max_div;div += 2) {

Update patch, help review.

>From 517077ffae66dd12c409b7c6b41a7014db66a3cc Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zgao6@marvell.com>
Date: Fri, 6 Aug 2010 07:10:01 +0800
Subject: [PATCH] sdhc: support 10-bit divided clock Mode

Signed-off-by: Zhangfei Gao <zgao6@marvell.com>
Reviewed-by: Matt Fleming <matt@console-pimps.org>
---
 drivers/mmc/host/sdhci.c |   14 ++++++++++----
 drivers/mmc/host/sdhci.h |    4 ++++
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c6d1bd8..b2d5e5e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -978,7 +978,7 @@ static void sdhci_finish_command(struct sdhci_host *host)

 static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 {
-	int div;
+	int div, max_div;
 	u16 clk;
 	unsigned long timeout;

@@ -996,13 +996,19 @@ static void sdhci_set_clock(struct sdhci_host
*host, unsigned int clock)
 	if (clock == 0)
 		goto out;

-	for (div = 1;div < 256;div *= 2) {
+	if (host->version >= SDHCI_SPEC_300)
+		max_div = 2046;
+	else
+		max_div = 256;
+
+	for (div = 1;div < max_div;div += 2) {
 		if ((host->max_clk / div) <= clock)
 			break;
 	}
 	div >>= 1;

-	clk = div << SDHCI_DIVIDER_SHIFT;
+	clk = (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
+	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIVIDER_SHIFT) <<
SDHCI_DIVIDER_SHIFT_HI;
 	clk |= SDHCI_CLOCK_INT_EN;
 	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);

@@ -1681,7 +1687,7 @@ int sdhci_add_host(struct sdhci_host *host)
 	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
 	host->version = (host->version & SDHCI_SPEC_VER_MASK)
 				>> SDHCI_SPEC_VER_SHIFT;
-	if (host->version > SDHCI_SPEC_200) {
+	if (host->version > SDHCI_SPEC_300) {
 		printk(KERN_ERR "%s: Unknown controller version (%d). "
 			"You may experience problems.\n", mmc_hostname(mmc),
 			host->version);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index c846813..7c09b4f 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -85,6 +85,9 @@

 #define SDHCI_CLOCK_CONTROL	0x2C
 #define  SDHCI_DIVIDER_SHIFT	8
+#define  SDHCI_DIVIDER_SHIFT_HI	6
+#define  SDHCI_DIV_MASK	0xFF
+#define  SDHCI_DIV_HI_MASK	0x300
 #define  SDHCI_CLOCK_CARD_EN	0x0004
 #define  SDHCI_CLOCK_INT_STABLE	0x0002
 #define  SDHCI_CLOCK_INT_EN	0x0001
@@ -177,6 +180,7 @@
 #define  SDHCI_SPEC_VER_SHIFT	0
 #define   SDHCI_SPEC_100	0
 #define   SDHCI_SPEC_200	1
+#define   SDHCI_SPEC_300	2

 struct sdhci_ops;

-- 
1.6.0.4

[-- Attachment #2: 0001-sdhc-support-10-bit-divided-clock-Mode.patch --]
[-- Type: application/octet-stream, Size: 2499 bytes --]

From 517077ffae66dd12c409b7c6b41a7014db66a3cc Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zgao6@marvell.com>
Date: Fri, 6 Aug 2010 07:10:01 +0800
Subject: [PATCH] sdhc: support 10-bit divided clock Mode

Signed-off-by: Zhangfei Gao <zgao6@marvell.com>
Reviewed-by: Matt Fleming <matt@console-pimps.org>
---
 drivers/mmc/host/sdhci.c |   14 ++++++++++----
 drivers/mmc/host/sdhci.h |    4 ++++
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c6d1bd8..b2d5e5e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -978,7 +978,7 @@ static void sdhci_finish_command(struct sdhci_host *host)
 
 static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 {
-	int div;
+	int div, max_div;
 	u16 clk;
 	unsigned long timeout;
 
@@ -996,13 +996,19 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 	if (clock == 0)
 		goto out;
 
-	for (div = 1;div < 256;div *= 2) {
+	if (host->version >= SDHCI_SPEC_300)
+		max_div = 2046;
+	else
+		max_div = 256;
+
+	for (div = 1;div < max_div;div += 2) {
 		if ((host->max_clk / div) <= clock)
 			break;
 	}
 	div >>= 1;
 
-	clk = div << SDHCI_DIVIDER_SHIFT;
+	clk = (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
+	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIVIDER_SHIFT) << SDHCI_DIVIDER_SHIFT_HI;
 	clk |= SDHCI_CLOCK_INT_EN;
 	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
 
@@ -1681,7 +1687,7 @@ int sdhci_add_host(struct sdhci_host *host)
 	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
 	host->version = (host->version & SDHCI_SPEC_VER_MASK)
 				>> SDHCI_SPEC_VER_SHIFT;
-	if (host->version > SDHCI_SPEC_200) {
+	if (host->version > SDHCI_SPEC_300) {
 		printk(KERN_ERR "%s: Unknown controller version (%d). "
 			"You may experience problems.\n", mmc_hostname(mmc),
 			host->version);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index c846813..7c09b4f 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -85,6 +85,9 @@
 
 #define SDHCI_CLOCK_CONTROL	0x2C
 #define  SDHCI_DIVIDER_SHIFT	8
+#define  SDHCI_DIVIDER_SHIFT_HI	6
+#define  SDHCI_DIV_MASK	0xFF
+#define  SDHCI_DIV_HI_MASK	0x300
 #define  SDHCI_CLOCK_CARD_EN	0x0004
 #define  SDHCI_CLOCK_INT_STABLE	0x0002
 #define  SDHCI_CLOCK_INT_EN	0x0001
@@ -177,6 +180,7 @@
 #define  SDHCI_SPEC_VER_SHIFT	0
 #define   SDHCI_SPEC_100	0
 #define   SDHCI_SPEC_200	1
+#define   SDHCI_SPEC_300	2
 
 struct sdhci_ops;
 
-- 
1.6.0.4

Re: [patch 1/1]sdhci: sdhc spec 3.0 add some modification

[zhangfei gao]

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

On Wed, Aug 11, 2010 at 3:44 AM, zhangfei gao <zhangfei.gao@gmail.com> wrote:
> On Wed, Aug 11, 2010 at 12:32 AM, Matt Fleming <matt@console-pimps.org> wrote:
>> On Mon, Aug 09, 2010 at 08:33:16PM +0800, zhangfei gao wrote:
>>>
>>> Thanks for your careful review, in our platform, we use max/min for
>>> the max_div, so miss this modification :(
>>> Update the patch, help review again.
>>
>> What do you mean? Where does max/min come from? It would definitely
>> be best if everybody uses the same code to calculate max_div. Have
>> you tested the max_div changes? Are you now using this patch on
>> your platform or are you still maintaining a patch to do it a
>> different way?
>>
>> If you've got a separate patch and it does this things better
>> than the way we currently do them, get it upstream! :-)
>>

Hi,  Matt

Originally, the div step by *2, and in fact the step should be +2.
However in such case, div start from 2 would be more accurate, so
modify to the following.
Though start from 1 is also OK, since >>2  would get the same result.


-       for (div = 1;div < 256;div *= 2) {
+       if (host->version >= SDHCI_SPEC_300)
+               max_div = 2046;
+       else
+               max_div = 256;
+
+       if(host->max_clk <= clock)
+               div = 1;
+       for (div = 2; div < max_div; div += 2) {

Thanks

updated patch:
>From b17eb3ca9dd23af63ada240e8f77bdd873e853a7 Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zgao6@marvell.com>
Date: Fri, 6 Aug 2010 07:10:01 +0800
Subject: [PATCH] sdhc: support 10-bit divided clock Mode

Signed-off-by: Zhangfei Gao <zgao6@marvell.com>
Reviewed-by: Matt Fleming <matt@console-pimps.org>
---
 drivers/mmc/host/sdhci.c |   16 ++++++++++++----
 drivers/mmc/host/sdhci.h |    4 ++++
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 7855121..ccfbc40 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -983,7 +983,7 @@ static void sdhci_finish_command(struct sdhci_host *host)

 static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 {
-	int div;
+	int div, max_div;
 	u16 clk;
 	unsigned long timeout;

@@ -1001,13 +1001,21 @@ static void sdhci_set_clock(struct sdhci_host
*host, unsigned int clock)
 	if (clock == 0)
 		goto out;

-	for (div = 1;div < 256;div *= 2) {
+	if (host->version >= SDHCI_SPEC_300)
+		max_div = 2046;
+	else
+		max_div = 256;
+
+	if(host->max_clk <= clock)
+		div = 1;
+	for (div = 2; div < max_div; div += 2) {
 		if ((host->max_clk / div) <= clock)
 			break;
 	}
 	div >>= 1;

-	clk = div << SDHCI_DIVIDER_SHIFT;
+	clk = (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
+	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIVIDER_SHIFT) <<
SDHCI_DIVIDER_SHIFT_HI;
 	clk |= SDHCI_CLOCK_INT_EN;
 	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);

@@ -1707,7 +1715,7 @@ int sdhci_add_host(struct sdhci_host *host)
 	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
 	host->version = (host->version & SDHCI_SPEC_VER_MASK)
 				>> SDHCI_SPEC_VER_SHIFT;
-	if (host->version > SDHCI_SPEC_200) {
+	if (host->version > SDHCI_SPEC_300) {
 		printk(KERN_ERR "%s: Unknown controller version (%d). "
 			"You may experience problems.\n", mmc_hostname(mmc),
 			host->version);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 036cfae..d7ef7e4 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -86,6 +86,9 @@

 #define SDHCI_CLOCK_CONTROL	0x2C
 #define  SDHCI_DIVIDER_SHIFT	8
+#define  SDHCI_DIVIDER_SHIFT_HI	6
+#define  SDHCI_DIV_MASK	0xFF
+#define  SDHCI_DIV_HI_MASK	0x300
 #define  SDHCI_CLOCK_CARD_EN	0x0004
 #define  SDHCI_CLOCK_INT_STABLE	0x0002
 #define  SDHCI_CLOCK_INT_EN	0x0001
@@ -178,6 +181,7 @@
 #define  SDHCI_SPEC_VER_SHIFT	0
 #define   SDHCI_SPEC_100	0
 #define   SDHCI_SPEC_200	1
+#define   SDHCI_SPEC_300	2

 struct sdhci_ops;

-- 
1.7.0.4

[-- Attachment #2: 0001-sdhc-support-10-bit-divided-clock-Mode.patch --]
[-- Type: text/x-patch, Size: 2546 bytes --]

From b17eb3ca9dd23af63ada240e8f77bdd873e853a7 Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zgao6@marvell.com>
Date: Fri, 6 Aug 2010 07:10:01 +0800
Subject: [PATCH] sdhc: support 10-bit divided clock Mode

Signed-off-by: Zhangfei Gao <zgao6@marvell.com>
Reviewed-by: Matt Fleming <matt@console-pimps.org>
---
 drivers/mmc/host/sdhci.c |   16 ++++++++++++----
 drivers/mmc/host/sdhci.h |    4 ++++
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 7855121..ccfbc40 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -983,7 +983,7 @@ static void sdhci_finish_command(struct sdhci_host *host)
 
 static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 {
-	int div;
+	int div, max_div;
 	u16 clk;
 	unsigned long timeout;
 
@@ -1001,13 +1001,21 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 	if (clock == 0)
 		goto out;
 
-	for (div = 1;div < 256;div *= 2) {
+	if (host->version >= SDHCI_SPEC_300)
+		max_div = 2046;
+	else
+		max_div = 256;
+
+	if(host->max_clk <= clock)
+		div = 1;
+	for (div = 2; div < max_div; div += 2) {
 		if ((host->max_clk / div) <= clock)
 			break;
 	}
 	div >>= 1;
 
-	clk = div << SDHCI_DIVIDER_SHIFT;
+	clk = (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
+	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIVIDER_SHIFT) << SDHCI_DIVIDER_SHIFT_HI;
 	clk |= SDHCI_CLOCK_INT_EN;
 	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
 
@@ -1707,7 +1715,7 @@ int sdhci_add_host(struct sdhci_host *host)
 	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
 	host->version = (host->version & SDHCI_SPEC_VER_MASK)
 				>> SDHCI_SPEC_VER_SHIFT;
-	if (host->version > SDHCI_SPEC_200) {
+	if (host->version > SDHCI_SPEC_300) {
 		printk(KERN_ERR "%s: Unknown controller version (%d). "
 			"You may experience problems.\n", mmc_hostname(mmc),
 			host->version);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 036cfae..d7ef7e4 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -86,6 +86,9 @@
 
 #define SDHCI_CLOCK_CONTROL	0x2C
 #define  SDHCI_DIVIDER_SHIFT	8
+#define  SDHCI_DIVIDER_SHIFT_HI	6
+#define  SDHCI_DIV_MASK	0xFF
+#define  SDHCI_DIV_HI_MASK	0x300
 #define  SDHCI_CLOCK_CARD_EN	0x0004
 #define  SDHCI_CLOCK_INT_STABLE	0x0002
 #define  SDHCI_CLOCK_INT_EN	0x0001
@@ -178,6 +181,7 @@
 #define  SDHCI_SPEC_VER_SHIFT	0
 #define   SDHCI_SPEC_100	0
 #define   SDHCI_SPEC_200	1
+#define   SDHCI_SPEC_300	2
 
 struct sdhci_ops;
 
-- 
1.7.0.4

Re: [patch 1/1]sdhci: sdhc spec 3.0 add some modification

[Michał Mirosław]

2010/8/12 zhangfei gao <zhangfei.gao@gmail.com>:
[...]
> updated patch:
> From b17eb3ca9dd23af63ada240e8f77bdd873e853a7 Mon Sep 17 00:00:00 2001
> From: Zhangfei Gao <zgao6@marvell.com>
> Date: Fri, 6 Aug 2010 07:10:01 +0800
> Subject: [PATCH] sdhc: support 10-bit divided clock Mode
>
> Signed-off-by: Zhangfei Gao <zgao6@marvell.com>
> Reviewed-by: Matt Fleming <matt@console-pimps.org>
> ---
>  drivers/mmc/host/sdhci.c |   16 ++++++++++++----
>  drivers/mmc/host/sdhci.h |    4 ++++
>  2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 7855121..ccfbc40 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -983,7 +983,7 @@ static void sdhci_finish_command(struct sdhci_host *host)
>
>  static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>  {
> -       int div;
> +       int div, max_div;
>        u16 clk;
>        unsigned long timeout;
>
> @@ -1001,13 +1001,21 @@ static void sdhci_set_clock(struct sdhci_host
> *host, unsigned int clock)
>        if (clock == 0)
>                goto out;
>
> -       for (div = 1;div < 256;div *= 2) {
> +       if (host->version >= SDHCI_SPEC_300)
> +               max_div = 2046;
> +       else
> +               max_div = 256;
> +
> +       if(host->max_clk <= clock)
> +               div = 1;
> +       for (div = 2; div < max_div; div += 2) {
>                if ((host->max_clk / div) <= clock)
>                        break;
>        }
>        div >>= 1;
>
> -       clk = div << SDHCI_DIVIDER_SHIFT;
> +       clk = (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;

> +       clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIVIDER_SHIFT) << SDHCI_DIVIDER_SHIFT_HI;

I think the right shift is using wrong #define, SDHCI_DIVIDER_SHIFT
(position of the bitfield in the register). It has a correct value of
8, though. This could be something like the following to reduce
confusion.

clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN) <<
SDHCI_DIVIDER_SHIFT_HI;
...
#define SDHCI_DIV_MASK 0xFF
#define SDHCI_DIV_MASK_LEN 8
#define  SDHCI_DIV_HI_MASK 0x300

or maybe:

clk |= ((div >> SDHCI_DIV_MASK_LEN) & SDHCI_DIV_HI_MASK) <<
SDHCI_DIVIDER_SHIFT_HI;
...
#define SDHCI_DIV_MASK 0xFF
#define SDHCI_DIV_MASK_LEN 8
#define  SDHCI_DIV_HI_MASK 0x03

>        clk |= SDHCI_CLOCK_INT_EN;
>        sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>
[...]
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -86,6 +86,9 @@
>
>  #define SDHCI_CLOCK_CONTROL    0x2C
>  #define  SDHCI_DIVIDER_SHIFT   8
> +#define  SDHCI_DIVIDER_SHIFT_HI        6
> +#define  SDHCI_DIV_MASK        0xFF
> +#define  SDHCI_DIV_HI_MASK     0x300
>  #define  SDHCI_CLOCK_CARD_EN   0x0004
>  #define  SDHCI_CLOCK_INT_STABLE        0x0002
>  #define  SDHCI_CLOCK_INT_EN    0x0001
[...]

SDHCI_DIVIDER_SHIFT_HI -> SDHCI_DIVIDER_HI_SHIFT

Best Regards,
Michał Mirosław

Re: [patch 1/1]sdhci: sdhc spec 3.0 add some modification

[zhangfei gao]

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

2010/8/13 Michał Mirosław <mirqus@gmail.com>:
> 2010/8/12 zhangfei gao <zhangfei.gao@gmail.com>:
> [...]
>> updated patch:
>> From b17eb3ca9dd23af63ada240e8f77bdd873e853a7 Mon Sep 17 00:00:00 2001
>> From: Zhangfei Gao <zgao6@marvell.com>
>> Date: Fri, 6 Aug 2010 07:10:01 +0800
>> Subject: [PATCH] sdhc: support 10-bit divided clock Mode
>>
>> Signed-off-by: Zhangfei Gao <zgao6@marvell.com>
>> Reviewed-by: Matt Fleming <matt@console-pimps.org>
>> ---
>>  drivers/mmc/host/sdhci.c |   16 ++++++++++++----
>>  drivers/mmc/host/sdhci.h |    4 ++++
>>  2 files changed, 16 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 7855121..ccfbc40 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -983,7 +983,7 @@ static void sdhci_finish_command(struct sdhci_host *host)
>>
>>  static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>  {
>> -       int div;
>> +       int div, max_div;
>>        u16 clk;
>>        unsigned long timeout;
>>
>> @@ -1001,13 +1001,21 @@ static void sdhci_set_clock(struct sdhci_host
>> *host, unsigned int clock)
>>        if (clock == 0)
>>                goto out;
>>
>> -       for (div = 1;div < 256;div *= 2) {
>> +       if (host->version >= SDHCI_SPEC_300)
>> +               max_div = 2046;
>> +       else
>> +               max_div = 256;
>> +
>> +       if(host->max_clk <= clock)
>> +               div = 1;
>> +       for (div = 2; div < max_div; div += 2) {
>>                if ((host->max_clk / div) <= clock)
>>                        break;
>>        }
>>        div >>= 1;
>>
>> -       clk = div << SDHCI_DIVIDER_SHIFT;
>> +       clk = (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
>
>> +       clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIVIDER_SHIFT) << SDHCI_DIVIDER_SHIFT_HI;
>
> I think the right shift is using wrong #define, SDHCI_DIVIDER_SHIFT
> (position of the bitfield in the register). It has a correct value of
> 8, though. This could be something like the following to reduce
> confusion.

agree though it would introduce new define
>
> clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN) <<
> SDHCI_DIVIDER_SHIFT_HI;
> ...
> #define SDHCI_DIV_MASK 0xFF
> #define SDHCI_DIV_MASK_LEN 8
> #define  SDHCI_DIV_HI_MASK 0x300
>
> or maybe:
>
> clk |= ((div >> SDHCI_DIV_MASK_LEN) & SDHCI_DIV_HI_MASK) <<
> SDHCI_DIVIDER_SHIFT_HI;
> ...
> #define SDHCI_DIV_MASK 0xFF
> #define SDHCI_DIV_MASK_LEN 8
> #define  SDHCI_DIV_HI_MASK 0x03
>
>>        clk |= SDHCI_CLOCK_INT_EN;
>>        sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>>
> [...]
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -86,6 +86,9 @@
>>
>>  #define SDHCI_CLOCK_CONTROL    0x2C
>>  #define  SDHCI_DIVIDER_SHIFT   8
>> +#define  SDHCI_DIVIDER_SHIFT_HI        6
>> +#define  SDHCI_DIV_MASK        0xFF
>> +#define  SDHCI_DIV_HI_MASK     0x300
>>  #define  SDHCI_CLOCK_CARD_EN   0x0004
>>  #define  SDHCI_CLOCK_INT_STABLE        0x0002
>>  #define  SDHCI_CLOCK_INT_EN    0x0001
> [...]
>
> SDHCI_DIVIDER_SHIFT_HI -> SDHCI_DIVIDER_HI_SHIFT

agree
>
> Best Regards,
> Michał Mirosław
>

Update the patch;

>From c0d23113e5056c9313407ac22d93349501b0b9b2 Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zgao6@marvell.com>
Date: Fri, 6 Aug 2010 07:10:01 +0800
Subject: [PATCH] sdhc: support 10-bit divided clock Mode

Signed-off-by: Zhangfei Gao <zgao6@marvell.com>
Reviewed-by: Matt Fleming <matt@console-pimps.org>
---
 drivers/mmc/host/sdhci.c |   16 ++++++++++++----
 drivers/mmc/host/sdhci.h |    5 +++++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 7855121..768fdc9 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -983,7 +983,7 @@ static void sdhci_finish_command(struct sdhci_host *host)

 static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 {
-	int div;
+	int div, max_div;
 	u16 clk;
 	unsigned long timeout;

@@ -1001,13 +1001,21 @@ static void sdhci_set_clock(struct sdhci_host
*host, unsigned int clock)
 	if (clock == 0)
 		goto out;

-	for (div = 1;div < 256;div *= 2) {
+	if (host->version >= SDHCI_SPEC_300)
+		max_div = 2046;
+	else
+		max_div = 256;
+
+	if(host->max_clk <= clock)
+		div = 1;
+	for (div = 2; div < max_div; div += 2) {
 		if ((host->max_clk / div) <= clock)
 			break;
 	}
 	div >>= 1;

-	clk = div << SDHCI_DIVIDER_SHIFT;
+	clk = (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
+	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN) <<
SDHCI_DIVIDER_HI_SHIFT;
 	clk |= SDHCI_CLOCK_INT_EN;
 	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);

@@ -1707,7 +1715,7 @@ int sdhci_add_host(struct sdhci_host *host)
 	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
 	host->version = (host->version & SDHCI_SPEC_VER_MASK)
 				>> SDHCI_SPEC_VER_SHIFT;
-	if (host->version > SDHCI_SPEC_200) {
+	if (host->version > SDHCI_SPEC_300) {
 		printk(KERN_ERR "%s: Unknown controller version (%d). "
 			"You may experience problems.\n", mmc_hostname(mmc),
 			host->version);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 036cfae..50860dc 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -86,6 +86,10 @@

 #define SDHCI_CLOCK_CONTROL	0x2C
 #define  SDHCI_DIVIDER_SHIFT	8
+#define  SDHCI_DIVIDER_HI_SHIFT	6
+#define  SDHCI_DIV_MASK	0xFF
+#define  SDHCI_DIV_MASK_LEN	8
+#define  SDHCI_DIV_HI_MASK	0x300
 #define  SDHCI_CLOCK_CARD_EN	0x0004
 #define  SDHCI_CLOCK_INT_STABLE	0x0002
 #define  SDHCI_CLOCK_INT_EN	0x0001
@@ -178,6 +182,7 @@
 #define  SDHCI_SPEC_VER_SHIFT	0
 #define   SDHCI_SPEC_100	0
 #define   SDHCI_SPEC_200	1
+#define   SDHCI_SPEC_300	2

 struct sdhci_ops;

-- 
1.7.0.4

[-- Attachment #2: 0001-sdhc-support-10-bit-divided-clock-Mode.patch --]
[-- Type: text/x-patch, Size: 2578 bytes --]

From c0d23113e5056c9313407ac22d93349501b0b9b2 Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zgao6@marvell.com>
Date: Fri, 6 Aug 2010 07:10:01 +0800
Subject: [PATCH] sdhc: support 10-bit divided clock Mode

Signed-off-by: Zhangfei Gao <zgao6@marvell.com>
Reviewed-by: Matt Fleming <matt@console-pimps.org>
---
 drivers/mmc/host/sdhci.c |   16 ++++++++++++----
 drivers/mmc/host/sdhci.h |    5 +++++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 7855121..768fdc9 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -983,7 +983,7 @@ static void sdhci_finish_command(struct sdhci_host *host)
 
 static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 {
-	int div;
+	int div, max_div;
 	u16 clk;
 	unsigned long timeout;
 
@@ -1001,13 +1001,21 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 	if (clock == 0)
 		goto out;
 
-	for (div = 1;div < 256;div *= 2) {
+	if (host->version >= SDHCI_SPEC_300)
+		max_div = 2046;
+	else
+		max_div = 256;
+
+	if(host->max_clk <= clock)
+		div = 1;
+	for (div = 2; div < max_div; div += 2) {
 		if ((host->max_clk / div) <= clock)
 			break;
 	}
 	div >>= 1;
 
-	clk = div << SDHCI_DIVIDER_SHIFT;
+	clk = (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
+	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN) << SDHCI_DIVIDER_HI_SHIFT;
 	clk |= SDHCI_CLOCK_INT_EN;
 	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
 
@@ -1707,7 +1715,7 @@ int sdhci_add_host(struct sdhci_host *host)
 	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
 	host->version = (host->version & SDHCI_SPEC_VER_MASK)
 				>> SDHCI_SPEC_VER_SHIFT;
-	if (host->version > SDHCI_SPEC_200) {
+	if (host->version > SDHCI_SPEC_300) {
 		printk(KERN_ERR "%s: Unknown controller version (%d). "
 			"You may experience problems.\n", mmc_hostname(mmc),
 			host->version);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 036cfae..50860dc 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -86,6 +86,10 @@
 
 #define SDHCI_CLOCK_CONTROL	0x2C
 #define  SDHCI_DIVIDER_SHIFT	8
+#define  SDHCI_DIVIDER_HI_SHIFT	6
+#define  SDHCI_DIV_MASK	0xFF
+#define  SDHCI_DIV_MASK_LEN	8
+#define  SDHCI_DIV_HI_MASK	0x300
 #define  SDHCI_CLOCK_CARD_EN	0x0004
 #define  SDHCI_CLOCK_INT_STABLE	0x0002
 #define  SDHCI_CLOCK_INT_EN	0x0001
@@ -178,6 +182,7 @@
 #define  SDHCI_SPEC_VER_SHIFT	0
 #define   SDHCI_SPEC_100	0
 #define   SDHCI_SPEC_200	1
+#define   SDHCI_SPEC_300	2
 
 struct sdhci_ops;
 
-- 
1.7.0.4