linux-tegra.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] mmc: tegra: use mmc_of_parse to get support of standard MMC DT binding
@ 2013-02-20  7:05 Joseph Lo
  2013-02-20  7:05 ` [PATCH 1/2] ARM: dts: tegra: fix the activate polarity of cd-gpio in mmc host Joseph Lo
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Joseph Lo @ 2013-02-20  7:05 UTC (permalink / raw)
  To: Chris Ball, Stephen Warren
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA, Joseph Lo

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(-)

-- 
1.8.1.1

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/2] ARM: dts: tegra: fix the activate polarity of cd-gpio in mmc host
  2013-02-20  7:05 [PATCH 0/2] mmc: tegra: use mmc_of_parse to get support of standard MMC DT binding Joseph Lo
@ 2013-02-20  7:05 ` Joseph Lo
       [not found]   ` <1361343902-15223-2-git-send-email-josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  2013-02-20  7:05 ` [PATCH 2/2] mmc: tegra: use mmc_of_parse to get the support of standard MMC DT bindings Joseph Lo
       [not found] ` <1361343902-15223-1-git-send-email-josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  2 siblings, 1 reply; 17+ messages in thread
From: Joseph Lo @ 2013-02-20  7:05 UTC (permalink / raw)
  To: Chris Ball, Stephen Warren; +Cc: linux-tegra, linux-mmc, Joseph Lo

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

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
 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 +-
 8 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/tegra20-harmony.dts b/arch/arm/boot/dts/tegra20-harmony.dts
index 61d027f..1f79c0d 100644
--- a/arch/arm/boot/dts/tegra20-harmony.dts
+++ b/arch/arm/boot/dts/tegra20-harmony.dts
@@ -437,7 +437,7 @@
 
 	sdhci@c8000200 {
 		status = "okay";
-		cd-gpios = <&gpio 69 0>; /* gpio PI5 */
+		cd-gpios = <&gpio 69 1>; /* gpio PI5 */
 		wp-gpios = <&gpio 57 0>; /* gpio PH1 */
 		power-gpios = <&gpio 155 0>; /* gpio PT3 */
 		bus-width = <4>;
@@ -445,7 +445,7 @@
 
 	sdhci@c8000600 {
 		status = "okay";
-		cd-gpios = <&gpio 58 0>; /* gpio PH2 */
+		cd-gpios = <&gpio 58 1>; /* gpio PH2 */
 		wp-gpios = <&gpio 59 0>; /* gpio PH3 */
 		power-gpios = <&gpio 70 0>; /* gpio PI6 */
 		bus-width = <8>;
diff --git a/arch/arm/boot/dts/tegra20-paz00.dts b/arch/arm/boot/dts/tegra20-paz00.dts
index 54d6fce..9db36da 100644
--- a/arch/arm/boot/dts/tegra20-paz00.dts
+++ b/arch/arm/boot/dts/tegra20-paz00.dts
@@ -436,7 +436,7 @@
 
 	sdhci@c8000000 {
 		status = "okay";
-		cd-gpios = <&gpio 173 0>; /* gpio PV5 */
+		cd-gpios = <&gpio 173 1>; /* gpio PV5 */
 		wp-gpios = <&gpio 57 0>;  /* gpio PH1 */
 		power-gpios = <&gpio 169 0>; /* gpio PV1 */
 		bus-width = <4>;
diff --git a/arch/arm/boot/dts/tegra20-seaboard.dts b/arch/arm/boot/dts/tegra20-seaboard.dts
index 37b3a57..715a8b8 100644
--- a/arch/arm/boot/dts/tegra20-seaboard.dts
+++ b/arch/arm/boot/dts/tegra20-seaboard.dts
@@ -584,7 +584,7 @@
 
 	sdhci@c8000400 {
 		status = "okay";
-		cd-gpios = <&gpio 69 0>; /* gpio PI5 */
+		cd-gpios = <&gpio 69 1>; /* gpio PI5 */
 		wp-gpios = <&gpio 57 0>; /* gpio PH1 */
 		power-gpios = <&gpio 70 0>; /* gpio PI6 */
 		bus-width = <4>;
diff --git a/arch/arm/boot/dts/tegra20-trimslice.dts b/arch/arm/boot/dts/tegra20-trimslice.dts
index 5d79e4f..98f3e44 100644
--- a/arch/arm/boot/dts/tegra20-trimslice.dts
+++ b/arch/arm/boot/dts/tegra20-trimslice.dts
@@ -325,7 +325,7 @@
 
 	sdhci@c8000600 {
 		status = "okay";
-		cd-gpios = <&gpio 121 0>; /* gpio PP1 */
+		cd-gpios = <&gpio 121 1>; /* gpio PP1 */
 		wp-gpios = <&gpio 122 0>; /* gpio PP2 */
 		bus-width = <4>;
 	};
diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts
index 425c890..4aef56f 100644
--- a/arch/arm/boot/dts/tegra20-ventana.dts
+++ b/arch/arm/boot/dts/tegra20-ventana.dts
@@ -520,7 +520,7 @@
 
 	sdhci@c8000400 {
 		status = "okay";
-		cd-gpios = <&gpio 69 0>; /* gpio PI5 */
+		cd-gpios = <&gpio 69 1>; /* gpio PI5 */
 		wp-gpios = <&gpio 57 0>; /* gpio PH1 */
 		power-gpios = <&gpio 70 0>; /* gpio PI6 */
 		bus-width = <4>;
diff --git a/arch/arm/boot/dts/tegra20-whistler.dts b/arch/arm/boot/dts/tegra20-whistler.dts
index ea57c0f..5762188 100644
--- a/arch/arm/boot/dts/tegra20-whistler.dts
+++ b/arch/arm/boot/dts/tegra20-whistler.dts
@@ -510,6 +510,7 @@
 
 	sdhci@c8000400 {
 		status = "okay";
+		cd-gpios = <&gpio 69 1>; /* gpio PI5 */
 		wp-gpios = <&gpio 173 0>; /* gpio PV5 */
 		bus-width = <8>;
 	};
diff --git a/arch/arm/boot/dts/tegra30-beaver.dts b/arch/arm/boot/dts/tegra30-beaver.dts
index 8ff2ff2..0a2cd24 100644
--- a/arch/arm/boot/dts/tegra30-beaver.dts
+++ b/arch/arm/boot/dts/tegra30-beaver.dts
@@ -257,7 +257,7 @@
 
 	sdhci@78000000 {
 		status = "okay";
-		cd-gpios = <&gpio 69 0>; /* gpio PI5 */
+		cd-gpios = <&gpio 69 1>; /* gpio PI5 */
 		wp-gpios = <&gpio 155 0>; /* gpio PT3 */
 		power-gpios = <&gpio 31 0>; /* gpio PD7 */
 		bus-width = <4>;
diff --git a/arch/arm/boot/dts/tegra30-cardhu.dtsi b/arch/arm/boot/dts/tegra30-cardhu.dtsi
index 1749927..3e2d210 100644
--- a/arch/arm/boot/dts/tegra30-cardhu.dtsi
+++ b/arch/arm/boot/dts/tegra30-cardhu.dtsi
@@ -311,7 +311,7 @@
 
 	sdhci@78000000 {
 		status = "okay";
-		cd-gpios = <&gpio 69 0>; /* gpio PI5 */
+		cd-gpios = <&gpio 69 1>; /* gpio PI5 */
 		wp-gpios = <&gpio 155 0>; /* gpio PT3 */
 		power-gpios = <&gpio 31 0>; /* gpio PD7 */
 		bus-width = <4>;
-- 
1.8.1.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/2] mmc: tegra: use mmc_of_parse to get the support of standard MMC DT bindings
  2013-02-20  7:05 [PATCH 0/2] mmc: tegra: use mmc_of_parse to get support of standard MMC DT binding Joseph Lo
  2013-02-20  7:05 ` [PATCH 1/2] ARM: dts: tegra: fix the activate polarity of cd-gpio in mmc host Joseph Lo
