Skip to contentSkip to conversation

mmc: usdhi6rol0: UHS support

PATCH Superseded Lars Persson 4 participants 8 messages Apr 21, 2016 – Apr 26, 2016 1 message not in the archive

Subsystems (3)
Device tree bindings MMC, SD and SDIO Arm/artpec machine support

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Lars Persson

author
#0 lore raw

v4 cover mmc: usdhi6rol0: UHS support

This patch set adds UHS support to the usdhi6rol0 driver, with control of the regulator and pin state settings.

Changes since v3:
- No error print for failure of mmc_regulator_set_vqmmc().
- Do not set UHS caps if the vqmmc regulator is missing.
- No not set UHS caps if the no-1-8-v property is set.

Changes since v2:
- dt: renamed pins_uhs to state_uhs.
- dt: made default optional in pinctrl-names.
- dt: document that use of state_uhs also requires a default pin state.
- add error handling for missing pin states.

Changes since v1:
- Use mmc_regulator_set_vqmmc().

Lars Persson (3):
mmc: dt: usdhi6rol0: add optional pinctrl binding
mmc: usdhi6rol0: add support for UHS modes
mmc: usdhi6rol0: add pinctrl to set pin drive strength

 .../devicetree/bindings/mmc/usdhi6rol0.txt         |  6 ++
 drivers/mmc/host/usdhi6rol0.c                      | 71 +++++++++++++++++++++-
 2 files changed, 76 insertions(+), 1 deletion(-)
Signature · 6 lines
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#2 lore raw

PATCH v4 1/3 mmc: dt: usdhi6rol0: add optional pinctrl binding

Add a pinctrl binding to specify different pin settings for high speed modes and UHS modes.

Signed-off-by Lars Persson <[email protected]>
---
 Documentation/devicetree/bindings/mmc/usdhi6rol0.txt | 6 ++++++
 1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/mmc/usdhi6rol0.txt +6 -0
index 8babdaa..6d1b797 100644
@@ -12,6 +12,12 @@ Optional properties:
 
 - vmmc-supply:	a phandle of a regulator, supplying Vcc to the card
 - vqmmc-supply:	a phandle of a regulator, supplying VccQ to the card
+- pinctrl-names: Can contain a "default" entry and a "state_uhs"
+                 entry. The state_uhs entry is used together with the default
+                 entry when the board requires distinct settings for UHS speeds.
+
+- pinctrl-N: One property for each name listed in pinctrl-names, see
+             ../pinctrl/pinctrl-bindings.txt.
 
 Additionally any standard mmc bindings from mmc.txt can be used.
Signature · 6 lines
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
maintainer
#3 lore raw

Re: [PATCH v4 1/3] mmc: dt: usdhi6rol0: add optional pinctrl binding

Quoted 13 lines · Lars Persson

Add a pinctrl binding to specify different pin settings for high speed modes and UHS modes.

Signed-off-by Lars Persson <[email protected]>
---
 Documentation/devicetree/bindings/mmc/usdhi6rol0.txt | 6 ++++++
 1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/mmc/usdhi6rol0.txt +0 -0
index 8babdaa..6d1b797 100644

Documentation/devicetree/bindings/mmc/usdhi6rol0.txt Optional properties: · line 15

Quoted 4 lines
Documentation/devicetree/bindings/mmc/usdhi6rol0.txt +1 -0
@@ -12,6 +12,12 @@ Optional properties:
 
 - vmmc-supply:	a phandle of a regulator, supplying Vcc to the card
 - vqmmc-supply:	a phandle of a regulator, supplying VccQ to the card
+- pinctrl-names: Can contain a "default" entry and a "state_uhs"

Don't use underscores. Just "uhs" should be sufficient, right?

Quoted 11 lines
Documentation/devicetree/bindings/mmc/usdhi6rol0.txt +5 -0
+                 entry. The state_uhs entry is used together with the default
+                 entry when the board requires distinct settings for UHS speeds.
+
+- pinctrl-N: One property for each name listed in pinctrl-names, see
+             ../pinctrl/pinctrl-bindings.txt.
 
 Additionally any standard mmc bindings from mmc.txt can be used.
