diff mbox

[5/5] ARM: dts: imx51-babbage: Fix esdhc setup

Message ID 1400848384-3226-6-git-send-email-s.hauer@pengutronix.de (mailing list archive)
State New, archived
Headers show

Commit Message

Sascha Hauer May 23, 2014, 12:33 p.m. UTC 
- Move cd/wp pinctrl from the hog group to the esdhc groups
- use gpio for card detection / write protection on esdhc2 since
  the controller based detection does not work
- Fix cd gpio polarity for esdhc1. This is wrong and currently
  only works because the imx esdhc driver ignores the polarity.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/boot/dts/imx51-babbage.dts | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Shawn Guo June 1, 2014, 3:22 p.m. UTC | #1 
On Fri, May 23, 2014 at 02:33:04PM +0200, Sascha Hauer wrote:
> - Move cd/wp pinctrl from the hog group to the esdhc groups
> - use gpio for card detection / write protection on esdhc2 since
>   the controller based detection does not work

I tracked it a little bit and found that the controller based detection
works fine with v3.13 and starts being broken from v3.14-rc1.  The
offending commit seems to be 89d7e5c13122 (mmc: sdhci-esdhc-imx: add
runtime pm support).

I will probably apply the patch as it is anyway.  But I'm wondering if
we should fix the regression and keep maintaining the support of
controller based cd/wp in sdhci-esdhc-imx driver.

Shawn