@ 2013-02-20  7:05 ` Joseph Lo
       [not found]   ` <1361343902-15223-3-git-send-email-josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
       [not found] ` <1361343902-15223-1-git-send-email-josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  2 siblings, 1 reply; 17+ messages in thread
From: Joseph Lo @ 2013-02-20  7:05 UTC (permalink / raw)
  To: Chris Ball, Stephen Warren; +Cc: linux-tegra, linux-mmc, 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
in generic MMC core.

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 85 ++++--------------------------------------
 1 file changed, 7 insertions(+), 78 deletions(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 08b06e9..4f6b5c4 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -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);
@@ -145,12 +129,11 @@ static void tegra_sdhci_reset_exit(struct sdhci_host *host, u8 mask)
 
 static int tegra_sdhci_buswidth(struct sdhci_host *host, int bus_width)
 {
-	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 &&
+	    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);
 
-- 
1.8.1.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] ARM: dts: tegra: fix the activate polarity of cd-gpio in mmc host
       [not found]   ` <1361343902-15223-2-git-send-email-josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2013-02-20 16:59     ` Stephen Warren
       [not found]       ` <512500EB.1070802-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Stephen Warren @ 2013-02-20 16:59 UTC (permalink / raw)
  To: Joseph Lo, Thierry Reding, Lucas Stach
  Cc: Chris Ball, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA

On 02/20/2013 12:05 AM, Joseph Lo wrote:
> 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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] mmc: tegra: use mmc_of_parse to get the support of standard MMC DT bindings
       [not found]   ` <1361343902-15223-3-git-send-email-josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2013-02-20 17:07     ` Stephen Warren
       [not found]       ` <512502D7.4090605-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  2013-02-21 22:44     ` Guennadi Liakhovetski
  1 sibling, 1 reply; 17+ messages in thread
From: Stephen Warren @ 2013-02-20 17:07 UTC (permalink / raw)
  To: Joseph Lo
  Cc: Chris Ball, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA

On 02/20/2013 12:05 AM, Joseph Lo wrote:
> 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)
>  {
> -	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.

> @@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
>  					struct sdhci_tegra *tegra_host)
>  {
...
> +	struct sdhci_host *host;
...
> +	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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] mmc: tegra: use mmc_of_parse to get the support of standard MMC DT bindings
       [not found]       ` <512502D7.4090605-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2013-02-21  2:23         ` Joseph Lo
       [not found]           ` <1361413425.5678.8.camel-yx3yKKdKkHfc7b1ADBJPm0n48jw8i0AO@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Joseph Lo @ 2013-02-21  2:23 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Chris Ball, linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

On Thu, 2013-02-21 at 01:07 +0800, Stephen Warren wrote:
> On 02/20/2013 12:05 AM, Joseph Lo wrote:
> > 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)
> >  {
> > -	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.
> 
> > @@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
> >  					struct sdhci_tegra *tegra_host)
> >  {
> ...
> > +	struct sdhci_host *host;
> ...
> > +	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.

Thanks,
Joseph

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] mmc: tegra: use mmc_of_parse to get the support of standard MMC DT bindings
       [not found]           ` <1361413425.5678.8.camel-yx3yKKdKkHfc7b1ADBJPm0n48jw8i0AO@public.gmane.org>
@ 2013-02-21  4:20             ` Stephen Warren
       [not found]               ` <5125A075.5000000-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Stephen Warren @ 2013-02-21  4:20 UTC (permalink / raw)
  To: Joseph Lo
  Cc: Chris Ball, linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

On 02/20/2013 07:23 PM, Joseph Lo wrote:
> On Thu, 2013-02-21 at 01:07 +0800, Stephen Warren wrote:
>> On 02/20/2013 12:05 AM, Joseph Lo wrote:
>>> 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

>>> @@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
>>>  					struct sdhci_tegra *tegra_host)
>>>  {
>> ...
>>> +	struct sdhci_host *host;
>> ...
>>> +	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.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] mmc: tegra: use mmc_of_parse to get the support of standard MMC DT bindings
       [not found]               ` <5125A075.5000000-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2013-02-21  6:26                 ` Joseph Lo
  2013-02-21  6:34                   ` Joseph Lo
                                     ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Joseph Lo @ 2013-02-21  6:26 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Chris Ball, linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

On Thu, 2013-02-21 at 12:20 +0800, Stephen Warren wrote:
> On 02/20/2013 07:23 PM, Joseph Lo wrote:
> > On Thu, 2013-02-21 at 01:07 +0800, Stephen Warren wrote:
> >> On 02/20/2013 12:05 AM, Joseph Lo wrote:
> >>> 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
> 
> >>> @@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
> >>>  					struct sdhci_tegra *tegra_host)
> >>>  {
> >> ...
> >>> +	struct sdhci_host *host;
> >> ...
> >>> +	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.
> 
> 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,
-                                       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.
-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);
 }
 
@@ -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);

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] mmc: tegra: use mmc_of_parse to get the support of standard MMC DT bindings
  2013-02-21  6:26                 ` Joseph Lo