Signature · 1 line
2.1.4
author
#4 lore raw

Re: [PATCH v4 1/3] mmc: dt: usdhi6rol0: add optional pinctrl binding

Quoted 15 lines · Rob Herring
Quoted 14 lines · Lars Persson

Add a pinctrl binding to specify different pin settings for high speed modes and UHS modes.

Signed-off-by Lars Persson <[email protected]>
---
  Documentation/devicetree/bindings/mmc/usdhi6rol0.txt | 6 ++++++
  1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/mmc/usdhi6rol0.txt +0 -0
index 8babdaa..6d1b797 100644
@@ -12,6 +12,12 @@ Optional properties:
Quoted 5 lines

Documentation/devicetree/bindings/mmc/usdhi6rol0.txt Optional properties: · line 15

Quoted 3 lines
Documentation/devicetree/bindings/mmc/usdhi6rol0.txt +1 -0
  - vmmc-supply:	a phandle of a regulator, supplying Vcc to the card
  - vqmmc-supply:	a phandle of a regulator, supplying VccQ to the card
+- pinctrl-names: Can contain a "default" entry and a "state_uhs"

Don't use underscores. Just "uhs" should be sufficient, right?

I do not object to this, but for consistency it seems better with state_uhs that existing bindings use.

Quoted 7 lines
Quoted 7 lines
Documentation/devicetree/bindings/mmc/usdhi6rol0.txt +5 -0
+                 entry. The state_uhs entry is used together with the default
+                 entry when the board requires distinct settings for UHS speeds.
+
+- pinctrl-N: One property for each name listed in pinctrl-names, see
+             ../pinctrl/pinctrl-bindings.txt.
Quoted 5 lines
Quoted 5 lines
Documentation/devicetree/bindings/mmc/usdhi6rol0.txt +0 -1
  Additionally any standard mmc bindings from mmc.txt can be used.
 
--

2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

maintainer
#5 lore raw

Re: [PATCH v4 1/3] mmc: dt: usdhi6rol0: add optional pinctrl binding

Quoted 31 lines · Lars Persson
Quoted 18 lines · Rob Herring
Quoted 16 lines · Lars Persson

Add a pinctrl binding to specify different pin settings for high speed modes and UHS modes.

Signed-off-by Lars Persson <[email protected]>
---
  Documentation/devicetree/bindings/mmc/usdhi6rol0.txt | 6 ++++++
  1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/mmc/usdhi6rol0.txt +0 -0
index 8babdaa..6d1b797 100644
@@ -12,6 +12,12 @@ Optional properties:
Quoted 8 lines
Quoted 5 lines
Documentation/devicetree/bindings/mmc/usdhi6rol0.txt +0 -0
  - vmmc-supply:        a phandle of a regulator, supplying Vcc to the

card
- vqmmc-supply: a phandle of a regulator, supplying VccQ to the
card
+- pinctrl-names: Can contain a "default" entry and a "state_uhs"

Don't use underscores. Just "uhs" should be sufficient, right?

Quoted 2 lines

I do not object to this, but for consistency it seems better with state_uhs that existing bindings use.

Okay. Consistency is better.

Rob

maintainer
#6 lore raw

PATCH v4 2/3 mmc: usdhi6rol0: add support for UHS modes

In reply to Lars Persson

Add a start_signal_voltage_switch() operation to support enabling of UHS modes.

Signed-off-by Lars Persson <[email protected]>
---
 drivers/mmc/host/usdhi6rol0.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)
drivers/mmc/host/usdhi6rol0.c +22 -1
index 807c06e..28fe5c7 100644
@@ -21,6 +21,7 @@
 #include <linux/mmc/sd.h>
 #include <linux/mmc/sdio.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/pagemap.h>
 #include <linux/platform_device.h>
 #include <linux/scatterlist.h>

@@ -1147,12 +1148,22 @@ static void usdhi6_enable_sdio_irq(struct mmc_host *mmc, int enable)
 	}
 }
 
