sdhci-esdhc-imx: use gpio for write protection and card detection

PATCH Superseded Wolfram Sang 3 participants 10 messages Feb 10, 2011 – Feb 11, 2011

Subsystems (6)

Arm (32-bit) Arm sub-architectures Arm i.MX and Layerscape MMC, SD and SDIO +2 more subsystems

lore

Share & tools

lore

Wolfram Sang

author

2011-02-10 19:14 UTC #0 lore raw

cover sdhci-esdhc-imx: use gpio for write protection and card detection

Hi,

I finally could do a second version of my write protect-patches and the first version of a card detect approach (thanks to Olof for the inspiration here). Note that these feature are only needed for i.mx25/35 as the i.mx5x-series _finally_ seem to be able to route the gpios to the controller directly.

These patches apply to mmc-next and have been tested with a mx35-based pcm043. I think the first three patches should go via Chris and the latter two via Sascha. The last one is more a cleanup-patch, but it depends on the former, so I included it here.

The branch can be found at:

	git://git.pengutronix.de/git/wsa/linux-2.6.git pcm043-mmc

Eric: Would be awesome if you could test these on your boards!

Looking forward to comments,

Wolfram

Wolfram Sang (5):
mmc: sdhci-esdhc-imx: add support for write protect on custom GPIO
mmc: sdhci-esdhc: broken card detection is not a default quirk
mmc: sdhci-esdhc-imx: add card detect on custom GPIO
arm: mach-mx3: pcm043: add write-protect and card-detect for SD1
arm: mach-mx3: use IMX_GPIO_NR instead of hard-coded values

 arch/arm/mach-mx3/eukrea_mbimxsd-baseboard.c |    4 +-
 arch/arm/mach-mx3/mach-cpuimx35.c            |    2 +-
 arch/arm/mach-mx3/mach-pcm043.c              |   19 +++-
 arch/arm/plat-mxc/include/mach/esdhc.h       |   12 ++-
 drivers/mmc/host/sdhci-esdhc-imx.c           |  129 +++++++++++++++++++++++---
 drivers/mmc/host/sdhci-esdhc.h               |    1 -
 drivers/mmc/host/sdhci-of-esdhc.c            |    3 +-
 7 files changed, 148 insertions(+), 22 deletions(-)

Signature · 1 line

1.7.2.3

Wolfram Sang

author

2011-02-10 19:14 UTC #1 lore raw

PATCH 1/5 mmc: sdhci-esdhc-imx: add support for write protect on custom GPIO

Signed-off-by Wolfram Sang <[email protected]>

---
 arch/arm/plat-mxc/include/mach/esdhc.h |   10 +++++-
 drivers/mmc/host/sdhci-esdhc-imx.c     |   51 +++++++++++++++++++++++++-------
 2 files changed, 49 insertions(+), 12 deletions(-)

arch/arm/plat-mxc/include/mach/esdhc.h +9 -1

index a48a9aa..dbf6d56 100644
@@ -10,7 +10,15 @@
 #ifndef __ASM_ARCH_IMX_ESDHC_H
 #define __ASM_ARCH_IMX_ESDHC_H
 
+/**
+ * struct esdhc_platform_data - optional platform data for esdhc on i.MX
+ *
+ * strongly recommended for i.MX25/35, not needed for other variants
+ *
+ * @wp_gpio:	gpio for write_protect
+ */
+
 struct esdhc_platform_data {
-	unsigned int wp_gpio;	/* write protect pin */
+	unsigned int wp_gpio;
 };
 #endif /* __ASM_ARCH_IMX_ESDHC_H */

drivers/mmc/host/sdhci-esdhc-imx.c +40 ~~-11~~

88 lines not on this page · raw message

Signature · 1 line

1.7.2.3

Marc Reilly

2011-02-10 22:24 UTC #2 lore raw

PATCH 1/5 mmc: sdhci-esdhc-imx: add support for write protect on custom GPIO

Hi Wolfram,

First three patches work well on my vpr200 board (mx35). One comment inline...

I ran some tests of insert/remove card with the card locked and unlocked, and trying to append a file. It behaved as expected, thus:

Tested-by Marc Reilly <[email protected]>

Quoted 10 lines