@ 2013-02-21  6:34                   ` Joseph Lo
  2013-02-21  8:03                   ` Thierry Reding
       [not found]                   ` <1361427997.5678.59.camel-yx3yKKdKkHfc7b1ADBJPm0n48jw8i0AO@public.gmane.org>
  2 siblings, 0 replies; 17+ messages in thread
From: Joseph Lo @ 2013-02-21  6:34 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Chris Ball, linux-tegra@vger.kernel.org,
	linux-mmc@vger.kernel.org

On Thu, 2013-02-21 at 14:26 +0800, Joseph Lo wrote:
> On Thu, 2013-02-21 at 12:20 +0800, Stephen Warren wrote:
> > On 02/20/2013 07:23 PM, Joseph Lo wrote:
> > > On Thu, 2013-02-21 at 01:07 +0800, Stephen Warren wrote:
> > >> On 02/20/2013 12:05 AM, Joseph Lo wrote:
> > >>> 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
> > 
> > >>> @@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
> > >>>  					struct sdhci_tegra *tegra_host)
> > >>>  {
> > >> ...
> > >>> +	struct sdhci_host *host;
> > >> ...
> > >>> +	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.
> > 
> > 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,
> -                                       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.
> -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);
>  }
>  
> @@ -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);
> 

or
4. This would be simpler.
@@ -203,12 +203,8 @@ static void sdhci_tegra_parse_dt(struct device
*dev,
                                        struct sdhci_tegra *tegra_host)
 {
        struct device_node *np = dev->of_node;
-       struct sdhci_host *host;
 
        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);
 }
 
 static int sdhci_tegra_probe(struct platform_device *pdev)
@@ -241,6 +237,7 @@ static int sdhci_tegra_probe(struct platform_device
*pdev)
        pltfm_host->priv = tegra_host;
 
        sdhci_tegra_parse_dt(&pdev->dev, tegra_host);
+       mmc_of_parse(host->mmc);



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] mmc: tegra: use mmc_of_parse to get the support of standard MMC DT bindings
  2013-02-21  6:26                 ` Joseph Lo
  2013-02-21  6:34                   ` Joseph Lo
@ 2013-02-21  8:03                   ` Thierry Reding
       [not found]                     ` <20130221080306.GA7718-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
       [not found]                   ` <1361427997.5678.59.camel-yx3yKKdKkHfc7b1ADBJPm0n48jw8i0AO@public.gmane.org>
  2 siblings, 1 reply; 17+ messages in thread
From: Thierry Reding @ 2013-02-21  8:03 UTC (permalink / raw)
  To: Joseph Lo
  Cc: Stephen Warren, Chris Ball, linux-tegra@vger.kernel.org,
	linux-mmc@vger.kernel.org

[-- Attachment #1: Type: text/plain, Size: 2590 bytes --]

On Thu, Feb 21, 2013 at 02:26:37PM +0800, Joseph Lo wrote:
> On Thu, 2013-02-21 at 12:20 +0800, Stephen Warren wrote:
> > On 02/20/2013 07:23 PM, Joseph Lo wrote:
> > > On Thu, 2013-02-21 at 01:07 +0800, Stephen Warren wrote:
> > >> On 02/20/2013 12:05 AM, Joseph Lo wrote:
> > >>> 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
> > 
> > >>> @@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
> > >>>  					struct sdhci_tegra *tegra_host)
> > >>>  {
> > >> ...
> > >>> +	struct sdhci_host *host;
> > >> ...
> > >>> +	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.
> > 
> > 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,
> -                                       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);
>  

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);

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] mmc: tegra: use mmc_of_parse to get the support of standard MMC DT bindings
       [not found]                     ` <20130221080306.GA7718-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
@ 2013-02-21  8:15                       ` Joseph Lo
  0 siblings, 0 replies; 17+ messages in thread
From: Joseph Lo @ 2013-02-21  8:15 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Stephen Warren, Chris Ball,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

