[PATCH RFC 0/3] mmc: mxs-mmc: Implement DDR support

[Stefan Wahren]

Based on these discussions: [1], [2] this patch series implements
DDR support for the MXS MMC host driver. This feature has never been
ported from the vendor kernel.

It has been tested on a i.MX28 board with eMMC which is currently
not in mainline (Duckbill 2).

* without DDR support
dd if=/dev/zero of=test bs=8k count=10000
81920000 bytes (82 MB) copied, 14.3321 s, 5.7 MB/s

* with DDR support:
dd if=/dev/zero of=test bs=8k count=10000
81920000 bytes (82 MB) copied, 13.4781 s, 6.1 MB/s

[1] - http://www.spinics.net/lists/linux-mmc/msg32285.html
[2] - http://www.spinics.net/lists/linux-mmc/msg34418.html

Stefan Wahren (3):
DT: bindings: mmc: Add property for 3.3V only support
mmc: core: add new cap for 3.3V only DDR MMCs
mmc: mxs-mmc: Implement DDR support

Documentation/devicetree/bindings/mmc/mmc.txt | 7 +++++--
drivers/mmc/core/host.c | 2 ++
drivers/mmc/core/mmc.c | 6 ++++++
drivers/mmc/host/mxs-mmc.c | 22 ++++++++++++++++++++++
include/linux/mmc/host.h | 1 +
5 files changed, 36 insertions(+), 2 deletions(-)

[Stefan Wahren]

This patch based on the work of Fabio Estevam:
"[PATCH v2] mmc: core: Do not set mmc voltage to 1.8V when
'no-1-8-v' is present"

It adds the support for 3.3V only DDR MMC hosts.

Signed-off-by: Stefan Wahren <***@i2se.com>
---
drivers/mmc/core/host.c | 2 ++
drivers/mmc/core/mmc.c | 6 ++++++
include/linux/mmc/host.h | 1 +
3 files changed, 9 insertions(+)

Changes to Fabio's patch:
- rebase to current linux-next
- rename DT property to mmc-ddr-3_3v
- use EXT_CSD_CARD_TYPE_DDR_52 instead of new define

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 98f25ff..4c971de 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -301,6 +301,8 @@ int mmc_of_parse(struct mmc_host *host)
if (of_property_read_bool(np, "wakeup-source") ||
of_property_read_bool(np, "enable-sdio-wakeup")) /* legacy */
host->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
+ if (of_property_read_bool(np, "mmc-ddr-3_3v"))
+ host->caps2 |= MMC_CAP2_3_3V_ONLY_DDR;
if (of_property_read_bool(np, "mmc-ddr-1_8v"))
host->caps |= MMC_CAP_1_8V_DDR;
if (of_property_read_bool(np, "mmc-ddr-1_2v"))
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f2d185c..8a933d5 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -210,6 +210,12 @@ static void mmc_select_card_type(struct mmc_card *card)
avail_type |= EXT_CSD_CARD_TYPE_HS_52;
}

