mmc: tegra: use mmc_of_parse to get support of standard MMC DT binding

The GPIO pin of SD slot card detection should active low.

Quoted 1 line · Joseph Lo

The GPIO pin of SD slot card detection should active low.

Thierry, Lucas, can you please check if the same change should be made to all the Avionic Design and Toradex boards? I assume MMC CD should be active low on all of them. Once you've confirmed this, Joseph can respin this first patch, and perhaps we can get it into 3.9/stable as a bug-fix. Thanks.

* PGP Signed by an unknown key

Quoted 8 lines · Stephen Warren

Quoted 1 line · Joseph Lo

The GPIO pin of SD slot card detection should active low.

Thierry, Lucas, can you please check if the same change should be made to all the Avionic Design and Toradex boards? I assume MMC CD should be active low on all of them. Once you've confirmed this, Joseph can respin this first patch, and perhaps we can get it into 3.9/stable as a bug-fix. Thanks.

The card-detect GPIO is certainly low-active on Tamonten. I'm not quite sure why things worked before, but I guess it was due to the MMC core assuming the CD GPIO to be low-active unless otherwise specified and the Tegra DT code didn't pass the GPIO flags to the core.

Anyway, I've verified that on Tamonten the same change doesn't break anything, card detect still works as expected with this patch series applied, so if Joseph could carry the Tamonten change as part of this patch it'd be great.

Updating the sdhci-tegra driver to use mmc_of_parse to support standard MMC DT bindings. Then we can remove the redundant code that already support

static unsigned int tegra_sdhci_get_ro(struct sdhci_host *host) {

drivers/mmc/host/sdhci-tegra.c +0 -0

@@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
 					struct sdhci_tegra *tegra_host)
 {

drivers/mmc/host/sdhci-tegra.c +1 -0

+	struct sdhci_host *host;

Quoted 5 lines · Joseph Lo

Updating the sdhci-tegra driver to use mmc_of_parse to support standard MMC DT bindings. Then we can remove the redundant code that already support

static unsigned int tegra_sdhci_get_ro(struct sdhci_host *host) {

drivers/mmc/host/sdhci-tegra.c static void tegra_sdhci_writel(struct sdhci_host *host, u32 val, int reg) · line 109

Quoted 9 lines

drivers/mmc/host/sdhci-tegra.c +1 -7

-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	struct sdhci_tegra *tegra_host = pltfm_host->priv;
-
-	if (!gpio_is_valid(tegra_host->wp_gpio))
-		return -1;
-
-	return gpio_get_value(tegra_host->wp_gpio);
+	return mmc_gpio_get_ro(host->mmc);
 }

It'd be nice if there was a standard version of this function that could be plugged directly into struct sdhci_ops, so that each individual driver doesn't have to re-invent this wrapper.

Quoted 3 lines

drivers/mmc/host/sdhci-tegra.c +0 -0

@@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
 					struct sdhci_tegra *tegra_host)
 {

...

Quoted 1 line

drivers/mmc/host/sdhci-tegra.c +1 -0

+	struct sdhci_host *host;

...

drivers/mmc/host/sdhci-tegra.c static void sdhci_tegra_parse_dt(struct device *dev, · line 212

Quoted 3 lines

drivers/mmc/host/sdhci-tegra.c +2 -0

+	host = platform_get_drvdata(to_platform_device(dev));
+	mmc_of_parse(host->mmc);
 }

It might be simpler to change the function prototype to simply pass in the host object too.

Quoted 8 lines · Stephen Warren

Quoted 6 lines · Joseph Lo

Updating the sdhci-tegra driver to use mmc_of_parse to support standard MMC DT bindings. Then we can remove the redundant code that already support

drivers/mmc/host/sdhci-tegra.c +0 -0

@@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
 					struct sdhci_tegra *tegra_host)
 {

...

Quoted 8 lines

Quoted 1 line

drivers/mmc/host/sdhci-tegra.c +1 -0

+	struct sdhci_host *host;

...

drivers/mmc/host/sdhci-tegra.c static void sdhci_tegra_parse_dt(struct device *dev, · line 212

Quoted 3 lines

drivers/mmc/host/sdhci-tegra.c +2 -0

+	host = platform_get_drvdata(to_platform_device(dev));
+	mmc_of_parse(host->mmc);
 }

It might be simpler to change the function prototype to simply pass in the host object too.

It's a interface problem that I can't fix now. If sdhci core is going to integrate mmc_of_parse into sdhci_get_of_property and mmc_gpio_get_ro into somethere sdhci_do_get_ro, then we can refine later.

Quoted 18 lines · Joseph Lo

Quoted 16 lines · Stephen Warren

Quoted 6 lines · Joseph Lo

Updating the sdhci-tegra driver to use mmc_of_parse to support standard MMC DT bindings. Then we can remove the redundant code that already support

(quoted diff) +0 -0

@@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
 					struct sdhci_tegra *tegra_host)
 {

...

Quoted 1 line

(quoted diff) +1 -0

+	struct sdhci_host *host;

...

Review

Quoted 3 lines

(quoted diff) +2 -0

+	host = platform_get_drvdata(to_platform_device(dev));
+	mmc_of_parse(host->mmc);
 }

