[U-Boot] [PATCH v2 0/4] mmc: fixes for HS200/UHS core support

[U-Boot] [PATCH v2 0/4] mmc: fixes for HS200/UHS core support

[Jean-Jacques Hiblot]

This series applies on top of "[PATCH v2 00/26] mmc: Add support for HS200
and UHS modes"

It fixes a bug with old SD and MMC cards that support only the legacy mode.
It also addresses the comments made on the mailing list:
* dump card and host capabilities in debug mode
* use 1-bit if the DTS property 'bus-width' is not present.
* recognize the "mmc-ddr-1_2v" and "mmc-hs200-1_2v" DTS properties
* convert mmc_of_parse to livetree

Tested on DRA7, DRA72 and AM335x

changes since v1:
* convert mmc_of_parse to livetree
* squashed all changes to mmc_of_parse in a single patch

Jean-Jacques Hiblot (4):
  dm: mmc: update mmc_of_parse()
  mmc: dump card and host capabilities if debug is enabled
  mmc: Fixed a problem with old sd or mmc that do not support High speed
  mmc: all hosts support 1-bit bus width and legacy timings

 drivers/mmc/mmc-uclass.c | 37 ++++++++++++++++++++++---------------
 drivers/mmc/mmc.c        | 24 +++++++++++++++++++-----
 include/mmc.h            | 11 ++++++++++-
 3 files changed, 51 insertions(+), 21 deletions(-)

-- 
1.9.1

[U-Boot] [PATCH v2 1/4] dm: mmc: update mmc_of_parse()

[Jean-Jacques Hiblot]

* convert to livetree API
* don't fail because of an invalid bus-width, instead default to 1-bit.
* recognize 1.2v DDR and 1.2v HS200 flags

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---
changes since v1:
* convert to livetree
* squashed all change to mmc_of_parse into this single commit
* Add a descriptive comment fo mmc_of_parse in the header

 drivers/mmc/mmc-uclass.c | 37 ++++++++++++++++++++++---------------
 include/mmc.h            | 11 ++++++++++-
 2 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
index e30cde7..f117923 100644
--- a/drivers/mmc/mmc-uclass.c
+++ b/drivers/mmc/mmc-uclass.c
@@ -10,6 +10,7 @@
 #include <dm.h>
 #include <dm/device-internal.h>
 #include <dm/lists.h>
+#include <dm/ofnode.h>
 #include <dm/root.h>
 #include "mmc_private.h"
 
@@ -120,11 +121,12 @@ int mmc_execute_tuning(struct mmc *mmc, uint opcode)
 	return dm_mmc_execute_tuning(mmc->dev, opcode);
 }
 
