[U-Boot] [PATCH v2 0/3] Add SDHCI card detection support

[U-Boot] [PATCH v2 0/3] Add SDHCI card detection support

[Michal Simek]

Hi,

This patch series adds card detection support in sdhci framework &
added functionality to read card detect dt properties.

Thanks,
Michal

Changes in v2:
 - Moved reading CD devicetree properties functionality from sdhci.c to
   mmc-uclass.c & moved mmc capability macros to mmc.h from sdhci.h.
 - Created a new patch for reading "cd-gpio" property from DT.
 - Used CARD_PRESENT instead of SDHCI_CARD_DETECT_PIN_LEVEL for reading
   sd card presence detection.

T Karthik Reddy (3):
  mmc: Read sd card detect properties from DT
  mmc: sdhci: Read cd-gpio from devicetree
  mmc: sdhci: Implement SDHCI card detect

 drivers/mmc/mmc-uclass.c |  9 +++++++++
 drivers/mmc/sdhci.c      | 38 ++++++++++++++++++++++++++++++++++++++
 include/mmc.h            |  4 ++++
 3 files changed, 51 insertions(+)

-- 
2.17.1

[U-Boot] [PATCH v2 1/3] mmc: Read sd card detect properties from DT

[Michal Simek]

From: T Karthik Reddy <t.karthik.reddy@xilinx.com>

This patch reads card detect properties from device tree &
added mmc capability macros in mmc.h.

Signed-off-by: T Karthik Reddy <t.karthik.reddy@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v2:
 - Moved reading CD devicetree properties functionality from sdhci.c to
   mmc-uclass.c & moved mmc capability macros to mmc.h from sdhci.h.

 drivers/mmc/mmc-uclass.c | 9 +++++++++
 include/mmc.h            | 4 ++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
index a9c8f335c142..1fba7a8c124d 100644
--- a/drivers/mmc/mmc-uclass.c
+++ b/drivers/mmc/mmc-uclass.c
@@ -171,6 +171,15 @@ int mmc_of_parse(struct udevice *dev, struct mmc_config *cfg)
 	if (dev_read_bool(dev, "mmc-hs400-1_2v"))
 		cfg->host_caps |= MMC_CAP(MMC_HS_400);
 
+	if (dev_read_bool(dev, "non-removable")) {
+		cfg->host_caps |= MMC_CAP_SD_NONREMOVABLE;
+	} else {
+		if (dev_read_bool(dev, "cd-inverted"))
+			cfg->host_caps |= MMC_CAP_CD_ACTIVE_HIGH;
+		if (dev_read_bool(dev, "broken-cd"))
+			cfg->host_caps |= MMC_CAP_SD_NEEDS_POLL;
+	}
+
 	return 0;
 }
 
diff --git a/include/mmc.h b/include/mmc.h
index 1f30f71d25f8..30d2b1036b69 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -66,6 +66,10 @@
 #define MMC_MODE_HS200		MMC_CAP(MMC_HS_200)
 #define MMC_MODE_HS400		MMC_CAP(MMC_HS_400)
 
+#define MMC_CAP_SD_NONREMOVABLE	BIT(14)
+#define MMC_CAP_SD_NEEDS_POLL	BIT(15)
+#define MMC_CAP_CD_ACTIVE_HIGH  BIT(16)
+
 #define MMC_MODE_8BIT		BIT(30)
 #define MMC_MODE_4BIT		BIT(29)
 #define MMC_MODE_1BIT		BIT(28)
-- 
2.17.1

[U-Boot] [PATCH v2 2/3] mmc: sdhci: Read cd-gpio from devicetree

[Michal Simek]

From: T Karthik Reddy <t.karthik.reddy@xilinx.com>

This patch reads cd-gpio property from devicetree

