Messages in this thread | | | Date | Sun, 12 Oct 2008 10:39:58 +0200 | From | Pierre Ossman <> | Subject | Re: [PATCH v2 4/7] atmel-mci: support multiple mmc slots |
| |
On Sun, 5 Oct 2008 18:21:27 +0200 Haavard Skinnemoen <haavard.skinnemoen@atmel.com> wrote:
> > if (ios->clock) { > + unsigned int clock_min = ~0U; > u32 clkdiv; > > - if (!host->mode_reg) > + spin_lock_bh(&host->lock); > + if (!host->mode_reg) { > clk_enable(host->mck); > + mci_writel(host, CR, MCI_CR_SWRST); > + mci_writel(host, CR, MCI_CR_MCIEN); > + } > > - /* Set clock rate */ > - clkdiv = DIV_ROUND_UP(host->bus_hz, 2 * ios->clock) - 1; > + /* > + * Use mirror of ios->clock to prevent race with mmc > + * core ios update when finding the minimum. > + */ > + slot->clock = ios->clock; > + for (i = 0; i < ATMEL_MCI_MAX_NR_SLOTS; i++) { > + if (host->slot[i] && host->slot[i]->clock > + && host->slot[i]->clock < clock_min) > + clock_min = host->slot[i]->clock; > + } > +
Hmm... This does not cover the power up case. You can only ignore the other slot's clock setting if it is unpowered.
> + /* Calculate clock divider */ > + clkdiv = DIV_ROUND_UP(host->bus_hz, 2 * clock_min) - 1; > if (clkdiv > 255) { > dev_warn(&mmc->class_dev, > "clock %u too slow; using %lu\n", > - ios->clock, host->bus_hz / (2 * 256)); > + clock_min, host->bus_hz / (2 * 256)); > clkdiv = 255; > } >
Has this ever happened, or is it just a bit defensive? :)
(not that there is something wrong with that)
> default: > /* > * TODO: None of the currently available AVR32-based > * boards allow MMC power to be turned off. Implement > * power control when this can be tested properly. > + * > + * We also need to hook this into the clock management > + * somehow so that newly inserted cards aren't > + * subjected to a fast clock before we have a chance > + * to figure out what the maximum rate is. Currently, > + * there's no way to avoid this, and there never will > + * be for boards that don't support power control. > */ > break; > }
Hmm again... As you've pointed out, this could be a problem. The card should survive getting jittery power during the insertion, but I wouldn't chance having the clock running at that point (which will happen if the other slot is populated).
I don't see a decent way of solving this, so I guess we'll have to stick our heads in the sand for now...
(did I mention that I think this slot multiplexing thing is a bad idea?)
> @@ -922,9 +1191,11 @@ static void atmci_write_data_pio(struct atmel_mci *host) > mci_writel(host, IDR, (MCI_NOTBUSY | MCI_TXRDY > | ATMCI_DATA_ERROR_FLAGS)); > host->data_status = status; > + data->bytes_xfered += nbytes; > + smp_wmb(); > atmci_set_pending(host, EVENT_DATA_ERROR); > tasklet_schedule(&host->tasklet); > - break; > + return; > } > } while (status & MCI_TXRDY); > > @@ -937,29 +1208,26 @@ done: > mci_writel(host, IDR, MCI_TXRDY); > mci_writel(host, IER, MCI_NOTBUSY); > data->bytes_xfered += nbytes; > + smp_wmb(); > atmci_set_pending(host, EVENT_XFER_COMPLETE); > } >
I'm not sure I commented on this during the last run, but bytes_xfered should only be updated upon confirmation from the card, not as it goes out over the bus (there might be CRC error for instance).
Rgds -- -- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org rdesktop, core developer http://www.rdesktop.org
WARNING: This correspondence is being monitored by the Swedish government. Make sure your server uses encryption for SMTP traffic and consider using PGP for end-to-end encryption. [unhandled content-type:application/pgp-signature] | |