[PATCH 0/6] mmc: omap_hsmmc: simplify cover/card detect logic

[PATCH 0/6] mmc: omap_hsmmc: simplify cover/card detect logic

[Andreas Fenkart]

These patches are trying to clean up the cover/card detect logic.

  Mobile phones (some) have no card detect pin, but
  can detect if the cover is removed. The purpose is the
  same; detect if card is being added/removed, but the
  details differ.
  When the cover is removed, it does not mean the card is
  gone. But it might, since it is accessible now. It's like
  a warning. All the driver does is to limit write access to
  the card, see protect_card flag. In contrast, card detect
  notifies us after the fact, e.g. card is gone, card is
  inserted.

While cover detect is only used by one platform (rx51), it
complicates the card detect logic. By separating the code
paths they both become easier to understand and maintain 

Patches have been tested by reverting: 95bebb5696ab
'mmc: omap_hsmmc: use mmc_of_parse to parse common mmc configuration'
otherwise gpio detection is handled by mmc_of_parse

compile tested
OMAP2:
CONFIG_MACH_OMAP2_TUSB6010=y
CONFIG_MACH_OMAP3_BEAGLE=y
CONFIG_MACH_DEVKIT8000=y
CONFIG_MACH_OMAP_LDP=y
CONFIG_MACH_OMAP3530_LV_SOM=y
CONFIG_MACH_OMAP3_TORPEDO=y
CONFIG_MACH_OVERO=y
CONFIG_MACH_OMAP3517EVM=y
CONFIG_MACH_CRANEBOARD=y
CONFIG_MACH_OMAP3_PANDORA=y
CONFIG_MACH_TOUCHBOOK=y
CONFIG_MACH_OMAP_3430SDP=y
CONFIG_MACH_NOKIA_N810=y
CONFIG_MACH_NOKIA_N810_WIMAX=y
CONFIG_MACH_NOKIA_N8X0=y
CONFIG_MACH_NOKIA_RX51=y
CONFIG_MACH_CM_T35=y
CONFIG_MACH_CM_T3517=y
CONFIG_MACH_CM_T3730=y
CONFIG_MACH_SBC3530=y
CONFIG_MACH_TI8168EVM=y
CONFIG_MACH_TI8148EVM=y

Andreas Fenkart (6):
  mmc: omap_hsmmc: remove unused fields from struct omap_hsmmc_host
  mmc: omap_hsmmc: use slot-gpio functions to manage read-only pin
    directly
  mmc: omap_hsmmc: use distinctive code paths for cover / card detect
    logic
  ARM: OMAP2: HSMMC: platform_data: explicit gpio_cover / gpio_cd fields
  mmc: omap_hsmmc: simplify card/cover detect isr
  mmc: omap_hsmmc: use generic slot-gpio isr to manage card detect pin

 arch/arm/mach-omap2/hsmmc.c              | 33 +++++++++----
 drivers/mmc/host/omap_hsmmc.c            | 80 +++++++++-----------------------
 include/linux/platform_data/hsmmc-omap.h |  6 +--
 3 files changed, 48 insertions(+), 71 deletions(-)

-- 
2.1.4

[PATCH 1/6] mmc: omap_hsmmc: remove unused fields from struct omap_hsmmc_host

[Andreas Fenkart]