Signed-off-by: T Karthik Reddy <t.karthik.reddy@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v2:
 - Created a new patch for reading "cd-gpio" property from DT.

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

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index e2bb90abbdf3..4ffe74e35e08 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -590,6 +590,12 @@ static int sdhci_set_ios(struct mmc *mmc)
 static int sdhci_init(struct mmc *mmc)
 {
 	struct sdhci_host *host = mmc->priv;
+#if CONFIG_IS_ENABLED(DM_MMC) && CONFIG_IS_ENABLED(DM_GPIO)
+	struct udevice *dev = mmc->dev;
+
+	gpio_request_by_name(dev, "cd-gpio", 0,
+			     &host->cd_gpio, GPIOD_IS_IN);
+#endif
 
 	sdhci_reset(host, SDHCI_RESET_ALL);
 
-- 
2.17.1

[U-Boot] [PATCH v2 3/3] mmc: sdhci: Implement SDHCI card detect

[Michal Simek]

From: T Karthik Reddy <t.karthik.reddy@xilinx.com>

Card detect function implemented for SDHCI framework.

Signed-off-by: T Karthik Reddy <t.karthik.reddy@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v2:
 - Used CARD_PRESENT instead of SDHCI_CARD_DETECT_PIN_LEVEL for reading
   sd card presence detection.

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

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 4ffe74e35e08..d66f9288dd91 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -12,6 +12,7 @@
 #include <malloc.h>
 #include <mmc.h>
 #include <sdhci.h>
+#include <dm.h>
 
 #if defined(CONFIG_FIXED_SDHCI_ALIGNED_BUFFER)
 void *aligned_buffer = (void *)CONFIG_FIXED_SDHCI_ALIGNED_BUFFER;
@@ -630,9 +631,40 @@ int sdhci_probe(struct udevice *dev)
 	return sdhci_init(mmc);
 }
 
+int sdhci_get_cd(struct udevice *dev)
+{
+	struct mmc *mmc = mmc_get_mmc_dev(dev);
+	struct sdhci_host *host = mmc->priv;
+	int value;
+
+	/* If nonremovable, assume that the card is always present. */
+	if (mmc->cfg->host_caps & MMC_CAP_SD_NONREMOVABLE)
+		return 1;
+	/* If polling, assume that the card is always present. */
+	if (mmc->cfg->host_caps & MMC_CAP_SD_NEEDS_POLL)
+		return 1;
+
+#if CONFIG_IS_ENABLED(DM_GPIO)
+	value = dm_gpio_get_value(&host->cd_gpio);
+	if (value >= 0) {
+		if (mmc->cfg->host_caps & MMC_CAP_CD_ACTIVE_HIGH)
+			return !value;
+		else
+			return value;
+	}
+#endif
+	value = !!(sdhci_readl(host, SDHCI_PRESENT_STATE) &
+		   SDHCI_CARD_PRESENT);
+	if (mmc->cfg->host_caps & MMC_CAP_CD_ACTIVE_HIGH)
+		return !value;
+	else
+		return value;
+}
+
 const struct dm_mmc_ops sdhci_ops = {
 	.send_cmd	= sdhci_send_command,
 	.set_ios	= sdhci_set_ios,
+	.get_cd		= sdhci_get_cd,
 #ifdef MMC_SUPPORTS_TUNING
 	.execute_tuning	= sdhci_execute_tuning,
 #endif
-- 
2.17.1

[U-Boot] [PATCH v2 1/3] mmc: Read sd card detect properties from DT

[Peng Fan]

> Subject: [PATCH v2 1/3] mmc: Read sd card detect properties from DT
> 
> From: T Karthik Reddy <t.karthik.reddy@xilinx.com>
> 
> This patch reads card detect properties from device tree & added mmc
> capability macros in mmc.h.
> 
> Signed-off-by: T Karthik Reddy <t.karthik.reddy@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
> Changes in v2:
>  - Moved reading CD devicetree properties functionality from sdhci.c to
>    mmc-uclass.c & moved mmc capability macros to mmc.h from sdhci.h.
> 
>  drivers/mmc/mmc-uclass.c | 9 +++++++++
>  include/mmc.h            | 4 ++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c index
> a9c8f335c142..1fba7a8c124d 100644
> --- a/drivers/mmc/mmc-uclass.c
> +++ b/drivers/mmc/mmc-uclass.c
> @@ -171,6 +171,15 @@ int mmc_of_parse(struct udevice *dev, struct
> mmc_config *cfg)
>  	if (dev_read_bool(dev, "mmc-hs400-1_2v"))
>  		cfg->host_caps |= MMC_CAP(MMC_HS_400);
> 
> +	if (dev_read_bool(dev, "non-removable")) {
> +		cfg->host_caps |= MMC_CAP_SD_NONREMOVABLE;
> +	} else {
> +		if (dev_read_bool(dev, "cd-inverted"))
> +			cfg->host_caps |= MMC_CAP_CD_ACTIVE_HIGH;
> +		if (dev_read_bool(dev, "broken-cd"))
> +			cfg->host_caps |= MMC_CAP_SD_NEEDS_POLL;

I think, it would be better if remove "SD" to align with Linux bit
definitions, saying MMC_CAP_NONREMOVEABLE.

Regards,
Peng.

> +	}
> +
>  	return 0;
>  }
> 
> diff --git a/include/mmc.h b/include/mmc.h index
> 1f30f71d25f8..30d2b1036b69 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -66,6 +66,10 @@
>  #define MMC_MODE_HS200		MMC_CAP(MMC_HS_200)
>  #define MMC_MODE_HS400		MMC_CAP(MMC_HS_400)
> 
> +#define MMC_CAP_SD_NONREMOVABLE	BIT(14)
> +#define MMC_CAP_SD_NEEDS_POLL	BIT(15)
> +#define MMC_CAP_CD_ACTIVE_HIGH  BIT(16)
> +
>  #define MMC_MODE_8BIT		BIT(30)
>  #define MMC_MODE_4BIT		BIT(29)
>  #define MMC_MODE_1BIT		BIT(28)
> --
> 2.17.1