On Thu, 2013-02-21 at 16:03 +0800, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Thu, Feb 21, 2013 at 02:26:37PM +0800, Joseph Lo wrote:
> > On Thu, 2013-02-21 at 12:20 +0800, Stephen Warren wrote:
> > > On 02/20/2013 07:23 PM, Joseph Lo wrote:
> > > > On Thu, 2013-02-21 at 01:07 +0800, Stephen Warren wrote:
> > > >> On 02/20/2013 12:05 AM, Joseph Lo wrote:
> > > >>> 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
> > > 
> > > >>> @@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
> > > >>>  					struct sdhci_tegra *tegra_host)
> > > >>>  {
> > > >> ...
> > > >>> +	struct sdhci_host *host;
> > > >> ...
> > > >>> +	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.
> > > 
> > > 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,
> > -                                       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);
> >  
> 
> 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);
> 
Indeed, it's simpler and works. Thanks for sharing. :)

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] ARM: dts: tegra: fix the activate polarity of cd-gpio in mmc host
       [not found]       ` <512500EB.1070802-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2013-02-21  8:18         ` Thierry Reding
       [not found]           ` <20130221081809.GB7718-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Thierry Reding @ 2013-02-21  8:18 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Joseph Lo, Lucas Stach, Chris Ball,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 1010 bytes --]

On Wed, Feb 20, 2013 at 09:59:23AM -0700, Stephen Warren wrote:
> On 02/20/2013 12:05 AM, Joseph Lo wrote:
> > 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.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/2] mmc: tegra: use mmc_of_parse to get support of standard MMC DT binding
       [not found] ` <1361343902-15223-1-git-send-email-josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2013-02-21  8:24   ` Thierry Reding
  0 siblings, 0 replies; 17+ messages in thread
From: Thierry Reding @ 2013-02-21  8:24 UTC (permalink / raw)
  To: Joseph Lo
  Cc: Chris Ball, Stephen Warren, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 1259 bytes --]

On Wed, Feb 20, 2013 at 03:05:00PM +0800, Joseph Lo wrote:
> 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(-)

With the same DTS change for Tamonten, the series:

Tested-by: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] ARM: dts: tegra: fix the activate polarity of cd-gpio in mmc host
       [not found]           ` <20130221081809.GB7718-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
@ 2013-02-21  8:30             ` Joseph Lo
  0 siblings, 0 replies; 17+ messages in thread
From: Joseph Lo @ 2013-02-21  8:30 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Stephen Warren, Lucas Stach, Chris Ball,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

Hi Thierry,

Thanks for your verification.

On Thu, 2013-02-21 at 16:18 +0800, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Wed, Feb 20, 2013 at 09:59:23AM -0700, Stephen Warren wrote:
> > On 02/20/2013 12:05 AM, Joseph Lo wrote:
> > > 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.
> 
Yes. And the original sdhci-tegra driver didn't use the GPIO flag to
config anything. After we use mmc_of_parse, it will be taken care in the
core. So both of the cp-gpio and wp-gpio should be config right in DT.

> 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.

Thanks. Will do.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] mmc: tegra: use mmc_of_parse to get the support of standard MMC DT bindings
       [not found]                   ` <1361427997.5678.59.camel-yx3yKKdKkHfc7b1ADBJPm0n48jw8i0AO@public.gmane.org>
@ 2013-02-21 18:50                     ` Stephen Warren
  0 siblings, 0 replies; 17+ messages in thread
From: Stephen Warren @ 2013-02-21 18:50 UTC (permalink / raw)
  To: Joseph Lo
  Cc: Chris Ball, linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

On 02/20/2013 11:26 PM, Joseph Lo wrote:
> On Thu, 2013-02-21 at 12:20 +0800, Stephen Warren wrote:
>> On 02/20/2013 07:23 PM, Joseph Lo wrote:
>>> On Thu, 2013-02-21 at 01:07 +0800, Stephen Warren wrote:
>>>> On 02/20/2013 12:05 AM, Joseph Lo wrote:
>>>>> 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
>>
>>>>> @@ -220,15 +203,12 @@ static void sdhci_tegra_parse_dt(struct device *dev,
>>>>>  					struct sdhci_tegra *tegra_host)
>>>>>  {
>>>> ...
>>>>> +	struct sdhci_host *host;
>>>> ...
>>>>> +	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.
>>
>> 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.

sdhci_tegra_probe() already has all the values you need; I was thinking
of just passing in all of the values that this function needs, rather
than deriving anything within the function.

>  static void sdhci_tegra_parse_dt(struct device *dev,
> -                                       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);

That seems fairly direct though. Is "dev" available inside host,
platfm_host or tegra_host? You could obtain it from there if you wanted
to reduce the number of parameters passed to the function.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] mmc: tegra: use mmc_of_parse to get the support of standard MMC DT bindings
       [not found]   ` <1361343902-15223-3-git-send-email-josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  2013-02-20 17:07     ` Stephen Warren
@ 2013-02-21 22:44     ` Guennadi Liakhovetski
       [not found]       ` <Pine.LNX.4.64.1302212333330.10403-0199iw4Nj15frtckUFj5Ag@public.gmane.org>
  1 sibling, 1 reply; 17+ messages in thread
From: Guennadi Liakhovetski @ 2013-02-21 22:44 UTC (permalink / raw)
  To: Joseph Lo
  Cc: Chris Ball, Stephen Warren, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA

Hi Joseph

On Wed, 20 Feb 2013, Joseph Lo wrote:

> 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 <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/mmc/host/sdhci-tegra.c | 85 ++++--------------------------------------
>  1 file changed, 7 insertions(+), 78 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 08b06e9..4f6b5c4 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -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);
> @@ -145,12 +129,11 @@ static void tegra_sdhci_reset_exit(struct sdhci_host *host, u8 mask)
>  
>  static int tegra_sdhci_buswidth(struct sdhci_host *host, int bus_width)
>  {
> -	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:

+	if ((host->mmc->caps & MMC_CAP_8_BIT_DATA) &&

Thanks
Guennadi

> +	    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);
>  
> -- 
> 1.8.1.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] mmc: tegra: use mmc_of_parse to get the support of standard MMC DT bindings
       [not found]       ` <Pine.LNX.4.64.1302212333330.10403-0199iw4Nj15frtckUFj5Ag@public.gmane.org>