+static int usdhi6_sig_volt_switch(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+	int ret;
+
+	ret = mmc_regulator_set_vqmmc(mmc, ios);
+
+	return ret;
+}
+
 static struct mmc_host_ops usdhi6_ops = {
 	.request	= usdhi6_request,
 	.set_ios	= usdhi6_set_ios,
 	.get_cd		= usdhi6_get_cd,
 	.get_ro		= usdhi6_get_ro,
 	.enable_sdio_irq = usdhi6_enable_sdio_irq,
+	.start_signal_voltage_switch = usdhi6_sig_volt_switch,
 };
 
 /*			State machine handlers				*/

@@ -1785,7 +1796,17 @@ static int usdhi6_probe(struct platform_device *pdev)
 
 	mmc->ops = &usdhi6_ops;
 	mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED |
-		MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_DDR50 | MMC_CAP_SDIO_IRQ;
+		     MMC_CAP_SDIO_IRQ;
+
+	/*
+	 * A vqmmc regulator with a 1.8V mode is mandatory to support
+	 * UHS modes.
+	 */
+	if (!of_property_read_bool(dev->of_node, "no-1-8-v") &&
+	    !IS_ERR(mmc->supply.vqmmc)) {
+		mmc->caps |= MMC_CAP_UHS_DDR50 | MMC_CAP_UHS_SDR50;
+	}
+
 	/* Set .max_segs to some random number. Feel free to adjust. */
 	mmc->max_segs = 32;
 	mmc->max_blk_size = 512;
Signature · 1 line
2.1.4
#7 lore raw

Re: [PATCH v4 2/3] mmc: usdhi6rol0: add support for UHS modes

Quoted 44 lines · Lars Persson

Add a start_signal_voltage_switch() operation to support enabling of UHS modes.

Signed-off-by Lars Persson <[email protected]>
---
 drivers/mmc/host/usdhi6rol0.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)
drivers/mmc/host/usdhi6rol0.c +11 -0
index 807c06e..28fe5c7 100644
@@ -21,6 +21,7 @@
 #include <linux/mmc/sd.h>
 #include <linux/mmc/sdio.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/pagemap.h>
 #include <linux/platform_device.h>
 #include <linux/scatterlist.h>

@@ -1147,12 +1148,22 @@ static void usdhi6_enable_sdio_irq(struct mmc_host *mmc, int enable)
        }
 }
 
+static int usdhi6_sig_volt_switch(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+       int ret;
+
+       ret = mmc_regulator_set_vqmmc(mmc, ios);
+
+       return ret;
+}
+
 static struct mmc_host_ops usdhi6_ops = {
        .request        = usdhi6_request,
        .set_ios        = usdhi6_set_ios,
        .get_cd         = usdhi6_get_cd,
        .get_ro         = usdhi6_get_ro,
        .enable_sdio_irq = usdhi6_enable_sdio_irq,
+       .start_signal_voltage_switch = usdhi6_sig_volt_switch,
 };
 
 /*                     State machine handlers                          */

drivers/mmc/host/usdhi6rol0.c static int usdhi6_probe(struct platform_device *pdev) · line 1788

Quoted 4 lines
drivers/mmc/host/usdhi6rol0.c +0 -1
@@ -1785,7 +1796,17 @@ static int usdhi6_probe(struct platform_device *pdev)
 
        mmc->ops = &usdhi6_ops;
        mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED |
-               MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_DDR50 | MMC_CAP_SDIO_IRQ;

Oh, this was weird. The driver already claimed to support UHS mode, but when in fact it didn't. :-)

Perhaps that change deserves its own fixup patch?

drivers/mmc/host/usdhi6rol0.c static int usdhi6_probe(struct platform_device *pdev) · line 1805