It might be simpler to change the function prototype to simply pass in the host object too.

Quoted 3 lines

It's a interface problem that I can't fix now. If sdhci core is going to integrate mmc_of_parse into sdhci_get_of_property and mmc_gpio_get_ro into somethere sdhci_do_get_ro, then we can refine later.

I meant to change the prototype of sdhci_tegra_parse_dt(). It would be simple to change that function in this patch.

Quoted 19 lines · Stephen Warren

Quoted 18 lines · Joseph Lo

Quoted 16 lines · Stephen Warren

Quoted 6 lines · Joseph Lo

Updating the sdhci-tegra driver to use mmc_of_parse to support standard MMC DT bindings. Then we can remove the redundant code that already support

(quoted diff) +0 -0

@@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
 					struct sdhci_tegra *tegra_host)
 {

...

Quoted 1 line

(quoted diff) +1 -0

+	struct sdhci_host *host;

...

Review

Quoted 3 lines

(quoted diff) +2 -0

+	host = platform_get_drvdata(to_platform_device(dev));
+	mmc_of_parse(host->mmc);
 }

It might be simpler to change the function prototype to simply pass in the host object too.

Quoted 6 lines

Quoted 3 lines

It's a interface problem that I can't fix now. If sdhci core is going to integrate mmc_of_parse into sdhci_get_of_property and mmc_gpio_get_ro into somethere sdhci_do_get_ro, then we can refine later.

I meant to change the prototype of sdhci_tegra_parse_dt(). It would be simple to change that function in this patch.

Hmm. It might not too much different. Which version you prefer?

1. Original version

2.
static void sdhci_tegra_parse_dt(struct device *dev,

(quoted diff) +4 -5

-                                       struct sdhci_tegra *tegra_host)
+                                       struct sdhci_host *host)
 {
        struct device_node *np = dev->of_node;
-       struct sdhci_host *host;
+       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+       struct sdhci_tegra *tegra_host = pltfm_host->priv;
 
        tegra_host->power_gpio = of_get_named_gpio(np, "power-gpios",0);
-
-       host = platform_get_drvdata(to_platform_device(dev));
        mmc_of_parse(host->mmc);
 }

@@ -240,7 +239,7 @@ static int sdhci_tegra_probe(struct platform_device
 *pdev)
        tegra_host->soc_data = soc_data;
        pltfm_host->priv = tegra_host;
 
-       sdhci_tegra_parse_dt(&pdev->dev, tegra_host);
+       sdhci_tegra_parse_dt(&pdev->dev, host);
 

3.

(quoted diff) +5 -6

-static void sdhci_tegra_parse_dt(struct device *dev,
-                                       struct sdhci_tegra *tegra_host)
+static void sdhci_tegra_parse_dt(struct platform_device *pdev)
 {
-       struct device_node *np = dev->of_node;
-       struct sdhci_host *host;
+       struct device_node *np = pdev->dev.of_node;
+       struct sdhci_host *host = platform_get_drvdata(pdev);
+       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+       struct sdhci_tegra *tegra_host = pltfm_host->priv;
 
        tegra_host->power_gpio = of_get_named_gpio(np, "power-gpios",0);
-
-       host = platform_get_drvdata(to_platform_device(dev));
        mmc_of_parse(host->mmc);
 }

Quoted 19 lines · Stephen Warren

Quoted 18 lines · Joseph Lo

Quoted 16 lines · Stephen Warren

Quoted 6 lines · Joseph Lo

Updating the sdhci-tegra driver to use mmc_of_parse to support standard MMC DT bindings. Then we can remove the redundant code that already support

(quoted diff) +0 -0

@@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
 					struct sdhci_tegra *tegra_host)
 {

...

Quoted 1 line

(quoted diff) +1 -0

+	struct sdhci_host *host;

...

Review

Quoted 3 lines

(quoted diff) +2 -0

+	host = platform_get_drvdata(to_platform_device(dev));
+	mmc_of_parse(host->mmc);
 }

It might be simpler to change the function prototype to simply pass in the host object too.