[U-Boot] [PATCH v2 2/3] mmc: sdhci: Read cd-gpio from devicetree

[Peng Fan]

> Subject: [PATCH v2 2/3] mmc: sdhci: Read cd-gpio from devicetree
> 
> From: T Karthik Reddy <t.karthik.reddy@xilinx.com>
> 
> This patch reads cd-gpio property from devicetree
> 
> Signed-off-by: T Karthik Reddy <t.karthik.reddy@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
> Changes in v2:
>  - Created a new patch for reading "cd-gpio" property from DT.
> 
>  drivers/mmc/sdhci.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index
> e2bb90abbdf3..4ffe74e35e08 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -590,6 +590,12 @@ static int sdhci_set_ios(struct mmc *mmc)  static
> int sdhci_init(struct mmc *mmc)  {
>  	struct sdhci_host *host = mmc->priv;
> +#if CONFIG_IS_ENABLED(DM_MMC) && CONFIG_IS_ENABLED(DM_GPIO)
> +	struct udevice *dev = mmc->dev;
> +
> +	gpio_request_by_name(dev, "cd-gpio", 0,
> +			     &host->cd_gpio, GPIOD_IS_IN);
> +#endif

Reviewed-by: Peng Fan <peng.fan@nxp.com>

> 
>  	sdhci_reset(host, SDHCI_RESET_ALL);
> 
> --
> 2.17.1

[U-Boot] [PATCH v2 1/3] mmc: Read sd card detect properties from DT

[Marek Vasut]

On 6/21/19 9:09 AM, Peng Fan wrote:
>> Subject: [PATCH v2 1/3] mmc: Read sd card detect properties from DT
>>
>> From: T Karthik Reddy <t.karthik.reddy@xilinx.com>
>>
>> This patch reads card detect properties from device tree & added mmc
>> capability macros in mmc.h.
>>
>> Signed-off-by: T Karthik Reddy <t.karthik.reddy@xilinx.com>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Since we're discussing CD handling, I think I might add a bit of an
unrelated idea here, which I was mulling about for a while.

I was pondering about mmc rescan. When you do something like
mmc rescan
mmc part
<remove the card, insert different card>
mmc part

Then the last "mmc part" still prints the partition table of the
previous card, because it's cached. Maybe we can add a hook into the
U-Boot main loop to poll the CD and if CD change is detected, deregister
the card ?

That way, we would have a simple way to detect that card was changed and
it would be nicer for the user too. Polling the CD should not be
expensive either, so it shouldn't cause any problems with interactivity
of the U-Boot shell.

Similar approach could possibly be extended to other subsystems, e.g.
USB for USB device insertion/removal detection.

Thoughts ?

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 1/3] mmc: Read sd card detect properties from DT