addon to: 09108968b7b72b6083a3bfc8f8259a74ed57255e
    mmc: omap_hsmmc: remove prepare/complete system suspend support

Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
---
 drivers/mmc/host/omap_hsmmc.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 8d87e08..2276d58 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -222,10 +222,6 @@ struct omap_hsmmc_host {
 	struct omap_hsmmc_next	next_data;
 	struct	omap_hsmmc_platform_data	*pdata;
 
-	/* To handle board related suspend/resume functionality for MMC */
-	int (*suspend)(struct device *dev);
-	int (*resume)(struct device *dev);
-
 	/* return MMC cover switch state, can be NULL if not supported.
 	 *
 	 * possible return values:
-- 
2.1.4

[PATCH 2/6] mmc: omap_hsmmc: use slot-gpio functions to manage read-only pin directly

[Andreas Fenkart]

The indirection via omap_hsmmc_get_ro and omap_hsmmc_get_wp is
redundant. Also dropped setting gpio_wp to EINVAL since platform date
is read-only
Untested: no device with ro pin was available, but change is fairly
simple

Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
---
 drivers/mmc/host/omap_hsmmc.c | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 2276d58..77ad471 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -234,8 +234,6 @@ struct omap_hsmmc_host {
 	int card_detect_irq;
 
 	int (*card_detect)(struct device *dev);
-	int (*get_ro)(struct device *dev);
-
 };
 
 struct omap_mmc_of_data {
@@ -252,13 +250,6 @@ static int omap_hsmmc_card_detect(struct device *dev)
 	return mmc_gpio_get_cd(host->mmc);
 }
 
-static int omap_hsmmc_get_wp(struct device *dev)
-{
-	struct omap_hsmmc_host *host = dev_get_drvdata(dev);
-
-	return mmc_gpio_get_ro(host->mmc);
-}
-
 static int omap_hsmmc_get_cover_state(struct device *dev)
 {
 	struct omap_hsmmc_host *host = dev_get_drvdata(dev);
@@ -455,12 +446,9 @@ static int omap_hsmmc_gpio_init(struct mmc_host *mmc,
 	}
 
 	if (gpio_is_valid(pdata->gpio_wp)) {
-		host->get_ro = omap_hsmmc_get_wp;
 		ret = mmc_gpio_request_ro(mmc, pdata->gpio_wp);
 		if (ret)
 			return ret;
-	} else {
-		pdata->gpio_wp = -EINVAL;
 	}
 
 	return 0;
@@ -1637,15 +1625,6 @@ static int omap_hsmmc_get_cd(struct mmc_host *mmc)
 	return host->card_detect(host->dev);
 }
 
-static int omap_hsmmc_get_ro(struct mmc_host *mmc)
-{
-	struct omap_hsmmc_host *host = mmc_priv(mmc);
-
-	if (!host->get_ro)
-		return -ENOSYS;
-	return host->get_ro(host->dev);
-}
-
 static void omap_hsmmc_init_card(struct mmc_host *mmc, struct mmc_card *card)
 {
 	struct omap_hsmmc_host *host = mmc_priv(mmc);
@@ -1811,7 +1790,7 @@ static struct mmc_host_ops omap_hsmmc_ops = {
 	.request = omap_hsmmc_request,
 	.set_ios = omap_hsmmc_set_ios,
 	.get_cd = omap_hsmmc_get_cd,
-	.get_ro = omap_hsmmc_get_ro,
+	.get_ro = mmc_gpio_get_ro,
 	.init_card = omap_hsmmc_init_card,
 	.enable_sdio_irq = omap_hsmmc_enable_sdio_irq,
 };
-- 
2.1.4

[PATCH 3/6] mmc: omap_hsmmc: use distinctive code paths for cover / card detect logic

[Andreas Fenkart]

Mobile phones (some) have no card detect pin, but can detect if the
cover is removed. The purpose is the same; detect if card is being
added/removed, but the details differ.
When the cover is removed, it does not mean the card is gone. But it
might, since it is accessible now. It's like a warning. All the driver
does is to limit write access to the card, see protect_card flag.
In contrast, card detect notifies us after the fact, e.g.
card is gone, card is inserted. We can't take precautions, but we can
rely on those events, -- the card is really gone, or do scan the card.
To summarize there is not much code sharing between cover and card
detect, it only increases confusion. By splitting, both will be
simplified in a followup patch.

Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
---
 drivers/mmc/host/omap_hsmmc.c | 56 +++++++++++++++++++++++++++++--------------
 1 file changed, 38 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 77ad471..38239fb 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -230,9 +230,6 @@ struct omap_hsmmc_host {
 	 */
 	int (*get_cover_state)(struct device *dev);
 
-	/* Card detection IRQs */
-	int card_detect_irq;
-
 	int (*card_detect)(struct device *dev);
 };
 
@@ -422,6 +419,7 @@ static inline int omap_hsmmc_have_reg(void)
 #endif
 
 static irqreturn_t omap_hsmmc_detect(int irq, void *dev_id);
