lkml.org 
[lkml]   [2008]   [Oct]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 4/7] atmel-mci: support multiple mmc slots
Pierre Ossman <drzeus-list@drzeus.cx> wrote:
> 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.

True. I don't have any boards which can control the power to the mmc
slots, so there's currently no way to avoid having a newly inserted
card exposed to a possibly high-speed clock while powered.

Once I get my hands on a board which can do that, I'll try to halt the
queue while powering up a new card and restart it when the clock speed
for the new card is known.

> > + /* 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)

It can happen if the controller is hooked up to a fast bus and fOD is
slow for whatever reason. And if it does, clkdiv will wrap around and
end up at a rate much faster than requested, so saturating it at the
maximum divider is the safe thing to do.

If the card ends up behaving strangely as a result of the clock
frequency being a bit off, at least you have a warning which may
indicate what's wrong.

> > 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...

I think we'll just have to say that using multiplexing without power
control might cause problems with some cards.

> (did I mention that I think this slot multiplexing thing is a bad idea?)

Yes, I think you did. But note that this feature is optional (most
boards only request one slot), and I think people who actually have
multiple slots would rather use both of them than just one.

> > @@ -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).

You're right. I'll remove it -- it's updated when the card finishes its
busy signaling anyway.

Haavard


\
 
 \ /
  Last update: 2008-10-12 15:17    [W:1.793 / U:0.068 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site