Quoted 6 lines

Quoted 3 lines

It's a interface problem that I can't fix now. If sdhci core is going to integrate mmc_of_parse into sdhci_get_of_property and mmc_gpio_get_ro into somethere sdhci_do_get_ro, then we can refine later.

I meant to change the prototype of sdhci_tegra_parse_dt(). It would be simple to change that function in this patch.

Hmm. It might not too much different. Which version you prefer?

1. Original version

2.
static void sdhci_tegra_parse_dt(struct device *dev,

(quoted diff) +3 -4

-                                       struct sdhci_tegra *tegra_host)
+                                       struct sdhci_host *host)
 {
        struct device_node *np = dev->of_node;
-       struct sdhci_host *host;
+       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+       struct sdhci_tegra *tegra_host = pltfm_host->priv;
 
        tegra_host->power_gpio = of_get_named_gpio(np, "power-gpios",0);
-
-       host = platform_get_drvdata(to_platform_device(dev));
        mmc_of_parse(host->mmc);
 }

* PGP Signed by an unknown key

Quoted 48 lines · Joseph Lo

Quoted 19 lines · Stephen Warren

Quoted 18 lines · Joseph Lo

Quoted 16 lines · Stephen Warren

Quoted 6 lines · Joseph Lo

Updating the sdhci-tegra driver to use mmc_of_parse to support standard MMC DT bindings. Then we can remove the redundant code that already support

(quoted diff) +0 -0

@@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
 					struct sdhci_tegra *tegra_host)
 {

...

Quoted 1 line

(quoted diff) +1 -0

+	struct sdhci_host *host;

...

Review

Quoted 3 lines

(quoted diff) +2 -0

+	host = platform_get_drvdata(to_platform_device(dev));
+	mmc_of_parse(host->mmc);
 }

It might be simpler to change the function prototype to simply pass in the host object too.

Quoted 6 lines

Quoted 3 lines

It's a interface problem that I can't fix now. If sdhci core is going to integrate mmc_of_parse into sdhci_get_of_property and mmc_gpio_get_ro into somethere sdhci_do_get_ro, then we can refine later.

I meant to change the prototype of sdhci_tegra_parse_dt(). It would be simple to change that function in this patch.

Hmm. It might not too much different. Which version you prefer?

1. Original version

2.
static void sdhci_tegra_parse_dt(struct device *dev,

(quoted diff) +3 -4

-                                       struct sdhci_tegra *tegra_host)
+                                       struct sdhci_host *host)
 {
        struct device_node *np = dev->of_node;
-       struct sdhci_host *host;
+       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+       struct sdhci_tegra *tegra_host = pltfm_host->priv;
 
        tegra_host->power_gpio = of_get_named_gpio(np, "power-gpios",0);
-
-       host = platform_get_drvdata(to_platform_device(dev));
        mmc_of_parse(host->mmc);
 }

Review static int sdhci_tegra_probe(struct platform_device · line 244

Quoted 7 lines

(quoted diff) +1 -1

@@ -240,7 +239,7 @@ static int sdhci_tegra_probe(struct platform_device
 *pdev)
        tegra_host->soc_data = soc_data;
        pltfm_host->priv = tegra_host;
 
-       sdhci_tegra_parse_dt(&pdev->dev, tegra_host);
+       sdhci_tegra_parse_dt(&pdev->dev, host);
 

I personally prefer this second version. It keeps with the tradition to pass a struct device into the *_parse_dt() and also provides easy access to the sdhci_host.

And on a side-note:

	host = platform_get_drvdata(to_platform_device(dev));

can be more simply written as:

	host = dev_get_drvdata(dev);

Quoted 23 lines · Stephen Warren

Quoted 21 lines · Joseph Lo

Quoted 8 lines · Stephen Warren

Quoted 6 lines · Joseph Lo

Updating the sdhci-tegra driver to use mmc_of_parse to support standard MMC DT bindings. Then we can remove the redundant code that already support

(quoted diff) +0 -0

@@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
 					struct sdhci_tegra *tegra_host)
 {

...

Quoted 8 lines

Quoted 1 line

(quoted diff) +1 -0

+	struct sdhci_host *host;

...

Review

Quoted 3 lines

(quoted diff) +2 -0

+	host = platform_get_drvdata(to_platform_device(dev));
+	mmc_of_parse(host->mmc);
 }

It might be simpler to change the function prototype to simply pass in the host object too.

It's a interface problem that I can't fix now. If sdhci core is going to integrate mmc_of_parse into sdhci_get_of_property and mmc_gpio_get_ro into somethere sdhci_do_get_ro, then we can refine later.