+ if (caps2 & MMC_CAP2_3_3V_ONLY_DDR &&
+ card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) {
+ hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
+ avail_type |= EXT_CSD_CARD_TYPE_DDR_52;
+ }
+
if (caps & MMC_CAP_1_8V_DDR &&
card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) {
hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index aa4bfbf..db0775d 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -311,6 +311,7 @@ struct mmc_host {
#define MMC_CAP2_HS400_ES (1 << 20) /* Host supports enhanced strobe */
#define MMC_CAP2_NO_SD (1 << 21) /* Do not send SD commands during initialization */
#define MMC_CAP2_NO_MMC (1 << 22) /* Do not send (e)MMC commands during initialization */
+#define MMC_CAP2_3_3V_ONLY_DDR (1 << 23) /* Only supports 3.3V DDR */

mmc_pm_flag_t pm_caps; /* supported pm features */

[Marek Vasut]

Post by Stefan Wahren
"[PATCH v2] mmc: core: Do not set mmc voltage to 1.8V when
'no-1-8-v' is present"
It adds the support for 3.3V only DDR MMC hosts.

Do such cards even exist ? Do you have a link where I can find some ?

Post by Stefan Wahren
---
drivers/mmc/core/host.c | 2 ++
drivers/mmc/core/mmc.c | 6 ++++++
include/linux/mmc/host.h | 1 +
3 files changed, 9 insertions(+)
- rebase to current linux-next
- rename DT property to mmc-ddr-3_3v
- use EXT_CSD_CARD_TYPE_DDR_52 instead of new define
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 98f25ff..4c971de 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -301,6 +301,8 @@ int mmc_of_parse(struct mmc_host *host)
if (of_property_read_bool(np, "wakeup-source") ||
of_property_read_bool(np, "enable-sdio-wakeup")) /* legacy */
host->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
+ if (of_property_read_bool(np, "mmc-ddr-3_3v"))
+ host->caps2 |= MMC_CAP2_3_3V_ONLY_DDR;
if (of_property_read_bool(np, "mmc-ddr-1_8v"))
host->caps |= MMC_CAP_1_8V_DDR;
if (of_property_read_bool(np, "mmc-ddr-1_2v"))
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f2d185c..8a933d5 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -210,6 +210,12 @@ static void mmc_select_card_type(struct mmc_card *card)
avail_type |= EXT_CSD_CARD_TYPE_HS_52;
}
+ if (caps2 & MMC_CAP2_3_3V_ONLY_DDR &&
+ card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) {
+ hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
+ avail_type |= EXT_CSD_CARD_TYPE_DDR_52;
+ }
+
if (caps & MMC_CAP_1_8V_DDR &&
card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) {
hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index aa4bfbf..db0775d 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -311,6 +311,7 @@ struct mmc_host {
#define MMC_CAP2_HS400_ES (1 << 20) /* Host supports enhanced strobe */
#define MMC_CAP2_NO_SD (1 << 21) /* Do not send SD commands during initialization */
#define MMC_CAP2_NO_MMC (1 << 22) /* Do not send (e)MMC commands during initialization */
+#define MMC_CAP2_3_3V_ONLY_DDR (1 << 23) /* Only supports 3.3V DDR */
mmc_pm_flag_t pm_caps; /* supported pm features */

[Stefan Wahren]

Hi Marek,

Post by Marek Vasut

Post by Stefan Wahren
"[PATCH v2] mmc: core: Do not set mmc voltage to 1.8V when
'no-1-8-v' is present"
It adds the support for 3.3V only DDR MMC hosts.

Do such cards even exist ? Do you have a link where I can find some ?

i never said anything about SD cards. I mean eMMC modules which usually have 8
data pins like this one [1].

Please don't blame me if it's not compatible to i.MX28. It's only an example.

[1] -
http://datasheet.octopart.com/THGBM3G4D1FBAIGH2H-Toshiba-datasheet-20748880.pdf

Post by Marek Vasut

Post by Stefan Wahren
---
drivers/mmc/core/host.c | 2 ++
drivers/mmc/core/mmc.c | 6 ++++++
include/linux/mmc/host.h | 1 +
3 files changed, 9 insertions(+)
- rebase to current linux-next
- rename DT property to mmc-ddr-3_3v
- use EXT_CSD_CARD_TYPE_DDR_52 instead of new define
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 98f25ff..4c971de 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -301,6 +301,8 @@ int mmc_of_parse(struct mmc_host *host)
if (of_property_read_bool(np, "wakeup-source") ||
of_property_read_bool(np, "enable-sdio-wakeup")) /* legacy */
host->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
+ if (of_property_read_bool(np, "mmc-ddr-3_3v"))
+ host->caps2 |= MMC_CAP2_3_3V_ONLY_DDR;
if (of_property_read_bool(np, "mmc-ddr-1_8v"))
host->caps |= MMC_CAP_1_8V_DDR;
if (of_property_read_bool(np, "mmc-ddr-1_2v"))
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f2d185c..8a933d5 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -210,6 +210,12 @@ static void mmc_select_card_type(struct mmc_card *card)
avail_type |= EXT_CSD_CARD_TYPE_HS_52;
}
+ if (caps2 & MMC_CAP2_3_3V_ONLY_DDR &&
+ card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) {
+ hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
+ avail_type |= EXT_CSD_CARD_TYPE_DDR_52;
+ }
+
if (caps & MMC_CAP_1_8V_DDR &&
card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) {
hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index aa4bfbf..db0775d 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -311,6 +311,7 @@ struct mmc_host {
#define MMC_CAP2_HS400_ES (1 << 20) /* Host supports enhanced strobe */
#define MMC_CAP2_NO_SD (1 << 21) /* Do not send SD commands during initialization */
#define MMC_CAP2_NO_MMC (1 << 22) /* Do not send (e)MMC commands during
initialization */
+#define MMC_CAP2_3_3V_ONLY_DDR (1 << 23) /* Only supports 3.3V DDR */
mmc_pm_flag_t pm_caps; /* supported pm features */

--
Best regards,
Marek Vasut

[Marek Vasut]

Post by Stefan Wahren
Hi Marek,

Post by Marek Vasut

Post by Stefan Wahren
"[PATCH v2] mmc: core: Do not set mmc voltage to 1.8V when
'no-1-8-v' is present"
It adds the support for 3.3V only DDR MMC hosts.

Do such cards even exist ? Do you have a link where I can find some ?

i never said anything about SD cards. I mean eMMC modules which usually have 8
data pins like this one [1].
Please don't blame me if it's not compatible to i.MX28. It's only an example.
[1] -
http://datasheet.octopart.com/THGBM3G4D1FBAIGH2H-Toshiba-datasheet-20748880.pdf

Interesting, thanks!

[Shawn Lin]

Post by Stefan Wahren
Hi Marek,

Post by Marek Vasut

Post by Stefan Wahren
"[PATCH v2] mmc: core: Do not set mmc voltage to 1.8V when
'no-1-8-v' is present"
It adds the support for 3.3V only DDR MMC hosts.

Do such cards even exist ? Do you have a link where I can find some ?

i never said anything about SD cards. I mean eMMC modules which usually have 8
data pins like this one [1].
Please don't blame me if it's not compatible to i.MX28. It's only an example.
[1] -
http://datasheet.octopart.com/THGBM3G4D1FBAIGH2H-Toshiba-datasheet-20748880.pdf

I download the datasheet you mentioned, and I explicitly see it
describe 1V8 everywhere, especially for the section of "ELECTRICAL
CHARACTERISTICS".

I never see one eMMC claiming DDR52 capability but doesn't support 1V8
so far. Could you kindly share me the part number of the eMMC you are
using? :)

Post by Stefan Wahren

Post by Marek Vasut

Post by Stefan Wahren
---
drivers/mmc/core/host.c | 2 ++
drivers/mmc/core/mmc.c | 6 ++++++
include/linux/mmc/host.h | 1 +
3 files changed, 9 insertions(+)
- rebase to current linux-next
- rename DT property to mmc-ddr-3_3v
- use EXT_CSD_CARD_TYPE_DDR_52 instead of new define
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 98f25ff..4c971de 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -301,6 +301,8 @@ int mmc_of_parse(struct mmc_host *host)
if (of_property_read_bool(np, "wakeup-source") ||
of_property_read_bool(np, "enable-sdio-wakeup")) /* legacy */
host->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
+ if (of_property_read_bool(np, "mmc-ddr-3_3v"))
+ host->caps2 |= MMC_CAP2_3_3V_ONLY_DDR;
if (of_property_read_bool(np, "mmc-ddr-1_8v"))
host->caps |= MMC_CAP_1_8V_DDR;
if (of_property_read_bool(np, "mmc-ddr-1_2v"))
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f2d185c..8a933d5 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -210,6 +210,12 @@ static void mmc_select_card_type(struct mmc_card *card)
avail_type |= EXT_CSD_CARD_TYPE_HS_52;
}
+ if (caps2 & MMC_CAP2_3_3V_ONLY_DDR &&
+ card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) {
+ hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
+ avail_type |= EXT_CSD_CARD_TYPE_DDR_52;
+ }
+
if (caps & MMC_CAP_1_8V_DDR &&
card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) {
hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index aa4bfbf..db0775d 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -311,6 +311,7 @@ struct mmc_host {
#define MMC_CAP2_HS400_ES (1 << 20) /* Host supports enhanced strobe */
#define MMC_CAP2_NO_SD (1 << 21) /* Do not send SD commands during initialization */
#define MMC_CAP2_NO_MMC (1 << 22) /* Do not send (e)MMC commands during
initialization */
+#define MMC_CAP2_3_3V_ONLY_DDR (1 << 23) /* Only supports 3.3V DDR */
mmc_pm_flag_t pm_caps; /* supported pm features */

--
Best regards,
Marek Vasut

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Stefan Wahren]

Hi Shawn,

Post by Shawn Lin

Post by Stefan Wahren
Hi Marek,

Post by Marek Vasut

Post by Stefan Wahren
"[PATCH v2] mmc: core: Do not set mmc voltage to 1.8V when
'no-1-8-v' is present"
It adds the support for 3.3V only DDR MMC hosts.

Do such cards even exist ? Do you have a link where I can find some ?

i never said anything about SD cards. I mean eMMC modules which usually have 8
data pins like this one [1].
Please don't blame me if it's not compatible to i.MX28. It's only an example.
[1] -
http://datasheet.octopart.com/THGBM3G4D1FBAIGH2H-Toshiba-datasheet-20748880.pdf

I download the datasheet you mentioned, and I explicitly see it
describe 1V8 everywhere, especially for the section of "ELECTRICAL
CHARACTERISTICS".
I never see one eMMC claiming DDR52 capability but doesn't support 1V8
so far. Could you kindly share me the part number of the eMMC you are
using? :)

sorry the subject line is misleading. The capability flag is about the host not
about the MMC itself. There are hosts which doesn't support 1.8V. I will clarify
this in the next version.

[Jaehoon Chung]

Post by Shawn Lin

Post by Stefan Wahren
Hi Marek,

Post by Marek Vasut

Post by Stefan Wahren
"[PATCH v2] mmc: core: Do not set mmc voltage to 1.8V when
'no-1-8-v' is present"
It adds the support for 3.3V only DDR MMC hosts.

Do such cards even exist ? Do you have a link where I can find some ?

i never said anything about SD cards. I mean eMMC modules which usually have 8
data pins like this one [1].
Please don't blame me if it's not compatible to i.MX28. It's only an example.
[1] -
http://datasheet.octopart.com/THGBM3G4D1FBAIGH2H-Toshiba-datasheet-20748880.pdf

I download the datasheet you mentioned, and I explicitly see it
describe 1V8 everywhere, especially for the section of "ELECTRICAL
CHARACTERISTICS".
I never see one eMMC claiming DDR52 capability but doesn't support 1V8
so far. Could you kindly share me the part number of the eMMC you are
using? :)

Post by Stefan Wahren

Post by Marek Vasut

Post by Stefan Wahren
---
drivers/mmc/core/host.c | 2 ++
drivers/mmc/core/mmc.c | 6 ++++++
include/linux/mmc/host.h | 1 +
3 files changed, 9 insertions(+)
- rebase to current linux-next
- rename DT property to mmc-ddr-3_3v
- use EXT_CSD_CARD_TYPE_DDR_52 instead of new define
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 98f25ff..4c971de 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -301,6 +301,8 @@ int mmc_of_parse(struct mmc_host *host)
if (of_property_read_bool(np, "wakeup-source") ||
of_property_read_bool(np, "enable-sdio-wakeup")) /* legacy */
host->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
+ if (of_property_read_bool(np, "mmc-ddr-3_3v"))
+ host->caps2 |= MMC_CAP2_3_3V_ONLY_DDR;
if (of_property_read_bool(np, "mmc-ddr-1_8v"))
host->caps |= MMC_CAP_1_8V_DDR;
if (of_property_read_bool(np, "mmc-ddr-1_2v"))
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f2d185c..8a933d5 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -210,6 +210,12 @@ static void mmc_select_card_type(struct mmc_card *card)
avail_type |= EXT_CSD_CARD_TYPE_HS_52;
}
+ if (caps2 & MMC_CAP2_3_3V_ONLY_DDR &&
+ card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) {
+ hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
+ avail_type |= EXT_CSD_CARD_TYPE_DDR_52;
+ }

