2014-10-14 16:33:20

by Doug Anderson

[permalink] [raw]
Subject: [PATCH] mmc: dw_mmc: Remove old card detect infrastructure

The dw_mmc driver had a bunch of code that ran whenever a card was
ejected and inserted. However, this code was old and crufty and
should be removed. Some evidence that it's really not needed:

1. Is is supposed to be legal to use 'cd-gpio' on dw_mmc instead of
using the built-in card detect mechanism. The 'cd-gpio' code
doesn't run any of the crufty old code but yet still works.

2. While looking at this, I realized that my old change (369ac86 mmc:
dw_mmc: don't queue up a card detect at slot startup) actually
castrated the old code a little bit already and nobody noticed.
Specifically "last_detect_state" was left as 0 at bootup. That
means that on the first card removal none of the crufty code ran.

3. I can run "while true; do dd if=/dev/mmcblk1 of=/dev/null; done"
while ejecting and inserting an SD Card and the world doesn't
explode.

If some of the crufty old code is actually needed, we should justify
it and also put it in some place where it will be run even with
"cd-gpio".

Note that in my case I'm using the "cd-gpio" mechanism but for various
reasons the hardware triggers a dw_mmc "card detect" at bootup. That
was actually causing a real bug. The card detect workqueue was
running while the system was trying to enumerate the card. The
"present != slot->last_detect_state" triggered and we were doing all
kinds of crazy stuff and messing up enumeration. The new mechanism of
just asking the core to check the card is much safer and then the
bogus interrupt doesn't hurt.

Signed-off-by: Doug Anderson <[email protected]>
---
drivers/mmc/host/dw_mmc.c | 121 ++++++++-------------------------------------
drivers/mmc/host/dw_mmc.h | 2 -
include/linux/mmc/dw_mmc.h | 2 -
3 files changed, 20 insertions(+), 105 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 69f0cc6..6a86495 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -34,7 +34,6 @@
#include <linux/mmc/dw_mmc.h>
#include <linux/bitops.h>
#include <linux/regulator/consumer.h>
-#include <linux/workqueue.h>
#include <linux/of.h>
#include <linux/of_gpio.h>
#include <linux/mmc/slot-gpio.h>
@@ -1954,6 +1953,23 @@ static void dw_mci_cmd_interrupt(struct dw_mci *host, u32 status)
tasklet_schedule(&host->tasklet);
}

+static void dw_mci_handle_cd(struct dw_mci *host)
+{
+ int i;
+
+ for (i = 0; i < host->num_slots; i++) {
+ struct dw_mci_slot *slot = host->slot[i];
+
+ if (!slot)
+ continue;
+
+ if (slot->mmc->ops->card_event)
+ slot->mmc->ops->card_event(slot->mmc);
+ mmc_detect_change(slot->mmc,
+ msecs_to_jiffies(host->pdata->detect_delay_ms));
+ }
+}
+
static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
{
struct dw_mci *host = dev_id;
@@ -2029,7 +2045,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)

if (pending & SDMMC_INT_CD) {
mci_writel(host, RINTSTS, SDMMC_INT_CD);
- queue_work(host->card_workqueue, &host->card_work);
+ dw_mci_handle_cd(host);
}

/* Handle SDIO Interrupts */
@@ -2056,88 +2072,6 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}

-static void dw_mci_work_routine_card(struct work_struct *work)
-{
- struct dw_mci *host = container_of(work, struct dw_mci, card_work);
- int i;
-
- for (i = 0; i < host->num_slots; i++) {
- struct dw_mci_slot *slot = host->slot[i];
- struct mmc_host *mmc = slot->mmc;
- struct mmc_request *mrq;
- int present;
-
- present = dw_mci_get_cd(mmc);
- while (present != slot->last_detect_state) {
- dev_dbg(&slot->mmc->class_dev, "card %s\n",
- present ? "inserted" : "removed");
-
- spin_lock_bh(&host->lock);
-
- /* Card change detected */
- slot->last_detect_state = present;
-
- /* Clean up queue if present */
- mrq = slot->mrq;
- if (mrq) {
- if (mrq == host->mrq) {
- host->data = NULL;
- host->cmd = NULL;
-
- switch (host->state) {
- case STATE_IDLE:
- case STATE_WAITING_CMD11_DONE:
- break;
- case STATE_SENDING_CMD11:
- case STATE_SENDING_CMD:
- mrq->cmd->error = -ENOMEDIUM;
- if (!mrq->data)
- break;
- /* fall through */
- case STATE_SENDING_DATA:
- mrq->data->error = -ENOMEDIUM;
- dw_mci_stop_dma(host);
- break;
- case STATE_DATA_BUSY:
- case STATE_DATA_ERROR:
- if (mrq->data->error == -EINPROGRESS)
- mrq->data->error = -ENOMEDIUM;
- /* fall through */
- case STATE_SENDING_STOP:
- if (mrq->stop)
- mrq->stop->error = -ENOMEDIUM;
- break;
- }
-
- dw_mci_request_end(host, mrq);
- } else {
- list_del(&slot->queue_node);
- mrq->cmd->error = -ENOMEDIUM;
- if (mrq->data)
- mrq->data->error = -ENOMEDIUM;
- if (mrq->stop)
- mrq->stop->error = -ENOMEDIUM;
-
- spin_unlock(&host->lock);
- mmc_request_done(slot->mmc, mrq);
- spin_lock(&host->lock);
- }
- }
-
- /* Power down slot */
- if (present == 0)
- dw_mci_reset(host);
-
- spin_unlock_bh(&host->lock);
-
- present = dw_mci_get_cd(mmc);
- }
-
- mmc_detect_change(slot->mmc,
- msecs_to_jiffies(host->pdata->detect_delay_ms));
- }
-}
-
#ifdef CONFIG_OF
/* given a slot id, find out the device node representing that slot */
static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
@@ -2289,9 +2223,6 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
dw_mci_init_debugfs(slot);
#endif

- /* Card initially undetected */
- slot->last_detect_state = 0;
-
return 0;

err_host_allocated:
@@ -2672,17 +2603,10 @@ int dw_mci_probe(struct dw_mci *host)
host->data_offset = DATA_240A_OFFSET;

tasklet_init(&host->tasklet, dw_mci_tasklet_func, (unsigned long)host);
- host->card_workqueue = alloc_workqueue("dw-mci-card",
- WQ_MEM_RECLAIM, 1);
- if (!host->card_workqueue) {
- ret = -ENOMEM;
- goto err_dmaunmap;
- }
- INIT_WORK(&host->card_work, dw_mci_work_routine_card);
ret = devm_request_irq(host->dev, host->irq, dw_mci_interrupt,
host->irq_flags, "dw-mci", host);
if (ret)
- goto err_workqueue;
+ goto err_dmaunmap;

if (host->pdata->num_slots)
host->num_slots = host->pdata->num_slots;
@@ -2718,7 +2642,7 @@ int dw_mci_probe(struct dw_mci *host)
} else {
dev_dbg(host->dev, "attempted to initialize %d slots, "
"but failed on all\n", host->num_slots);
- goto err_workqueue;
+ goto err_dmaunmap;
}

if (host->quirks & DW_MCI_QUIRK_IDMAC_DTO)
@@ -2726,9 +2650,6 @@ int dw_mci_probe(struct dw_mci *host)

return 0;

-err_workqueue:
- destroy_workqueue(host->card_workqueue);
-
err_dmaunmap:
if (host->use_dma && host->dma_ops->exit)
host->dma_ops->exit(host);
@@ -2762,8 +2683,6 @@ void dw_mci_remove(struct dw_mci *host)
mci_writel(host, CLKENA, 0);
mci_writel(host, CLKSRC, 0);

- destroy_workqueue(host->card_workqueue);
-
if (host->use_dma && host->dma_ops->exit)
host->dma_ops->exit(host);

diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 01b99e8..71d4995 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -214,7 +214,6 @@ extern int dw_mci_resume(struct dw_mci *host);
* with CONFIG_MMC_CLKGATE.
* @flags: Random state bits associated with the slot.
* @id: Number of this slot.
- * @last_detect_state: Most recently observed card detect state.
*/
struct dw_mci_slot {
struct mmc_host *mmc;
@@ -234,7 +233,6 @@ struct dw_mci_slot {
#define DW_MMC_CARD_PRESENT 0
#define DW_MMC_CARD_NEED_INIT 1
int id;
- int last_detect_state;
};

struct dw_mci_tuning_data {
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 0013669..69d0814 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -135,7 +135,6 @@ struct dw_mci {
struct mmc_command stop_abort;
unsigned int prev_blksz;
unsigned char timing;
- struct workqueue_struct *card_workqueue;

/* DMA interface members*/
int use_dma;
@@ -154,7 +153,6 @@ struct dw_mci {
u32 stop_cmdr;
u32 dir_status;
struct tasklet_struct tasklet;
- struct work_struct card_work;
unsigned long pending_events;
unsigned long completed_events;
enum dw_mci_state state;
--
2.1.0.rc2.206.gedb03e5


2014-10-16 08:19:36

by Jaehoon Chung

[permalink] [raw]
Subject: Re: [PATCH] mmc: dw_mmc: Remove old card detect infrastructure

Hi, Doug.

Looks good to me.

Tested-by: Jaehoon Chung <[email protected]>
with exynos3/4 series.

Need to check exynos5 with using other "card detect" mechanism.

Best Regards,
Jaehoon Chung

On 10/15/2014 01:33 AM, Doug Anderson wrote:
> The dw_mmc driver had a bunch of code that ran whenever a card was
> ejected and inserted. However, this code was old and crufty and
> should be removed. Some evidence that it's really not needed:
>
> 1. Is is supposed to be legal to use 'cd-gpio' on dw_mmc instead of
> using the built-in card detect mechanism. The 'cd-gpio' code
> doesn't run any of the crufty old code but yet still works.
>
> 2. While looking at this, I realized that my old change (369ac86 mmc:
> dw_mmc: don't queue up a card detect at slot startup) actually
> castrated the old code a little bit already and nobody noticed.
> Specifically "last_detect_state" was left as 0 at bootup. That
> means that on the first card removal none of the crufty code ran.
>
> 3. I can run "while true; do dd if=/dev/mmcblk1 of=/dev/null; done"
> while ejecting and inserting an SD Card and the world doesn't
> explode.
>
> If some of the crufty old code is actually needed, we should justify
> it and also put it in some place where it will be run even with
> "cd-gpio".
>
> Note that in my case I'm using the "cd-gpio" mechanism but for various
> reasons the hardware triggers a dw_mmc "card detect" at bootup. That
> was actually causing a real bug. The card detect workqueue was
> running while the system was trying to enumerate the card. The
> "present != slot->last_detect_state" triggered and we were doing all
> kinds of crazy stuff and messing up enumeration. The new mechanism of
> just asking the core to check the card is much safer and then the
> bogus interrupt doesn't hurt.
>
> Signed-off-by: Doug Anderson <[email protected]>
> ---
> drivers/mmc/host/dw_mmc.c | 121 ++++++++-------------------------------------
> drivers/mmc/host/dw_mmc.h | 2 -
> include/linux/mmc/dw_mmc.h | 2 -
> 3 files changed, 20 insertions(+), 105 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 69f0cc6..6a86495 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -34,7 +34,6 @@
> #include <linux/mmc/dw_mmc.h>
> #include <linux/bitops.h>
> #include <linux/regulator/consumer.h>
> -#include <linux/workqueue.h>
> #include <linux/of.h>
> #include <linux/of_gpio.h>
> #include <linux/mmc/slot-gpio.h>
> @@ -1954,6 +1953,23 @@ static void dw_mci_cmd_interrupt(struct dw_mci *host, u32 status)
> tasklet_schedule(&host->tasklet);
> }
>
> +static void dw_mci_handle_cd(struct dw_mci *host)
> +{
> + int i;
> +
> + for (i = 0; i < host->num_slots; i++) {
> + struct dw_mci_slot *slot = host->slot[i];
> +
> + if (!slot)
> + continue;
> +
> + if (slot->mmc->ops->card_event)
> + slot->mmc->ops->card_event(slot->mmc);
> + mmc_detect_change(slot->mmc,
> + msecs_to_jiffies(host->pdata->detect_delay_ms));
> + }
> +}
> +
> static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
> {
> struct dw_mci *host = dev_id;
> @@ -2029,7 +2045,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>
> if (pending & SDMMC_INT_CD) {
> mci_writel(host, RINTSTS, SDMMC_INT_CD);
> - queue_work(host->card_workqueue, &host->card_work);
> + dw_mci_handle_cd(host);
> }
>
> /* Handle SDIO Interrupts */
> @@ -2056,88 +2072,6 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> -static void dw_mci_work_routine_card(struct work_struct *work)
> -{
> - struct dw_mci *host = container_of(work, struct dw_mci, card_work);
> - int i;
> -
> - for (i = 0; i < host->num_slots; i++) {
> - struct dw_mci_slot *slot = host->slot[i];
> - struct mmc_host *mmc = slot->mmc;
> - struct mmc_request *mrq;
> - int present;
> -
> - present = dw_mci_get_cd(mmc);
> - while (present != slot->last_detect_state) {
> - dev_dbg(&slot->mmc->class_dev, "card %s\n",
> - present ? "inserted" : "removed");
> -
> - spin_lock_bh(&host->lock);
> -
> - /* Card change detected */
> - slot->last_detect_state = present;
> -
> - /* Clean up queue if present */
> - mrq = slot->mrq;
> - if (mrq) {
> - if (mrq == host->mrq) {
> - host->data = NULL;
> - host->cmd = NULL;
> -
> - switch (host->state) {
> - case STATE_IDLE:
> - case STATE_WAITING_CMD11_DONE:
> - break;
> - case STATE_SENDING_CMD11:
> - case STATE_SENDING_CMD:
> - mrq->cmd->error = -ENOMEDIUM;
> - if (!mrq->data)
> - break;
> - /* fall through */
> - case STATE_SENDING_DATA:
> - mrq->data->error = -ENOMEDIUM;
> - dw_mci_stop_dma(host);
> - break;
> - case STATE_DATA_BUSY:
> - case STATE_DATA_ERROR:
> - if (mrq->data->error == -EINPROGRESS)
> - mrq->data->error = -ENOMEDIUM;
> - /* fall through */
> - case STATE_SENDING_STOP:
> - if (mrq->stop)
> - mrq->stop->error = -ENOMEDIUM;
> - break;
> - }
> -
> - dw_mci_request_end(host, mrq);
> - } else {
> - list_del(&slot->queue_node);
> - mrq->cmd->error = -ENOMEDIUM;
> - if (mrq->data)
> - mrq->data->error = -ENOMEDIUM;
> - if (mrq->stop)
> - mrq->stop->error = -ENOMEDIUM;
> -
> - spin_unlock(&host->lock);
> - mmc_request_done(slot->mmc, mrq);
> - spin_lock(&host->lock);
> - }
> - }
> -
> - /* Power down slot */
> - if (present == 0)
> - dw_mci_reset(host);
> -
> - spin_unlock_bh(&host->lock);
> -
> - present = dw_mci_get_cd(mmc);
> - }
> -
> - mmc_detect_change(slot->mmc,
> - msecs_to_jiffies(host->pdata->detect_delay_ms));
> - }
> -}
> -
> #ifdef CONFIG_OF
> /* given a slot id, find out the device node representing that slot */
> static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
> @@ -2289,9 +2223,6 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
> dw_mci_init_debugfs(slot);
> #endif
>
> - /* Card initially undetected */
> - slot->last_detect_state = 0;
> -
> return 0;
>
> err_host_allocated:
> @@ -2672,17 +2603,10 @@ int dw_mci_probe(struct dw_mci *host)
> host->data_offset = DATA_240A_OFFSET;
>
> tasklet_init(&host->tasklet, dw_mci_tasklet_func, (unsigned long)host);
> - host->card_workqueue = alloc_workqueue("dw-mci-card",
> - WQ_MEM_RECLAIM, 1);
> - if (!host->card_workqueue) {
> - ret = -ENOMEM;
> - goto err_dmaunmap;
> - }
> - INIT_WORK(&host->card_work, dw_mci_work_routine_card);
> ret = devm_request_irq(host->dev, host->irq, dw_mci_interrupt,
> host->irq_flags, "dw-mci", host);
> if (ret)
> - goto err_workqueue;
> + goto err_dmaunmap;
>
> if (host->pdata->num_slots)
> host->num_slots = host->pdata->num_slots;
> @@ -2718,7 +2642,7 @@ int dw_mci_probe(struct dw_mci *host)
> } else {
> dev_dbg(host->dev, "attempted to initialize %d slots, "
> "but failed on all\n", host->num_slots);
> - goto err_workqueue;
> + goto err_dmaunmap;
> }
>
> if (host->quirks & DW_MCI_QUIRK_IDMAC_DTO)
> @@ -2726,9 +2650,6 @@ int dw_mci_probe(struct dw_mci *host)
>
> return 0;
>
> -err_workqueue:
> - destroy_workqueue(host->card_workqueue);
> -
> err_dmaunmap:
> if (host->use_dma && host->dma_ops->exit)
> host->dma_ops->exit(host);
> @@ -2762,8 +2683,6 @@ void dw_mci_remove(struct dw_mci *host)
> mci_writel(host, CLKENA, 0);
> mci_writel(host, CLKSRC, 0);
>
> - destroy_workqueue(host->card_workqueue);
> -
> if (host->use_dma && host->dma_ops->exit)
> host->dma_ops->exit(host);
>
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 01b99e8..71d4995 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -214,7 +214,6 @@ extern int dw_mci_resume(struct dw_mci *host);
> * with CONFIG_MMC_CLKGATE.
> * @flags: Random state bits associated with the slot.
> * @id: Number of this slot.
> - * @last_detect_state: Most recently observed card detect state.
> */
> struct dw_mci_slot {
> struct mmc_host *mmc;
> @@ -234,7 +233,6 @@ struct dw_mci_slot {
> #define DW_MMC_CARD_PRESENT 0
> #define DW_MMC_CARD_NEED_INIT 1
> int id;
> - int last_detect_state;
> };
>
> struct dw_mci_tuning_data {
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index 0013669..69d0814 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -135,7 +135,6 @@ struct dw_mci {
> struct mmc_command stop_abort;
> unsigned int prev_blksz;
> unsigned char timing;
> - struct workqueue_struct *card_workqueue;
>
> /* DMA interface members*/
> int use_dma;
> @@ -154,7 +153,6 @@ struct dw_mci {
> u32 stop_cmdr;
> u32 dir_status;
> struct tasklet_struct tasklet;
> - struct work_struct card_work;
> unsigned long pending_events;
> unsigned long completed_events;
> enum dw_mci_state state;
>

2014-10-16 09:38:16

by Jaehoon Chung

[permalink] [raw]
Subject: Re: [PATCH] mmc: dw_mmc: Remove old card detect infrastructure

Hi, Doug.

Add one comment.

On 10/16/2014 05:19 PM, Jaehoon Chung wrote:
> Hi, Doug.
>
> Looks good to me.
>
> Tested-by: Jaehoon Chung <[email protected]>
> with exynos3/4 series.
>
> Need to check exynos5 with using other "card detect" mechanism.
>
> Best Regards,
> Jaehoon Chung
>
> On 10/15/2014 01:33 AM, Doug Anderson wrote:
>> The dw_mmc driver had a bunch of code that ran whenever a card was
>> ejected and inserted. However, this code was old and crufty and
>> should be removed. Some evidence that it's really not needed:
>>
>> 1. Is is supposed to be legal to use 'cd-gpio' on dw_mmc instead of
>> using the built-in card detect mechanism. The 'cd-gpio' code
>> doesn't run any of the crufty old code but yet still works.
>>
>> 2. While looking at this, I realized that my old change (369ac86 mmc:
>> dw_mmc: don't queue up a card detect at slot startup) actually
>> castrated the old code a little bit already and nobody noticed.
>> Specifically "last_detect_state" was left as 0 at bootup. That
>> means that on the first card removal none of the crufty code ran.
>>
>> 3. I can run "while true; do dd if=/dev/mmcblk1 of=/dev/null; done"
>> while ejecting and inserting an SD Card and the world doesn't
>> explode.
>>
>> If some of the crufty old code is actually needed, we should justify
>> it and also put it in some place where it will be run even with
>> "cd-gpio".
>>
>> Note that in my case I'm using the "cd-gpio" mechanism but for various
>> reasons the hardware triggers a dw_mmc "card detect" at bootup. That
>> was actually causing a real bug. The card detect workqueue was
>> running while the system was trying to enumerate the card. The
>> "present != slot->last_detect_state" triggered and we were doing all
>> kinds of crazy stuff and messing up enumeration. The new mechanism of
>> just asking the core to check the card is much safer and then the
>> bogus interrupt doesn't hurt.

Did you read the Card-detect Recommendation?
There is a Recommendation for Card-detect(CDT) at TRM.
We don't read the CDETECT register(0x50) anywhere. Could you share this register value after detecting?

And I think "last_detect_state" is used for "software should read card detect register and store in memory."

Best Regards,
Jaehoon Chung

>>
>> Signed-off-by: Doug Anderson <[email protected]>
>> ---
>> drivers/mmc/host/dw_mmc.c | 121 ++++++++-------------------------------------
>> drivers/mmc/host/dw_mmc.h | 2 -
>> include/linux/mmc/dw_mmc.h | 2 -
>> 3 files changed, 20 insertions(+), 105 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 69f0cc6..6a86495 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -34,7 +34,6 @@
>> #include <linux/mmc/dw_mmc.h>
>> #include <linux/bitops.h>
>> #include <linux/regulator/consumer.h>
>> -#include <linux/workqueue.h>
>> #include <linux/of.h>
>> #include <linux/of_gpio.h>
>> #include <linux/mmc/slot-gpio.h>
>> @@ -1954,6 +1953,23 @@ static void dw_mci_cmd_interrupt(struct dw_mci *host, u32 status)
>> tasklet_schedule(&host->tasklet);
>> }
>>
>> +static void dw_mci_handle_cd(struct dw_mci *host)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < host->num_slots; i++) {
>> + struct dw_mci_slot *slot = host->slot[i];
>> +
>> + if (!slot)
>> + continue;
>> +
>> + if (slot->mmc->ops->card_event)
>> + slot->mmc->ops->card_event(slot->mmc);
>> + mmc_detect_change(slot->mmc,
>> + msecs_to_jiffies(host->pdata->detect_delay_ms));
>> + }
>> +}
>> +
>> static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>> {
>> struct dw_mci *host = dev_id;
>> @@ -2029,7 +2045,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>>
>> if (pending & SDMMC_INT_CD) {
>> mci_writel(host, RINTSTS, SDMMC_INT_CD);
>> - queue_work(host->card_workqueue, &host->card_work);
>> + dw_mci_handle_cd(host);
>> }
>>
>> /* Handle SDIO Interrupts */
>> @@ -2056,88 +2072,6 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>> return IRQ_HANDLED;
>> }
>>
>> -static void dw_mci_work_routine_card(struct work_struct *work)
>> -{
>> - struct dw_mci *host = container_of(work, struct dw_mci, card_work);
>> - int i;
>> -
>> - for (i = 0; i < host->num_slots; i++) {
>> - struct dw_mci_slot *slot = host->slot[i];
>> - struct mmc_host *mmc = slot->mmc;
>> - struct mmc_request *mrq;
>> - int present;
>> -
>> - present = dw_mci_get_cd(mmc);
>> - while (present != slot->last_detect_state) {
>> - dev_dbg(&slot->mmc->class_dev, "card %s\n",
>> - present ? "inserted" : "removed");
>> -
>> - spin_lock_bh(&host->lock);
>> -
>> - /* Card change detected */
>> - slot->last_detect_state = present;
>> -
>> - /* Clean up queue if present */
>> - mrq = slot->mrq;
>> - if (mrq) {
>> - if (mrq == host->mrq) {
>> - host->data = NULL;
>> - host->cmd = NULL;
>> -
>> - switch (host->state) {
>> - case STATE_IDLE:
>> - case STATE_WAITING_CMD11_DONE:
>> - break;
>> - case STATE_SENDING_CMD11:
>> - case STATE_SENDING_CMD:
>> - mrq->cmd->error = -ENOMEDIUM;
>> - if (!mrq->data)
>> - break;
>> - /* fall through */
>> - case STATE_SENDING_DATA:
>> - mrq->data->error = -ENOMEDIUM;
>> - dw_mci_stop_dma(host);
>> - break;
>> - case STATE_DATA_BUSY:
>> - case STATE_DATA_ERROR:
>> - if (mrq->data->error == -EINPROGRESS)
>> - mrq->data->error = -ENOMEDIUM;
>> - /* fall through */
>> - case STATE_SENDING_STOP:
>> - if (mrq->stop)
>> - mrq->stop->error = -ENOMEDIUM;
>> - break;
>> - }
>> -
>> - dw_mci_request_end(host, mrq);
>> - } else {
>> - list_del(&slot->queue_node);
>> - mrq->cmd->error = -ENOMEDIUM;
>> - if (mrq->data)
>> - mrq->data->error = -ENOMEDIUM;
>> - if (mrq->stop)
>> - mrq->stop->error = -ENOMEDIUM;
>> -
>> - spin_unlock(&host->lock);
>> - mmc_request_done(slot->mmc, mrq);
>> - spin_lock(&host->lock);
>> - }
>> - }
>> -
>> - /* Power down slot */
>> - if (present == 0)
>> - dw_mci_reset(host);
>> -
>> - spin_unlock_bh(&host->lock);
>> -
>> - present = dw_mci_get_cd(mmc);
>> - }
>> -
>> - mmc_detect_change(slot->mmc,
>> - msecs_to_jiffies(host->pdata->detect_delay_ms));
>> - }
>> -}
>> -
>> #ifdef CONFIG_OF
>> /* given a slot id, find out the device node representing that slot */
>> static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
>> @@ -2289,9 +2223,6 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>> dw_mci_init_debugfs(slot);
>> #endif
>>
>> - /* Card initially undetected */
>> - slot->last_detect_state = 0;
>> -
>> return 0;
>>
>> err_host_allocated:
>> @@ -2672,17 +2603,10 @@ int dw_mci_probe(struct dw_mci *host)
>> host->data_offset = DATA_240A_OFFSET;
>>
>> tasklet_init(&host->tasklet, dw_mci_tasklet_func, (unsigned long)host);
>> - host->card_workqueue = alloc_workqueue("dw-mci-card",
>> - WQ_MEM_RECLAIM, 1);
>> - if (!host->card_workqueue) {
>> - ret = -ENOMEM;
>> - goto err_dmaunmap;
>> - }
>> - INIT_WORK(&host->card_work, dw_mci_work_routine_card);
>> ret = devm_request_irq(host->dev, host->irq, dw_mci_interrupt,
>> host->irq_flags, "dw-mci", host);
>> if (ret)
>> - goto err_workqueue;
>> + goto err_dmaunmap;
>>
>> if (host->pdata->num_slots)
>> host->num_slots = host->pdata->num_slots;
>> @@ -2718,7 +2642,7 @@ int dw_mci_probe(struct dw_mci *host)
>> } else {
>> dev_dbg(host->dev, "attempted to initialize %d slots, "
>> "but failed on all\n", host->num_slots);
>> - goto err_workqueue;
>> + goto err_dmaunmap;
>> }
>>
>> if (host->quirks & DW_MCI_QUIRK_IDMAC_DTO)
>> @@ -2726,9 +2650,6 @@ int dw_mci_probe(struct dw_mci *host)
>>
>> return 0;
>>
>> -err_workqueue:
>> - destroy_workqueue(host->card_workqueue);
>> -
>> err_dmaunmap:
>> if (host->use_dma && host->dma_ops->exit)
>> host->dma_ops->exit(host);
>> @@ -2762,8 +2683,6 @@ void dw_mci_remove(struct dw_mci *host)
>> mci_writel(host, CLKENA, 0);
>> mci_writel(host, CLKSRC, 0);
>>
>> - destroy_workqueue(host->card_workqueue);
>> -
>> if (host->use_dma && host->dma_ops->exit)
>> host->dma_ops->exit(host);
>>
>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>> index 01b99e8..71d4995 100644
>> --- a/drivers/mmc/host/dw_mmc.h
>> +++ b/drivers/mmc/host/dw_mmc.h
>> @@ -214,7 +214,6 @@ extern int dw_mci_resume(struct dw_mci *host);
>> * with CONFIG_MMC_CLKGATE.
>> * @flags: Random state bits associated with the slot.
>> * @id: Number of this slot.
>> - * @last_detect_state: Most recently observed card detect state.
>> */
>> struct dw_mci_slot {
>> struct mmc_host *mmc;
>> @@ -234,7 +233,6 @@ struct dw_mci_slot {
>> #define DW_MMC_CARD_PRESENT 0
>> #define DW_MMC_CARD_NEED_INIT 1
>> int id;
>> - int last_detect_state;
>> };
>>
>> struct dw_mci_tuning_data {
>> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
>> index 0013669..69d0814 100644
>> --- a/include/linux/mmc/dw_mmc.h
>> +++ b/include/linux/mmc/dw_mmc.h
>> @@ -135,7 +135,6 @@ struct dw_mci {
>> struct mmc_command stop_abort;
>> unsigned int prev_blksz;
>> unsigned char timing;
>> - struct workqueue_struct *card_workqueue;
>>
>> /* DMA interface members*/
>> int use_dma;
>> @@ -154,7 +153,6 @@ struct dw_mci {
>> u32 stop_cmdr;
>> u32 dir_status;
>> struct tasklet_struct tasklet;
>> - struct work_struct card_work;
>> unsigned long pending_events;
>> unsigned long completed_events;
>> enum dw_mci_state state;
>>
>

2014-10-16 12:57:56

by Alim Akhtar

[permalink] [raw]
Subject: Re: [PATCH] mmc: dw_mmc: Remove old card detect infrastructure

Hi Doug,

On Tue, Oct 14, 2014 at 10:03 PM, Doug Anderson <[email protected]> wrote:
> The dw_mmc driver had a bunch of code that ran whenever a card was
> ejected and inserted. However, this code was old and crufty and
> should be removed. Some evidence that it's really not needed:
>
> 1. Is is supposed to be legal to use 'cd-gpio' on dw_mmc instead of
> using the built-in card detect mechanism. The 'cd-gpio' code
> doesn't run any of the crufty old code but yet still works.
>
> 2. While looking at this, I realized that my old change (369ac86 mmc:
> dw_mmc: don't queue up a card detect at slot startup) actually
> castrated the old code a little bit already and nobody noticed.
> Specifically "last_detect_state" was left as 0 at bootup. That
> means that on the first card removal none of the crufty code ran.
>
Yes, right most of these codes are _almost_ never call. But I see
dw_mci_reset() being called on card removal (after first
insert/removal).
I tested this on exynos5800 and this looks working fine. We need to
test once cross suspend/resume as well.
And as Jaehoon pointed out,probably lets look in TRM if there are some
recommended steps for cd-detect.
Otherwise this looks good to me.

> 3. I can run "while true; do dd if=/dev/mmcblk1 of=/dev/null; done"
> while ejecting and inserting an SD Card and the world doesn't
> explode.
>
> If some of the crufty old code is actually needed, we should justify
> it and also put it in some place where it will be run even with
> "cd-gpio".
>
> Note that in my case I'm using the "cd-gpio" mechanism but for various
> reasons the hardware triggers a dw_mmc "card detect" at bootup. That
> was actually causing a real bug. The card detect workqueue was
> running while the system was trying to enumerate the card. The
> "present != slot->last_detect_state" triggered and we were doing all
> kinds of crazy stuff and messing up enumeration. The new mechanism of
> just asking the core to check the card is much safer and then the
> bogus interrupt doesn't hurt.
>
> Signed-off-by: Doug Anderson <[email protected]>
> ---
> drivers/mmc/host/dw_mmc.c | 121 ++++++++-------------------------------------
> drivers/mmc/host/dw_mmc.h | 2 -
> include/linux/mmc/dw_mmc.h | 2 -
> 3 files changed, 20 insertions(+), 105 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 69f0cc6..6a86495 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -34,7 +34,6 @@
> #include <linux/mmc/dw_mmc.h>
> #include <linux/bitops.h>
> #include <linux/regulator/consumer.h>
> -#include <linux/workqueue.h>
> #include <linux/of.h>
> #include <linux/of_gpio.h>
> #include <linux/mmc/slot-gpio.h>
> @@ -1954,6 +1953,23 @@ static void dw_mci_cmd_interrupt(struct dw_mci *host, u32 status)
> tasklet_schedule(&host->tasklet);
> }
>
> +static void dw_mci_handle_cd(struct dw_mci *host)
> +{
> + int i;
> +
> + for (i = 0; i < host->num_slots; i++) {
> + struct dw_mci_slot *slot = host->slot[i];
> +
> + if (!slot)
> + continue;
> +
> + if (slot->mmc->ops->card_event)
> + slot->mmc->ops->card_event(slot->mmc);
> + mmc_detect_change(slot->mmc,
> + msecs_to_jiffies(host->pdata->detect_delay_ms));
> + }
> +}
> +
> static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
> {
> struct dw_mci *host = dev_id;
> @@ -2029,7 +2045,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>
> if (pending & SDMMC_INT_CD) {
> mci_writel(host, RINTSTS, SDMMC_INT_CD);
> - queue_work(host->card_workqueue, &host->card_work);
> + dw_mci_handle_cd(host);
> }
>
> /* Handle SDIO Interrupts */
> @@ -2056,88 +2072,6 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> -static void dw_mci_work_routine_card(struct work_struct *work)
> -{
> - struct dw_mci *host = container_of(work, struct dw_mci, card_work);
> - int i;
> -
> - for (i = 0; i < host->num_slots; i++) {
> - struct dw_mci_slot *slot = host->slot[i];
> - struct mmc_host *mmc = slot->mmc;
> - struct mmc_request *mrq;
> - int present;
> -
> - present = dw_mci_get_cd(mmc);
> - while (present != slot->last_detect_state) {
> - dev_dbg(&slot->mmc->class_dev, "card %s\n",
> - present ? "inserted" : "removed");
> -
> - spin_lock_bh(&host->lock);
> -
> - /* Card change detected */
> - slot->last_detect_state = present;
> -
> - /* Clean up queue if present */
> - mrq = slot->mrq;
> - if (mrq) {
> - if (mrq == host->mrq) {
> - host->data = NULL;
> - host->cmd = NULL;
> -
> - switch (host->state) {
> - case STATE_IDLE:
> - case STATE_WAITING_CMD11_DONE:
> - break;
> - case STATE_SENDING_CMD11:
> - case STATE_SENDING_CMD:
> - mrq->cmd->error = -ENOMEDIUM;
> - if (!mrq->data)
> - break;
> - /* fall through */
> - case STATE_SENDING_DATA:
> - mrq->data->error = -ENOMEDIUM;
> - dw_mci_stop_dma(host);
> - break;
> - case STATE_DATA_BUSY:
> - case STATE_DATA_ERROR:
> - if (mrq->data->error == -EINPROGRESS)
> - mrq->data->error = -ENOMEDIUM;
> - /* fall through */
> - case STATE_SENDING_STOP:
> - if (mrq->stop)
> - mrq->stop->error = -ENOMEDIUM;
> - break;
> - }
> -
> - dw_mci_request_end(host, mrq);
> - } else {
> - list_del(&slot->queue_node);
> - mrq->cmd->error = -ENOMEDIUM;
> - if (mrq->data)
> - mrq->data->error = -ENOMEDIUM;
> - if (mrq->stop)
> - mrq->stop->error = -ENOMEDIUM;
> -
> - spin_unlock(&host->lock);
> - mmc_request_done(slot->mmc, mrq);
> - spin_lock(&host->lock);
> - }
> - }
> -
> - /* Power down slot */
> - if (present == 0)
> - dw_mci_reset(host);
> -
> - spin_unlock_bh(&host->lock);
> -
> - present = dw_mci_get_cd(mmc);
> - }
> -
> - mmc_detect_change(slot->mmc,
> - msecs_to_jiffies(host->pdata->detect_delay_ms));
> - }
> -}
> -
> #ifdef CONFIG_OF
> /* given a slot id, find out the device node representing that slot */
> static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
> @@ -2289,9 +2223,6 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
> dw_mci_init_debugfs(slot);
> #endif
>
> - /* Card initially undetected */
> - slot->last_detect_state = 0;
> -
> return 0;
>
> err_host_allocated:
> @@ -2672,17 +2603,10 @@ int dw_mci_probe(struct dw_mci *host)
> host->data_offset = DATA_240A_OFFSET;
>
> tasklet_init(&host->tasklet, dw_mci_tasklet_func, (unsigned long)host);
> - host->card_workqueue = alloc_workqueue("dw-mci-card",
> - WQ_MEM_RECLAIM, 1);
> - if (!host->card_workqueue) {
> - ret = -ENOMEM;
> - goto err_dmaunmap;
> - }
> - INIT_WORK(&host->card_work, dw_mci_work_routine_card);
> ret = devm_request_irq(host->dev, host->irq, dw_mci_interrupt,
> host->irq_flags, "dw-mci", host);
> if (ret)
> - goto err_workqueue;
> + goto err_dmaunmap;
>
> if (host->pdata->num_slots)
> host->num_slots = host->pdata->num_slots;
> @@ -2718,7 +2642,7 @@ int dw_mci_probe(struct dw_mci *host)
> } else {
> dev_dbg(host->dev, "attempted to initialize %d slots, "
> "but failed on all\n", host->num_slots);
> - goto err_workqueue;
> + goto err_dmaunmap;
> }
>
> if (host->quirks & DW_MCI_QUIRK_IDMAC_DTO)
> @@ -2726,9 +2650,6 @@ int dw_mci_probe(struct dw_mci *host)
>
> return 0;
>
> -err_workqueue:
> - destroy_workqueue(host->card_workqueue);
> -
> err_dmaunmap:
> if (host->use_dma && host->dma_ops->exit)
> host->dma_ops->exit(host);
> @@ -2762,8 +2683,6 @@ void dw_mci_remove(struct dw_mci *host)
> mci_writel(host, CLKENA, 0);
> mci_writel(host, CLKSRC, 0);
>
> - destroy_workqueue(host->card_workqueue);
> -
> if (host->use_dma && host->dma_ops->exit)
> host->dma_ops->exit(host);
>
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 01b99e8..71d4995 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -214,7 +214,6 @@ extern int dw_mci_resume(struct dw_mci *host);
> * with CONFIG_MMC_CLKGATE.
> * @flags: Random state bits associated with the slot.
> * @id: Number of this slot.
> - * @last_detect_state: Most recently observed card detect state.
> */
> struct dw_mci_slot {
> struct mmc_host *mmc;
> @@ -234,7 +233,6 @@ struct dw_mci_slot {
> #define DW_MMC_CARD_PRESENT 0
> #define DW_MMC_CARD_NEED_INIT 1
> int id;
> - int last_detect_state;
> };
>
> struct dw_mci_tuning_data {
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index 0013669..69d0814 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -135,7 +135,6 @@ struct dw_mci {
> struct mmc_command stop_abort;
> unsigned int prev_blksz;
> unsigned char timing;
> - struct workqueue_struct *card_workqueue;
>
> /* DMA interface members*/
> int use_dma;
> @@ -154,7 +153,6 @@ struct dw_mci {
> u32 stop_cmdr;
> u32 dir_status;
> struct tasklet_struct tasklet;
> - struct work_struct card_work;
> unsigned long pending_events;
> unsigned long completed_events;
> enum dw_mci_state state;
> --
> 2.1.0.rc2.206.gedb03e5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Regards,
Alim

2014-10-16 16:05:37

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH] mmc: dw_mmc: Remove old card detect infrastructure

Jaehoon,

On Thu, Oct 16, 2014 at 2:38 AM, Jaehoon Chung <[email protected]> wrote:
> Hi, Doug.
>
> Add one comment.
>
> On 10/16/2014 05:19 PM, Jaehoon Chung wrote:
>> Hi, Doug.
>>
>> Looks good to me.
>>
>> Tested-by: Jaehoon Chung <[email protected]>
>> with exynos3/4 series.
>>
>> Need to check exynos5 with using other "card detect" mechanism.
>>
>> Best Regards,
>> Jaehoon Chung
>>
>> On 10/15/2014 01:33 AM, Doug Anderson wrote:
>>> The dw_mmc driver had a bunch of code that ran whenever a card was
>>> ejected and inserted. However, this code was old and crufty and
>>> should be removed. Some evidence that it's really not needed:
>>>
>>> 1. Is is supposed to be legal to use 'cd-gpio' on dw_mmc instead of
>>> using the built-in card detect mechanism. The 'cd-gpio' code
>>> doesn't run any of the crufty old code but yet still works.
>>>
>>> 2. While looking at this, I realized that my old change (369ac86 mmc:
>>> dw_mmc: don't queue up a card detect at slot startup) actually
>>> castrated the old code a little bit already and nobody noticed.
>>> Specifically "last_detect_state" was left as 0 at bootup. That
>>> means that on the first card removal none of the crufty code ran.
>>>
>>> 3. I can run "while true; do dd if=/dev/mmcblk1 of=/dev/null; done"
>>> while ejecting and inserting an SD Card and the world doesn't
>>> explode.
>>>
>>> If some of the crufty old code is actually needed, we should justify
>>> it and also put it in some place where it will be run even with
>>> "cd-gpio".
>>>
>>> Note that in my case I'm using the "cd-gpio" mechanism but for various
>>> reasons the hardware triggers a dw_mmc "card detect" at bootup. That
>>> was actually causing a real bug. The card detect workqueue was
>>> running while the system was trying to enumerate the card. The
>>> "present != slot->last_detect_state" triggered and we were doing all
>>> kinds of crazy stuff and messing up enumeration. The new mechanism of
>>> just asking the core to check the card is much safer and then the
>>> bogus interrupt doesn't hurt.
>
> Did you read the Card-detect Recommendation?
> There is a Recommendation for Card-detect(CDT) at TRM.
> We don't read the CDETECT register(0x50) anywhere. Could you share this register value after detecting?

Sure we do. Look at dw_mci_get_cd(). I see "mci_readl(slot->host, CDETECT)".


> And I think "last_detect_state" is used for "software should read card detect register and store in memory."

Well yeah, last_detect_state DID store the result of the card detect
register in memory. ...but it wasn't the only place that the card
detect state was stored in memory. The MMC core handles things for
you, right? Said another way: put a printout in dw_mci_get_cd() when
the CDETECT is read. Insert a card. You should see your printout.
Now try to access your card. Did your printout happen? It shouldn't.
Linux knows that the card is still inserted because it stored that
fact in memory (in some MMC core data structure).

The MMC core handles polling the card detect at boot time (calling
dw_mci_get_cd()) to see if there is a card inserted. The Linux core
also handles calling dw_mci_get_cd() as appropriate when you tell it
something may have changed with mmc_detect_change(). Let's let the
core do its work.

If we _really_ need to follow the recommendation in the Designware
TRM, we should:

1. Before enabling interrupts cache the CDETECT (per slot)

2. In the IRQ handler read the CDETECT.

3. Never read CDETECT in dw_mci_get_cd(). Only use the cached value.

If you insist I'll spin the patch for that, but I'd rather not since:

* The old code definitely didn't get CDETECT in the irq handler (it
read it in the work routine), so it already violated the
recommendation.

* The designware guide shows this as a recommendation, not a
requirement. I think they are just trying to make things simpler for
you, but nothing could be simpler than letting the MMC core do all the
heavy lifting.

2014-10-16 16:10:42

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH] mmc: dw_mmc: Remove old card detect infrastructure

Alim,

On Thu, Oct 16, 2014 at 5:57 AM, Alim Akhtar <[email protected]> wrote:
> Hi Doug,
>
> On Tue, Oct 14, 2014 at 10:03 PM, Doug Anderson <[email protected]> wrote:
>> The dw_mmc driver had a bunch of code that ran whenever a card was
>> ejected and inserted. However, this code was old and crufty and
>> should be removed. Some evidence that it's really not needed:
>>
>> 1. Is is supposed to be legal to use 'cd-gpio' on dw_mmc instead of
>> using the built-in card detect mechanism. The 'cd-gpio' code
>> doesn't run any of the crufty old code but yet still works.
>>
>> 2. While looking at this, I realized that my old change (369ac86 mmc:
>> dw_mmc: don't queue up a card detect at slot startup) actually
>> castrated the old code a little bit already and nobody noticed.
>> Specifically "last_detect_state" was left as 0 at bootup. That
>> means that on the first card removal none of the crufty code ran.
>>
> Yes, right most of these codes are _almost_ never call. But I see
> dw_mci_reset() being called on card removal (after first
> insert/removal).

Right. The old crufty code was called on the 2nd removal, not the
1st. That meant that the two were accidentally different. My point
was that if the old code was really required that someone would have
noticed crashes on the 1st removal after each boot. Since nobody is
reporting crashes with that then it means it can't be too terrible.

One thing to note: I remember in the last Chromebook project you were
trying to track down crashes associated with constant eject / insert
of SD Cards. I wonder if my patch will fix these crashes?


> I tested this on exynos5800 and this looks working fine. We need to
> test once cross suspend/resume as well.

Good idea. Can you test that? I know that there's been lots of flux
with suspend/resume on exynos and I'm not sure I have all the latest
patches, but I'll search for them if you are unable to test easily.


> And as Jaehoon pointed out,probably lets look in TRM if there are some
> recommended steps for cd-detect.
> Otherwise this looks good to me.

If you see some other requirement than the one I addressed in my email
to Jaehoon, please let me know.


-Doug

2014-10-17 12:45:27

by Alim Akhtar

[permalink] [raw]
Subject: Re: [PATCH] mmc: dw_mmc: Remove old card detect infrastructure

Hi Doug,

On Thu, Oct 16, 2014 at 9:40 PM, Doug Anderson <[email protected]> wrote:
> Alim,
>
> On Thu, Oct 16, 2014 at 5:57 AM, Alim Akhtar <[email protected]> wrote:
>> Hi Doug,
>>
>> On Tue, Oct 14, 2014 at 10:03 PM, Doug Anderson <[email protected]> wrote:
>>> The dw_mmc driver had a bunch of code that ran whenever a card was
>>> ejected and inserted. However, this code was old and crufty and
>>> should be removed. Some evidence that it's really not needed:
>>>
>>> 1. Is is supposed to be legal to use 'cd-gpio' on dw_mmc instead of
>>> using the built-in card detect mechanism. The 'cd-gpio' code
>>> doesn't run any of the crufty old code but yet still works.
>>>
>>> 2. While looking at this, I realized that my old change (369ac86 mmc:
>>> dw_mmc: don't queue up a card detect at slot startup) actually
>>> castrated the old code a little bit already and nobody noticed.
>>> Specifically "last_detect_state" was left as 0 at bootup. That
>>> means that on the first card removal none of the crufty code ran.
>>>
>> Yes, right most of these codes are _almost_ never call. But I see
>> dw_mci_reset() being called on card removal (after first
>> insert/removal).
>
> Right. The old crufty code was called on the 2nd removal, not the
> 1st. That meant that the two were accidentally different. My point
> was that if the old code was really required that someone would have
> noticed crashes on the 1st removal after each boot. Since nobody is
> reporting crashes with that then it means it can't be too terrible.
>
> One thing to note: I remember in the last Chromebook project you were
> trying to track down crashes associated with constant eject / insert
> of SD Cards. I wonder if my patch will fix these crashes?
>
Ah, yes, reproducing that and checking with this patch will be really
interesting.

>
>> I tested this on exynos5800 and this looks working fine. We need to
>> test once cross suspend/resume as well.
>
> Good idea. Can you test that? I know that there's been lots of flux
> with suspend/resume on exynos and I'm not sure I have all the latest
> patches, but I'll search for them if you are unable to test easily.
>
Sure, I will do that..but probably sometime next week, as I will out
of office for few days.
>
>> And as Jaehoon pointed out,probably lets look in TRM if there are some
>> recommended steps for cd-detect.
>> Otherwise this looks good to me.
>
> If you see some other requirement than the one I addressed in my email
> to Jaehoon, please let me know.
>
Well, as most of the current CD detect code are dead code, so lets see
more test results, specially across a suspend/resume and warm reboot
test and take this forward.
>
> -Doug



--
Regards,
Alim

2014-10-20 03:23:21

by Jaehoon Chung

[permalink] [raw]
Subject: Re: [PATCH] mmc: dw_mmc: Remove old card detect infrastructure

Hi.

On 10/17/2014 09:44 PM, Alim Akhtar wrote:
> Hi Doug,
>
> On Thu, Oct 16, 2014 at 9:40 PM, Doug Anderson <[email protected]> wrote:
>> Alim,
>>
>> On Thu, Oct 16, 2014 at 5:57 AM, Alim Akhtar <[email protected]> wrote:
>>> Hi Doug,
>>>
>>> On Tue, Oct 14, 2014 at 10:03 PM, Doug Anderson <[email protected]> wrote:
>>>> The dw_mmc driver had a bunch of code that ran whenever a card was
>>>> ejected and inserted. However, this code was old and crufty and
>>>> should be removed. Some evidence that it's really not needed:
>>>>
>>>> 1. Is is supposed to be legal to use 'cd-gpio' on dw_mmc instead of
>>>> using the built-in card detect mechanism. The 'cd-gpio' code
>>>> doesn't run any of the crufty old code but yet still works.
>>>>
>>>> 2. While looking at this, I realized that my old change (369ac86 mmc:
>>>> dw_mmc: don't queue up a card detect at slot startup) actually
>>>> castrated the old code a little bit already and nobody noticed.
>>>> Specifically "last_detect_state" was left as 0 at bootup. That
>>>> means that on the first card removal none of the crufty code ran.
>>>>
>>> Yes, right most of these codes are _almost_ never call. But I see
>>> dw_mci_reset() being called on card removal (after first
>>> insert/removal).
>>
>> Right. The old crufty code was called on the 2nd removal, not the
>> 1st. That meant that the two were accidentally different. My point
>> was that if the old code was really required that someone would have
>> noticed crashes on the 1st removal after each boot. Since nobody is
>> reporting crashes with that then it means it can't be too terrible.
>>
>> One thing to note: I remember in the last Chromebook project you were
>> trying to track down crashes associated with constant eject / insert
>> of SD Cards. I wonder if my patch will fix these crashes?
>>
> Ah, yes, reproducing that and checking with this patch will be really
> interesting.
>
>>
>>> I tested this on exynos5800 and this looks working fine. We need to
>>> test once cross suspend/resume as well.
>>
>> Good idea. Can you test that? I know that there's been lots of flux
>> with suspend/resume on exynos and I'm not sure I have all the latest
>> patches, but I'll search for them if you are unable to test easily.
>>
> Sure, I will do that..but probably sometime next week, as I will out
> of office for few days.
>>
>>> And as Jaehoon pointed out,probably lets look in TRM if there are some
>>> recommended steps for cd-detect.
>>> Otherwise this looks good to me.
>>
>> If you see some other requirement than the one I addressed in my email
>> to Jaehoon, please let me know.

I know there is no other requirement for detecting card.
So this patch can be applied after testing the above case(suspend/resume).

Best Regards,
Jaehoon Chung

>>
> Well, as most of the current CD detect code are dead code, so lets see
> more test results, specially across a suspend/resume and warm reboot
> test and take this forward.
>>
>> -Doug
>
>
>

2014-10-22 16:37:02

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH] mmc: dw_mmc: Remove old card detect infrastructure

Hi,

On Sun, Oct 19, 2014 at 8:23 PM, Jaehoon Chung <[email protected]> wrote:
> Hi.
>
> On 10/17/2014 09:44 PM, Alim Akhtar wrote:
>> Hi Doug,
>>
>> On Thu, Oct 16, 2014 at 9:40 PM, Doug Anderson <[email protected]> wrote:
>>> Alim,
>>>
>>> On Thu, Oct 16, 2014 at 5:57 AM, Alim Akhtar <[email protected]> wrote:
>>>> Hi Doug,
>>>>
>>>> On Tue, Oct 14, 2014 at 10:03 PM, Doug Anderson <[email protected]> wrote:
>>>>> The dw_mmc driver had a bunch of code that ran whenever a card was
>>>>> ejected and inserted. However, this code was old and crufty and
>>>>> should be removed. Some evidence that it's really not needed:
>>>>>
>>>>> 1. Is is supposed to be legal to use 'cd-gpio' on dw_mmc instead of
>>>>> using the built-in card detect mechanism. The 'cd-gpio' code
>>>>> doesn't run any of the crufty old code but yet still works.
>>>>>
>>>>> 2. While looking at this, I realized that my old change (369ac86 mmc:
>>>>> dw_mmc: don't queue up a card detect at slot startup) actually
>>>>> castrated the old code a little bit already and nobody noticed.
>>>>> Specifically "last_detect_state" was left as 0 at bootup. That
>>>>> means that on the first card removal none of the crufty code ran.
>>>>>
>>>> Yes, right most of these codes are _almost_ never call. But I see
>>>> dw_mci_reset() being called on card removal (after first
>>>> insert/removal).
>>>
>>> Right. The old crufty code was called on the 2nd removal, not the
>>> 1st. That meant that the two were accidentally different. My point
>>> was that if the old code was really required that someone would have
>>> noticed crashes on the 1st removal after each boot. Since nobody is
>>> reporting crashes with that then it means it can't be too terrible.
>>>
>>> One thing to note: I remember in the last Chromebook project you were
>>> trying to track down crashes associated with constant eject / insert
>>> of SD Cards. I wonder if my patch will fix these crashes?
>>>
>> Ah, yes, reproducing that and checking with this patch will be really
>> interesting.
>>
>>>
>>>> I tested this on exynos5800 and this looks working fine. We need to
>>>> test once cross suspend/resume as well.
>>>
>>> Good idea. Can you test that? I know that there's been lots of flux
>>> with suspend/resume on exynos and I'm not sure I have all the latest
>>> patches, but I'll search for them if you are unable to test easily.
>>>
>> Sure, I will do that..but probably sometime next week, as I will out
>> of office for few days.
>>>
>>>> And as Jaehoon pointed out,probably lets look in TRM if there are some
>>>> recommended steps for cd-detect.
>>>> Otherwise this looks good to me.
>>>
>>> If you see some other requirement than the one I addressed in my email
>>> to Jaehoon, please let me know.
>
> I know there is no other requirement for detecting card.
> So this patch can be applied after testing the above case(suspend/resume).

I put a kernel based upon 3.17 on an exynos5250-snow (specifically
git://git.collabora.co.uk/git/user/javier/linux.git branch
max77802-op-modes-v3, git hash 98cf5a0). Snow uses the builtin card
detect on dw_mmc. Resume wasn't terribly reliable to start with even
without my patch (it often woke up right after suspend), but it worked
well enough for testing. I tested the following scenarios:

1. Leave card in and mounted. Suspend/resume. Card is still usable
after resume

2. Suspend and insert card. Resume. Card is detected upon resume.

3. Suspend and remove card. Resume. Card is removed upon resume.

How does that sound?

-Doug

2014-10-23 01:13:16

by Jaehoon Chung

[permalink] [raw]
Subject: Re: [PATCH] mmc: dw_mmc: Remove old card detect infrastructure

Dear, Doug.

On 10/23/2014 01:36 AM, Doug Anderson wrote:
> Hi,
>
> On Sun, Oct 19, 2014 at 8:23 PM, Jaehoon Chung <[email protected]> wrote:
>> Hi.
>>
>> On 10/17/2014 09:44 PM, Alim Akhtar wrote:
>>> Hi Doug,
>>>
>>> On Thu, Oct 16, 2014 at 9:40 PM, Doug Anderson <[email protected]> wrote:
>>>> Alim,
>>>>
>>>> On Thu, Oct 16, 2014 at 5:57 AM, Alim Akhtar <[email protected]> wrote:
>>>>> Hi Doug,
>>>>>
>>>>> On Tue, Oct 14, 2014 at 10:03 PM, Doug Anderson <[email protected]> wrote:
>>>>>> The dw_mmc driver had a bunch of code that ran whenever a card was
>>>>>> ejected and inserted. However, this code was old and crufty and
>>>>>> should be removed. Some evidence that it's really not needed:
>>>>>>
>>>>>> 1. Is is supposed to be legal to use 'cd-gpio' on dw_mmc instead of
>>>>>> using the built-in card detect mechanism. The 'cd-gpio' code
>>>>>> doesn't run any of the crufty old code but yet still works.
>>>>>>
>>>>>> 2. While looking at this, I realized that my old change (369ac86 mmc:
>>>>>> dw_mmc: don't queue up a card detect at slot startup) actually
>>>>>> castrated the old code a little bit already and nobody noticed.
>>>>>> Specifically "last_detect_state" was left as 0 at bootup. That
>>>>>> means that on the first card removal none of the crufty code ran.
>>>>>>
>>>>> Yes, right most of these codes are _almost_ never call. But I see
>>>>> dw_mci_reset() being called on card removal (after first
>>>>> insert/removal).
>>>>
>>>> Right. The old crufty code was called on the 2nd removal, not the
>>>> 1st. That meant that the two were accidentally different. My point
>>>> was that if the old code was really required that someone would have
>>>> noticed crashes on the 1st removal after each boot. Since nobody is
>>>> reporting crashes with that then it means it can't be too terrible.
>>>>
>>>> One thing to note: I remember in the last Chromebook project you were
>>>> trying to track down crashes associated with constant eject / insert
>>>> of SD Cards. I wonder if my patch will fix these crashes?
>>>>
>>> Ah, yes, reproducing that and checking with this patch will be really
>>> interesting.
>>>
>>>>
>>>>> I tested this on exynos5800 and this looks working fine. We need to
>>>>> test once cross suspend/resume as well.
>>>>
>>>> Good idea. Can you test that? I know that there's been lots of flux
>>>> with suspend/resume on exynos and I'm not sure I have all the latest
>>>> patches, but I'll search for them if you are unable to test easily.
>>>>
>>> Sure, I will do that..but probably sometime next week, as I will out
>>> of office for few days.
>>>>
>>>>> And as Jaehoon pointed out,probably lets look in TRM if there are some
>>>>> recommended steps for cd-detect.
>>>>> Otherwise this looks good to me.
>>>>
>>>> If you see some other requirement than the one I addressed in my email
>>>> to Jaehoon, please let me know.
>>
>> I know there is no other requirement for detecting card.
>> So this patch can be applied after testing the above case(suspend/resume).
>
> I put a kernel based upon 3.17 on an exynos5250-snow (specifically
> git://git.collabora.co.uk/git/user/javier/linux.git branch
> max77802-op-modes-v3, git hash 98cf5a0). Snow uses the builtin card
> detect on dw_mmc. Resume wasn't terribly reliable to start with even
> without my patch (it often woke up right after suspend), but it worked
> well enough for testing. I tested the following scenarios:
>
> 1. Leave card in and mounted. Suspend/resume. Card is still usable
> after resume
>
> 2. Suspend and insert card. Resume. Card is detected upon resume.
>
> 3. Suspend and remove card. Resume. Card is removed upon resume.
>
> How does that sound?

I think these test cases are enough, and if it's working fine, sounds good.

Acked-by: Jaehoon Chung <[email protected]>

Best Regards,
Jaehoon Chung

>
> -Doug
>

2014-10-23 12:44:29

by Alim Akhtar

[permalink] [raw]
Subject: Re: [PATCH] mmc: dw_mmc: Remove old card detect infrastructure

Hi Doug,

On Wed, Oct 22, 2014 at 10:06 PM, Doug Anderson <[email protected]> wrote:
> Hi,
>
> On Sun, Oct 19, 2014 at 8:23 PM, Jaehoon Chung <[email protected]> wrote:
>> Hi.
>>
>> On 10/17/2014 09:44 PM, Alim Akhtar wrote:
>>> Hi Doug,
>>>
>>> On Thu, Oct 16, 2014 at 9:40 PM, Doug Anderson <[email protected]> wrote:
>>>> Alim,
>>>>
>>>> On Thu, Oct 16, 2014 at 5:57 AM, Alim Akhtar <[email protected]> wrote:
>>>>> Hi Doug,
>>>>>
>>>>> On Tue, Oct 14, 2014 at 10:03 PM, Doug Anderson <[email protected]> wrote:
>>>>>> The dw_mmc driver had a bunch of code that ran whenever a card was
>>>>>> ejected and inserted. However, this code was old and crufty and
>>>>>> should be removed. Some evidence that it's really not needed:
>>>>>>
>>>>>> 1. Is is supposed to be legal to use 'cd-gpio' on dw_mmc instead of
>>>>>> using the built-in card detect mechanism. The 'cd-gpio' code
>>>>>> doesn't run any of the crufty old code but yet still works.
>>>>>>
>>>>>> 2. While looking at this, I realized that my old change (369ac86 mmc:
>>>>>> dw_mmc: don't queue up a card detect at slot startup) actually
>>>>>> castrated the old code a little bit already and nobody noticed.
>>>>>> Specifically "last_detect_state" was left as 0 at bootup. That
>>>>>> means that on the first card removal none of the crufty code ran.
>>>>>>
>>>>> Yes, right most of these codes are _almost_ never call. But I see
>>>>> dw_mci_reset() being called on card removal (after first
>>>>> insert/removal).
>>>>
>>>> Right. The old crufty code was called on the 2nd removal, not the
>>>> 1st. That meant that the two were accidentally different. My point
>>>> was that if the old code was really required that someone would have
>>>> noticed crashes on the 1st removal after each boot. Since nobody is
>>>> reporting crashes with that then it means it can't be too terrible.
>>>>
>>>> One thing to note: I remember in the last Chromebook project you were
>>>> trying to track down crashes associated with constant eject / insert
>>>> of SD Cards. I wonder if my patch will fix these crashes?
>>>>
>>> Ah, yes, reproducing that and checking with this patch will be really
>>> interesting.
>>>
>>>>
>>>>> I tested this on exynos5800 and this looks working fine. We need to
>>>>> test once cross suspend/resume as well.
>>>>
>>>> Good idea. Can you test that? I know that there's been lots of flux
>>>> with suspend/resume on exynos and I'm not sure I have all the latest
>>>> patches, but I'll search for them if you are unable to test easily.
>>>>
>>> Sure, I will do that..but probably sometime next week, as I will out
>>> of office for few days.
>>>>
>>>>> And as Jaehoon pointed out,probably lets look in TRM if there are some
>>>>> recommended steps for cd-detect.
>>>>> Otherwise this looks good to me.
>>>>
>>>> If you see some other requirement than the one I addressed in my email
>>>> to Jaehoon, please let me know.
>>
>> I know there is no other requirement for detecting card.
>> So this patch can be applied after testing the above case(suspend/resume).
>
> I put a kernel based upon 3.17 on an exynos5250-snow (specifically
> git://git.collabora.co.uk/git/user/javier/linux.git branch
> max77802-op-modes-v3, git hash 98cf5a0). Snow uses the builtin card
> detect on dw_mmc. Resume wasn't terribly reliable to start with even
> without my patch (it often woke up right after suspend), but it worked
> well enough for testing. I tested the following scenarios:
>
> 1. Leave card in and mounted. Suspend/resume. Card is still usable
> after resume
>
> 2. Suspend and insert card. Resume. Card is detected upon resume.
>
> 3. Suspend and remove card. Resume. Card is removed upon resume.
>
> How does that sound?
>
Sounds good to me.
I tested this on Peach-Pi with v3.17 based kernel across suspend
resume and cards works fine.
Even I back-ported this to chromium 3.8 and tested on peach-pi, no
issue over a hundred s2r cycles.

Tested-by: alim.akhtar <[email protected]>
> -Doug



--
Regards,
Alim

2014-10-27 15:42:29

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH] mmc: dw_mmc: Remove old card detect infrastructure

On 14 October 2014 18:33, Doug Anderson <[email protected]> wrote:
> The dw_mmc driver had a bunch of code that ran whenever a card was
> ejected and inserted. However, this code was old and crufty and
> should be removed. Some evidence that it's really not needed:
>
> 1. Is is supposed to be legal to use 'cd-gpio' on dw_mmc instead of
> using the built-in card detect mechanism. The 'cd-gpio' code
> doesn't run any of the crufty old code but yet still works.
>
> 2. While looking at this, I realized that my old change (369ac86 mmc:
> dw_mmc: don't queue up a card detect at slot startup) actually
> castrated the old code a little bit already and nobody noticed.
> Specifically "last_detect_state" was left as 0 at bootup. That
> means that on the first card removal none of the crufty code ran.
>
> 3. I can run "while true; do dd if=/dev/mmcblk1 of=/dev/null; done"
> while ejecting and inserting an SD Card and the world doesn't
> explode.
>
> If some of the crufty old code is actually needed, we should justify
> it and also put it in some place where it will be run even with
> "cd-gpio".
>
> Note that in my case I'm using the "cd-gpio" mechanism but for various
> reasons the hardware triggers a dw_mmc "card detect" at bootup. That
> was actually causing a real bug. The card detect workqueue was
> running while the system was trying to enumerate the card. The
> "present != slot->last_detect_state" triggered and we were doing all
> kinds of crazy stuff and messing up enumeration. The new mechanism of
> just asking the core to check the card is much safer and then the
> bogus interrupt doesn't hurt.
>
> Signed-off-by: Doug Anderson <[email protected]>

Thanks! Applied for next.

Kind regards
Uffe

> ---
> drivers/mmc/host/dw_mmc.c | 121 ++++++++-------------------------------------
> drivers/mmc/host/dw_mmc.h | 2 -
> include/linux/mmc/dw_mmc.h | 2 -
> 3 files changed, 20 insertions(+), 105 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 69f0cc6..6a86495 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -34,7 +34,6 @@
> #include <linux/mmc/dw_mmc.h>
> #include <linux/bitops.h>
> #include <linux/regulator/consumer.h>
> -#include <linux/workqueue.h>
> #include <linux/of.h>
> #include <linux/of_gpio.h>
> #include <linux/mmc/slot-gpio.h>
> @@ -1954,6 +1953,23 @@ static void dw_mci_cmd_interrupt(struct dw_mci *host, u32 status)
> tasklet_schedule(&host->tasklet);
> }
>
> +static void dw_mci_handle_cd(struct dw_mci *host)
> +{
> + int i;
> +
> + for (i = 0; i < host->num_slots; i++) {
> + struct dw_mci_slot *slot = host->slot[i];
> +
> + if (!slot)
> + continue;
> +
> + if (slot->mmc->ops->card_event)
> + slot->mmc->ops->card_event(slot->mmc);
> + mmc_detect_change(slot->mmc,
> + msecs_to_jiffies(host->pdata->detect_delay_ms));
> + }
> +}
> +
> static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
> {
> struct dw_mci *host = dev_id;
> @@ -2029,7 +2045,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>
> if (pending & SDMMC_INT_CD) {
> mci_writel(host, RINTSTS, SDMMC_INT_CD);
> - queue_work(host->card_workqueue, &host->card_work);
> + dw_mci_handle_cd(host);
> }
>
> /* Handle SDIO Interrupts */
> @@ -2056,88 +2072,6 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> -static void dw_mci_work_routine_card(struct work_struct *work)
> -{
> - struct dw_mci *host = container_of(work, struct dw_mci, card_work);
> - int i;
> -
> - for (i = 0; i < host->num_slots; i++) {
> - struct dw_mci_slot *slot = host->slot[i];
> - struct mmc_host *mmc = slot->mmc;
> - struct mmc_request *mrq;
> - int present;
> -
> - present = dw_mci_get_cd(mmc);
> - while (present != slot->last_detect_state) {
> - dev_dbg(&slot->mmc->class_dev, "card %s\n",
> - present ? "inserted" : "removed");
> -
> - spin_lock_bh(&host->lock);
> -
> - /* Card change detected */
> - slot->last_detect_state = present;
> -
> - /* Clean up queue if present */
> - mrq = slot->mrq;
> - if (mrq) {
> - if (mrq == host->mrq) {
> - host->data = NULL;
> - host->cmd = NULL;
> -
> - switch (host->state) {
> - case STATE_IDLE:
> - case STATE_WAITING_CMD11_DONE:
> - break;
> - case STATE_SENDING_CMD11:
> - case STATE_SENDING_CMD:
> - mrq->cmd->error = -ENOMEDIUM;
> - if (!mrq->data)
> - break;
> - /* fall through */
> - case STATE_SENDING_DATA:
> - mrq->data->error = -ENOMEDIUM;
> - dw_mci_stop_dma(host);
> - break;
> - case STATE_DATA_BUSY:
> - case STATE_DATA_ERROR:
> - if (mrq->data->error == -EINPROGRESS)
> - mrq->data->error = -ENOMEDIUM;
> - /* fall through */
> - case STATE_SENDING_STOP:
> - if (mrq->stop)
> - mrq->stop->error = -ENOMEDIUM;
> - break;
> - }
> -
> - dw_mci_request_end(host, mrq);
> - } else {
> - list_del(&slot->queue_node);
> - mrq->cmd->error = -ENOMEDIUM;
> - if (mrq->data)
> - mrq->data->error = -ENOMEDIUM;
> - if (mrq->stop)
> - mrq->stop->error = -ENOMEDIUM;
> -
> - spin_unlock(&host->lock);
> - mmc_request_done(slot->mmc, mrq);
> - spin_lock(&host->lock);
> - }
> - }
> -
> - /* Power down slot */
> - if (present == 0)
> - dw_mci_reset(host);
> -
> - spin_unlock_bh(&host->lock);
> -
> - present = dw_mci_get_cd(mmc);
> - }
> -
> - mmc_detect_change(slot->mmc,
> - msecs_to_jiffies(host->pdata->detect_delay_ms));
> - }
> -}
> -
> #ifdef CONFIG_OF
> /* given a slot id, find out the device node representing that slot */
> static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
> @@ -2289,9 +2223,6 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
> dw_mci_init_debugfs(slot);
> #endif
>
> - /* Card initially undetected */
> - slot->last_detect_state = 0;
> -
> return 0;
>
> err_host_allocated:
> @@ -2672,17 +2603,10 @@ int dw_mci_probe(struct dw_mci *host)
> host->data_offset = DATA_240A_OFFSET;
>
> tasklet_init(&host->tasklet, dw_mci_tasklet_func, (unsigned long)host);
> - host->card_workqueue = alloc_workqueue("dw-mci-card",
> - WQ_MEM_RECLAIM, 1);
> - if (!host->card_workqueue) {
> - ret = -ENOMEM;
> - goto err_dmaunmap;
> - }
> - INIT_WORK(&host->card_work, dw_mci_work_routine_card);
> ret = devm_request_irq(host->dev, host->irq, dw_mci_interrupt,
> host->irq_flags, "dw-mci", host);
> if (ret)
> - goto err_workqueue;
> + goto err_dmaunmap;
>
> if (host->pdata->num_slots)
> host->num_slots = host->pdata->num_slots;
> @@ -2718,7 +2642,7 @@ int dw_mci_probe(struct dw_mci *host)
> } else {
> dev_dbg(host->dev, "attempted to initialize %d slots, "
> "but failed on all\n", host->num_slots);
> - goto err_workqueue;
> + goto err_dmaunmap;
> }
>
> if (host->quirks & DW_MCI_QUIRK_IDMAC_DTO)
> @@ -2726,9 +2650,6 @@ int dw_mci_probe(struct dw_mci *host)
>
> return 0;
>
> -err_workqueue:
> - destroy_workqueue(host->card_workqueue);
> -
> err_dmaunmap:
> if (host->use_dma && host->dma_ops->exit)
> host->dma_ops->exit(host);
> @@ -2762,8 +2683,6 @@ void dw_mci_remove(struct dw_mci *host)
> mci_writel(host, CLKENA, 0);
> mci_writel(host, CLKSRC, 0);
>
> - destroy_workqueue(host->card_workqueue);
> -
> if (host->use_dma && host->dma_ops->exit)
> host->dma_ops->exit(host);
>
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 01b99e8..71d4995 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -214,7 +214,6 @@ extern int dw_mci_resume(struct dw_mci *host);
> * with CONFIG_MMC_CLKGATE.
> * @flags: Random state bits associated with the slot.
> * @id: Number of this slot.
> - * @last_detect_state: Most recently observed card detect state.
> */
> struct dw_mci_slot {
> struct mmc_host *mmc;
> @@ -234,7 +233,6 @@ struct dw_mci_slot {
> #define DW_MMC_CARD_PRESENT 0
> #define DW_MMC_CARD_NEED_INIT 1
> int id;
> - int last_detect_state;
> };
>
> struct dw_mci_tuning_data {
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index 0013669..69d0814 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -135,7 +135,6 @@ struct dw_mci {
> struct mmc_command stop_abort;
> unsigned int prev_blksz;
> unsigned char timing;
> - struct workqueue_struct *card_workqueue;
>
> /* DMA interface members*/
> int use_dma;
> @@ -154,7 +153,6 @@ struct dw_mci {
> u32 stop_cmdr;
> u32 dir_status;
> struct tasklet_struct tasklet;
> - struct work_struct card_work;
> unsigned long pending_events;
> unsigned long completed_events;
> enum dw_mci_state state;
> --
> 2.1.0.rc2.206.gedb03e5
>