Quoted 2 lines

I meant to change the prototype of sdhci_tegra_parse_dt(). It would be simple to change that function in this patch.

Hmm. It might not too much different. Which version you prefer?

1. Original version

2.

static void sdhci_tegra_parse_dt(struct device *dev,

(quoted diff) +3 -4

-                                       struct sdhci_tegra *tegra_host)
+                                       struct sdhci_host *host)
 {
        struct device_node *np = dev->of_node;
-       struct sdhci_host *host;
+       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+       struct sdhci_tegra *tegra_host = pltfm_host->priv;
 
        tegra_host->power_gpio = of_get_named_gpio(np, "power-gpios",0);
-
-       host = platform_get_drvdata(to_platform_device(dev));
        mmc_of_parse(host->mmc);
 }

Updating the sdhci-tegra driver to use mmc_of_parse to support standard MMC DT bindings. Then we can remove the redundant code that already support in generic MMC core.

Signed-off-by Joseph Lo <[email protected]>

---
 drivers/mmc/host/sdhci-tegra.c | 85 ++++--------------------------------------
 1 file changed, 7 insertions(+), 78 deletions(-)

drivers/mmc/host/sdhci-tegra.c +2 -18

index 08b06e9..4f6b5c4 100644
@@ -24,6 +24,7 @@
 #include <linux/gpio.h>
 #include <linux/mmc/card.h>
 #include <linux/mmc/host.h>
+#include <linux/mmc/slot-gpio.h>
 
 #include <asm/gpio.h>
 

@@ -44,10 +45,7 @@ struct sdhci_tegra_soc_data {
 
 struct sdhci_tegra {
 	const struct sdhci_tegra_soc_data *soc_data;
-	int cd_gpio;
-	int wp_gpio;
 	int power_gpio;
-	int is_8bit;
 };
 
 static u32 tegra_sdhci_readl(struct sdhci_host *host, int reg)

@@ -107,23 +105,9 @@ static void tegra_sdhci_writel(struct sdhci_host *host, u32 val, int reg)
 
 static unsigned int tegra_sdhci_get_ro(struct sdhci_host *host)
 {
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	struct sdhci_tegra *tegra_host = pltfm_host->priv;
-
-	if (!gpio_is_valid(tegra_host->wp_gpio))
-		return -1;
-
-	return gpio_get_value(tegra_host->wp_gpio);
+	return mmc_gpio_get_ro(host->mmc);
 }
 
-static irqreturn_t carddetect_irq(int irq, void *data)
-{
-	struct sdhci_host *sdhost = (struct sdhci_host *)data;
-
-	tasklet_schedule(&sdhost->card_tasklet);
-	return IRQ_HANDLED;
-};
-
 static void tegra_sdhci_reset_exit(struct sdhci_host *host, u8 mask)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);

drivers/mmc/host/sdhci-tegra.c +4 -57

+	    bus_width == MMC_BUS_WIDTH_8) {
 		ctrl &= ~SDHCI_CTRL_4BITBUS;
 		ctrl |= SDHCI_CTRL_8BITBUS;
 	} else {

@@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
 					struct sdhci_tegra *tegra_host)
 {
 	struct device_node *np = dev->of_node;
-	u32 bus_width;
+	struct sdhci_host *host;
 
-	tegra_host->cd_gpio = of_get_named_gpio(np, "cd-gpios", 0);
-	tegra_host->wp_gpio = of_get_named_gpio(np, "wp-gpios", 0);
 	tegra_host->power_gpio = of_get_named_gpio(np, "power-gpios", 0);
 
-	if (of_property_read_u32(np, "bus-width", &bus_width) == 0 &&
-	    bus_width == 8)
-		tegra_host->is_8bit = 1;
+	host = platform_get_drvdata(to_platform_device(dev));
+	mmc_of_parse(host->mmc);
 }
 
 static int sdhci_tegra_probe(struct platform_device *pdev)

@@ -272,37 +252,6 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
 		gpio_direction_output(tegra_host->power_gpio, 1);
 	}
 