drivers/mmc/host/sdhci-esdhc-imx.c +10 -0

+static unsigned int esdhc_pltfm_get_ro(struct sdhci_host *host)
+{
+	struct esdhc_platform_data *boarddata = host->mmc->parent->platform_data;
+
+	if (boarddata && gpio_is_valid(boarddata->wp_gpio))
+		return gpio_get_value(boarddata->wp_gpio);
+	else
+		return -ENOSYS;
+}
+

..snip..

Quoted 4 lines

drivers/mmc/host/sdhci-esdhc-imx.c +0 -0

@@ -116,9 +139,20 @@ static int esdhc_pltfm_init(struct sdhci_host *host,
 struct sdhci_pltfm_data *pd if (cpu_is_mx35() || cpu_is_mx51())
 		host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;

drivers/mmc/host/sdhci-esdhc-imx.c static int esdhc_pltfm_init(struct sdhci_host *host, · line 149

Quoted 10 lines

drivers/mmc/host/sdhci-esdhc-imx.c +7 -2

-	/* Fix errata ENGcm07207 which is present on i.MX25 and i.MX35 */
-	if (cpu_is_mx25() || cpu_is_mx35())
+	if (cpu_is_mx25() || cpu_is_mx35()) {
+		/* Fix errata ENGcm07207 present on i.MX25 and i.MX35 */
 		host->quirks |= SDHCI_QUIRK_NO_MULTIBLOCK;
+		/* write_protect can't be routed to controller, use gpio */
+		sdhci_esdhc_ops.get_ro = esdhc_pltfm_get_ro;
+	}
+
+	if (boarddata) {

Perhaps (boarddata && gpio_is_valid(boarddata->wp_gpio) as above?

For example what if someone sets up the boarddata for a card detect, but not write protect.

drivers/mmc/host/sdhci-esdhc-imx.c static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pd · line 158

Quoted 9 lines

drivers/mmc/host/sdhci-esdhc-imx.c +6 -0

+		err = gpio_request_one(boarddata->wp_gpio, GPIOF_IN, "ESDHC_WP");
+		if (err) {
+			dev_warn(mmc_dev(host->mmc), "can't get wp_pin!\n");
+			boarddata->wp_gpio = err;
+		}
+	}
 
 	return 0;
 }

Cheers,
Marc

Wolfram Sang

author

2011-02-11 09:42 UTC #3 lore raw

PATCH 1/5 mmc: sdhci-esdhc-imx: add support for write protect on custom GPIO

Marc,

Quoted 1 line · Marc Reilly

Tested-by Marc Reilly <[email protected]>

Thanks a lot!

Quoted 6 lines

drivers/mmc/host/sdhci-esdhc-imx.c static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pd · line 149
Quoted 1 line
drivers/mmc/host/sdhci-esdhc-imx.c +1 -0
+	if (boarddata) {
Perhaps (boarddata && gpio_is_valid(boarddata->wp_gpio) as above?
For example what if someone sets up the boarddata for a card detect, but not write protect.

Well, I couldn't do this here, because later the cd_pin-stuff will also be added to the if-block. However, I see what you mean and that could still be added before the gpio_request below. That would add another level of indentation, though, for a case I'd consider to be more theoretical (and definately not recommended). Maybe I will adjust the warning message to be also understandable for the case a GPIO is preset with an invalid number. I think the warning itself is justified.

Quoted 7 lines

drivers/mmc/host/sdhci-esdhc-imx.c static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pd · line 155
Quoted 7 lines
drivers/mmc/host/sdhci-esdhc-imx.c +6 -0
+		err = gpio_request_one(boarddata->wp_gpio, GPIOF_IN, "ESDHC_WP");
+		if (err) {
+			dev_warn(mmc_dev(host->mmc), "can't get wp_pin!\n");
+			boarddata->wp_gpio = err;
+		}
+	}

Regards,

Wolfram

Signature · 2 lines

Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Wolfram Sang

author

2011-02-10 19:14 UTC #4 lore raw

PATCH 2/5 mmc: sdhci-esdhc: broken card detection is not a default quirk

In reply to Wolfram Sang

It can be worked around using a GPIO which will be done for i.MX later.

Signed-off-by Wolfram Sang <[email protected]>

---
 drivers/mmc/host/sdhci-esdhc-imx.c |    3 ++-
 drivers/mmc/host/sdhci-esdhc.h     |    1 -
 drivers/mmc/host/sdhci-of-esdhc.c  |    3 ++-
 3 files changed, 4 insertions(+), 3 deletions(-)

drivers/mmc/host/sdhci-esdhc-imx.c +2 -1

index 83d178b..e0fa1f3 100644
@@ -170,7 +170,8 @@ static void esdhc_pltfm_exit(struct sdhci_host *host)
 }
 
 struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
-	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_ADMA,
+	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_ADMA
+			| SDHCI_QUIRK_BROKEN_CARD_DETECTION,
 	/* ADMA has issues. Might be fixable */
 	.ops = &sdhci_esdhc_ops,
 	.init = esdhc_pltfm_init,

drivers/mmc/host/sdhci-esdhc.h +0 -1

index afaf1bc..c55aae8 100644
@@ -19,7 +19,6 @@
  */
 
 #define ESDHC_DEFAULT_QUIRKS	(SDHCI_QUIRK_FORCE_BLK_SZ_2048 | \
-				SDHCI_QUIRK_BROKEN_CARD_DETECTION | \
 				SDHCI_QUIRK_NO_BUSY_IRQ | \
 				SDHCI_QUIRK_NONSTANDARD_CLOCK | \
 				SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | \

drivers/mmc/host/sdhci-of-esdhc.c +2 -1

index fcd0e1f..08161f6 100644
@@ -73,7 +73,8 @@ static unsigned int esdhc_of_get_min_clock(struct sdhci_host *host)
 }
 
 struct sdhci_of_data sdhci_esdhc = {
-	.quirks = ESDHC_DEFAULT_QUIRKS,
+	/* card detection could be handled via GPIO */
+	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_CARD_DETECTION,
 	.ops = {
 		.read_l = sdhci_be32bs_readl,
 		.read_w = esdhc_readw,

Signature · 1 line

1.7.2.3

Wolfram Sang

author

2011-02-10 19:18 UTC #5 lore raw

PATCH 2/5 mmc: sdhci-esdhc: broken card detection is not a default quirk

Adding Anton to Cc...

Quoted 52 lines · Wolfram Sang

It can be worked around using a GPIO which will be done for i.MX later.

Signed-off-by Wolfram Sang <[email protected]>

---
 drivers/mmc/host/sdhci-esdhc-imx.c |    3 ++-
 drivers/mmc/host/sdhci-esdhc.h     |    1 -
 drivers/mmc/host/sdhci-of-esdhc.c  |    3 ++-
 3 files changed, 4 insertions(+), 3 deletions(-)

drivers/mmc/host/sdhci-esdhc-imx.c +2 -1

index 83d178b..e0fa1f3 100644
@@ -170,7 +170,8 @@ static void esdhc_pltfm_exit(struct sdhci_host *host)
 }
 
 struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
-	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_ADMA,
+	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_ADMA
+			| SDHCI_QUIRK_BROKEN_CARD_DETECTION,
 	/* ADMA has issues. Might be fixable */
 	.ops = &sdhci_esdhc_ops,
 	.init = esdhc_pltfm_init,

drivers/mmc/host/sdhci-esdhc.h +0 -1

index afaf1bc..c55aae8 100644
@@ -19,7 +19,6 @@
  */
 
 #define ESDHC_DEFAULT_QUIRKS	(SDHCI_QUIRK_FORCE_BLK_SZ_2048 | \
-				SDHCI_QUIRK_BROKEN_CARD_DETECTION | \
 				SDHCI_QUIRK_NO_BUSY_IRQ | \
 				SDHCI_QUIRK_NONSTANDARD_CLOCK | \
 				SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | \

drivers/mmc/host/sdhci-of-esdhc.c +2 -1

index fcd0e1f..08161f6 100644
@@ -73,7 +73,8 @@ static unsigned int esdhc_of_get_min_clock(struct sdhci_host *host)
 }
 
 struct sdhci_of_data sdhci_esdhc = {
-	.quirks = ESDHC_DEFAULT_QUIRKS,
+	/* card detection could be handled via GPIO */
+	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_CARD_DETECTION,
 	.ops = {
 		.read_l = sdhci_be32bs_readl,
 		.read_w = esdhc_readw,

Signature · 1 line

1.7.2.3

Signature · 9 lines

Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110210/398800b4/attachment-0001.sig>

Anton Vorontsov

2011-02-11 10:07 UTC #6 lore raw

PATCH 2/5 mmc: sdhci-esdhc: broken card detection is not a default quirk

Quoted 2 lines · Wolfram Sang

Adding Anton to Cc...

[...]

Quoted 4 lines

drivers/mmc/host/sdhci-esdhc-imx.c static void esdhc_pltfm_exit(struct sdhci_host *host) · line 174
Quoted 4 lines
struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
drivers/mmc/host/sdhci-esdhc-imx.c +2 -1
-	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_ADMA,
+	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_ADMA
+			| SDHCI_QUIRK_BROKEN_CARD_DETECTION,

Minor nit: "|" should be on the previous line.

Otherwise,

Acked-by Anton Vorontsov <[email protected]>

Thanks!

Signature · 2 lines

Anton Vorontsov
Email: cbouatmailru at gmail.com

Wolfram Sang

author

2011-02-10 19:14 UTC #7 lore raw

PATCH 3/5 mmc: sdhci-esdhc-imx: add card detect on custom GPIO

In reply to Wolfram Sang

Signed-off-by Wolfram Sang <[email protected]>

---
 arch/arm/plat-mxc/include/mach/esdhc.h |    2 +
 drivers/mmc/host/sdhci-esdhc-imx.c     |   75 ++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 0 deletions(-)

arch/arm/plat-mxc/include/mach/esdhc.h +2 -0

index dbf6d56..7501d4e 100644
@@ -16,9 +16,11 @@
  * strongly recommended for i.MX25/35, not needed for other variants
  *
  * @wp_gpio:	gpio for write_protect
+ * @cd_gpio:	gpio for card_detect interrupt
  */
 
 struct esdhc_platform_data {
 	unsigned int wp_gpio;
+	unsigned int cd_gpio;
 };
 #endif /* __ASM_ARCH_IMX_ESDHC_H */

drivers/mmc/host/sdhci-esdhc-imx.c +75 -0

108 lines not on this page · raw message

Signature · 1 line

1.7.2.3

Wolfram Sang

author

2011-02-10 19:14 UTC #8 lore raw

PATCH 4/5 arm: mach-mx3: pcm043: add write-protect and card-detect for SD1

In reply to Wolfram Sang

Signed-off-by Wolfram Sang <[email protected]>

---
 arch/arm/mach-mx3/mach-pcm043.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

arch/arm/mach-mx3/mach-pcm043.c +12 -1

index bcf83fc..26b686c 100644
@@ -40,6 +40,7 @@
 #include <mach/mx3fb.h>
 #include <mach/ulpi.h>
 #include <mach/audmux.h>
+#include <mach/esdhc.h>
 
 #include "devices-imx35.h"
 #include "devices.h"

@@ -219,12 +220,17 @@ static iomux_v3_cfg_t pcm043_pads[] = {
 	MX35_PAD_SD1_DATA1__ESDHC1_DAT1,
 	MX35_PAD_SD1_DATA2__ESDHC1_DAT2,
 	MX35_PAD_SD1_DATA3__ESDHC1_DAT3,
+	MX35_PAD_ATA_DATA10__GPIO2_23, /* WriteProtect */
+	MX35_PAD_ATA_DATA11__GPIO2_24, /* CardDetect */
 };
 
 #define AC97_GPIO_TXFS	(1 * 32 + 31)
 #define AC97_GPIO_TXD	(1 * 32 + 28)
 #define AC97_GPIO_RESET	(1 * 32 + 0)
 
+#define SD1_GPIO_WP	(1 * 32 + 23)
+#define SD1_GPIO_CD	(1 * 32 + 24)
+
 static void pcm043_ac97_warm_reset(struct snd_ac97 *ac97)
 {
 	iomux_v3_cfg_t txfs_gpio = MX35_PAD_STXFS4__GPIO2_31;

@@ -307,6 +313,11 @@ pcm037_nand_board_info __initconst = {
 	.hw_ecc = 1,
 };
 
+static struct esdhc_platform_data sd1_pdata = {
+	.wp_gpio = SD1_GPIO_WP,
+	.cd_gpio = SD1_GPIO_CD,
+};
+
 #if defined(CONFIG_USB_ULPI)
 static struct mxc_usbh_platform_data otg_pdata __initdata = {
 	.portsc	= MXC_EHCI_MODE_UTMI,

@@ -393,7 +404,7 @@ static void __init mxc_board_init(void)
 		imx35_add_fsl_usb2_udc(&otg_device_pdata);
 
 	imx35_add_flexcan1(NULL);
-	imx35_add_sdhci_esdhc_imx(0, NULL);
+	imx35_add_sdhci_esdhc_imx(0, &sd1_pdata);
 }
 
 static void __init pcm043_timer_init(void)

Signature · 1 line

1.7.2.3

Wolfram Sang

author

2011-02-10 19:14 UTC #9 lore raw

PATCH 5/5 arm: mach-mx3: use IMX_GPIO_NR instead of hard-coded values

In reply to Wolfram Sang

The latter are error-prone because the bank number is one less than one would read in the documentation.

Signed-off-by Wolfram Sang <[email protected]>

Cc Eric B?nard <[email protected]>

Cc Sascha Hauer <[email protected]>

---
 arch/arm/mach-mx3/eukrea_mbimxsd-baseboard.c |    4 ++--
 arch/arm/mach-mx3/mach-cpuimx35.c            |    2 +-
 arch/arm/mach-mx3/mach-pcm043.c              |   10 +++++-----
 3 files changed, 8 insertions(+), 8 deletions(-)

arch/arm/mach-mx3/eukrea_mbimxsd-baseboard.c +2 -2

index 14a5ffc..8076147 100644
@@ -165,8 +165,8 @@ static iomux_v3_cfg_t eukrea_mbimxsd_pads[] = {
 	MX35_PAD_SD1_DATA3__ESDHC1_DAT3,
 };
 
-#define GPIO_LED1	(2 * 32 + 29)
-#define GPIO_SWITCH1	(2 * 32 + 25)
+#define GPIO_LED1	IMX_GPIO_NR(3, 29)
+#define GPIO_SWITCH1	IMX_GPIO_NR(3, 25)
 #define GPIO_LCDPWR	(4)
 
 static void eukrea_mbimxsd_lcd_power_set(struct plat_lcd_data *pd,

arch/arm/mach-mx3/mach-cpuimx35.c +1 -1

index 26ae90f..892c3a9 100644
@@ -60,7 +60,7 @@ static struct tsc2007_platform_data tsc2007_info = {
 	.x_plate_ohms		= 180,
 };
 
-#define TSC2007_IRQGPIO		(2 * 32 + 2)
+#define TSC2007_IRQGPIO		IMX_GPIO_NR(3, 2)
 static struct i2c_board_info eukrea_cpuimx35_i2c_devices[] = {
 	{
 		I2C_BOARD_INFO("pcf8563", 0x51),

arch/arm/mach-mx3/mach-pcm043.c +5 -5

index 26b686c..51542f7 100644
@@ -224,12 +224,12 @@ static iomux_v3_cfg_t pcm043_pads[] = {
 	MX35_PAD_ATA_DATA11__GPIO2_24, /* CardDetect */
 };
 
-#define AC97_GPIO_TXFS	(1 * 32 + 31)
-#define AC97_GPIO_TXD	(1 * 32 + 28)
-#define AC97_GPIO_RESET	(1 * 32 + 0)
+#define AC97_GPIO_TXFS	IMX_GPIO_NR(2, 31)
+#define AC97_GPIO_TXD	IMX_GPIO_NR(2, 28)
+#define AC97_GPIO_RESET	IMX_GPIO_NR(2, 0)
 
-#define SD1_GPIO_WP	(1 * 32 + 23)
-#define SD1_GPIO_CD	(1 * 32 + 24)
+#define SD1_GPIO_WP	IMX_GPIO_NR(2, 23)
+#define SD1_GPIO_CD	IMX_GPIO_NR(2, 24)
 
 static void pcm043_ac97_warm_reset(struct snd_ac97 *ac97)
 {

Signature · 1 line

1.7.2.3