+static irqreturn_t omap_hsmmc_cover_irq(int irq, void *dev_id);
 
 static int omap_hsmmc_gpio_init(struct mmc_host *mmc,
 				struct omap_hsmmc_host *host,
@@ -429,20 +427,20 @@ static int omap_hsmmc_gpio_init(struct mmc_host *mmc,
 {
 	int ret;
 
-	if (gpio_is_valid(pdata->switch_pin)) {
-		if (pdata->cover)
-			host->get_cover_state =
-				omap_hsmmc_get_cover_state;
-		else
-			host->card_detect = omap_hsmmc_card_detect;
-		host->card_detect_irq =
-				gpio_to_irq(pdata->switch_pin);
-		mmc_gpio_set_cd_isr(mmc, omap_hsmmc_detect);
+	if (pdata->cover && gpio_is_valid(pdata->switch_pin)) {
 		ret = mmc_gpio_request_cd(mmc, pdata->switch_pin, 0);
 		if (ret)
 			return ret;
-	} else {
-		pdata->switch_pin = -EINVAL;
+
+		host->get_cover_state = omap_hsmmc_get_cover_state;
+		mmc_gpio_set_cd_isr(mmc, omap_hsmmc_cover_irq);
+	} else if (!pdata->cover && gpio_is_valid(pdata->switch_pin)) {
+		ret = mmc_gpio_request_cd(mmc, pdata->switch_pin, 0);
+		if (ret)
+			return ret;
+
+		host->card_detect = omap_hsmmc_card_detect;
+		mmc_gpio_set_cd_isr(mmc, omap_hsmmc_detect);
 	}
 
 	if (gpio_is_valid(pdata->gpio_wp)) {
@@ -1236,15 +1234,37 @@ static void omap_hsmmc_protect_card(struct omap_hsmmc_host *host)
 }
 
 /*
- * irq handler to notify the core about card insertion/removal
+ * irq handler when (cell-phone) cover is mounted/removed
  */
-static irqreturn_t omap_hsmmc_detect(int irq, void *dev_id)
+static irqreturn_t omap_hsmmc_cover_irq(int irq, void *dev_id)
 {
 	struct omap_hsmmc_host *host = dev_id;
 	int carddetect;
 
 	sysfs_notify(&host->mmc->class_dev.kobj, NULL, "cover_switch");
 
+	if (host->card_detect) {
+		carddetect = host->card_detect(host->dev);
+	} else {
+		omap_hsmmc_protect_card(host);
+		carddetect = -ENOSYS;
+	}
+
+	if (carddetect)
+		mmc_detect_change(host->mmc, (HZ * 200) / 1000);
+	else
+		mmc_detect_change(host->mmc, (HZ * 50) / 1000);
+	return IRQ_HANDLED;
+}
+
+/*
+ * irq handler to notify the core about card insertion/removal
+ */
+static irqreturn_t omap_hsmmc_detect(int irq, void *dev_id)
+{
+	struct omap_hsmmc_host *host = dev_id;
+	int carddetect;
+
 	if (host->card_detect)
 		carddetect = host->card_detect(host->dev);
 	else {
@@ -2164,9 +2184,9 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
 		if (ret < 0)
 			goto err_slot_name;
 	}
-	if (host->card_detect_irq && host->get_cover_state) {
+	if (host->get_cover_state) {
 		ret = device_create_file(&mmc->class_dev,
-					&dev_attr_cover_switch);
+					 &dev_attr_cover_switch);
 		if (ret < 0)
 			goto err_slot_name;
 	}
-- 
2.1.4

[PATCH 4/6] ARM: OMAP2: HSMMC: platform_data: explicit gpio_cover / gpio_cd fields

[Andreas Fenkart]

Cover detection and card detection are not equivalent, cover detection
is like a warning that something might happen (cover removed, card is
accessible), card detection a notification that something has happened.
You could use both in parallel.
Technically this is not possible, since there is only one gpio for both
and a 'cover' flag to indicate either or. With this commit we push that
de-multiplexing of that gpio + flag out of the driver into the platform
specific init code.
It's not pushed down into omap2_hsmmc_info which is used to initialize
the platform data, since we would have to go over all board files
and set the new gpio_cover pin to -EINVAL. That would be dangerous
since '0' is a valid pin number.
FYI: only board-rx51 uses cover_only detection

Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
---
 arch/arm/mach-omap2/hsmmc.c              | 33 ++++++++++++++++++++++++--------
 drivers/mmc/host/omap_hsmmc.c            | 11 ++++++-----
 include/linux/platform_data/hsmmc-omap.h |  6 ++----
 3 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-omap2/hsmmc.c b/arch/arm/mach-omap2/hsmmc.c
index dc6e79c..0e54aac 100644
--- a/arch/arm/mach-omap2/hsmmc.c
+++ b/arch/arm/mach-omap2/hsmmc.c
@@ -150,9 +150,13 @@ static int nop_mmc_set_power(struct device *dev, int power_on, int vdd)
 static inline void omap_hsmmc_mux(struct omap_hsmmc_platform_data
 				  *mmc_controller, int controller_nr)
 {
-	if (gpio_is_valid(mmc_controller->switch_pin) &&
-	    (mmc_controller->switch_pin < OMAP_MAX_GPIO_LINES))
-		omap_mux_init_gpio(mmc_controller->switch_pin,
+	if (gpio_is_valid(mmc_controller->gpio_cd) &&
+	    (mmc_controller->gpio_cd < OMAP_MAX_GPIO_LINES))
+		omap_mux_init_gpio(mmc_controller->gpio_cd,
+				   OMAP_PIN_INPUT_PULLUP);
+	if (gpio_is_valid(mmc_controller->gpio_cover) &&
+	    (mmc_controller->gpio_cover < OMAP_MAX_GPIO_LINES))
+		omap_mux_init_gpio(mmc_controller->gpio_cover,
 				   OMAP_PIN_INPUT_PULLUP);
 	if (gpio_is_valid(mmc_controller->gpio_wp) &&
 	    (mmc_controller->gpio_wp < OMAP_MAX_GPIO_LINES))
@@ -250,15 +254,20 @@ static int __init omap_hsmmc_pdata_init(struct omap2_hsmmc_info *c,
 	mmc->internal_clock = !c->ext_clock;
 	mmc->reg_offset = 0;
 
-	mmc->switch_pin = c->gpio_cd;
+	if (c->cover_only) {
+		/* detect if mobile phone cover removed */
+		mmc->gpio_cd = -EINVAL;
+		mmc->gpio_cover = c->gpio_cd;
+	} else {
+		/* card detect pin on the mmc socket itself */
+		mmc->gpio_cd = c->gpio_cd;
+		mmc->gpio_cover = -EINVAL;
+	}
 	mmc->gpio_wp = c->gpio_wp;
 
 	mmc->remux = c->remux;
 	mmc->init_card = c->init_card;
 
-	if (c->cover_only)
-		mmc->cover = 1;
-
 	if (c->nonremovable)
 		mmc->nonremovable = 1;
 
@@ -358,7 +367,15 @@ void omap_hsmmc_late_init(struct omap2_hsmmc_info *c)
 		if (!mmc_pdata)
 			continue;
 
-		mmc_pdata->switch_pin = c->gpio_cd;
+		if (c->cover_only) {
+			/* detect if mobile phone cover removed */
+			mmc_pdata->gpio_cd = -EINVAL;
+			mmc_pdata->gpio_cover = c->gpio_cd;
+		} else {
+			/* card detect pin on the mmc socket itself */
+			mmc_pdata->gpio_cd = c->gpio_cd;
+			mmc_pdata->gpio_cover = -EINVAL;
+		}
 		mmc_pdata->gpio_wp = c->gpio_wp;
 
 		res = omap_device_register(pdev);
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 38239fb..ed68e55 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -427,15 +427,15 @@ static int omap_hsmmc_gpio_init(struct mmc_host *mmc,
 {
 	int ret;
 
-	if (pdata->cover && gpio_is_valid(pdata->switch_pin)) {
-		ret = mmc_gpio_request_cd(mmc, pdata->switch_pin, 0);
+	if (gpio_is_valid(pdata->gpio_cover)) {
+		ret = mmc_gpio_request_cd(mmc, pdata->gpio_cover, 0);
 		if (ret)
 			return ret;
 
 		host->get_cover_state = omap_hsmmc_get_cover_state;
 		mmc_gpio_set_cd_isr(mmc, omap_hsmmc_cover_irq);
-	} else if (!pdata->cover && gpio_is_valid(pdata->switch_pin)) {
-		ret = mmc_gpio_request_cd(mmc, pdata->switch_pin, 0);
+	} else if (gpio_is_valid(pdata->gpio_cd)) {
+		ret = mmc_gpio_request_cd(mmc, pdata->gpio_cd, 0);
 		if (ret)
 			return ret;
 
@@ -1932,7 +1932,8 @@ static struct omap_hsmmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
 	if (of_find_property(np, "ti,dual-volt", NULL))
 		pdata->controller_flags |= OMAP_HSMMC_SUPPORTS_DUAL_VOLT;
 
-	pdata->switch_pin = -EINVAL;
+	pdata->gpio_cd = -EINVAL;
+	pdata->gpio_cover = -EINVAL;
 	pdata->gpio_wp = -EINVAL;
 
 	if (of_find_property(np, "ti,non-removable", NULL)) {
diff --git a/include/linux/platform_data/hsmmc-omap.h b/include/linux/platform_data/hsmmc-omap.h
index 67bbcf0..9bd6703 100644
--- a/include/linux/platform_data/hsmmc-omap.h
+++ b/include/linux/platform_data/hsmmc-omap.h
@@ -55,9 +55,6 @@ struct omap_hsmmc_platform_data {
 	u32 caps;	/* Used for the MMC driver on 2430 and later */
 	u32 pm_caps;	/* PM capabilities of the mmc */
 
-	/* switch pin can be for card detect (default) or card cover */
-	unsigned cover:1;
-
 	/* use the internal clock */
 	unsigned internal_clock:1;
 
@@ -73,7 +70,8 @@ struct omap_hsmmc_platform_data {
 #define HSMMC_HAS_HSPE_SUPPORT	(1 << 2)
 	unsigned features;
 
-	int switch_pin;			/* gpio (card detect) */
+	int gpio_cd;			/* gpio (card detect) */
+	int gpio_cover;			/* gpio (cover detect) */
 	int gpio_wp;			/* gpio (write protect) */
 
 	int (*set_power)(struct device *dev, int power_on, int vdd);
-- 
2.1.4

[PATCH 5/6] mmc: omap_hsmmc: simplify card/cover detect isr

[Andreas Fenkart]

strip the card dectet logic from cover detect isr and vice versa
the generic mmc_gpio_cd_irqt isr, uses 200ms on removal/insertion,
hence that should be fine here as well

Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
---
 drivers/mmc/host/omap_hsmmc.c | 27 +++------------------------
 1 file changed, 3 insertions(+), 24 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index ed68e55..4f6fbe5 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1239,21 +1239,11 @@ static void omap_hsmmc_protect_card(struct omap_hsmmc_host *host)
 static irqreturn_t omap_hsmmc_cover_irq(int irq, void *dev_id)
 {
 	struct omap_hsmmc_host *host = dev_id;
-	int carddetect;
 
 	sysfs_notify(&host->mmc->class_dev.kobj, NULL, "cover_switch");
 
-	if (host->card_detect) {
-		carddetect = host->card_detect(host->dev);
-	} else {
-		omap_hsmmc_protect_card(host);
-		carddetect = -ENOSYS;
-	}
-
-	if (carddetect)
-		mmc_detect_change(host->mmc, (HZ * 200) / 1000);
-	else
-		mmc_detect_change(host->mmc, (HZ * 50) / 1000);
+	omap_hsmmc_protect_card(host);
+	mmc_detect_change(host->mmc, (HZ * 200) / 1000);
 	return IRQ_HANDLED;
 }
 
@@ -1263,19 +1253,8 @@ static irqreturn_t omap_hsmmc_cover_irq(int irq, void *dev_id)
 static irqreturn_t omap_hsmmc_detect(int irq, void *dev_id)
 {
 	struct omap_hsmmc_host *host = dev_id;
-	int carddetect;
-
-	if (host->card_detect)
-		carddetect = host->card_detect(host->dev);
-	else {
-		omap_hsmmc_protect_card(host);
-		carddetect = -ENOSYS;
-	}
 
-	if (carddetect)
-		mmc_detect_change(host->mmc, (HZ * 200) / 1000);
-	else
-		mmc_detect_change(host->mmc, (HZ * 50) / 1000);
+	mmc_detect_change(host->mmc, (HZ * 200) / 1000);
 	return IRQ_HANDLED;
 }
 
-- 
2.1.4

[PATCH 6/6] mmc: omap_hsmmc: use generic slot-gpio isr to manage card detect pin

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
---
 drivers/mmc/host/omap_hsmmc.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 4f6fbe5..0c3368e 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -418,7 +418,6 @@ static inline int omap_hsmmc_have_reg(void)
 
 #endif
 
-static irqreturn_t omap_hsmmc_detect(int irq, void *dev_id);
 static irqreturn_t omap_hsmmc_cover_irq(int irq, void *dev_id);
 
 static int omap_hsmmc_gpio_init(struct mmc_host *mmc,
@@ -440,7 +439,6 @@ static int omap_hsmmc_gpio_init(struct mmc_host *mmc,
 			return ret;
 
 		host->card_detect = omap_hsmmc_card_detect;
-		mmc_gpio_set_cd_isr(mmc, omap_hsmmc_detect);
 	}
 
 	if (gpio_is_valid(pdata->gpio_wp)) {
@@ -1247,17 +1245,6 @@ static irqreturn_t omap_hsmmc_cover_irq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-/*
- * irq handler to notify the core about card insertion/removal
- */
-static irqreturn_t omap_hsmmc_detect(int irq, void *dev_id)
-{
-	struct omap_hsmmc_host *host = dev_id;
-
-	mmc_detect_change(host->mmc, (HZ * 200) / 1000);
-	return IRQ_HANDLED;
-}
-
 static void omap_hsmmc_dma_callback(void *param)
 {
 	struct omap_hsmmc_host *host = param;
-- 
2.1.4

Re: [PATCH 0/6] mmc: omap_hsmmc: simplify cover/card detect logic

[NeilBrown]

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

On Tue,  3 Mar 2015 13:28:12 +0100 Andreas Fenkart <afenkart@gmail.com> wrote:

> These patches are trying to clean up the cover/card detect logic.
> 
>   Mobile phones (some) have no card detect pin, but
>   can detect if the cover is removed. The purpose is the
>   same; detect if card is being added/removed, but the
>   details differ.
>   When the cover is removed, it does not mean the card is
>   gone. But it might, since it is accessible now. It's like
>   a warning. All the driver does is to limit write access to
>   the card, see protect_card flag. In contrast, card detect
>   notifies us after the fact, e.g. card is gone, card is
>   inserted.
> 
> While cover detect is only used by one platform (rx51), it
> complicates the card detect logic. By separating the code
> paths they both become easier to understand and maintain 
> 
> Patches have been tested by reverting: 95bebb5696ab
> 'mmc: omap_hsmmc: use mmc_of_parse to parse common mmc configuration'
> otherwise gpio detection is handled by mmc_of_parse

Wouldn't it make more sense to put this core  in mmc/core rather than in just
one host controller?  That way it would be available to all hosts, and you
wouldn't need to revert that patch.

NeilBrown

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCH 0/6] mmc: omap_hsmmc: simplify cover/card detect logic

[Andreas Fenkart]

2015-03-04 6:31 GMT+01:00 NeilBrown <neilb@suse.de>:
> On Tue,  3 Mar 2015 13:28:12 +0100 Andreas Fenkart <afenkart@gmail.com> wrote:
>> While cover detect is only used by one platform (rx51), it
>> complicates the card detect logic. By separating the code
>> paths they both become easier to understand and maintain
>>
>> Patches have been tested by reverting: 95bebb5696ab
>> 'mmc: omap_hsmmc: use mmc_of_parse to parse common mmc configuration'
>> otherwise gpio detection is handled by mmc_of_parse
>
> Wouldn't it make more sense to put this core  in mmc/core rather than in just
> one host controller?
Yes, I would very welcome that, since it would free the omap_hsmmc of the
'protection' mechanism that makes the driver clumsy. But as an initial step
separating cover from card detect already seems enough for this series.
Another thought was, that since only one platform is using it, the prospect of
getting it merged into the core was minimal.

> That way it would be available to all hosts, and you
> wouldn't need to revert that patch.
Oh sorry, seems I was unclear. I definitely do not want to revert that patch
permanently, the opposite is true, I very much welcome it. Thanks for your work
by the way.
I only reverted it temporarily so I could test that the card detect
logic still works
with legacy board files, non-device-tree platforms. But since mmc_of_parse is
setting the card detect pin, my changes could not be tested with my
device-tree platform (bbone). So I reverted the above patch only for testing,
afterwards I rebased the patches on top of it, before submitting


Andreas

Re: [PATCH 0/6] mmc: omap_hsmmc: simplify cover/card detect logic

[Ulf Hansson]

On 3 March 2015 at 13:28, Andreas Fenkart <afenkart@gmail.com> wrote:
> These patches are trying to clean up the cover/card detect logic.
>
>   Mobile phones (some) have no card detect pin, but
>   can detect if the cover is removed. The purpose is the
>   same; detect if card is being added/removed, but the
>   details differ.
>   When the cover is removed, it does not mean the card is
>   gone. But it might, since it is accessible now. It's like
>   a warning. All the driver does is to limit write access to
>   the card, see protect_card flag. In contrast, card detect
>   notifies us after the fact, e.g. card is gone, card is
>   inserted.
>
> While cover detect is only used by one platform (rx51), it
> complicates the card detect logic. By separating the code
> paths they both become easier to understand and maintain
>
> Patches have been tested by reverting: 95bebb5696ab
> 'mmc: omap_hsmmc: use mmc_of_parse to parse common mmc configuration'
> otherwise gpio detection is handled by mmc_of_parse
>
> compile tested
> OMAP2:
> CONFIG_MACH_OMAP2_TUSB6010=y
> CONFIG_MACH_OMAP3_BEAGLE=y
> CONFIG_MACH_DEVKIT8000=y
> CONFIG_MACH_OMAP_LDP=y
> CONFIG_MACH_OMAP3530_LV_SOM=y
> CONFIG_MACH_OMAP3_TORPEDO=y
> CONFIG_MACH_OVERO=y
> CONFIG_MACH_OMAP3517EVM=y
> CONFIG_MACH_CRANEBOARD=y
> CONFIG_MACH_OMAP3_PANDORA=y
> CONFIG_MACH_TOUCHBOOK=y
> CONFIG_MACH_OMAP_3430SDP=y
> CONFIG_MACH_NOKIA_N810=y
> CONFIG_MACH_NOKIA_N810_WIMAX=y
> CONFIG_MACH_NOKIA_N8X0=y
> CONFIG_MACH_NOKIA_RX51=y
> CONFIG_MACH_CM_T35=y
> CONFIG_MACH_CM_T3517=y
> CONFIG_MACH_CM_T3730=y
> CONFIG_MACH_SBC3530=y
> CONFIG_MACH_TI8168EVM=y
> CONFIG_MACH_TI8148EVM=y
>
> Andreas Fenkart (6):
>   mmc: omap_hsmmc: remove unused fields from struct omap_hsmmc_host
>   mmc: omap_hsmmc: use slot-gpio functions to manage read-only pin
>     directly
>   mmc: omap_hsmmc: use distinctive code paths for cover / card detect
>     logic
>   ARM: OMAP2: HSMMC: platform_data: explicit gpio_cover / gpio_cd fields
>   mmc: omap_hsmmc: simplify card/cover detect isr
>   mmc: omap_hsmmc: use generic slot-gpio isr to manage card detect pin
>
>  arch/arm/mach-omap2/hsmmc.c              | 33 +++++++++----
>  drivers/mmc/host/omap_hsmmc.c            | 80 +++++++++-----------------------
>  include/linux/platform_data/hsmmc-omap.h |  6 +--
>  3 files changed, 48 insertions(+), 71 deletions(-)
>
> --
> 2.1.4
>

Applied patch 1->3, thanks!

If I get an ack for patch 4 (ARM) and can take them all through my tree.

Kind regards
Uffe

Re: [PATCH 5/6] mmc: omap_hsmmc: simplify card/cover detect isr

[Ulf Hansson]

On 3 March 2015 at 13:28, Andreas Fenkart <afenkart@gmail.com> wrote:
> strip the card dectet logic from cover detect isr and vice versa
> the generic mmc_gpio_cd_irqt isr, uses 200ms on removal/insertion,
> hence that should be fine here as well
>
> Signed-off-by: Andreas Fenkart <afenkart@gmail.com>

Thanks! Applied.

Kind regards
Uffe

> ---
>  drivers/mmc/host/omap_hsmmc.c | 27 +++------------------------
>  1 file changed, 3 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index ed68e55..4f6fbe5 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1239,21 +1239,11 @@ static void omap_hsmmc_protect_card(struct omap_hsmmc_host *host)
>  static irqreturn_t omap_hsmmc_cover_irq(int irq, void *dev_id)
>  {
>         struct omap_hsmmc_host *host = dev_id;
> -       int carddetect;
>
>         sysfs_notify(&host->mmc->class_dev.kobj, NULL, "cover_switch");
>
> -       if (host->card_detect) {
> -               carddetect = host->card_detect(host->dev);
> -       } else {
> -               omap_hsmmc_protect_card(host);
> -               carddetect = -ENOSYS;
> -       }
> -
> -       if (carddetect)
> -               mmc_detect_change(host->mmc, (HZ * 200) / 1000);
> -       else
> -               mmc_detect_change(host->mmc, (HZ * 50) / 1000);
> +       omap_hsmmc_protect_card(host);
> +       mmc_detect_change(host->mmc, (HZ * 200) / 1000);
>         return IRQ_HANDLED;
>  }
>
> @@ -1263,19 +1253,8 @@ static irqreturn_t omap_hsmmc_cover_irq(int irq, void *dev_id)
>  static irqreturn_t omap_hsmmc_detect(int irq, void *dev_id)
>  {
>         struct omap_hsmmc_host *host = dev_id;
> -       int carddetect;
> -
> -       if (host->card_detect)
> -               carddetect = host->card_detect(host->dev);
> -       else {
> -               omap_hsmmc_protect_card(host);
> -               carddetect = -ENOSYS;
> -       }
>
> -       if (carddetect)
> -               mmc_detect_change(host->mmc, (HZ * 200) / 1000);
> -       else
> -               mmc_detect_change(host->mmc, (HZ * 50) / 1000);
> +       mmc_detect_change(host->mmc, (HZ * 200) / 1000);
>         return IRQ_HANDLED;
>  }
>
> --
> 2.1.4
>

Re: [PATCH 6/6] mmc: omap_hsmmc: use generic slot-gpio isr to manage card detect pin

[Ulf Hansson]

On 3 March 2015 at 13:28, Andreas Fenkart <afenkart@gmail.com> wrote:
> Signed-off-by: Andreas Fenkart <afenkart@gmail.com>

Thanks! Applied.

Kind regards
Uffe


> ---
>  drivers/mmc/host/omap_hsmmc.c | 13 -------------
>  1 file changed, 13 deletions(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 4f6fbe5..0c3368e 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -418,7 +418,6 @@ static inline int omap_hsmmc_have_reg(void)
>
>  #endif
>
> -static irqreturn_t omap_hsmmc_detect(int irq, void *dev_id);
>  static irqreturn_t omap_hsmmc_cover_irq(int irq, void *dev_id);
>
>  static int omap_hsmmc_gpio_init(struct mmc_host *mmc,
> @@ -440,7 +439,6 @@ static int omap_hsmmc_gpio_init(struct mmc_host *mmc,
>                         return ret;
>
>                 host->card_detect = omap_hsmmc_card_detect;
> -               mmc_gpio_set_cd_isr(mmc, omap_hsmmc_detect);
>         }
>
>         if (gpio_is_valid(pdata->gpio_wp)) {
> @@ -1247,17 +1245,6 @@ static irqreturn_t omap_hsmmc_cover_irq(int irq, void *dev_id)
>         return IRQ_HANDLED;
>  }
>
> -/*
> - * irq handler to notify the core about card insertion/removal
> - */
> -static irqreturn_t omap_hsmmc_detect(int irq, void *dev_id)
> -{
> -       struct omap_hsmmc_host *host = dev_id;
> -
> -       mmc_detect_change(host->mmc, (HZ * 200) / 1000);
> -       return IRQ_HANDLED;
> -}
> -
>  static void omap_hsmmc_dma_callback(void *param)
>  {
>         struct omap_hsmmc_host *host = param;
> --
> 2.1.4
>