-	if (gpio_is_valid(tegra_host->cd_gpio)) {
-		rc = gpio_request(tegra_host->cd_gpio, "sdhci_cd");
-		if (rc) {
-			dev_err(mmc_dev(host->mmc),
-				"failed to allocate cd gpio\n");
-			goto err_cd_req;
-		}
-		gpio_direction_input(tegra_host->cd_gpio);
-
-		rc = request_irq(gpio_to_irq(tegra_host->cd_gpio),
-				 carddetect_irq,
-				 IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
-				 mmc_hostname(host->mmc), host);
-
-		if (rc)	{
-			dev_err(mmc_dev(host->mmc), "request irq error\n");
-			goto err_cd_irq_req;
-		}
-
-	}
-
-	if (gpio_is_valid(tegra_host->wp_gpio)) {
-		rc = gpio_request(tegra_host->wp_gpio, "sdhci_wp");
-		if (rc) {
-			dev_err(mmc_dev(host->mmc),
-				"failed to allocate wp gpio\n");
-			goto err_wp_req;
-		}
-		gpio_direction_input(tegra_host->wp_gpio);
-	}
-
 	clk = clk_get(mmc_dev(host->mmc), NULL);
 	if (IS_ERR(clk)) {
 		dev_err(mmc_dev(host->mmc), "clk err\n");

@@ -312,9 +261,6 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
 	clk_prepare_enable(clk);
 	pltfm_host->clk = clk;
 
-	if (tegra_host->is_8bit)
-		host->mmc->caps |= MMC_CAP_8_BIT_DATA;
-
 	rc = sdhci_add_host(host);
 	if (rc)
 		goto err_add_host;

@@ -325,15 +271,6 @@ err_add_host:
 	clk_disable_unprepare(pltfm_host->clk);
 	clk_put(pltfm_host->clk);
 err_clk_get:
-	if (gpio_is_valid(tegra_host->wp_gpio))
-		gpio_free(tegra_host->wp_gpio);
-err_wp_req:
-	if (gpio_is_valid(tegra_host->cd_gpio))
-		free_irq(gpio_to_irq(tegra_host->cd_gpio), host);
-err_cd_irq_req:
-	if (gpio_is_valid(tegra_host->cd_gpio))
-		gpio_free(tegra_host->cd_gpio);
-err_cd_req:
 	if (gpio_is_valid(tegra_host->power_gpio))
 		gpio_free(tegra_host->power_gpio);
 err_power_req:

@@ -351,14 +288,6 @@ static int sdhci_tegra_remove(struct platform_device *pdev)
 
 	sdhci_remove_host(host, dead);
 
-	if (gpio_is_valid(tegra_host->wp_gpio))
-		gpio_free(tegra_host->wp_gpio);
-
-	if (gpio_is_valid(tegra_host->cd_gpio)) {
-		free_irq(gpio_to_irq(tegra_host->cd_gpio), host);
-		gpio_free(tegra_host->cd_gpio);
-	}
-
 	if (gpio_is_valid(tegra_host->power_gpio))
 		gpio_free(tegra_host->power_gpio);
 

Signature · 6 lines

1.8.1.1

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

Hi Joseph

drivers/mmc/host/sdhci-tegra.c static void tegra_sdhci_reset_exit(struct sdhci_host *host, u8 mask) · line 135

Quoted 9 lines · Joseph Lo

static int tegra_sdhci_buswidth(struct sdhci_host *host, int bus_width) {

drivers/mmc/host/sdhci-tegra.c +1 -3

-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	struct sdhci_tegra *tegra_host = pltfm_host->priv;
 	u32 ctrl;
 
 	ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
-	if (tegra_host->is_8bit && bus_width == MMC_BUS_WIDTH_8) {
+	if (host->mmc->caps == MMC_CAP_8_BIT_DATA &&

Don't you want to check for just one bit? -Šcaps can contain multiple flags set:

The 2nd patch of this series depends on two patches below.

"mmc: provide a standard MMC device-tree binding parser centrally" (available on next-20130219)
mmc: tegra: assume CONFIG_OF, remove platform data
(https://patchwork.kernel.org/patch/2150351/)

Verified on Seaboard and Cardhu.

Joseph Lo (2):
ARM: dts: tegra: fix the activate polarity of cd-gpio in mmc host
mmc: tegra: use mmc_of_parse to get the support of standard MMC DT
bindings

 arch/arm/boot/dts/tegra20-harmony.dts   |  4 +-
 arch/arm/boot/dts/tegra20-paz00.dts     |  2 +-
 arch/arm/boot/dts/tegra20-seaboard.dts  |  2 +-
 arch/arm/boot/dts/tegra20-trimslice.dts |  2 +-
 arch/arm/boot/dts/tegra20-ventana.dts   |  2 +-
 arch/arm/boot/dts/tegra20-whistler.dts  |  1 +
 arch/arm/boot/dts/tegra30-beaver.dts    |  2 +-
 arch/arm/boot/dts/tegra30-cardhu.dtsi   |  2 +-
 drivers/mmc/host/sdhci-tegra.c          | 85 +++------------------------------
 9 files changed, 16 insertions(+), 86 deletions(-)