* [PATCH 0/2 v2] mmc: tmio: handle missing HW interrupts
@ 2010-12-29 13:21 ` Arnd Hannemann
0 siblings, 0 replies; 14+ messages in thread
From: Arnd Hannemann @ 2010-12-29 13:21 UTC (permalink / raw)
To: linux-mmc, Ian Molton; +Cc: linux-sh
[PATCH 1/2 v2] mmc: tmio: handle missing HW interrupts
[PATCH 2/2 v2] mmc: tmio: fix CMD irq handling
This are rebased equivalents to the patches:
[1/2] tmio_mmc: handle missing HW interrupts
https://patchwork.kernel.org/patch/201962/
[2/2] tmio_mmc: fix CMD irq handling
https://patchwork.kernel.org/patch/201982/
They now depend on -rc7 and:
mmc: tmio: merge the private header into the driver
https://patchwork.kernel.org/patch/349931/
Signed-off-by: Arnd Hannemann <arnd@arndnet.de>
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 0/2 v2] mmc: tmio: handle missing HW interrupts @ 2010-12-29 13:21 ` Arnd Hannemann 0 siblings, 0 replies; 14+ messages in thread From: Arnd Hannemann @ 2010-12-29 13:21 UTC (permalink / raw) To: linux-mmc, Ian Molton; +Cc: linux-sh [PATCH 1/2 v2] mmc: tmio: handle missing HW interrupts [PATCH 2/2 v2] mmc: tmio: fix CMD irq handling This are rebased equivalents to the patches: [1/2] tmio_mmc: handle missing HW interrupts https://patchwork.kernel.org/patch/201962/ [2/2] tmio_mmc: fix CMD irq handling https://patchwork.kernel.org/patch/201982/ They now depend on -rc7 and: mmc: tmio: merge the private header into the driver https://patchwork.kernel.org/patch/349931/ Signed-off-by: Arnd Hannemann <arnd@arndnet.de> ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2 v2] mmc: tmio: handle missing HW interrupts 2010-12-29 13:21 ` Arnd Hannemann @ 2010-12-29 13:21 ` Arnd Hannemann -1 siblings, 0 replies; 14+ messages in thread From: Arnd Hannemann @ 2010-12-29 13:21 UTC (permalink / raw) To: linux-mmc, Ian Molton; +Cc: linux-sh, Arnd Hannemann When doing excessive hotplug, e.g., repeated insert/eject operations, the hardware may get confused to a point where no CMDTIMEOUT/CMDRESPEND interrupts are generated any more. As a result requests get stuck, e.g.: [ 360.351562] INFO: task kworker/u:0:5 blocked for more than 120 seconds. [ 360.351562] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 360.359375] kworker/u:0 D c020c2b4 0 5 2 0x00000000 [ 360.367187] Backtrace: [ 360.367187] [<c020bfb0>] (schedule+0x0/0x340) from [<c020c480>] (schedule_timeout+0x20/0x190) [ 360.375000] r8:c702fd70 r7:00000002 r6:c702e000 r5:c702fdc4 r4:7fffffff [ 360.375000] r3:c701e040 [ 360.382812] [<c020c460>] (schedule_timeout+0x0/0x190) from [<c020be78>] (wait_for_common+0xc4/0x150) [ 360.390625] r6:c702e000 r5:c702fdc4 r4:7fffffff [ 360.390625] [<c020bdb4>] (wait_for_common+0x0/0x150) from [<c020bfac>] (wait_for_completion+0x18/0x1c) [ 360.398437] [<c020bf94>] (wait_for_completion+0x0/0x1c) from [<c0185590>] (mmc_wait_for_req+0x214/0x234) [ 360.406250] [<c018537c>] (mmc_wait_for_req+0x0/0x234) from [<c01889d0>] (mmc_sd_switch+0xfc/0x114) [ 360.414062] r7:c702fe4c r6:c702fe20 r5:c7179800 r4:00fffff0 [ 360.421875] [<c01888d4>] (mmc_sd_switch+0x0/0x114) from [<c0187f70>] (mmc_sd_setup_card+0x260/0x384) [ 360.429687] [<c0187d10>] (mmc_sd_setup_card+0x0/0x384) from [<c01885e0>] (mmc_sd_init_card+0x13c/0x1e0) [ 360.437500] [<c01884a4>] (mmc_sd_init_card+0x0/0x1e0) from [<c01887a8>] (mmc_attach_sd+0x124/0x1a8) [ 360.445312] r8:c02db404 r7:ffffff92 r6:c702ff34 r5:c6007da8 r4:c6007c00 [ 360.453125] [<c0188684>] (mmc_attach_sd+0x0/0x1a8) from [<c0185140>] (mmc_rescan+0x248/0x2f0) [ 360.460937] r5:c6007da8 r4:c6007c00 [ 360.468750] [<c0184ef8>] (mmc_rescan+0x0/0x2f0) from [<c00467f0>] (process_one_work+0x1ec/0x318) [ 360.476562] r7:c6007da8 r6:00000000 r5:c710ec00 r4:c701bde0 [ 360.484375] [<c0046604>] (process_one_work+0x0/0x318) from [<c0047fb0>] (worker_thread+0x1b0/0x2cc) [ 360.492187] [<c0047e00>] (worker_thread+0x0/0x2cc) from [<c004b338>] (kthread+0x8c/0x94) [ 360.500000] [<c004b2ac>] (kthread+0x0/0x94) from [<c0037fc4>] (do_exit+0x0/0x590) [ 360.507812] r7:00000013 r6:c0037fc4 r5:c004b2ac r4:c7021f00 This patch addresses this problem by introducing timeouts for outstanding interrupts. If a hardware interrupt is missing, a soft reset will be performed to bring the hardware back to a working state. Tested with the SDHI hardware block in sh7372 / AP4EVB. Signed-off-by: Arnd Hannemann <arnd@arndnet.de> --- Changes in v2: - rebased against -rc7 - depends on: mmc: tmio: merge the private header into the driver https://patchwork.kernel.org/patch/349931/ drivers/mmc/host/tmio_mmc.c | 90 +++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 86 insertions(+), 4 deletions(-) diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c index a0cf04a..f980042 100644 --- a/drivers/mmc/host/tmio_mmc.c +++ b/drivers/mmc/host/tmio_mmc.c @@ -39,6 +39,8 @@ #include <linux/module.h> #include <linux/pagemap.h> #include <linux/scatterlist.h> +#include <linux/workqueue.h> +#include <linux/spinlock.h> #define CTL_SD_CMD 0x00 #define CTL_ARG_REG 0x04 @@ -154,6 +156,11 @@ struct tmio_mmc_host { u8 bounce_buf[PAGE_CACHE_SIZE] __attribute__((aligned(MAX_ALIGN))); struct scatterlist bounce_sg; #endif + + /* Track lost interrupts */ + struct delayed_work delayed_reset_work; + spinlock_t lock; + unsigned long last_req_ts; }; static u16 sd_ctrl_read16(struct tmio_mmc_host *host, int addr) @@ -342,15 +349,60 @@ static void reset(struct tmio_mmc_host *host) msleep(10); } +static void tmio_mmc_reset_work(struct work_struct *work) +{ + struct tmio_mmc_host *host = container_of(work, struct tmio_mmc_host, + delayed_reset_work.work); + struct mmc_request *mrq; + unsigned long flags; + + spin_lock_irqsave(&host->lock, flags); + mrq = host->mrq; + + /* request already finished */ + if (!mrq + || time_is_after_jiffies(host->last_req_ts + + msecs_to_jiffies(2000))) { + spin_unlock_irqrestore(&host->lock, flags); + return; + } + + dev_warn(&host->pdev->dev, + "timeout waiting for hardware interrupt (CMD%u)\n", + mrq->cmd->opcode); + + if (host->data) + host->data->error = -ETIMEDOUT; + else if (host->cmd) + host->cmd->error = -ETIMEDOUT; + else + mrq->cmd->error = -ETIMEDOUT; + + host->cmd = NULL; + host->data = NULL; + host->mrq = NULL; + + spin_unlock_irqrestore(&host->lock, flags); + + reset(host); + + mmc_request_done(host->mmc, mrq); +} + static void tmio_mmc_finish_request(struct tmio_mmc_host *host) { struct mmc_request *mrq = host->mrq; + if (!mrq) + return; + host->mrq = NULL; host->cmd = NULL; host->data = NULL; + cancel_delayed_work(&host->delayed_reset_work); + mmc_request_done(host->mmc, mrq); } @@ -460,6 +512,7 @@ static void tmio_mmc_pio_irq(struct tmio_mmc_host *host) return; } +/* needs to be called with host->lock held */ static void tmio_mmc_do_data_irq(struct tmio_mmc_host *host) { struct mmc_data *data = host->data; @@ -520,10 +573,12 @@ static void tmio_mmc_do_data_irq(struct tmio_mmc_host *host) static void tmio_mmc_data_irq(struct tmio_mmc_host *host) { - struct mmc_data *data = host->data; + struct mmc_data *data; + spin_lock(&host->lock); + data = host->data; if (!data) - return; + goto out; if (host->chan_tx && (data->flags & MMC_DATA_WRITE)) { /* @@ -544,6 +599,8 @@ static void tmio_mmc_data_irq(struct tmio_mmc_host *host) } else { tmio_mmc_do_data_irq(host); } +out: + spin_unlock(&host->lock); } static void tmio_mmc_cmd_irq(struct tmio_mmc_host *host, @@ -552,9 +609,11 @@ static void tmio_mmc_cmd_irq(struct tmio_mmc_host *host, struct mmc_command *cmd = host->cmd; int i, addr; + spin_lock(&host->lock); + if (!host->cmd) { pr_debug("Spurious CMD irq\n"); - return; + goto out; } host->cmd = NULL; @@ -599,6 +658,9 @@ static void tmio_mmc_cmd_irq(struct tmio_mmc_host *host, tmio_mmc_finish_request(host); } +out: + spin_unlock(&host->lock); + return; } @@ -899,6 +961,12 @@ static void tmio_issue_tasklet_fn(unsigned long priv) static void tmio_tasklet_fn(unsigned long arg) { struct tmio_mmc_host *host = (struct tmio_mmc_host *)arg; + unsigned long flags; + + spin_lock_irqsave(&host->lock, flags); + + if (!host->data) + goto out; if (host->data->flags & MMC_DATA_READ) dma_unmap_sg(&host->pdev->dev, host->sg_ptr, host->dma_sglen, @@ -908,6 +976,8 @@ static void tmio_tasklet_fn(unsigned long arg) DMA_TO_DEVICE); tmio_mmc_do_data_irq(host); +out: + spin_unlock_irqrestore(&host->lock, flags); } /* It might be necessary to make filter MFD specific */ @@ -1026,6 +1096,8 @@ static void tmio_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq) if (host->mrq) pr_debug("request not null\n"); + host->last_req_ts = jiffies; + wmb(); host->mrq = mrq; if (mrq->data) { @@ -1035,10 +1107,14 @@ static void tmio_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq) } ret = tmio_mmc_start_command(host, mrq->cmd); - if (!ret) + if (!ret) { + schedule_delayed_work(&host->delayed_reset_work, + msecs_to_jiffies(2000)); return; + } fail: + host->mrq = NULL; mrq->cmd->error = ret; mmc_request_done(mmc, mrq); } @@ -1235,6 +1311,11 @@ static int __devinit tmio_mmc_probe(struct platform_device *dev) if (ret) goto cell_disable; + spin_lock_init(&host->lock); + + /* Init delayed work for request timeouts */ + INIT_DELAYED_WORK(&host->delayed_reset_work, tmio_mmc_reset_work); + /* See if we also get DMA */ tmio_mmc_request_dma(host, pdata); @@ -1273,6 +1354,7 @@ static int __devexit tmio_mmc_remove(struct platform_device *dev) if (mmc) { struct tmio_mmc_host *host = mmc_priv(mmc); mmc_remove_host(mmc); + cancel_delayed_work_sync(&host->delayed_reset_work); tmio_mmc_release_dma(host); free_irq(host->irq, host); if (cell->disable) -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 1/2 v2] mmc: tmio: handle missing HW interrupts @ 2010-12-29 13:21 ` Arnd Hannemann 0 siblings, 0 replies; 14+ messages in thread From: Arnd Hannemann @ 2010-12-29 13:21 UTC (permalink / raw) To: linux-mmc, Ian Molton; +Cc: linux-sh, Arnd Hannemann When doing excessive hotplug, e.g., repeated insert/eject operations, the hardware may get confused to a point where no CMDTIMEOUT/CMDRESPEND interrupts are generated any more. As a result requests get stuck, e.g.: [ 360.351562] INFO: task kworker/u:0:5 blocked for more than 120 seconds. [ 360.351562] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 360.359375] kworker/u:0 D c020c2b4 0 5 2 0x00000000 [ 360.367187] Backtrace: [ 360.367187] [<c020bfb0>] (schedule+0x0/0x340) from [<c020c480>] (schedule_timeout+0x20/0x190) [ 360.375000] r8:c702fd70 r7:00000002 r6:c702e000 r5:c702fdc4 r4:7fffffff [ 360.375000] r3:c701e040 [ 360.382812] [<c020c460>] (schedule_timeout+0x0/0x190) from [<c020be78>] (wait_for_common+0xc4/0x150) [ 360.390625] r6:c702e000 r5:c702fdc4 r4:7fffffff [ 360.390625] [<c020bdb4>] (wait_for_common+0x0/0x150) from [<c020bfac>] (wait_for_completion+0x18/0x1c) [ 360.398437] [<c020bf94>] (wait_for_completion+0x0/0x1c) from [<c0185590>] (mmc_wait_for_req+0x214/0x234) [ 360.406250] [<c018537c>] (mmc_wait_for_req+0x0/0x234) from [<c01889d0>] (mmc_sd_switch+0xfc/0x114) [ 360.414062] r7:c702fe4c r6:c702fe20 r5:c7179800 r4:00fffff0 [ 360.421875] [<c01888d4>] (mmc_sd_switch+0x0/0x114) from [<c0187f70>] (mmc_sd_setup_card+0x260/0x384) [ 360.429687] [<c0187d10>] (mmc_sd_setup_card+0x0/0x384) from [<c01885e0>] (mmc_sd_init_card+0x13c/0x1e0) [ 360.437500] [<c01884a4>] (mmc_sd_init_card+0x0/0x1e0) from [<c01887a8>] (mmc_attach_sd+0x124/0x1a8) [ 360.445312] r8:c02db404 r7:ffffff92 r6:c702ff34 r5:c6007da8 r4:c6007c00 [ 360.453125] [<c0188684>] (mmc_attach_sd+0x0/0x1a8) from [<c0185140>] (mmc_rescan+0x248/0x2f0) [ 360.460937] r5:c6007da8 r4:c6007c00 [ 360.468750] [<c0184ef8>] (mmc_rescan+0x0/0x2f0) from [<c00467f0>] (process_one_work+0x1ec/0x318) [ 360.476562] r7:c6007da8 r6:00000000 r5:c710ec00 r4:c701bde0 [ 360.484375] [<c0046604>] (process_one_work+0x0/0x318) from [<c0047fb0>] (worker_thread+0x1b0/0x2cc) [ 360.492187] [<c0047e00>] (worker_thread+0x0/0x2cc) from [<c004b338>] (kthread+0x8c/0x94) [ 360.500000] [<c004b2ac>] (kthread+0x0/0x94) from [<c0037fc4>] (do_exit+0x0/0x590) [ 360.507812] r7:00000013 r6:c0037fc4 r5:c004b2ac r4:c7021f00 This patch addresses this problem by introducing timeouts for outstanding interrupts. If a hardware interrupt is missing, a soft reset will be performed to bring the hardware back to a working state. Tested with the SDHI hardware block in sh7372 / AP4EVB. Signed-off-by: Arnd Hannemann <arnd@arndnet.de> --- Changes in v2: - rebased against -rc7 - depends on: mmc: tmio: merge the private header into the driver https://patchwork.kernel.org/patch/349931/ drivers/mmc/host/tmio_mmc.c | 90 +++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 86 insertions(+), 4 deletions(-) diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c index a0cf04a..f980042 100644 --- a/drivers/mmc/host/tmio_mmc.c +++ b/drivers/mmc/host/tmio_mmc.c @@ -39,6 +39,8 @@ #include <linux/module.h> #include <linux/pagemap.h> #include <linux/scatterlist.h> +#include <linux/workqueue.h> +#include <linux/spinlock.h> #define CTL_SD_CMD 0x00 #define CTL_ARG_REG 0x04 @@ -154,6 +156,11 @@ struct tmio_mmc_host { u8 bounce_buf[PAGE_CACHE_SIZE] __attribute__((aligned(MAX_ALIGN))); struct scatterlist bounce_sg; #endif + + /* Track lost interrupts */ + struct delayed_work delayed_reset_work; + spinlock_t lock; + unsigned long last_req_ts; }; static u16 sd_ctrl_read16(struct tmio_mmc_host *host, int addr) @@ -342,15 +349,60 @@ static void reset(struct tmio_mmc_host *host) msleep(10); } +static void tmio_mmc_reset_work(struct work_struct *work) +{ + struct tmio_mmc_host *host = container_of(work, struct tmio_mmc_host, + delayed_reset_work.work); + struct mmc_request *mrq; + unsigned long flags; + + spin_lock_irqsave(&host->lock, flags); + mrq = host->mrq; + + /* request already finished */ + if (!mrq + || time_is_after_jiffies(host->last_req_ts + + msecs_to_jiffies(2000))) { + spin_unlock_irqrestore(&host->lock, flags); + return; + } + + dev_warn(&host->pdev->dev, + "timeout waiting for hardware interrupt (CMD%u)\n", + mrq->cmd->opcode); + + if (host->data) + host->data->error = -ETIMEDOUT; + else if (host->cmd) + host->cmd->error = -ETIMEDOUT; + else + mrq->cmd->error = -ETIMEDOUT; + + host->cmd = NULL; + host->data = NULL; + host->mrq = NULL; + + spin_unlock_irqrestore(&host->lock, flags); + + reset(host); + + mmc_request_done(host->mmc, mrq); +} + static void tmio_mmc_finish_request(struct tmio_mmc_host *host) { struct mmc_request *mrq = host->mrq; + if (!mrq) + return; + host->mrq = NULL; host->cmd = NULL; host->data = NULL; + cancel_delayed_work(&host->delayed_reset_work); + mmc_request_done(host->mmc, mrq); } @@ -460,6 +512,7 @@ static void tmio_mmc_pio_irq(struct tmio_mmc_host *host) return; } +/* needs to be called with host->lock held */ static void tmio_mmc_do_data_irq(struct tmio_mmc_host *host) { struct mmc_data *data = host->data; @@ -520,10 +573,12 @@ static void tmio_mmc_do_data_irq(struct tmio_mmc_host *host) static void tmio_mmc_data_irq(struct tmio_mmc_host *host) { - struct mmc_data *data = host->data; + struct mmc_data *data; + spin_lock(&host->lock); + data = host->data; if (!data) - return; + goto out; if (host->chan_tx && (data->flags & MMC_DATA_WRITE)) { /* @@ -544,6 +599,8 @@ static void tmio_mmc_data_irq(struct tmio_mmc_host *host) } else { tmio_mmc_do_data_irq(host); } +out: + spin_unlock(&host->lock); } static void tmio_mmc_cmd_irq(struct tmio_mmc_host *host, @@ -552,9 +609,11 @@ static void tmio_mmc_cmd_irq(struct tmio_mmc_host *host, struct mmc_command *cmd = host->cmd; int i, addr; + spin_lock(&host->lock); + if (!host->cmd) { pr_debug("Spurious CMD irq\n"); - return; + goto out; } host->cmd = NULL; @@ -599,6 +658,9 @@ static void tmio_mmc_cmd_irq(struct tmio_mmc_host *host, tmio_mmc_finish_request(host); } +out: + spin_unlock(&host->lock); + return; } @@ -899,6 +961,12 @@ static void tmio_issue_tasklet_fn(unsigned long priv) static void tmio_tasklet_fn(unsigned long arg) { struct tmio_mmc_host *host = (struct tmio_mmc_host *)arg; + unsigned long flags; + + spin_lock_irqsave(&host->lock, flags); + + if (!host->data) + goto out; if (host->data->flags & MMC_DATA_READ) dma_unmap_sg(&host->pdev->dev, host->sg_ptr, host->dma_sglen, @@ -908,6 +976,8 @@ static void tmio_tasklet_fn(unsigned long arg) DMA_TO_DEVICE); tmio_mmc_do_data_irq(host); +out: + spin_unlock_irqrestore(&host->lock, flags); } /* It might be necessary to make filter MFD specific */ @@ -1026,6 +1096,8 @@ static void tmio_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq) if (host->mrq) pr_debug("request not null\n"); + host->last_req_ts = jiffies; + wmb(); host->mrq = mrq; if (mrq->data) { @@ -1035,10 +1107,14 @@ static void tmio_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq) } ret = tmio_mmc_start_command(host, mrq->cmd); - if (!ret) + if (!ret) { + schedule_delayed_work(&host->delayed_reset_work, + msecs_to_jiffies(2000)); return; + } fail: + host->mrq = NULL; mrq->cmd->error = ret; mmc_request_done(mmc, mrq); } @@ -1235,6 +1311,11 @@ static int __devinit tmio_mmc_probe(struct platform_device *dev) if (ret) goto cell_disable; + spin_lock_init(&host->lock); + + /* Init delayed work for request timeouts */ + INIT_DELAYED_WORK(&host->delayed_reset_work, tmio_mmc_reset_work); + /* See if we also get DMA */ tmio_mmc_request_dma(host, pdata); @@ -1273,6 +1354,7 @@ static int __devexit tmio_mmc_remove(struct platform_device *dev) if (mmc) { struct tmio_mmc_host *host = mmc_priv(mmc); mmc_remove_host(mmc); + cancel_delayed_work_sync(&host->delayed_reset_work); tmio_mmc_release_dma(host); free_irq(host->irq, host); if (cell->disable) -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2 v2] mmc: tmio: handle missing HW interrupts 2010-12-29 13:21 ` Arnd Hannemann @ 2011-01-05 21:22 ` Chris Ball -1 siblings, 0 replies; 14+ messages in thread From: Chris Ball @ 2011-01-05 21:22 UTC (permalink / raw) To: Arnd Hannemann; +Cc: linux-mmc, Ian Molton, linux-sh Hi Arnd, On Wed, Dec 29, 2010 at 02:21:13PM +0100, Arnd Hannemann wrote: > This patch addresses this problem by introducing timeouts for outstanding > interrupts. If a hardware interrupt is missing, a soft reset will be performed > to bring the hardware back to a working state. > Tested with the SDHI hardware block in sh7372 / AP4EVB. > > Signed-off-by: Arnd Hannemann <arnd@arndnet.de> This breaks compilation without CONFIG_TMIO_MMC_DMA=y, because it attempts to compile tmio_mmc_reset_work() unconditionally even though delayed_reset_work is only a member of tmio_mmc_host if TMIO_MMC_DMA=y: drivers/mmc/host/tmio_mmc.c: In function ‘tmio_mmc_reset_work’: drivers/mmc/host/tmio_mmc.c:357:31: error: ‘struct tmio_mmc_host’ has no member named ‘delayed_reset_work’ -- Chris Ball <cjb@laptop.org> <http://printf.net/> One Laptop Per Child ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2 v2] mmc: tmio: handle missing HW interrupts @ 2011-01-05 21:22 ` Chris Ball 0 siblings, 0 replies; 14+ messages in thread From: Chris Ball @ 2011-01-05 21:22 UTC (permalink / raw) To: Arnd Hannemann; +Cc: linux-mmc, Ian Molton, linux-sh Hi Arnd, On Wed, Dec 29, 2010 at 02:21:13PM +0100, Arnd Hannemann wrote: > This patch addresses this problem by introducing timeouts for outstanding > interrupts. If a hardware interrupt is missing, a soft reset will be performed > to bring the hardware back to a working state. > Tested with the SDHI hardware block in sh7372 / AP4EVB. > > Signed-off-by: Arnd Hannemann <arnd@arndnet.de> This breaks compilation without CONFIG_TMIO_MMC_DMA=y, because it attempts to compile tmio_mmc_reset_work() unconditionally even though delayed_reset_work is only a member of tmio_mmc_host if TMIO_MMC_DMA=y: drivers/mmc/host/tmio_mmc.c: In function ‘tmio_mmc_reset_work’: drivers/mmc/host/tmio_mmc.c:357:31: error: ‘struct tmio_mmc_host’ has no member named ‘delayed_reset_work’ -- Chris Ball <cjb@laptop.org> <http://printf.net/> One Laptop Per Child ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2 v2] mmc: tmio: handle missing HW interrupts 2011-01-05 21:22 ` Chris Ball @ 2011-01-05 22:31 ` Arnd Hannemann -1 siblings, 0 replies; 14+ messages in thread From: Arnd Hannemann @ 2011-01-05 22:31 UTC (permalink / raw) To: Chris Ball; +Cc: linux-mmc, Ian Molton, linux-sh Hi Chris, Am 05.01.2011 22:22, schrieb Chris Ball: > Hi Arnd, > > On Wed, Dec 29, 2010 at 02:21:13PM +0100, Arnd Hannemann wrote: >> This patch addresses this problem by introducing timeouts for outstanding >> interrupts. If a hardware interrupt is missing, a soft reset will be performed >> to bring the hardware back to a working state. >> Tested with the SDHI hardware block in sh7372 / AP4EVB. >> >> Signed-off-by: Arnd Hannemann <arnd@arndnet.de> > > This breaks compilation without CONFIG_TMIO_MMC_DMA=y, because it > attempts to compile tmio_mmc_reset_work() unconditionally even though > delayed_reset_work is only a member of tmio_mmc_host if TMIO_MMC_DMA=y: > > drivers/mmc/host/tmio_mmc.c: In function ‘tmio_mmc_reset_work’: > drivers/mmc/host/tmio_mmc.c:357:31: error: ‘struct tmio_mmc_host’ has no member named ‘delayed_reset_work’ Hmm, I could not reproduce this. delayed_reset_work should not be in the #ifdef TMIO_MMC_DMA #endif scope. And it isn't according to the patch. I tried with mmc-next last commit "549bad416ef62f09711cb22e77adff029e27ce07". The patch would apply with some fuzz, but compilation without CONFIG_TMIO_MMC_DMA=y works. With what tree and .config did you try? Maybe I confused you with my email mentioning the older patches, did you try the (rebased) v2: https://patchwork.kernel.org/patch/439421/ https://patchwork.kernel.org/patch/439431/ Thanks Arnd ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2 v2] mmc: tmio: handle missing HW interrupts @ 2011-01-05 22:31 ` Arnd Hannemann 0 siblings, 0 replies; 14+ messages in thread From: Arnd Hannemann @ 2011-01-05 22:31 UTC (permalink / raw) To: Chris Ball; +Cc: linux-mmc, Ian Molton, linux-sh Hi Chris, Am 05.01.2011 22:22, schrieb Chris Ball: > Hi Arnd, > > On Wed, Dec 29, 2010 at 02:21:13PM +0100, Arnd Hannemann wrote: >> This patch addresses this problem by introducing timeouts for outstanding >> interrupts. If a hardware interrupt is missing, a soft reset will be performed >> to bring the hardware back to a working state. >> Tested with the SDHI hardware block in sh7372 / AP4EVB. >> >> Signed-off-by: Arnd Hannemann <arnd@arndnet.de> > > This breaks compilation without CONFIG_TMIO_MMC_DMA=y, because it > attempts to compile tmio_mmc_reset_work() unconditionally even though > delayed_reset_work is only a member of tmio_mmc_host if TMIO_MMC_DMA=y: > > drivers/mmc/host/tmio_mmc.c: In function ‘tmio_mmc_reset_work’: > drivers/mmc/host/tmio_mmc.c:357:31: error: ‘struct tmio_mmc_host’ has no member named ‘delayed_reset_work’ Hmm, I could not reproduce this. delayed_reset_work should not be in the #ifdef TMIO_MMC_DMA #endif scope. And it isn't according to the patch. I tried with mmc-next last commit "549bad416ef62f09711cb22e77adff029e27ce07". The patch would apply with some fuzz, but compilation without CONFIG_TMIO_MMC_DMA=y works. With what tree and .config did you try? Maybe I confused you with my email mentioning the older patches, did you try the (rebased) v2: https://patchwork.kernel.org/patch/439421/ https://patchwork.kernel.org/patch/439431/ Thanks Arnd ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2 v2] mmc: tmio: handle missing HW interrupts 2011-01-05 22:31 ` Arnd Hannemann @ 2011-01-05 22:44 ` Chris Ball -1 siblings, 0 replies; 14+ messages in thread From: Chris Ball @ 2011-01-05 22:44 UTC (permalink / raw) To: Arnd Hannemann; +Cc: linux-mmc, Ian Molton, linux-sh Hi Arnd, On Wed, Jan 05, 2011 at 11:31:00PM +0100, Arnd Hannemann wrote: > Hmm, I could not reproduce this. > delayed_reset_work should not be in the #ifdef TMIO_MMC_DMA #endif scope. > And it isn't according to the patch. > > I tried with mmc-next last commit "549bad416ef62f09711cb22e77adff029e27ce07". > The patch would apply with some fuzz, but compilation without CONFIG_TMIO_MMC_DMA=y works. Oops, my apologies -- I mishandled the fuzz. The patch is fine as-is. I've pushed it to mmc-next now. -- Chris Ball <cjb@laptop.org> <http://printf.net/> One Laptop Per Child ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2 v2] mmc: tmio: handle missing HW interrupts @ 2011-01-05 22:44 ` Chris Ball 0 siblings, 0 replies; 14+ messages in thread From: Chris Ball @ 2011-01-05 22:44 UTC (permalink / raw) To: Arnd Hannemann; +Cc: linux-mmc, Ian Molton, linux-sh Hi Arnd, On Wed, Jan 05, 2011 at 11:31:00PM +0100, Arnd Hannemann wrote: > Hmm, I could not reproduce this. > delayed_reset_work should not be in the #ifdef TMIO_MMC_DMA #endif scope. > And it isn't according to the patch. > > I tried with mmc-next last commit "549bad416ef62f09711cb22e77adff029e27ce07". > The patch would apply with some fuzz, but compilation without CONFIG_TMIO_MMC_DMA=y works. Oops, my apologies -- I mishandled the fuzz. The patch is fine as-is. I've pushed it to mmc-next now. -- Chris Ball <cjb@laptop.org> <http://printf.net/> One Laptop Per Child ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2 v2] mmc: tmio: fix CMD irq handling 2010-12-29 13:21 ` Arnd Hannemann @ 2010-12-29 13:21 ` Arnd Hannemann -1 siblings, 0 replies; 14+ messages in thread From: Arnd Hannemann @ 2010-12-29 13:21 UTC (permalink / raw) To: linux-mmc, Ian Molton; +Cc: linux-sh, Arnd Hannemann With current code card insert/eject interrupts will acknowledge outstanding commands. Normally this seems to be no problem, however if the hardware gets stuck and no interrupts for CMD_TIMEOUT or CMD_RESPEND are generated, then inserting and ejecting cards will falsely acknowledge outstanding commands from the core. This patch changes the behavior so that CMDs are only acked, if CMD_TIMEOUT or CMD_RESPEND is received. Signed-off-by: Arnd Hannemann <arnd@arndnet.de> --- drivers/mmc/host/tmio_mmc.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c index f980042..e4926c7 100644 --- a/drivers/mmc/host/tmio_mmc.c +++ b/drivers/mmc/host/tmio_mmc.c @@ -728,8 +728,10 @@ static irqreturn_t tmio_mmc_irq(int irq, void *devid) */ /* Command completion */ - if (ireg & TMIO_MASK_CMD) { - ack_mmc_irqs(host, TMIO_MASK_CMD); + if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) { + ack_mmc_irqs(host, + TMIO_STAT_CMDRESPEND | + TMIO_STAT_CMDTIMEOUT); tmio_mmc_cmd_irq(host, status); } -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2 v2] mmc: tmio: fix CMD irq handling @ 2010-12-29 13:21 ` Arnd Hannemann 0 siblings, 0 replies; 14+ messages in thread From: Arnd Hannemann @ 2010-12-29 13:21 UTC (permalink / raw) To: linux-mmc, Ian Molton; +Cc: linux-sh, Arnd Hannemann With current code card insert/eject interrupts will acknowledge outstanding commands. Normally this seems to be no problem, however if the hardware gets stuck and no interrupts for CMD_TIMEOUT or CMD_RESPEND are generated, then inserting and ejecting cards will falsely acknowledge outstanding commands from the core. This patch changes the behavior so that CMDs are only acked, if CMD_TIMEOUT or CMD_RESPEND is received. Signed-off-by: Arnd Hannemann <arnd@arndnet.de> --- drivers/mmc/host/tmio_mmc.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c index f980042..e4926c7 100644 --- a/drivers/mmc/host/tmio_mmc.c +++ b/drivers/mmc/host/tmio_mmc.c @@ -728,8 +728,10 @@ static irqreturn_t tmio_mmc_irq(int irq, void *devid) */ /* Command completion */ - if (ireg & TMIO_MASK_CMD) { - ack_mmc_irqs(host, TMIO_MASK_CMD); + if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) { + ack_mmc_irqs(host, + TMIO_STAT_CMDRESPEND | + TMIO_STAT_CMDTIMEOUT); tmio_mmc_cmd_irq(host, status); } -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2 v2] mmc: tmio: fix CMD irq handling 2010-12-29 13:21 ` Arnd Hannemann @ 2011-01-05 22:48 ` Chris Ball -1 siblings, 0 replies; 14+ messages in thread From: Chris Ball @ 2011-01-05 22:48 UTC (permalink / raw) To: Arnd Hannemann; +Cc: linux-mmc, Ian Molton, linux-sh Hi Arnd, On Wed, Dec 29, 2010 at 02:21:14PM +0100, Arnd Hannemann wrote: > With current code card insert/eject interrupts will acknowledge outstanding > commands. Normally this seems to be no problem, however if the hardware gets > stuck and no interrupts for CMD_TIMEOUT or CMD_RESPEND are generated, then > inserting and ejecting cards will falsely acknowledge outstanding commands from > the core. > > This patch changes the behavior so that CMDs are only acked, if > CMD_TIMEOUT or CMD_RESPEND is received. > > Signed-off-by: Arnd Hannemann <arnd@arndnet.de> > --- > drivers/mmc/host/tmio_mmc.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c > index f980042..e4926c7 100644 > --- a/drivers/mmc/host/tmio_mmc.c > +++ b/drivers/mmc/host/tmio_mmc.c > @@ -728,8 +728,10 @@ static irqreturn_t tmio_mmc_irq(int irq, void *devid) > */ > > /* Command completion */ > - if (ireg & TMIO_MASK_CMD) { > - ack_mmc_irqs(host, TMIO_MASK_CMD); > + if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) { > + ack_mmc_irqs(host, > + TMIO_STAT_CMDRESPEND | > + TMIO_STAT_CMDTIMEOUT); > tmio_mmc_cmd_irq(host, status); > } > Thanks, pushed to mmc-next for .38. -- Chris Ball <cjb@laptop.org> <http://printf.net/> One Laptop Per Child ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2 v2] mmc: tmio: fix CMD irq handling @ 2011-01-05 22:48 ` Chris Ball 0 siblings, 0 replies; 14+ messages in thread From: Chris Ball @ 2011-01-05 22:48 UTC (permalink / raw) To: Arnd Hannemann; +Cc: linux-mmc, Ian Molton, linux-sh Hi Arnd, On Wed, Dec 29, 2010 at 02:21:14PM +0100, Arnd Hannemann wrote: > With current code card insert/eject interrupts will acknowledge outstanding > commands. Normally this seems to be no problem, however if the hardware gets > stuck and no interrupts for CMD_TIMEOUT or CMD_RESPEND are generated, then > inserting and ejecting cards will falsely acknowledge outstanding commands from > the core. > > This patch changes the behavior so that CMDs are only acked, if > CMD_TIMEOUT or CMD_RESPEND is received. > > Signed-off-by: Arnd Hannemann <arnd@arndnet.de> > --- > drivers/mmc/host/tmio_mmc.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c > index f980042..e4926c7 100644 > --- a/drivers/mmc/host/tmio_mmc.c > +++ b/drivers/mmc/host/tmio_mmc.c > @@ -728,8 +728,10 @@ static irqreturn_t tmio_mmc_irq(int irq, void *devid) > */ > > /* Command completion */ > - if (ireg & TMIO_MASK_CMD) { > - ack_mmc_irqs(host, TMIO_MASK_CMD); > + if (ireg & (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT)) { > + ack_mmc_irqs(host, > + TMIO_STAT_CMDRESPEND | > + TMIO_STAT_CMDTIMEOUT); > tmio_mmc_cmd_irq(host, status); > } > Thanks, pushed to mmc-next for .38. -- Chris Ball <cjb@laptop.org> <http://printf.net/> One Laptop Per Child ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2011-01-05 22:48 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-12-29 13:21 [PATCH 0/2 v2] mmc: tmio: handle missing HW interrupts Arnd Hannemann 2010-12-29 13:21 ` Arnd Hannemann 2010-12-29 13:21 ` [PATCH 1/2 " Arnd Hannemann 2010-12-29 13:21 ` Arnd Hannemann 2011-01-05 21:22 ` Chris Ball 2011-01-05 21:22 ` Chris Ball 2011-01-05 22:31 ` Arnd Hannemann 2011-01-05 22:31 ` Arnd Hannemann 2011-01-05 22:44 ` Chris Ball 2011-01-05 22:44 ` Chris Ball 2010-12-29 13:21 ` [PATCH 2/2 v2] mmc: tmio: fix CMD irq handling Arnd Hannemann 2010-12-29 13:21 ` Arnd Hannemann 2011-01-05 22:48 ` Chris Ball 2011-01-05 22:48 ` Chris Ball
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.