Quoted 7 lines
drivers/mmc/host/usdhi6rol0.c +7 -0
+                    MMC_CAP_SDIO_IRQ;
+
+       /*
+        * A vqmmc regulator with a 1.8V mode is mandatory to support
+        * UHS modes.
+        */
+       if (!of_property_read_bool(dev->of_node, "no-1-8-v") &&

No, please don't use this binding.

Instead there is already bindings for MMC_CAP_UHS_DDR50 and MMC_CAP_UHS_SDR50, please use them instead.

Quoted 10 lines
drivers/mmc/host/usdhi6rol0.c +4 -1
+           !IS_ERR(mmc->supply.vqmmc)) {
+               mmc->caps |= MMC_CAP_UHS_DDR50 | MMC_CAP_UHS_SDR50;
+       }
+
        /* Set .max_segs to some random number. Feel free to adjust. */
        mmc->max_segs = 32;
        mmc->max_blk_size = 512;
--

2.1.4

Kind regards
Uffe

maintainer
#8 lore raw

PATCH v4 3/3 mmc: usdhi6rol0: add pinctrl to set pin drive strength

In reply to Lars Persson

Some boards need different pin drive strength for the UHS mode. Add an optional pinctrl setting with two pin states covering UHS speeds and other speeds.

Signed-off-by Lars Persson <[email protected]>
---
 drivers/mmc/host/usdhi6rol0.c | 48 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
drivers/mmc/host/usdhi6rol0.c +48 -0
index 28fe5c7..38c418c 100644
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/pagemap.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
 #include <linux/scatterlist.h>
 #include <linux/string.h>

@@ -199,6 +200,11 @@ struct usdhi6_host {
 	struct dma_chan *chan_rx;
 	struct dma_chan *chan_tx;
 	bool dma_active;
+
+	/* Pin control */
+	struct pinctrl *pinctrl;
+	struct pinctrl_state *pins_default;
+	struct pinctrl_state *pins_uhs;
 };
 
 /*			I/O primitives					*/

@@ -1148,12 +1154,35 @@ static void usdhi6_enable_sdio_irq(struct mmc_host *mmc, int enable)
 	}
 }
 
+static int usdhi6_set_pinstates(struct usdhi6_host *host, int voltage)
+{
+	if (IS_ERR(host->pins_uhs))
+		return 0;
+
+	switch (voltage) {
+	case MMC_SIGNAL_VOLTAGE_180:
+	case MMC_SIGNAL_VOLTAGE_120:
+		return pinctrl_select_state(host->pinctrl,
+					    host->pins_uhs);
+
+	default:
+		return pinctrl_select_state(host->pinctrl,
+					    host->pins_default);
+	}
+}
+
 static int usdhi6_sig_volt_switch(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	int ret;
 
 	ret = mmc_regulator_set_vqmmc(mmc, ios);
+	if (ret < 0)
+		return ret;
 
+	ret = usdhi6_set_pinstates(mmc_priv(mmc), ios->signal_voltage);
+	if (ret)
+		dev_warn_once(mmc_dev(mmc),
+			      "Failed to set pinstate err=%d\n", ret);
 	return ret;
 }
 

@@ -1741,6 +1770,25 @@ static int usdhi6_probe(struct platform_device *pdev)
 	host->wait	= USDHI6_WAIT_FOR_REQUEST;
 	host->timeout	= msecs_to_jiffies(4000);
 
+	host->pinctrl = devm_pinctrl_get(&pdev->dev);
+	if (IS_ERR(host->pinctrl)) {
+		ret = PTR_ERR(host->pinctrl);
+		goto e_free_mmc;
+	}
+
+	host->pins_uhs = pinctrl_lookup_state(host->pinctrl, "state_uhs");
+	if (!IS_ERR(host->pins_uhs)) {
+		host->pins_default = pinctrl_lookup_state(host->pinctrl,
+							  PINCTRL_STATE_DEFAULT);
+
+		if (IS_ERR(host->pins_default)) {
+			dev_err(dev,
+				"UHS pinctrl requires a default pin state.\n");
+			ret = PTR_ERR(host->pins_default);
+			goto e_free_mmc;
+		}
+	}
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	host->base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(host->base)) {
Signature · 1 line
2.1.4