did you know EXT_CSD_CARD_TYPE_DDR_52 -> TYPE_DDR_1_8V and TYPE_DDR_1_2V?
It means card can use to the 1.8v and 1.2v ddr mode.
So this patch is strange..You added capability for only supporting 3_3v.

Confusing...

Best Regards,
Jaehoon Chung

Post by Shawn Lin

Post by Stefan Wahren

Post by Marek Vasut

Post by Stefan Wahren
+
if (caps & MMC_CAP_1_8V_DDR &&
card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) {
hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index aa4bfbf..db0775d 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -311,6 +311,7 @@ struct mmc_host {
#define MMC_CAP2_HS400_ES (1 << 20) /* Host supports enhanced strobe */
#define MMC_CAP2_NO_SD (1 << 21) /* Do not send SD commands during
initialization */
#define MMC_CAP2_NO_MMC (1 << 22) /* Do not send (e)MMC commands during
initialization */
+#define MMC_CAP2_3_3V_ONLY_DDR (1 << 23) /* Only supports 3.3V DDR */
mmc_pm_flag_t pm_caps; /* supported pm features */

--
Best regards,
Marek Vasut

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Stefan Wahren]

Post by Jaehoon Chung

Post by Shawn Lin

Post by Stefan Wahren
Hi Marek,

Post by Marek Vasut

Post by Stefan Wahren
"[PATCH v2] mmc: core: Do not set mmc voltage to 1.8V when
'no-1-8-v' is present"
It adds the support for 3.3V only DDR MMC hosts.

Do such cards even exist ? Do you have a link where I can find some ?

i never said anything about SD cards. I mean eMMC modules which usually have 8
data pins like this one [1].
Please don't blame me if it's not compatible to i.MX28. It's only an example.
[1] -
http://datasheet.octopart.com/THGBM3G4D1FBAIGH2H-Toshiba-datasheet-20748880.pdf

I download the datasheet you mentioned, and I explicitly see it
describe 1V8 everywhere, especially for the section of "ELECTRICAL
CHARACTERISTICS".
I never see one eMMC claiming DDR52 capability but doesn't support 1V8
so far. Could you kindly share me the part number of the eMMC you are
using? :)

Post by Stefan Wahren

Post by Marek Vasut