-int mmc_of_parse(const void *fdt, int node, struct mmc_config *cfg)
+int mmc_of_parse(struct udevice *dev, struct mmc_config *cfg)
 {
 	int val;
+	ofnode ref = dev_ofnode(dev);
 
-	val = fdtdec_get_int(fdt, node, "bus-width", 1);
+	val = ofnode_read_u32_default(ref, "bus-width", 1);
 
 	switch (val) {
 	case 0x8:
@@ -137,30 +139,35 @@ int mmc_of_parse(const void *fdt, int node, struct mmc_config *cfg)
 		cfg->host_caps |= MMC_MODE_1BIT;
 		break;
 	default:
-		printf("error: %s invalid bus-width property %d\n",
-		       fdt_get_name(fdt, node, NULL), val);
-		return -ENOENT;
+		debug("warning: %s invalid bus-width property. using 1-bit\n",
+		      ofnode_get_name(ref));
+		cfg->host_caps |= MMC_MODE_1BIT;
+		break;
 	}
 
-	cfg->f_max = fdtdec_get_int(fdt, node, "max-frequency", 52000000);
+	cfg->f_max = ofnode_read_u32_default(ref, "max-frequency", 52000000);
 
-	if (fdtdec_get_bool(fdt, node, "cap-sd-highspeed"))
+	if (ofnode_read_bool(ref, "cap-sd-highspeed"))
 		cfg->host_caps |= MMC_CAP(SD_HS);
-	if (fdtdec_get_bool(fdt, node, "cap-mmc-highspeed"))
+	if (ofnode_read_bool(ref, "cap-mmc-highspeed"))
 		cfg->host_caps |= MMC_CAP(MMC_HS);
-	if (fdtdec_get_bool(fdt, node, "sd-uhs-sdr12"))
+	if (ofnode_read_bool(ref, "sd-uhs-sdr12"))
 		cfg->host_caps |= MMC_CAP(UHS_SDR12);
-	if (fdtdec_get_bool(fdt, node, "sd-uhs-sdr25"))
+	if (ofnode_read_bool(ref, "sd-uhs-sdr25"))
 		cfg->host_caps |= MMC_CAP(UHS_SDR25);
-	if (fdtdec_get_bool(fdt, node, "sd-uhs-sdr50"))
+	if (ofnode_read_bool(ref, "sd-uhs-sdr50"))
 		cfg->host_caps |= MMC_CAP(UHS_SDR50);
-	if (fdtdec_get_bool(fdt, node, "sd-uhs-sdr104"))
+	if (ofnode_read_bool(ref, "sd-uhs-sdr104"))
 		cfg->host_caps |= MMC_CAP(UHS_SDR104);
-	if (fdtdec_get_bool(fdt, node, "sd-uhs-ddr50"))
+	if (ofnode_read_bool(ref, "sd-uhs-ddr50"))
 		cfg->host_caps |= MMC_CAP(UHS_DDR50);
-	if (fdtdec_get_bool(fdt, node, "mmc-ddr-1_8v"))
+	if (ofnode_read_bool(ref, "mmc-ddr-1_8v"))
 		cfg->host_caps |= MMC_CAP(MMC_DDR_52);
-	if (fdtdec_get_bool(fdt, node, "mmc-hs200-1_8v"))
+	if (ofnode_read_bool(ref, "mmc-ddr-1_2v"))
+		cfg->host_caps |= MMC_CAP(MMC_DDR_52);
+	if (ofnode_read_bool(ref, "mmc-hs200-1_8v"))
+		cfg->host_caps |= MMC_CAP(MMC_HS_200);
+	if (ofnode_read_bool(ref, "mmc-hs200-1_2v"))
 		cfg->host_caps |= MMC_CAP(MMC_HS_200);
 
 	return 0;
diff --git a/include/mmc.h b/include/mmc.h
index 6230a32..e3f777f 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -651,7 +651,16 @@ int mmc_unbind(struct udevice *dev);
 int mmc_initialize(bd_t *bis);
 int mmc_init(struct mmc *mmc);
 int mmc_send_tuning(struct mmc *mmc, u32 opcode, int *cmd_error);
-int mmc_of_parse(const void *fdt, int node, struct mmc_config *cfg);
+
+/**
+ * mmc_of_parse() - Parse the device tree to get the capabilities of the host
+ *
+ * @dev:	MMC device
+ * @cfg:	MMC configuration
+ * @return 0 if OK, -ve on error
+ */
+int mmc_of_parse(struct udevice *dev, struct mmc_config *cfg);
+
 int mmc_read(struct mmc *mmc, u64 src, uchar *dst, int size);
 
 /**
-- 
1.9.1

[U-Boot] [PATCH v2 2/4] mmc: dump card and host capabilities if debug is enabled

[Jean-Jacques Hiblot]

This is a useful information while debugging the initialization process or
performance issues.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
Reviewed-by: Lukasz Majewski <lukma@denx.de>
---

no change since v1

 drivers/mmc/mmc.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index a5a521e..a30b6a2 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -1502,6 +1502,10 @@ void mmc_dump_capabilities(const char *text, uint caps)
 			printf("%s, ", mmc_mode_name(mode));
 	printf("\b\b]\n");
 }
+#else
+void mmc_dump_capabilities(const char *text, uint caps)
+{
+}
 #endif
 
 struct mode_width_tuning {
@@ -1582,6 +1586,8 @@ static int sd_select_mode_and_width(struct mmc *mmc, uint card_caps)
 	bool uhs_en = (mmc->ocr & OCR_S18R) ? true : false;
 	uint caps;
 
+	mmc_dump_capabilities("sd card", card_caps);
+	mmc_dump_capabilities("host", mmc->host_caps | MMC_MODE_1BIT);
 
 	/* Restrict card's capabilities by what the host can do */
 	caps = card_caps & (mmc->host_caps | MMC_MODE_1BIT);
@@ -1764,6 +1770,9 @@ static int mmc_select_mode_and_width(struct mmc *mmc, uint card_caps)
 	const struct mode_width_tuning *mwt;
 	const struct ext_csd_bus_width *ecbw;
 
+	mmc_dump_capabilities("mmc", card_caps);
+	mmc_dump_capabilities("host", mmc->host_caps | MMC_MODE_1BIT);
+
 	/* Restrict card's capabilities by what the host can do */
 	card_caps &= (mmc->host_caps | MMC_MODE_1BIT);
 
-- 
1.9.1

[U-Boot] [PATCH v2 3/4] mmc: Fixed a problem with old sd or mmc that do not support High speed

[Jean-Jacques Hiblot]

As the legacy modes were not added to the list of supported modes, old
cards that do not support other modes could not be used.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
Reviewed-by: Lukasz Majewski <lukma@denx.de>
---

no change since v1


 drivers/mmc/mmc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index a30b6a2..67f21ff 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -818,7 +818,7 @@ static int mmc_get_capabilities(struct mmc *mmc)
 	u8 *ext_csd = mmc->ext_csd;
 	char cardtype;
 
-	mmc->card_caps = MMC_MODE_1BIT;
+	mmc->card_caps = MMC_MODE_1BIT | MMC_CAP(MMC_LEGACY);
 
 	if (mmc_host_is_spi(mmc))
 		return 0;
@@ -1171,7 +1171,7 @@ static int sd_get_capabilities(struct mmc *mmc)
 	int timeout;
 	u32 sd3_bus_mode;
 
-	mmc->card_caps = MMC_MODE_1BIT;
+	mmc->card_caps = MMC_MODE_1BIT | MMC_CAP(SD_LEGACY);
 
 	if (mmc_host_is_spi(mmc))
 		return 0;
-- 
1.9.1

[U-Boot] [PATCH v2 4/4] mmc: all hosts support 1-bit bus width and legacy timings

[Jean-Jacques Hiblot]

Make sure that those basic capabilities are advertised by the host.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
Reviewed-by: Lukasz Majewski <lukma@denx.de>
---

no change since v1

 drivers/mmc/mmc.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 67f21ff..ec1dc49 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -1587,10 +1587,10 @@ static int sd_select_mode_and_width(struct mmc *mmc, uint card_caps)
 	uint caps;
 
 	mmc_dump_capabilities("sd card", card_caps);
-	mmc_dump_capabilities("host", mmc->host_caps | MMC_MODE_1BIT);
+	mmc_dump_capabilities("host", mmc->host_caps);
 
 	/* Restrict card's capabilities by what the host can do */
-	caps = card_caps & (mmc->host_caps | MMC_MODE_1BIT);
+	caps = card_caps & mmc->host_caps;
 
 	if (!uhs_en)
 		caps &= ~UHS_CAPS;
@@ -1771,10 +1771,10 @@ static int mmc_select_mode_and_width(struct mmc *mmc, uint card_caps)
 	const struct ext_csd_bus_width *ecbw;
 
 	mmc_dump_capabilities("mmc", card_caps);
-	mmc_dump_capabilities("host", mmc->host_caps | MMC_MODE_1BIT);
+	mmc_dump_capabilities("host", mmc->host_caps);
 
 	/* Restrict card's capabilities by what the host can do */
-	card_caps &= (mmc->host_caps | MMC_MODE_1BIT);
+	card_caps &= mmc->host_caps;
 
 	/* Only version 4 of MMC supports wider bus widths */
 	if (mmc->version < MMC_VERSION_4)
@@ -2389,7 +2389,12 @@ int mmc_start_init(struct mmc *mmc)
 	bool uhs_en = supports_uhs(mmc->cfg->host_caps);
 	int err;
 
-	mmc->host_caps = mmc->cfg->host_caps;
+	/*
+	 * all hosts are capable of 1 bit bus-width and able to use the legacy
+	 * timings.
+	 */
+	mmc->host_caps = mmc->cfg->host_caps | MMC_CAP(SD_LEGACY) |
+			 MMC_CAP(MMC_LEGACY) | MMC_MODE_1BIT;
 
 	/* we pretend there's no card when init is NULL */
 	no_card = mmc_getcd(mmc) == 0;
-- 
1.9.1

[U-Boot] [PATCH v2 2/4] mmc: dump card and host capabilities if debug is enabled

[Jaehoon Chung]

Hi JJ,

On 11/27/2017 06:59 PM, Jean-Jacques Hiblot wrote:
> This is a useful information while debugging the initialization process or
> performance issues.
> 
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> Reviewed-by: Lukasz Majewski <lukma@denx.de>
> ---
> 
> no change since v1
> 
>  drivers/mmc/mmc.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index a5a521e..a30b6a2 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -1502,6 +1502,10 @@ void mmc_dump_capabilities(const char *text, uint caps)
>  			printf("%s, ", mmc_mode_name(mode));
>  	printf("\b\b]\n");
>  }
> +#else
> +void mmc_dump_capabilities(const char *text, uint caps)
> +{
> +}
>  #endif
>  
>  struct mode_width_tuning {
> @@ -1582,6 +1586,8 @@ static int sd_select_mode_and_width(struct mmc *mmc, uint card_caps)
>  	bool uhs_en = (mmc->ocr & OCR_S18R) ? true : false;
>  	uint caps;
>  
> +	mmc_dump_capabilities("sd card", card_caps);
> +	mmc_dump_capabilities("host", mmc->host_caps | MMC_MODE_1BIT);
>  
>  	/* Restrict card's capabilities by what the host can do */
>  	caps = card_caps & (mmc->host_caps | MMC_MODE_1BIT);
> @@ -1764,6 +1770,9 @@ static int mmc_select_mode_and_width(struct mmc *mmc, uint card_caps)
>  	const struct mode_width_tuning *mwt;
>  	const struct ext_csd_bus_width *ecbw;
>  
> +	mmc_dump_capabilities("mmc", card_caps);
> +	mmc_dump_capabilities("host", mmc->host_caps | MMC_MODE_1BIT);
> +

I'm not sure...but
MMC_VERBOSE is enabled by default. So this dump message is always displayed.

Best Regards,
Jaehoon Chung

>  	/* Restrict card's capabilities by what the host can do */
>  	card_caps &= (mmc->host_caps | MMC_MODE_1BIT);
>  
> 

[U-Boot] [PATCH v2 1/4] dm: mmc: update mmc_of_parse()

[Simon Glass]

Hi Jean-Jacques,

On 27 November 2017 at 02:59, Jean-Jacques Hiblot <jjhiblot@ti.com> wrote:
> * convert to livetree API
> * don't fail because of an invalid bus-width, instead default to 1-bit.
> * recognize 1.2v DDR and 1.2v HS200 flags
>
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> ---
> changes since v1:
> * convert to livetree
> * squashed all change to mmc_of_parse into this single commit
> * Add a descriptive comment fo mmc_of_parse in the header
>
>  drivers/mmc/mmc-uclass.c | 37 ++++++++++++++++++++++---------------
>  include/mmc.h            | 11 ++++++++++-
>  2 files changed, 32 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
> index e30cde7..f117923 100644
> --- a/drivers/mmc/mmc-uclass.c
> +++ b/drivers/mmc/mmc-uclass.c
> @@ -10,6 +10,7 @@
>  #include <dm.h>
>  #include <dm/device-internal.h>
>  #include <dm/lists.h>
> +#include <dm/ofnode.h>
>  #include <dm/root.h>
>  #include "mmc_private.h"
>
> @@ -120,11 +121,12 @@ int mmc_execute_tuning(struct mmc *mmc, uint opcode)
>         return dm_mmc_execute_tuning(mmc->dev, opcode);
>  }
>
> -int mmc_of_parse(const void *fdt, int node, struct mmc_config *cfg)
> +int mmc_of_parse(struct udevice *dev, struct mmc_config *cfg)
>  {
>         int val;
> +       ofnode ref = dev_ofnode(dev);

Do you need this? I think you can just use dev_read_...() below
instead of ofnode_...() ?

Regards,
Simon

[U-Boot] [PATCH v2 3/4] mmc: Fixed a problem with old sd or mmc that do not support High speed

[Simon Glass]

On 27 November 2017 at 02:59, Jean-Jacques Hiblot <jjhiblot@ti.com> wrote:
> As the legacy modes were not added to the list of supported modes, old
> cards that do not support other modes could not be used.
>
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> Reviewed-by: Lukasz Majewski <lukma@denx.de>
> ---
>
> no change since v1
>
>
>  drivers/mmc/mmc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 4/4] mmc: all hosts support 1-bit bus width and legacy timings