[Michal Simek]

On 21. 06. 19 9:09, Peng Fan wrote:
>> Subject: [PATCH v2 1/3] mmc: Read sd card detect properties from DT
>>
>> From: T Karthik Reddy <t.karthik.reddy@xilinx.com>
>>
>> This patch reads card detect properties from device tree & added mmc
>> capability macros in mmc.h.
>>
>> Signed-off-by: T Karthik Reddy <t.karthik.reddy@xilinx.com>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> Changes in v2:
>>  - Moved reading CD devicetree properties functionality from sdhci.c to
>>    mmc-uclass.c & moved mmc capability macros to mmc.h from sdhci.h.
>>
>>  drivers/mmc/mmc-uclass.c | 9 +++++++++
>>  include/mmc.h            | 4 ++++
>>  2 files changed, 13 insertions(+)
>>
>> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c index
>> a9c8f335c142..1fba7a8c124d 100644
>> --- a/drivers/mmc/mmc-uclass.c
>> +++ b/drivers/mmc/mmc-uclass.c
>> @@ -171,6 +171,15 @@ int mmc_of_parse(struct udevice *dev, struct
>> mmc_config *cfg)
>>  	if (dev_read_bool(dev, "mmc-hs400-1_2v"))
>>  		cfg->host_caps |= MMC_CAP(MMC_HS_400);
>>
>> +	if (dev_read_bool(dev, "non-removable")) {
>> +		cfg->host_caps |= MMC_CAP_SD_NONREMOVABLE;
>> +	} else {
>> +		if (dev_read_bool(dev, "cd-inverted"))
>> +			cfg->host_caps |= MMC_CAP_CD_ACTIVE_HIGH;
>> +		if (dev_read_bool(dev, "broken-cd"))
>> +			cfg->host_caps |= MMC_CAP_SD_NEEDS_POLL;
> 
> I think, it would be better if remove "SD" to align with Linux bit
> definitions, saying MMC_CAP_NONREMOVEABLE.

This macro is in  include/mvebu_mmc.h but it is unused. It shouldn't be
a problem to remove it and use it here.

Thanks,
Michal

[U-Boot] [PATCH v2 1/3] mmc: Read sd card detect properties from DT

[Suniel Mahesh]

Hi Marek,

Looks like CD is working fine with TI AM3358X SOC based targets.

Ran a quick test after looking at the discussion, posting the results which might be of some help.

In this test, mmc part is not printing the partition table of the previous card, instead it prints
the partition table of the latest card which is inserted.

Hope the test is inline with the discussion. Please ignore if the results doesn't make any sense.

Tested on U-Boot v2019.07-rc1

8GB card inserted

=> mmc rescan
=> mmc part

Partition Map for MMC device 0  --   Partition Type: DOS

Part    Start Sector    Num Sectors     UUID            Type
  1     2048            262144          7d8cd224-01     0b Boot
  2     264192          15259648        7d8cd224-02     83

=> mmc info
Device: OMAP SD/MMC
Manufacturer ID: 3
OEM: 5344
Name: SS08G 
Bus Speed: 48000000
Mode : SD High Speed (50MHz)
Rd Block Len: 512
SD version 3.0
High Capacity: Yes
Capacity: 7.4 GiB
Bus Width: 4-bit
Erase Group Size: 512 Bytes


removed 8GB card and inserted 2GB card

=> mmc rescan
=> mmc part

Partition Map for MMC device 0  --   Partition Type: DOS

Part    Start Sector    Num Sectors     UUID            Type
  1     2048            262144          000c0116-01     0b Boot
  2     264192          3647488         000c0116-02     83
=> mmc info  
Device: OMAP SD/MMC
Manufacturer ID: 1b
OEM: 534d
Name: 00000 
Bus Speed: 48000000
Mode : SD High Speed (50MHz)
Rd Block Len: 512
SD version 3.0
High Capacity: No
Capacity: 1.9 GiB
Bus Width: 4-bit
Erase Group Size: 512 Bytes

Thanks

Regards
-- 
Suniel Mahesh

[U-Boot] [PATCH v2 1/3] mmc: Read sd card detect properties from DT

[Marek Vasut]

On 6/22/19 8:58 PM, Suniel Mahesh wrote:
> Hi Marek,

Hi,

> Looks like CD is working fine with TI AM3358X SOC based targets.

Yes, it does.

> Ran a quick test after looking at the discussion, posting the results which might be of some help.

I think you misunderstood my proposal.

mmc rescan ; mmc part ; <replace card> ; mmc rescan ; mmc part obviously
works, because of the second mmc rescan. Try without the second mmc
rescan, you'll get the old results.

My proposal was suggesting that we add the ability to poll CD into the
u-boot main loop and detect the card replacement in the background, thus
making "mmc rescan" basically a no-op. Then, if the user replaced the
card, the user won't have to worry about running "mmc rescan" or
forgetting to do so.

> In this test, mmc part is not printing the partition table of the previous card, instead it prints
> the partition table of the latest card which is inserted.
> 
> Hope the test is inline with the discussion. Please ignore if the results doesn't make any sense.
> 
> Tested on U-Boot v2019.07-rc1
> 
> 8GB card inserted
> 
> => mmc rescan
> => mmc part
> 
> Partition Map for MMC device 0  --   Partition Type: DOS
> 
> Part    Start Sector    Num Sectors     UUID            Type
>   1     2048            262144          7d8cd224-01     0b Boot
>   2     264192          15259648        7d8cd224-02     83
> 
> => mmc info
> Device: OMAP SD/MMC
> Manufacturer ID: 3
> OEM: 5344
> Name: SS08G 
> Bus Speed: 48000000
> Mode : SD High Speed (50MHz)
> Rd Block Len: 512
> SD version 3.0
> High Capacity: Yes
> Capacity: 7.4 GiB
> Bus Width: 4-bit
> Erase Group Size: 512 Bytes
> 
> 
> removed 8GB card and inserted 2GB card
> 
> => mmc rescan
> => mmc part
> 
> Partition Map for MMC device 0  --   Partition Type: DOS
> 
> Part    Start Sector    Num Sectors     UUID            Type
>   1     2048            262144          000c0116-01     0b Boot
>   2     264192          3647488         000c0116-02     83
> => mmc info  
> Device: OMAP SD/MMC
> Manufacturer ID: 1b
> OEM: 534d
> Name: 00000 
> Bus Speed: 48000000
> Mode : SD High Speed (50MHz)
> Rd Block Len: 512
> SD version 3.0
> High Capacity: No
> Capacity: 1.9 GiB
> Bus Width: 4-bit
> Erase Group Size: 512 Bytes
> 
> Thanks
> 
> Regards
> 


-- 
Best regards,
Marek Vasut