Post by Stefan Wahren
---
drivers/mmc/core/host.c | 2 ++
drivers/mmc/core/mmc.c | 6 ++++++
include/linux/mmc/host.h | 1 +
3 files changed, 9 insertions(+)
- rebase to current linux-next
- rename DT property to mmc-ddr-3_3v
- use EXT_CSD_CARD_TYPE_DDR_52 instead of new define
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 98f25ff..4c971de 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -301,6 +301,8 @@ int mmc_of_parse(struct mmc_host *host)
if (of_property_read_bool(np, "wakeup-source") ||
of_property_read_bool(np, "enable-sdio-wakeup")) /* legacy */
host->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
+ if (of_property_read_bool(np, "mmc-ddr-3_3v"))
+ host->caps2 |= MMC_CAP2_3_3V_ONLY_DDR;
if (of_property_read_bool(np, "mmc-ddr-1_8v"))
host->caps |= MMC_CAP_1_8V_DDR;
if (of_property_read_bool(np, "mmc-ddr-1_2v"))
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f2d185c..8a933d5 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -210,6 +210,12 @@ static void mmc_select_card_type(struct mmc_card *card)
avail_type |= EXT_CSD_CARD_TYPE_HS_52;
}
+ if (caps2 & MMC_CAP2_3_3V_ONLY_DDR &&
+ card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) {
+ hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
+ avail_type |= EXT_CSD_CARD_TYPE_DDR_52;
+ }

did you know EXT_CSD_CARD_TYPE_DDR_52 -> TYPE_DDR_1_8V and TYPE_DDR_1_2V?

Thanks for pointing out. Fabio's patch was better here.

As i wrote to Shawn the subject line is incorrect. I want to add 3.3V
only support for hosts not for "card".

According to the comments EXT_CSD_CARD_TYPE_DDR_1_8V stands for 1.8V or
3.3V. So i better add EXT_CSD_CARD_TYPE_DDR_3_3V_ONLY even the "card"
supports 1.8V.

[Stefan Wahren]

Currently there is no proper way to define that a MMC host supports
only 3.3V. The property no-1-8-v is broken and has different meanings
for different sdhci variants. So add a new property for 3.3V only
support and mark no-1-8-v as deprecated.

Signed-off-by: Stefan Wahren <***@i2se.com>
---
Documentation/devicetree/bindings/mmc/mmc.txt | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index 22d1e1f..b2b8960 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -27,8 +27,6 @@ Optional properties:
logic it is sufficient to not specify wp-gpios property in the absence of a WP
line.
- max-frequency: maximum operating clock frequency
-- no-1-8-v: when present, denotes that 1.8v card voltage is not supported on
- this system, even if the controller claims it is.
- cap-sd-highspeed: SD high-speed timing is supported
- cap-mmc-highspeed: MMC high-speed timing is supported
- sd-uhs-sdr12: SD UHS SDR12 speed is supported
@@ -40,6 +38,7 @@ Optional properties:
- cap-mmc-hw-reset: eMMC hardware reset is supported
- cap-sdio-irq: enable SDIO IRQ signalling on this interface
- full-pwr-cycle: full power cycle of the card is supported
+- mmc-ddr-3_3v: eMMC high-speed DDR mode(3.3V I/O) is supported
- mmc-ddr-1_8v: eMMC high-speed DDR mode(1.8V I/O) is supported
- mmc-ddr-1_2v: eMMC high-speed DDR mode(1.2V I/O) is supported
- mmc-hs200-1_8v: eMMC HS200 mode(1.8V I/O) is supported
@@ -53,6 +52,10 @@ Optional properties:
- no-sd: controller is limited to send sd cmd during initialization
- no-mmc: controller is limited to send mmc cmd during initialization

+Deprecated properties:
+- no-1-8-v: when present, denotes that 1.8v card voltage is not supported on
+ this system, even if the controller claims it is.
+
*NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line
polarity properties, we have to fix the meaning of the "normal" and "inverted"
line levels. We choose to follow the SDHCI standard, which specifies both those

[Rob Herring]

Post by Stefan Wahren
Currently there is no proper way to define that a MMC host supports
only 3.3V. The property no-1-8-v is broken and has different meanings
for different sdhci variants. So add a new property for 3.3V only
support and mark no-1-8-v as deprecated.

Why is it broken? How would I override a controller saying 1.8V is
supported and it is not?

Rob

[Stefan Wahren]

Hi,

Post by Rob Herring

Post by Stefan Wahren
Currently there is no proper way to define that a MMC host supports
only 3.3V. The property no-1-8-v is broken and has different meanings
for different sdhci variants. So add a new property for 3.3V only
support and mark no-1-8-v as deprecated.

Why is it broken?

i want to quote Ulf Hansson here [1]:

The problem with the "no-1-8-v" binding is that it's describing what
the hardware *can't* do. It thus becomes easy to abuse it.

I suggest we stop using it, we should mark it deprecated.

[1] - http://www.spinics.net/lists/linux-mmc/msg34604.html

Post by Rob Herring
How would I override a controller saying 1.8V is
supported and it is not?

Sorry, i'm not sure that i understand your question. In case a board or a MMC
controller doesn't support 1.8V, it usually supports only 3.3V which is the
intension of this patch.

Post by Rob Herring
Rob
_______________________________________________
linux-arm-kernel mailing list
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[Rob Herring]

Post by Stefan Wahren
Hi,

Post by Rob Herring

Post by Stefan Wahren
Currently there is no proper way to define that a MMC host supports
only 3.3V. The property no-1-8-v is broken and has different meanings
for different sdhci variants. So add a new property for 3.3V only
support and mark no-1-8-v as deprecated.

Why is it broken?

The problem with the "no-1-8-v" binding is that it's describing what
the hardware *can't* do. It thus becomes easy to abuse it.

Sounds like a quirk which is perfectly normal for a property. I'd
agree it should be what the h/w *can* do if h/w didn't have capability
bit that does that.

Post by Stefan Wahren
I suggest we stop using it, we should mark it deprecated.
[1] - http://www.spinics.net/lists/linux-mmc/msg34604.html

Post by Rob Herring
How would I override a controller saying 1.8V is
supported and it is not?

Sorry, i'm not sure that i understand your question. In case a board or a MMC
controller doesn't support 1.8V, it usually supports only 3.3V which is the
intension of this patch.