[Simon Glass]

On 27 November 2017 at 02:59, Jean-Jacques Hiblot <jjhiblot@ti.com> wrote:
> Make sure that those basic capabilities are advertised by the host.
>
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> Reviewed-by: Lukasz Majewski <lukma@denx.de>
> ---
>
> no change since v1
>
>  drivers/mmc/mmc.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 1/4] dm: mmc: update mmc_of_parse()

[Jean-Jacques Hiblot]

On 27/11/2017 18:13, Simon Glass wrote:
> Hi Jean-Jacques,
>
> On 27 November 2017 at 02:59, Jean-Jacques Hiblot <jjhiblot@ti.com> wrote:
>> * convert to livetree API
>> * don't fail because of an invalid bus-width, instead default to 1-bit.
>> * recognize 1.2v DDR and 1.2v HS200 flags
>>
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
>> ---
>> changes since v1:
>> * convert to livetree
>> * squashed all change to mmc_of_parse into this single commit
>> * Add a descriptive comment fo mmc_of_parse in the header
>>
>>   drivers/mmc/mmc-uclass.c | 37 ++++++++++++++++++++++---------------
>>   include/mmc.h            | 11 ++++++++++-
>>   2 files changed, 32 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
>> index e30cde7..f117923 100644
>> --- a/drivers/mmc/mmc-uclass.c
>> +++ b/drivers/mmc/mmc-uclass.c
>> @@ -10,6 +10,7 @@
>>   #include <dm.h>
>>   #include <dm/device-internal.h>
>>   #include <dm/lists.h>
>> +#include <dm/ofnode.h>
>>   #include <dm/root.h>
>>   #include "mmc_private.h"
>>
>> @@ -120,11 +121,12 @@ int mmc_execute_tuning(struct mmc *mmc, uint opcode)
>>          return dm_mmc_execute_tuning(mmc->dev, opcode);
>>   }
>>
>> -int mmc_of_parse(const void *fdt, int node, struct mmc_config *cfg)
>> +int mmc_of_parse(struct udevice *dev, struct mmc_config *cfg)
>>   {
>>          int val;
>> +       ofnode ref = dev_ofnode(dev);
> Do you need this? I think you can just use dev_read_...() below
> instead of ofnode_...() ?
Sure. I'll send the v3 shortly
>
> Regards,
> Simon
>

[U-Boot] [PATCH v2 2/4] mmc: dump card and host capabilities if debug is enabled

[Jean-Jacques Hiblot]

On 27/11/2017 11:10, Jaehoon Chung wrote:
> Hi JJ,
>
> On 11/27/2017 06:59 PM, Jean-Jacques Hiblot wrote:
>> This is a useful information while debugging the initialization process or
>> performance issues.
>>
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
>> Reviewed-by: Lukasz Majewski <lukma@denx.de>
>> ---
>>
>> no change since v1
>>
>>   drivers/mmc/mmc.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>> index a5a521e..a30b6a2 100644
>> --- a/drivers/mmc/mmc.c
>> +++ b/drivers/mmc/mmc.c
>> @@ -1502,6 +1502,10 @@ void mmc_dump_capabilities(const char *text, uint caps)
>>   			printf("%s, ", mmc_mode_name(mode));
>>   	printf("\b\b]\n");
>>   }
>> +#else
>> +void mmc_dump_capabilities(const char *text, uint caps)
>> +{
>> +}
>>   #endif
>>   
>>   struct mode_width_tuning {
>> @@ -1582,6 +1586,8 @@ static int sd_select_mode_and_width(struct mmc *mmc, uint card_caps)
>>   	bool uhs_en = (mmc->ocr & OCR_S18R) ? true : false;
>>   	uint caps;
>>   
>> +	mmc_dump_capabilities("sd card", card_caps);
>> +	mmc_dump_capabilities("host", mmc->host_caps | MMC_MODE_1BIT);
>>   
>>   	/* Restrict card's capabilities by what the host can do */
>>   	caps = card_caps & (mmc->host_caps | MMC_MODE_1BIT);
>> @@ -1764,6 +1770,9 @@ static int mmc_select_mode_and_width(struct mmc *mmc, uint card_caps)
>>   	const struct mode_width_tuning *mwt;
>>   	const struct ext_csd_bus_width *ecbw;
>>   
>> +	mmc_dump_capabilities("mmc", card_caps);
>> +	mmc_dump_capabilities("host", mmc->host_caps | MMC_MODE_1BIT);
>> +
> I'm not sure...but
> MMC_VERBOSE is enabled by default. So this dump message is always displayed.
Yes you're right. I'll fix that by printing it only if debug is enabled
Thanks
>
> Best Regards,
> Jaehoon Chung
>
>>   	/* Restrict card's capabilities by what the host can do */
>>   	card_caps &= (mmc->host_caps | MMC_MODE_1BIT);
>>   
>>
>