| Message ID | 1400848384-3226-6-git-send-email-s.hauer@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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
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
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 >
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
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
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
- 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(-)