Some controllers have capability bits saying what voltages they
support, right? And those bits can be wrong (unless firmware sets them
I'd expect that is the common case) which as I read it is what
"no-1-8-v" was for. So with the "mmc-ddr-*v" properties, what does not
present mean and how do they relate to controller capability bits? I
assume they would override the controller bits, but you can only
override the capability bit not set case. I would think the property
not present means use the capability bit, not that that voltage is not
supported. I think you probably need tri-state properties here where
value 1 means supported, 0 means not supported, and not present means
use capability bit (or other method).

Rob

[Shawn Lin]

+ Adrian

Let's queue Adrian here who now maintains SDHCI stuff.

Post by Rob Herring

Post by Stefan Wahren
Hi,

Post by Rob Herring

Post by Stefan Wahren
Currently there is no proper way to define that a MMC host supports
only 3.3V. The property no-1-8-v is broken and has different meanings
for different sdhci variants. So add a new property for 3.3V only
support and mark no-1-8-v as deprecated.

Why is it broken?

The problem with the "no-1-8-v" binding is that it's describing what
the hardware *can't* do. It thus becomes easy to abuse it.

Sounds like a quirk which is perfectly normal for a property. I'd
agree it should be what the h/w *can* do if h/w didn't have capability
bit that does that.

Post by Stefan Wahren
I suggest we stop using it, we should mark it deprecated.
[1] - http://www.spinics.net/lists/linux-mmc/msg34604.html

Post by Rob Herring
How would I override a controller saying 1.8V is
supported and it is not?

Sorry, i'm not sure that i understand your question. In case a board or a MMC
controller doesn't support 1.8V, it usually supports only 3.3V which is the
intension of this patch.

Some controllers have capability bits saying what voltages they
support, right? And those bits can be wrong (unless firmware sets them
I'd expect that is the common case) which as I read it is what
"no-1-8-v" was for. So with the "mmc-ddr-*v" properties, what does not
present mean and how do they relate to controller capability bits? I
assume they would override the controller bits, but you can only
override the capability bit not set case. I would think the property
not present means use the capability bit, not that that voltage is not
supported. I think you probably need tri-state properties here where
value 1 means supported, 0 means not supported, and not present means
use capability bit (or other method).
Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Adrian Hunter]

Post by Shawn Lin
+ Adrian
Let's queue Adrian here who now maintains SDHCI stuff.

SDHCI drivers may not implement no-1-8-v in a consistent manner, but as far
as I can see, the meaning is still clear: 1.8V will not be used for either
supply or signaling.

SDHCI is complicated because the SDHCI specification does not cover eMMC.
From the perspective of SDHCI, the only 1.8V modes are the UHS-I modes, so
support for 1.8V signaling is the same as support for one of those modes
(the spec even says as much). But what happens is that the host controller
can support those modes but the board can't supply 1.8V so the drivers
remove capability for the modes. Support for 1.8V supply has a capability
bit which drivers can override if necessary but removable SD cards don't
support 1.8V supply anyway, so the issue doesn't arise if the host
controller is only used for uSD cards.

Post by Shawn Lin

Post by Rob Herring

Post by Stefan Wahren
Hi,

Post by Rob Herring

Post by Stefan Wahren
Currently there is no proper way to define that a MMC host supports
only 3.3V. The property no-1-8-v is broken and has different meanings
for different sdhci variants. So add a new property for 3.3V only
support and mark no-1-8-v as deprecated.

Why is it broken?

The problem with the "no-1-8-v" binding is that it's describing what
the hardware *can't* do. It thus becomes easy to abuse it.

Sounds like a quirk which is perfectly normal for a property. I'd
agree it should be what the h/w *can* do if h/w didn't have capability
bit that does that.

Post by Stefan Wahren
I suggest we stop using it, we should mark it deprecated.
[1] - http://www.spinics.net/lists/linux-mmc/msg34604.html

Post by Rob Herring
How would I override a controller saying 1.8V is
supported and it is not?

Sorry, i'm not sure that i understand your question. In case a board or a MMC
controller doesn't support 1.8V, it usually supports only 3.3V which is the
intension of this patch.

Some controllers have capability bits saying what voltages they
support, right? And those bits can be wrong (unless firmware sets them
I'd expect that is the common case) which as I read it is what
"no-1-8-v" was for. So with the "mmc-ddr-*v" properties, what does not
present mean and how do they relate to controller capability bits? I
assume they would override the controller bits, but you can only
override the capability bit not set case. I would think the property
not present means use the capability bit, not that that voltage is not
supported. I think you probably need tri-state properties here where
value 1 means supported, 0 means not supported, and not present means
use capability bit (or other method).
Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Ulf Hansson]

Post by Adrian Hunter

Post by Shawn Lin
+ Adrian
Let's queue Adrian here who now maintains SDHCI stuff.

SDHCI drivers may not implement no-1-8-v in a consistent manner, but as far
as I can see, the meaning is still clear: 1.8V will not be used for either
supply or signaling.

Okay.

Post by Adrian Hunter
SDHCI is complicated because the SDHCI specification does not cover eMMC.
From the perspective of SDHCI, the only 1.8V modes are the UHS-I modes, so
support for 1.8V signaling is the same as support for one of those modes
(the spec even says as much). But what happens is that the host controller
can support those modes but the board can't supply 1.8V so the drivers
remove capability for the modes. Support for 1.8V supply has a capability
bit which drivers can override if necessary but removable SD cards don't
support 1.8V supply anyway, so the issue doesn't arise if the host
controller is only used for uSD cards.

By looking how SDHCI uses the SDHCI_SUPPORT_DDR50 in conjunction with
SDHCI_QUIRK2_NO_1_8_V (which is set when no-1-8-v DT property is
provided), this becomes a bit messy.

From Adrian's summary above, it then seems appropriate to limit the
no-1-8-v DT property to apply only to capabilities related to SD
cards, as I assume that also was the original purpose.

Do you think it's possible to clean up this in sdhci when assigning
the caps masks, and then also clarify the no-1-8-v DT binding in the
documentation?

Regarding the new DT binding proposed to be added, mmc-ddr-3_3v, it
seems we need this to be able to properly describe the HW.
Rob, do you have an issue with adding this binding? I am thinking that
we already have mmc-ddr-1_8v and mmc-ddr-1_2v, so it just follow
existing pattern.

[...]

Kind regards
Uffe

[Stefan Wahren]

Hi Ulf, hi Rob

Post by Ulf Hansson

Post by Adrian Hunter

Post by Shawn Lin
+ Adrian
Let's queue Adrian here who now maintains SDHCI stuff.

SDHCI drivers may not implement no-1-8-v in a consistent manner, but as far
as I can see, the meaning is still clear: 1.8V will not be used for either
supply or signaling.

Okay.

Post by Adrian Hunter
SDHCI is complicated because the SDHCI specification does not cover eMMC.
From the perspective of SDHCI, the only 1.8V modes are the UHS-I modes, so
support for 1.8V signaling is the same as support for one of those modes
(the spec even says as much). But what happens is that the host controller
can support those modes but the board can't supply 1.8V so the drivers
remove capability for the modes. Support for 1.8V supply has a capability
bit which drivers can override if necessary but removable SD cards don't
support 1.8V supply anyway, so the issue doesn't arise if the host
controller is only used for uSD cards.

By looking how SDHCI uses the SDHCI_SUPPORT_DDR50 in conjunction with
SDHCI_QUIRK2_NO_1_8_V (which is set when no-1-8-v DT property is
provided), this becomes a bit messy.
From Adrian's summary above, it then seems appropriate to limit the
no-1-8-v DT property to apply only to capabilities related to SD
cards, as I assume that also was the original purpose.
Do you think it's possible to clean up this in sdhci when assigning
the caps masks, and then also clarify the no-1-8-v DT binding in the
documentation?

was the question addressed to me? I think this clean up should be a separate
patch series. Unfortunately i don't have a clue about what exactly and how it
should be fixed.

Post by Ulf Hansson
Regarding the new DT binding proposed to be added, mmc-ddr-3_3v, it
seems we need this to be able to properly describe the HW.
Rob, do you have an issue with adding this binding? I am thinking that
we already have mmc-ddr-1_8v and mmc-ddr-1_2v, so it just follow
existing pattern.

@Rob: gently ping ...

Regards
Stefan

Post by Ulf Hansson
[...]
Kind regards
Uffe
_______________________________________________
linux-arm-kernel mailing list
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[Rob Herring]

Post by Stefan Wahren
Hi Ulf, hi Rob

Post by Ulf Hansson

Post by Adrian Hunter

Post by Shawn Lin
+ Adrian
Let's queue Adrian here who now maintains SDHCI stuff.

SDHCI drivers may not implement no-1-8-v in a consistent manner, but as far
as I can see, the meaning is still clear: 1.8V will not be used for either
supply or signaling.

Okay.

Post by Adrian Hunter
SDHCI is complicated because the SDHCI specification does not cover eMMC.
From the perspective of SDHCI, the only 1.8V modes are the UHS-I modes, so
support for 1.8V signaling is the same as support for one of those modes
(the spec even says as much). But what happens is that the host controller
can support those modes but the board can't supply 1.8V so the drivers
remove capability for the modes. Support for 1.8V supply has a capability
bit which drivers can override if necessary but removable SD cards don't
support 1.8V supply anyway, so the issue doesn't arise if the host
controller is only used for uSD cards.

By looking how SDHCI uses the SDHCI_SUPPORT_DDR50 in conjunction with
SDHCI_QUIRK2_NO_1_8_V (which is set when no-1-8-v DT property is
provided), this becomes a bit messy.
From Adrian's summary above, it then seems appropriate to limit the
no-1-8-v DT property to apply only to capabilities related to SD
cards, as I assume that also was the original purpose.
Do you think it's possible to clean up this in sdhci when assigning
the caps masks, and then also clarify the no-1-8-v DT binding in the
documentation?

was the question addressed to me? I think this clean up should be a separate
patch series. Unfortunately i don't have a clue about what exactly and how it
should be fixed.

Post by Ulf Hansson
Regarding the new DT binding proposed to be added, mmc-ddr-3_3v, it
seems we need this to be able to properly describe the HW.
Rob, do you have an issue with adding this binding? I am thinking that
we already have mmc-ddr-1_8v and mmc-ddr-1_2v, so it just follow
existing pattern.

@Rob: gently ping ...

Yes, this seems fine. I was only the no-1-8-v removal I had issue with.

Rob

[Stefan Wahren]

This patch implements driver support for 3.3V DDR eMMCs.

Signed-off-by: Stefan Wahren <***@i2se.com>
---
drivers/mmc/host/mxs-mmc.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index d839147..84019d5 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -71,6 +71,7 @@ struct mxs_mmc_host {
spinlock_t lock;
int sdio_irq_en;
bool broken_cd;
+ bool is_ddr;
};

static int mxs_mmc_get_cd(struct mmc_host *mmc)
@@ -411,6 +412,9 @@ static void mxs_mmc_adtc(struct mxs_mmc_host *host)
cmd0 |= BF_SSP(log2_blksz, CMD0_BLOCK_SIZE) |
BF_SSP(blocks - 1, CMD0_BLOCK_COUNT);
} else {
+ if (host->is_ddr)
+ cmd0 |= BM_SSP_CMD0_DBL_DATA_RATE_EN;
+
writel(data_size, ssp->base + HW_SSP_XFER_SIZE);
writel(BF_SSP(log2_blksz, BLOCK_SIZE_BLOCK_SIZE) |
BF_SSP(blocks - 1, BLOCK_SIZE_BLOCK_COUNT),
@@ -499,6 +503,7 @@ static void mxs_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
static void mxs_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
{
struct mxs_mmc_host *host = mmc_priv(mmc);
+ struct mxs_ssp *ssp = &host->ssp;

if (ios->bus_width == MMC_BUS_WIDTH_8)
host->bus_width = 2;
@@ -509,6 +514,23 @@ static void mxs_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)

if (ios->clock)
mxs_ssp_set_clk_rate(&host->ssp, ios->clock);
+
+ if (ssp_is_old(ssp))
+ return;
+
+ if (ios->timing == MMC_TIMING_MMC_DDR52) {
+ /*
+ * ENGR00133481-1: In DDR mode the host send the data at
+ * negative edge and the MMC receive the data at positive edge.
+ */
+ host->is_ddr = true;
+ writel(BM_SSP_CTRL1_POLARITY, ssp->base +
+ HW_SSP_CTRL1(ssp) + STMP_OFFSET_REG_CLR);
+ } else {
+ host->is_ddr = false;
+ writel(BM_SSP_CTRL1_POLARITY, ssp->base + HW_SSP_CTRL1(ssp) +
+ STMP_OFFSET_REG_SET);
+ }
}

static void mxs_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)

