1 From: Haavard Skinnemoen <hskinnemoen@atmel.com>
2 Subject: [PATCH 2/2] atmel_mci: Fix two subtle but deadly races
4 This patch fixes two possible races in the atmel_mci driver, at least
5 one of which may cause card probing to fail.
7 The first one may happen if a command fails and the next command is
8 queued before the controller is ready to accept a new one. Fix this by
9 not enabling error interrupts for commands and instead do any error
10 handling when the CMDRDY bit has been set.
12 The second one may happen after a successful read data transfer where
13 then next command is queued after the DMA transfer is complete, but
14 before the whole data transfer from the card is complete (i.e. the
15 card is still sending CRC, for example.) Fix this by waiting for the
16 NOTBUSY bit to be set before considering the request to be done. This
17 will also ensure that we actually see any CRC failures before
18 completing the request.
20 Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
22 drivers/mmc/host/atmel-mci.c | 172 +++++++++++++-----------------------------
23 1 files changed, 54 insertions(+), 118 deletions(-)
25 diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
26 index 1dc91b4..45323c9 100644
27 --- a/drivers/mmc/host/atmel-mci.c
28 +++ b/drivers/mmc/host/atmel-mci.c
31 #define DRIVER_NAME "atmel_mci"
33 -#define MCI_CMD_ERROR_FLAGS (MCI_BIT(RINDE) | MCI_BIT(RDIRE) | \
34 - MCI_BIT(RCRCE) | MCI_BIT(RENDE) | \
36 #define MCI_DATA_ERROR_FLAGS (MCI_BIT(DCRCE) | MCI_BIT(DTOE) | \
37 MCI_BIT(OVRE) | MCI_BIT(UNRE))
40 EVENT_CMD_COMPLETE = 0,
50 @@ -61,13 +56,14 @@ struct atmel_mci {
51 struct mmc_command *cmd;
52 struct mmc_data *data;
60 struct tasklet_struct tasklet;
61 unsigned long pending_events;
62 unsigned long completed_events;
67 @@ -99,8 +95,6 @@ MODULE_PARM_DESC(fmax, "Max frequency in Hz of the MMC bus clock");
68 /* Test bit macros for completed events */
69 #define mci_cmd_is_complete(host) \
70 test_bit(EVENT_CMD_COMPLETE, &host->completed_events)
71 -#define mci_cmd_error_is_complete(host) \
72 - test_bit(EVENT_CMD_ERROR, &host->completed_events)
73 #define mci_data_is_complete(host) \
74 test_bit(EVENT_DATA_COMPLETE, &host->completed_events)
75 #define mci_data_error_is_complete(host) \
76 @@ -109,8 +103,6 @@ MODULE_PARM_DESC(fmax, "Max frequency in Hz of the MMC bus clock");
77 test_bit(EVENT_STOP_SENT, &host->completed_events)
78 #define mci_stop_is_complete(host) \
79 test_bit(EVENT_STOP_COMPLETE, &host->completed_events)
80 -#define mci_stop_error_is_complete(host) \
81 - test_bit(EVENT_STOP_ERROR, &host->completed_events)
82 #define mci_dma_error_is_complete(host) \
83 test_bit(EVENT_DMA_ERROR, &host->completed_events)
84 #define mci_card_detect_is_complete(host) \
85 @@ -119,8 +111,6 @@ MODULE_PARM_DESC(fmax, "Max frequency in Hz of the MMC bus clock");
86 /* Test and clear bit macros for pending events */
87 #define mci_clear_cmd_is_pending(host) \
88 test_and_clear_bit(EVENT_CMD_COMPLETE, &host->pending_events)
89 -#define mci_clear_cmd_error_is_pending(host) \
90 - test_and_clear_bit(EVENT_CMD_ERROR, &host->pending_events)
91 #define mci_clear_data_is_pending(host) \
92 test_and_clear_bit(EVENT_DATA_COMPLETE, &host->pending_events)
93 #define mci_clear_data_error_is_pending(host) \
94 @@ -129,8 +119,6 @@ MODULE_PARM_DESC(fmax, "Max frequency in Hz of the MMC bus clock");
95 test_and_clear_bit(EVENT_STOP_SENT, &host->pending_events)
96 #define mci_clear_stop_is_pending(host) \
97 test_and_clear_bit(EVENT_STOP_COMPLETE, &host->pending_events)
98 -#define mci_clear_stop_error_is_pending(host) \
99 - test_and_clear_bit(EVENT_STOP_ERROR, &host->pending_events)
100 #define mci_clear_dma_error_is_pending(host) \
101 test_and_clear_bit(EVENT_DMA_ERROR, &host->pending_events)
102 #define mci_clear_card_detect_is_pending(host) \
103 @@ -139,8 +127,6 @@ MODULE_PARM_DESC(fmax, "Max frequency in Hz of the MMC bus clock");
104 /* Test and set bit macros for completed events */
105 #define mci_set_cmd_is_completed(host) \
106 test_and_set_bit(EVENT_CMD_COMPLETE, &host->completed_events)
107 -#define mci_set_cmd_error_is_completed(host) \
108 - test_and_set_bit(EVENT_CMD_ERROR, &host->completed_events)
109 #define mci_set_data_is_completed(host) \
110 test_and_set_bit(EVENT_DATA_COMPLETE, &host->completed_events)
111 #define mci_set_data_error_is_completed(host) \
112 @@ -149,8 +135,6 @@ MODULE_PARM_DESC(fmax, "Max frequency in Hz of the MMC bus clock");
113 test_and_set_bit(EVENT_STOP_SENT, &host->completed_events)
114 #define mci_set_stop_is_completed(host) \
115 test_and_set_bit(EVENT_STOP_COMPLETE, &host->completed_events)
116 -#define mci_set_stop_error_is_completed(host) \
117 - test_and_set_bit(EVENT_STOP_ERROR, &host->completed_events)
118 #define mci_set_dma_error_is_completed(host) \
119 test_and_set_bit(EVENT_DMA_ERROR, &host->completed_events)
120 #define mci_set_card_detect_is_completed(host) \
121 @@ -159,8 +143,6 @@ MODULE_PARM_DESC(fmax, "Max frequency in Hz of the MMC bus clock");
122 /* Set bit macros for completed events */
123 #define mci_set_cmd_complete(host) \
124 set_bit(EVENT_CMD_COMPLETE, &host->completed_events)
125 -#define mci_set_cmd_error_complete(host) \
126 - set_bit(EVENT_CMD_ERROR, &host->completed_events)
127 #define mci_set_data_complete(host) \
128 set_bit(EVENT_DATA_COMPLETE, &host->completed_events)
129 #define mci_set_data_error_complete(host) \
130 @@ -169,8 +151,6 @@ MODULE_PARM_DESC(fmax, "Max frequency in Hz of the MMC bus clock");
131 set_bit(EVENT_STOP_SENT, &host->completed_events)
132 #define mci_set_stop_complete(host) \
133 set_bit(EVENT_STOP_COMPLETE, &host->completed_events)
134 -#define mci_set_stop_error_complete(host) \
135 - set_bit(EVENT_STOP_ERROR, &host->completed_events)
136 #define mci_set_dma_error_complete(host) \
137 set_bit(EVENT_DMA_ERROR, &host->completed_events)
138 #define mci_set_card_detect_complete(host) \
139 @@ -179,8 +159,6 @@ MODULE_PARM_DESC(fmax, "Max frequency in Hz of the MMC bus clock");
140 /* Set bit macros for pending events */
141 #define mci_set_cmd_pending(host) \
142 set_bit(EVENT_CMD_COMPLETE, &host->pending_events)
143 -#define mci_set_cmd_error_pending(host) \
144 - set_bit(EVENT_CMD_ERROR, &host->pending_events)
145 #define mci_set_data_pending(host) \
146 set_bit(EVENT_DATA_COMPLETE, &host->pending_events)
147 #define mci_set_data_error_pending(host) \
148 @@ -189,8 +167,6 @@ MODULE_PARM_DESC(fmax, "Max frequency in Hz of the MMC bus clock");
149 set_bit(EVENT_STOP_SENT, &host->pending_events)
150 #define mci_set_stop_pending(host) \
151 set_bit(EVENT_STOP_COMPLETE, &host->pending_events)
152 -#define mci_set_stop_error_pending(host) \
153 - set_bit(EVENT_STOP_ERROR, &host->pending_events)
154 #define mci_set_dma_error_pending(host) \
155 set_bit(EVENT_DMA_ERROR, &host->pending_events)
156 #define mci_set_card_detect_pending(host) \
157 @@ -199,8 +175,6 @@ MODULE_PARM_DESC(fmax, "Max frequency in Hz of the MMC bus clock");
158 /* Clear bit macros for pending events */
159 #define mci_clear_cmd_pending(host) \
160 clear_bit(EVENT_CMD_COMPLETE, &host->pending_events)
161 -#define mci_clear_cmd_error_pending(host) \
162 - clear_bit(EVENT_CMD_ERROR, &host->pending_events)
163 #define mci_clear_data_pending(host) \
164 clear_bit(EVENT_DATA_COMPLETE, &host->pending_events)
165 #define mci_clear_data_error_pending(host) \
166 @@ -209,8 +183,6 @@ MODULE_PARM_DESC(fmax, "Max frequency in Hz of the MMC bus clock");
167 clear_bit(EVENT_STOP_SENT, &host->pending_events)
168 #define mci_clear_stop_pending(host) \
169 clear_bit(EVENT_STOP_COMPLETE, &host->pending_events)
170 -#define mci_clear_stop_error_pending(host) \
171 - clear_bit(EVENT_STOP_ERROR, &host->pending_events)
172 #define mci_clear_dma_error_pending(host) \
173 clear_bit(EVENT_DMA_ERROR, &host->pending_events)
174 #define mci_clear_card_detect_pending(host) \
175 @@ -471,20 +443,16 @@ static void atmci_set_timeout(struct atmel_mci *host,
179 - * Return mask with interrupt flags to be handled for this command.
180 + * Return mask with command flags to be enabled for this command.
182 static u32 atmci_prepare_command(struct mmc_host *mmc,
183 - struct mmc_command *cmd,
185 + struct mmc_command *cmd)
190 cmd->error = MMC_ERR_NONE;
193 - BUG_ON(MCI_BFEXT(CMDNB, cmdr) != 0);
194 - cmdr = MCI_BFINS(CMDNB, cmd->opcode, cmdr);
195 + cmdr = MCI_BF(CMDNB, cmd->opcode);
197 if (cmd->flags & MMC_RSP_PRESENT) {
198 if (cmd->flags & MMC_RSP_136)
199 @@ -503,16 +471,11 @@ static u32 atmci_prepare_command(struct mmc_host *mmc,
200 if (mmc->ios.bus_mode == MMC_BUSMODE_OPENDRAIN)
201 cmdr |= MCI_BIT(OPDCMD);
203 - iflags = MCI_BIT(CMDRDY) | MCI_CMD_ERROR_FLAGS;
204 - if (!(cmd->flags & MMC_RSP_CRC))
205 - iflags &= ~MCI_BIT(RCRCE);
207 dev_dbg(&mmc->class_dev,
208 "cmd: op %02x arg %08x flags %08x, cmdflags %08lx\n",
209 cmd->opcode, cmd->arg, cmd->flags, (unsigned long)cmdr);
216 static void atmci_start_command(struct atmel_mci *host,
217 @@ -596,13 +559,13 @@ static void atmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
218 host->pending_events = 0;
219 host->completed_events = 0;
221 - iflags = atmci_prepare_command(mmc, mrq->cmd, &cmdflags);
222 + iflags = MCI_BIT(CMDRDY);
223 + cmdflags = atmci_prepare_command(mmc, mrq->cmd);
229 - host->stop_iflags = atmci_prepare_command(mmc, mrq->stop,
231 + host->stop_cmdr = atmci_prepare_command(mmc, mrq->stop);
232 host->stop_cmdr |= MCI_BF(TRCMD, MCI_TRCMD_STOP_TRANS);
233 if (!(data->flags & MMC_DATA_WRITE))
234 host->stop_cmdr |= MCI_BIT(TRDIR);
235 @@ -716,7 +679,7 @@ static void send_stop_cmd(struct mmc_host *mmc, struct mmc_data *data,
236 struct atmel_mci *host = mmc_priv(mmc);
238 atmci_start_command(host, data->stop, host->stop_cmdr | flags);
239 - mci_writel(host, IER, host->stop_iflags);
240 + mci_writel(host, IER, MCI_BIT(CMDRDY));
243 static void atmci_data_complete(struct atmel_mci *host, struct mmc_data *data)
244 @@ -735,18 +698,30 @@ static void atmci_data_complete(struct atmel_mci *host, struct mmc_data *data)
245 atmci_request_end(host->mmc, data->mrq);
248 -static void atmci_command_error(struct mmc_host *mmc,
249 - struct mmc_command *cmd,
251 +static void atmci_command_complete(struct atmel_mci *host,
252 + struct mmc_command *cmd, u32 status)
254 - dev_dbg(&mmc->class_dev, "command error: status=0x%08x\n", status);
256 if (status & MCI_BIT(RTOE))
257 cmd->error = MMC_ERR_TIMEOUT;
258 - else if (status & MCI_BIT(RCRCE))
259 + else if ((cmd->flags & MMC_RSP_CRC)
260 + && (status & MCI_BIT(RCRCE)))
261 cmd->error = MMC_ERR_BADCRC;
263 + else if (status & (MCI_BIT(RINDE) | MCI_BIT(RDIRE) | MCI_BIT(RENDE)))
264 cmd->error = MMC_ERR_FAILED;
266 + if (cmd->error != MMC_ERR_NONE) {
267 + dev_dbg(&host->mmc->class_dev,
268 + "command error: op=0x%x status=0x%08x\n",
269 + cmd->opcode, status);
272 + dma_stop_request(host->dma.req.req.dmac,
273 + host->dma.req.req.channel);
274 + mci_writel(host, IDR, MCI_BIT(NOTBUSY)
275 + | MCI_DATA_ERROR_FLAGS);
281 static void atmci_tasklet_func(unsigned long priv)
282 @@ -761,38 +736,16 @@ static void atmci_tasklet_func(unsigned long priv)
283 host->pending_events, host->completed_events,
284 mci_readl(host, IMR));
286 - if (mci_clear_cmd_error_is_pending(host)) {
287 - struct mmc_command *cmd;
289 - mci_set_cmd_error_complete(host);
290 - mci_clear_cmd_pending(host);
291 - cmd = host->mrq->cmd;
294 - dma_stop_request(host->dma.req.req.dmac,
295 - host->dma.req.req.channel);
299 - atmci_command_error(mmc, cmd, host->error_status);
300 - atmci_request_end(mmc, cmd->mrq);
302 - if (mci_clear_stop_error_is_pending(host)) {
303 - mci_set_stop_error_complete(host);
304 - mci_clear_stop_pending(host);
305 - atmci_command_error(mmc, host->mrq->stop,
306 - host->error_status);
308 - atmci_request_end(mmc, host->mrq);
310 if (mci_clear_cmd_is_pending(host)) {
311 mci_set_cmd_complete(host);
312 - if (!mrq->data || mci_data_is_complete(host)
313 + atmci_command_complete(host, mrq->cmd, host->cmd_status);
314 + if (!host->data || mci_data_is_complete(host)
315 || mci_data_error_is_complete(host))
316 atmci_request_end(mmc, mrq);
318 if (mci_clear_stop_is_pending(host)) {
319 mci_set_stop_complete(host);
320 + atmci_command_complete(host, mrq->stop, host->stop_status);
321 if (mci_data_is_complete(host)
322 || mci_data_error_is_complete(host))
323 atmci_request_end(mmc, mrq);
324 @@ -815,7 +768,7 @@ static void atmci_tasklet_func(unsigned long priv)
325 atmci_data_complete(host, data);
327 if (mci_clear_data_error_is_pending(host)) {
328 - u32 status = host->error_status;
329 + u32 status = host->data_status;
331 mci_set_data_error_complete(host);
332 mci_clear_data_pending(host);
333 @@ -858,10 +811,8 @@ static void atmci_tasklet_func(unsigned long priv)
335 /* Clean up queue if present */
337 - if (!mci_cmd_is_complete(host)
338 - && !mci_cmd_error_is_complete(host)) {
339 + if (!mci_cmd_is_complete(host))
340 mrq->cmd->error = MMC_ERR_TIMEOUT;
342 if (mrq->data && !mci_data_is_complete(host)
343 && !mci_data_error_is_complete(host)) {
344 dma_stop_request(host->dma.req.req.dmac,
345 @@ -869,10 +820,8 @@ static void atmci_tasklet_func(unsigned long priv)
346 host->data->error = MMC_ERR_TIMEOUT;
347 atmci_data_complete(host, data);
349 - if (mrq->stop && !mci_stop_is_complete(host)
350 - && !mci_stop_error_is_complete(host)) {
351 + if (mrq->stop && !mci_stop_is_complete(host))
352 mrq->stop->error = MMC_ERR_TIMEOUT;
356 atmci_request_end(mmc, mrq);
357 @@ -895,13 +844,16 @@ static void atmci_cmd_interrupt(struct mmc_host *mmc, u32 status)
358 cmd->resp[2] = mci_readl(host, RSPR);
359 cmd->resp[3] = mci_readl(host, RSPR);
361 - mci_writel(host, IDR, MCI_BIT(CMDRDY) | MCI_CMD_ERROR_FLAGS);
362 + mci_writel(host, IDR, MCI_BIT(CMDRDY));
365 - if (mci_stop_sent_is_complete(host))
366 + if (mci_stop_sent_is_complete(host)) {
367 + host->stop_status = status;
368 mci_set_stop_pending(host);
371 + host->cmd_status = status;
372 mci_set_cmd_pending(host);
375 tasklet_schedule(&host->tasklet);
377 @@ -920,18 +872,16 @@ static void atmci_xfer_complete(struct dma_request *_req)
378 if (data->stop && !mci_set_stop_sent_is_completed(host))
379 send_stop_cmd(host->mmc, data, 0);
381 - if (data->flags & MMC_DATA_READ) {
382 - mci_writel(host, IDR, MCI_DATA_ERROR_FLAGS);
383 - mci_set_data_pending(host);
384 - tasklet_schedule(&host->tasklet);
387 - * For the WRITE case, wait for NOTBUSY. This function
388 - * is called when everything has been written to the
389 - * controller, not when the card is done programming.
391 - mci_writel(host, IER, MCI_BIT(NOTBUSY));
394 + * Regardless of what the documentation says, we have to wait
395 + * for NOTBUSY even after block read operations.
397 + * When the DMA transfer is complete, the controller may still
398 + * be reading the CRC from the card, i.e. the data transfer is
399 + * still in progress and we haven't seen all the potential
402 + mci_writel(host, IER, MCI_BIT(NOTBUSY));
405 static void atmci_dma_error(struct dma_request *_req)
406 @@ -963,24 +913,10 @@ static irqreturn_t atmci_interrupt(int irq, void *dev_id)
407 pending = status & mask;
410 - if (pending & MCI_CMD_ERROR_FLAGS) {
411 - mci_writel(host, IDR, (MCI_BIT(CMDRDY)
413 - | MCI_CMD_ERROR_FLAGS
414 - | MCI_DATA_ERROR_FLAGS));
415 - host->error_status = status;
417 - if (mci_stop_sent_is_complete(host))
418 - mci_set_stop_error_pending(host);
420 - mci_set_cmd_error_pending(host);
421 - tasklet_schedule(&host->tasklet);
424 if (pending & MCI_DATA_ERROR_FLAGS) {
425 mci_writel(host, IDR, (MCI_BIT(NOTBUSY)
426 | MCI_DATA_ERROR_FLAGS));
427 - host->error_status = status;
428 + host->data_status = status;
429 mci_set_data_error_pending(host);
430 tasklet_schedule(&host->tasklet);
435 _______________________________________________
437 Kernel@avr32linux.org
438 http://duppen.flaskehals.net/cgi-bin/mailman/listinfo/kernel