> - Fix cd gpio polarity for esdhc1. This is wrong and currently
>   only works because the imx esdhc driver ignores the polarity.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  arch/arm/boot/dts/imx51-babbage.dts | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
> index 9e9deb2..28d4553 100644
> --- a/arch/arm/boot/dts/imx51-babbage.dts
> +++ b/arch/arm/boot/dts/imx51-babbage.dts
> @@ -134,15 +134,15 @@
>  &esdhc1 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_esdhc1>;
> -	fsl,cd-controller;
> -	fsl,wp-controller;
> +	cd-gpios = <&gpio1 0 GPIO_ACTIVE_LOW>;
> +	wp-gpios = <&gpio1 1 GPIO_ACTIVE_HIGH>;
>  	status = "okay";
>  };
>  
>  &esdhc2 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_esdhc2>;
> -	cd-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
> +	cd-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
>  	wp-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
>  	status = "okay";
>  };
> @@ -300,10 +300,6 @@
>  	imx51-babbage {
>  		pinctrl_hog: hoggrp {
>  			fsl,pins = <
> -				MX51_PAD_GPIO1_0__SD1_CD     0x20d5
> -				MX51_PAD_GPIO1_1__SD1_WP     0x20d5
> -				MX51_PAD_GPIO1_5__GPIO1_5    0x100
> -				MX51_PAD_GPIO1_6__GPIO1_6    0x100
>  				MX51_PAD_EIM_A27__GPIO2_21   0x5
>  				MX51_PAD_CSPI1_SS0__GPIO4_24 0x85
>  				MX51_PAD_CSPI1_SS1__GPIO4_25 0x85
> @@ -330,6 +326,8 @@
>  
>  		pinctrl_esdhc1: esdhc1grp {
>  			fsl,pins = <
> +				MX51_PAD_GPIO1_0__GPIO1_0		0x20d5
> +				MX51_PAD_GPIO1_1__GPIO1_1		0x20d5
>  				MX51_PAD_SD1_CMD__SD1_CMD		0x400020d5
>  				MX51_PAD_SD1_CLK__SD1_CLK		0x20d5
>  				MX51_PAD_SD1_DATA0__SD1_DATA0		0x20d5
> @@ -341,6 +339,8 @@
>  
>  		pinctrl_esdhc2: esdhc2grp {
>  			fsl,pins = <
> +				MX51_PAD_GPIO1_5__GPIO1_5		0x100
> +				MX51_PAD_GPIO1_6__GPIO1_6		0x100
>  				MX51_PAD_SD2_CMD__SD2_CMD		0x400020d5
>  				MX51_PAD_SD2_CLK__SD2_CLK		0x20d5
>  				MX51_PAD_SD2_DATA0__SD2_DATA0		0x20d5
> -- 
> 2.0.0.rc0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sascha Hauer June 5, 2014, 10:39 a.m. UTC | #2 
On Sun, Jun 01, 2014 at 11:22:19PM +0800, Shawn Guo wrote:
> On Fri, May 23, 2014 at 02:33:04PM +0200, Sascha Hauer wrote:
> > - Move cd/wp pinctrl from the hog group to the esdhc groups
> > - use gpio for card detection / write protection on esdhc2 since
> >   the controller based detection does not work
> 
> I tracked it a little bit and found that the controller based detection
> works fine with v3.13 and starts being broken from v3.14-rc1.  The
> offending commit seems to be 89d7e5c13122 (mmc: sdhci-esdhc-imx: add
> runtime pm support).

This makes sense. When the controller is disabled it obviously can't
detect card insertion/removal events anymore. So you can only have one:
controller based card detection or runtime pm for the controller.
So it probably makes sense to use gpio card detection whenever possible
to get additional power savings.

Sascha
Shawn Guo June 9, 2014, 3:38 a.m. UTC | #3 
On Thu, Jun 05, 2014 at 12:39:49PM +0200, Sascha Hauer wrote:
> On Sun, Jun 01, 2014 at 11:22:19PM +0800, Shawn Guo wrote:
> > On Fri, May 23, 2014 at 02:33:04PM +0200, Sascha Hauer wrote:
> > > - Move cd/wp pinctrl from the hog group to the esdhc groups
> > > - use gpio for card detection / write protection on esdhc2 since
> > >   the controller based detection does not work
> > 
> > I tracked it a little bit and found that the controller based detection
> > works fine with v3.13 and starts being broken from v3.14-rc1.  The
> > offending commit seems to be 89d7e5c13122 (mmc: sdhci-esdhc-imx: add
> > runtime pm support).
> 
> This makes sense. When the controller is disabled it obviously can't
> detect card insertion/removal events anymore. So you can only have one:
> controller based card detection or runtime pm for the controller.
> So it probably makes sense to use gpio card detection whenever possible
> to get additional power savings.

Can we have something like that in commit log to explain the breakage?
Also, the patch does not apply to my branch, so you may need to rebase
anyway.

Lothar, Denis,

FYI.  The controller based card detection / write protection are broken
right now, and I see your boards (imx53-tx53 and
imx51-eukrea-mbimxsd51-baseboard) are using them.  You may want to
switch to use GPIO if available.

Shawn
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shawn Guo June 9, 2014, 3:41 a.m. UTC | #4 
On Fri, May 23, 2014 at 02:33:04PM +0200, Sascha Hauer wrote:
> - Move cd/wp pinctrl from the hog group to the esdhc groups
> - use gpio for card detection / write protection on esdhc2 since
>   the controller based detection does not work
> - Fix cd gpio polarity for esdhc1. This is wrong and currently
>   only works because the imx esdhc driver ignores the polarity.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  arch/arm/boot/dts/imx51-babbage.dts | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
> index 9e9deb2..28d4553 100644
> --- a/arch/arm/boot/dts/imx51-babbage.dts
> +++ b/arch/arm/boot/dts/imx51-babbage.dts
> @@ -134,15 +134,15 @@
>  &esdhc1 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_esdhc1>;
> -	fsl,cd-controller;
> -	fsl,wp-controller;
> +	cd-gpios = <&gpio1 0 GPIO_ACTIVE_LOW>;
> +	wp-gpios = <&gpio1 1 GPIO_ACTIVE_HIGH>;
>  	status = "okay";
>  };
>  
>  &esdhc2 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_esdhc2>;
> -	cd-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
> +	cd-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
>  	wp-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
>  	status = "okay";
>  };
> @@ -300,10 +300,6 @@
>  	imx51-babbage {
>  		pinctrl_hog: hoggrp {
>  			fsl,pins = <
> -				MX51_PAD_GPIO1_0__SD1_CD     0x20d5
> -				MX51_PAD_GPIO1_1__SD1_WP     0x20d5
> -				MX51_PAD_GPIO1_5__GPIO1_5    0x100
> -				MX51_PAD_GPIO1_6__GPIO1_6    0x100
>  				MX51_PAD_EIM_A27__GPIO2_21   0x5
>  				MX51_PAD_CSPI1_SS0__GPIO4_24 0x85
>  				MX51_PAD_CSPI1_SS1__GPIO4_25 0x85
> @@ -330,6 +326,8 @@
>  
>  		pinctrl_esdhc1: esdhc1grp {
>  			fsl,pins = <
> +				MX51_PAD_GPIO1_0__GPIO1_0		0x20d5
> +				MX51_PAD_GPIO1_1__GPIO1_1		0x20d5

The pad config value 0x20d5 still makes sense here when switching to
GPIO function?  Probably should be 0x100 like what we have in esdhc2grp?

Shawn

>  				MX51_PAD_SD1_CMD__SD1_CMD		0x400020d5
>  				MX51_PAD_SD1_CLK__SD1_CLK		0x20d5
>  				MX51_PAD_SD1_DATA0__SD1_DATA0		0x20d5
> @@ -341,6 +339,8 @@
>  
>  		pinctrl_esdhc2: esdhc2grp {
>  			fsl,pins = <
> +				MX51_PAD_GPIO1_5__GPIO1_5		0x100
> +				MX51_PAD_GPIO1_6__GPIO1_6		0x100
>  				MX51_PAD_SD2_CMD__SD2_CMD		0x400020d5
>  				MX51_PAD_SD2_CLK__SD2_CLK		0x20d5
>  				MX51_PAD_SD2_DATA0__SD2_DATA0		0x20d5
> -- 
> 2.0.0.rc0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lothar Waßmann June 10, 2014, 8:01 a.m. UTC | #5 
Hi,

Shawn Guo wrote:
> On Fri, May 23, 2014 at 02:33:04PM +0200, Sascha Hauer wrote:
> > - Move cd/wp pinctrl from the hog group to the esdhc groups
> > - use gpio for card detection / write protection on esdhc2 since
> >   the controller based detection does not work
> > - Fix cd gpio polarity for esdhc1. This is wrong and currently
> >   only works because the imx esdhc driver ignores the polarity.
> > 
[...]
> > @@ -330,6 +326,8 @@
> >  
> >  		pinctrl_esdhc1: esdhc1grp {
> >  			fsl,pins = <
> > +				MX51_PAD_GPIO1_0__GPIO1_0		0x20d5
> > +				MX51_PAD_GPIO1_1__GPIO1_1		0x20d5
> 
> The pad config value 0x20d5 still makes sense here when switching to
> GPIO function?  Probably should be 0x100 like what we have in esdhc2grp?
>
The pad config 0x20d5 doesn't make any sense at all because according
to the Ref. Manual only bits 0..2 and 4..8 (mask 0x01f7) are defined for
those pads. Thus 0x2000 doesn't serve any purpose at all.


Lothar Waßmann
Shawn Guo June 21, 2014, 7:56 a.m. UTC | #6 
On Fri, May 23, 2014 at 02:33:04PM +0200, Sascha Hauer wrote:
> - Move cd/wp pinctrl from the hog group to the esdhc groups
> - use gpio for card detection / write protection on esdhc2 since
>   the controller based detection does not work
> - Fix cd gpio polarity for esdhc1. This is wrong and currently
>   only works because the imx esdhc driver ignores the polarity.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Applied it after fixing up the commit log and pad value.

Shawn
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
index 9e9deb2..28d4553 100644
--- a/arch/arm/boot/dts/imx51-babbage.dts
+++ b/arch/arm/boot/dts/imx51-babbage.dts
@@ -134,15 +134,15 @@ 
 &esdhc1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_esdhc1>;
-	fsl,cd-controller;
-	fsl,wp-controller;
+	cd-gpios = <&gpio1 0 GPIO_ACTIVE_LOW>;
+	wp-gpios = <&gpio1 1 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 };
 
 &esdhc2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_esdhc2>;
-	cd-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
+	cd-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
 	wp-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 };
@@ -300,10 +300,6 @@ 
 	imx51-babbage {
 		pinctrl_hog: hoggrp {
 			fsl,pins = <
-				MX51_PAD_GPIO1_0__SD1_CD     0x20d5
-				MX51_PAD_GPIO1_1__SD1_WP     0x20d5
-				MX51_PAD_GPIO1_5__GPIO1_5    0x100
-				MX51_PAD_GPIO1_6__GPIO1_6    0x100
 				MX51_PAD_EIM_A27__GPIO2_21   0x5
 				MX51_PAD_CSPI1_SS0__GPIO4_24 0x85
 				MX51_PAD_CSPI1_SS1__GPIO4_25 0x85
@@ -330,6 +326,8 @@ 
 
 		pinctrl_esdhc1: esdhc1grp {
 			fsl,pins = <
+				MX51_PAD_GPIO1_0__GPIO1_0		0x20d5
+				MX51_PAD_GPIO1_1__GPIO1_1		0x20d5
 				MX51_PAD_SD1_CMD__SD1_CMD		0x400020d5
 				MX51_PAD_SD1_CLK__SD1_CLK		0x20d5
 				MX51_PAD_SD1_DATA0__SD1_DATA0		0x20d5
@@ -341,6 +339,8 @@ 
 
 		pinctrl_esdhc2: esdhc2grp {
 			fsl,pins = <
+				MX51_PAD_GPIO1_5__GPIO1_5		0x100
+				MX51_PAD_GPIO1_6__GPIO1_6		0x100
 				MX51_PAD_SD2_CMD__SD2_CMD		0x400020d5
 				MX51_PAD_SD2_CLK__SD2_CLK		0x20d5
 				MX51_PAD_SD2_DATA0__SD2_DATA0		0x20d5