[Marek Vasut]

Post by Stefan Wahren
This patch implements driver support for 3.3V DDR eMMCs.
---
drivers/mmc/host/mxs-mmc.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index d839147..84019d5 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -71,6 +71,7 @@ struct mxs_mmc_host {
spinlock_t lock;
int sdio_irq_en;
bool broken_cd;
+ bool is_ddr;
};
static int mxs_mmc_get_cd(struct mmc_host *mmc)
@@ -411,6 +412,9 @@ static void mxs_mmc_adtc(struct mxs_mmc_host *host)
cmd0 |= BF_SSP(log2_blksz, CMD0_BLOCK_SIZE) |
BF_SSP(blocks - 1, CMD0_BLOCK_COUNT);
} else {
+ if (host->is_ddr)
+ cmd0 |= BM_SSP_CMD0_DBL_DATA_RATE_EN;
+
writel(data_size, ssp->base + HW_SSP_XFER_SIZE);
writel(BF_SSP(log2_blksz, BLOCK_SIZE_BLOCK_SIZE) |
BF_SSP(blocks - 1, BLOCK_SIZE_BLOCK_COUNT),
@@ -499,6 +503,7 @@ static void mxs_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
static void mxs_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
{
struct mxs_mmc_host *host = mmc_priv(mmc);
+ struct mxs_ssp *ssp = &host->ssp;
if (ios->bus_width == MMC_BUS_WIDTH_8)
host->bus_width = 2;
@@ -509,6 +514,23 @@ static void mxs_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
if (ios->clock)
mxs_ssp_set_clk_rate(&host->ssp, ios->clock);
+
+ if (ssp_is_old(ssp))
+ return;
+
+ if (ios->timing == MMC_TIMING_MMC_DDR52) {

Shouldn't you validate that the clock are set to at least 52MHz before
neabling the DDR mode ?

Post by Stefan Wahren
+ /*
+ * ENGR00133481-1: In DDR mode the host send the data at
+ * negative edge and the MMC receive the data at positive edge.
+ */
+ host->is_ddr = true;
+ writel(BM_SSP_CTRL1_POLARITY, ssp->base +
+ HW_SSP_CTRL1(ssp) + STMP_OFFSET_REG_CLR);
+ } else {

Is it by any chance possible that this else branch will catch not only
the non-DDR options, but also some DDR ones , thus causing problems ?

Post by Stefan Wahren
+ host->is_ddr = false;
+ writel(BM_SSP_CTRL1_POLARITY, ssp->base + HW_SSP_CTRL1(ssp) +
+ STMP_OFFSET_REG_SET);
+ }
}
static void mxs_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)

[Stefan Wahren]

Hi Marek,

Post by Marek Vasut

Post by Stefan Wahren
This patch implements driver support for 3.3V DDR eMMCs.
---
drivers/mmc/host/mxs-mmc.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index d839147..84019d5 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -71,6 +71,7 @@ struct mxs_mmc_host {
spinlock_t lock;
int sdio_irq_en;
bool broken_cd;
+ bool is_ddr;
};
static int mxs_mmc_get_cd(struct mmc_host *mmc)
@@ -411,6 +412,9 @@ static void mxs_mmc_adtc(struct mxs_mmc_host *host)
cmd0 |= BF_SSP(log2_blksz, CMD0_BLOCK_SIZE) |
BF_SSP(blocks - 1, CMD0_BLOCK_COUNT);
} else {
+ if (host->is_ddr)
+ cmd0 |= BM_SSP_CMD0_DBL_DATA_RATE_EN;
+
writel(data_size, ssp->base + HW_SSP_XFER_SIZE);
writel(BF_SSP(log2_blksz, BLOCK_SIZE_BLOCK_SIZE) |
BF_SSP(blocks - 1, BLOCK_SIZE_BLOCK_COUNT),
@@ -499,6 +503,7 @@ static void mxs_mmc_request(struct mmc_host *mmc, struct
mmc_request *mrq)
static void mxs_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
{
struct mxs_mmc_host *host = mmc_priv(mmc);
+ struct mxs_ssp *ssp = &host->ssp;
if (ios->bus_width == MMC_BUS_WIDTH_8)
host->bus_width = 2;
@@ -509,6 +514,23 @@ static void mxs_mmc_set_ios(struct mmc_host *mmc,
struct mmc_ios *ios)
if (ios->clock)
mxs_ssp_set_clk_rate(&host->ssp, ios->clock);
+
+ if (ssp_is_old(ssp))
+ return;
+
+ if (ios->timing == MMC_TIMING_MMC_DDR52) {

Shouldn't you validate that the clock are set to at least 52MHz before
neabling the DDR mode ?

according to the i.MX28 datasheet IMX28CEC
3.5.14.2 MMC4.4 (Dual Data Rate) AC Timing
the minimum clock frequency is 0 and maximum is 52 MHz. So i assume the core
take care of the right clock rate.

Post by Marek Vasut

Post by Stefan Wahren
+ /*
+ * ENGR00133481-1: In DDR mode the host send the data at
+ * negative edge and the MMC receive the data at positive edge.
+ */
+ host->is_ddr = true;
+ writel(BM_SSP_CTRL1_POLARITY, ssp->base +
+ HW_SSP_CTRL1(ssp) + STMP_OFFSET_REG_CLR);
+ } else {

Is it by any chance possible that this else branch will catch not only
the non-DDR options, but also some DDR ones , thus causing problems ?

No, because the polarity bit has been set all the time by mxs_mmc_reset().

But it's possible that i didn't catch all DDR cases.

Post by Marek Vasut

Post by Stefan Wahren
+ host->is_ddr = false;
+ writel(BM_SSP_CTRL1_POLARITY, ssp->base + HW_SSP_CTRL1(ssp) +
+ STMP_OFFSET_REG_SET);
+ }
}
static void mxs_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)

--
Best regards,
Marek Vasut

[Marek Vasut]

Post by Stefan Wahren
Based on these discussions: [1], [2] this patch series implements
DDR support for the MXS MMC host driver. This feature has never been
ported from the vendor kernel.
It has been tested on a i.MX28 board with eMMC which is currently
not in mainline (Duckbill 2).
* without DDR support
dd if=/dev/zero of=test bs=8k count=10000
81920000 bytes (82 MB) copied, 14.3321 s, 5.7 MB/s
dd if=/dev/zero of=test bs=8k count=10000
81920000 bytes (82 MB) copied, 13.4781 s, 6.1 MB/s

Not much of a speedup, how so ?

Post by Stefan Wahren
[1] - http://www.spinics.net/lists/linux-mmc/msg32285.html
[2] - http://www.spinics.net/lists/linux-mmc/msg34418.html
DT: bindings: mmc: Add property for 3.3V only support
mmc: core: add new cap for 3.3V only DDR MMCs
mmc: mxs-mmc: Implement DDR support
Documentation/devicetree/bindings/mmc/mmc.txt | 7 +++++--
drivers/mmc/core/host.c | 2 ++
drivers/mmc/core/mmc.c | 6 ++++++
drivers/mmc/host/mxs-mmc.c | 22 ++++++++++++++++++++++
include/linux/mmc/host.h | 1 +
5 files changed, 36 insertions(+), 2 deletions(-)

[Stefan Wahren]

Hi Marek,

Post by Marek Vasut

Post by Stefan Wahren
Based on these discussions: [1], [2] this patch series implements
DDR support for the MXS MMC host driver. This feature has never been
ported from the vendor kernel.
It has been tested on a i.MX28 board with eMMC which is currently
not in mainline (Duckbill 2).
* without DDR support
dd if=/dev/zero of=test bs=8k count=10000
81920000 bytes (82 MB) copied, 14.3321 s, 5.7 MB/s
dd if=/dev/zero of=test bs=8k count=10000
81920000 bytes (82 MB) copied, 13.4781 s, 6.1 MB/s

Not much of a speedup, how so ?

i agree. Unfortunately i never had the time for a deeper analysis, but i noticed
big write performance differences between the vendor kernel 2.6.35 and current
mainline kernel in case of writing 100 MB directly to an eMMC partition (factor
2). Implementation of DDR mode was the lower fruit.

I've the suspicion that is related to the usage of DMA engine. Maybe the
overhead for AC/BC commands is to big. In the Freescale kernel is an
optimization [1].

[1] -
http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/commit/?h=imx_2.6.35_maintain&id=b09358887fb4b67f6d497fac8cc48475c8bd292d

Post by Marek Vasut

Post by Stefan Wahren
[1] - http://www.spinics.net/lists/linux-mmc/msg32285.html
[2] - http://www.spinics.net/lists/linux-mmc/msg34418.html
DT: bindings: mmc: Add property for 3.3V only support
mmc: core: add new cap for 3.3V only DDR MMCs
mmc: mxs-mmc: Implement DDR support
Documentation/devicetree/bindings/mmc/mmc.txt | 7 +++++--
drivers/mmc/core/host.c | 2 ++
drivers/mmc/core/mmc.c | 6 ++++++
drivers/mmc/host/mxs-mmc.c | 22 ++++++++++++++++++++++
include/linux/mmc/host.h | 1 +
5 files changed, 36 insertions(+), 2 deletions(-)

--
Best regards,
Marek Vasut

[Stefan Wahren]

Hi Marek,

Post by Stefan Wahren
Hi Marek,

Post by Marek Vasut

Post by Stefan Wahren
Based on these discussions: [1], [2] this patch series implements
DDR support for the MXS MMC host driver. This feature has never been
ported from the vendor kernel.
It has been tested on a i.MX28 board with eMMC which is currently
not in mainline (Duckbill 2).
* without DDR support
dd if=/dev/zero of=test bs=8k count=10000
81920000 bytes (82 MB) copied, 14.3321 s, 5.7 MB/s
dd if=/dev/zero of=test bs=8k count=10000
81920000 bytes (82 MB) copied, 13.4781 s, 6.1 MB/s

Not much of a speedup, how so ?

i agree. Unfortunately i never had the time for a deeper analysis, but i noticed
big write performance differences between the vendor kernel 2.6.35 and current
mainline kernel in case of writing 100 MB directly to an eMMC partition (factor
2). Implementation of DDR mode was the lower fruit.
I've the suspicion that is related to the usage of DMA engine. Maybe the
overhead for AC/BC commands is to big. In the Freescale kernel is an
optimization [1].
[1] -
http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/commit/?h=imx_2.6.35_maintain&id=b09358887fb4b67f6d497fac8cc48475c8bd292d

here are some additional thoughts about the too little performance gain of DDR
mode:

* The DDR mode applies only to the DATA lines not to the CMD line. So the plain
MMC commands can't benefit from that.

* The i.MX28 reference manual mentions a relevant setting:

17.8.2.2 eMMC DDR operation
To boost the performance of SSP running in DDR mode and at the maximum
frequency,
the DMA interface allows a burst of 4 or 8 32-bit APB transfers per DMA
request.

I tried to change HW_SSP_DDR_CTRL_DMA_BURST_TYPE with no luck (in case of 8
bursts the communication to the MMC get lost). AFAIK this register is never
touched in the vendor kernel.

Regards
Stefan

[Stefan Wahren]

Hi Ulf,

Post by Stefan Wahren
Based on these discussions: [1], [2] this patch series implements
DDR support for the MXS MMC host driver. This feature has never been
ported from the vendor kernel.
It has been tested on a i.MX28 board with eMMC which is currently
not in mainline (Duckbill 2).
* without DDR support
dd if=/dev/zero of=test bs=8k count=10000
81920000 bytes (82 MB) copied, 14.3321 s, 5.7 MB/s
dd if=/dev/zero of=test bs=8k count=10000
81920000 bytes (82 MB) copied, 13.4781 s, 6.1 MB/s
[1] - http://www.spinics.net/lists/linux-mmc/msg32285.html
[2] - http://www.spinics.net/lists/linux-mmc/msg34418.html

do you want to add some comments about this series?

Regards
Stefan

Post by Stefan Wahren
DT: bindings: mmc: Add property for 3.3V only support
mmc: core: add new cap for 3.3V only DDR MMCs
mmc: mxs-mmc: Implement DDR support
Documentation/devicetree/bindings/mmc/mmc.txt | 7 +++++--
drivers/mmc/core/host.c | 2 ++
drivers/mmc/core/mmc.c | 6 ++++++
drivers/mmc/host/mxs-mmc.c | 22 ++++++++++++++++++++++
include/linux/mmc/host.h | 1 +
5 files changed, 36 insertions(+), 2 deletions(-)
--
1.7.9.5