@ 2013-02-22  2:07         ` Joseph Lo
  0 siblings, 0 replies; 17+ messages in thread
From: Joseph Lo @ 2013-02-22  2:07 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Chris Ball, Stephen Warren,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

On Fri, 2013-02-22 at 06:44 +0800, Guennadi Liakhovetski wrote:
> Hi Joseph
> 
> On Wed, 20 Feb 2013, Joseph Lo wrote:
> >  static int tegra_sdhci_buswidth(struct sdhci_host *host, int bus_width)
> >  {
> > -	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:
> 
> +	if ((host->mmc->caps & MMC_CAP_8_BIT_DATA) &&
> 
Ah, yes. Thanks for your review.

Joseph

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2013-02-22  2:07 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-20  7:05 [PATCH 0/2] mmc: tegra: use mmc_of_parse to get support of standard MMC DT binding Joseph Lo
2013-02-20  7:05 ` [PATCH 1/2] ARM: dts: tegra: fix the activate polarity of cd-gpio in mmc host Joseph Lo
     [not found]   ` <1361343902-15223-2-git-send-email-josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-02-20 16:59     ` Stephen Warren
     [not found]       ` <512500EB.1070802-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-02-21  8:18         ` Thierry Reding
     [not found]           ` <20130221081809.GB7718-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
2013-02-21  8:30             ` Joseph Lo
2013-02-20  7:05 ` [PATCH 2/2] mmc: tegra: use mmc_of_parse to get the support of standard MMC DT bindings Joseph Lo
     [not found]   ` <1361343902-15223-3-git-send-email-josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-02-20 17:07     ` Stephen Warren
     [not found]       ` <512502D7.4090605-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-02-21  2:23         ` Joseph Lo
     [not found]           ` <1361413425.5678.8.camel-yx3yKKdKkHfc7b1ADBJPm0n48jw8i0AO@public.gmane.org>
2013-02-21  4:20             ` Stephen Warren
     [not found]               ` <5125A075.5000000-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-02-21  6:26                 ` Joseph Lo
2013-02-21  6:34                   ` Joseph Lo
2013-02-21  8:03                   ` Thierry Reding
     [not found]                     ` <20130221080306.GA7718-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
2013-02-21  8:15                       ` Joseph Lo
     [not found]                   ` <1361427997.5678.59.camel-yx3yKKdKkHfc7b1ADBJPm0n48jw8i0AO@public.gmane.org>
2013-02-21 18:50                     ` Stephen Warren
2013-02-21 22:44     ` Guennadi Liakhovetski
     [not found]       ` <Pine.LNX.4.64.1302212333330.10403-0199iw4Nj15frtckUFj5Ag@public.gmane.org>
2013-02-22  2:07         ` Joseph Lo
     [not found] ` <1361343902-15223-1-git-send-email-josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-02-21  8:24   ` [PATCH 0/2] mmc: tegra: use mmc_of_parse to get support of standard MMC DT binding Thierry Reding

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).