Re: [PATCH][Fix] New Unified AVR32/AT91 MCI Driver that supportsboth MCI slots used at the same time
From: Haavard Skinnemoen
Date: Thu Jun 11 2009 - 03:54:58 EST
[cutting the redundant bits from Subject]
Hi Rob,
Rob Emanuele wrote:
> This patch unifies the at91 changes I had into the atmel-mci
> (originally for AVR32) driver.
That doesn't look so bad...but I suspect PDC support hasn't been
integrated yet?
> This also fixes an important bug where in 4-bit mode you could only
> select Slot A.
It has already been fixed in mainline.
> As with the at91 port I had of this driver, I had to add more flags to
> the ATMCI_DATA_ERROR_FLAGS as other communication errors were
> occurring and they were not be reported back. Can anyone add more
> insight into this?
Adding them to the data error bits doesn't sound like the right thing
to do...but I guess there might be some sort of timing issue in there
where we think we're done sending the command but the controller may
still raise errors.
> Again, anyone who can, please test (on either or both the AT91 and
> AVR32) and comment.
I haven't looked very closely at it yet, but I spotted a few things
which might prevent the patch from being accepted as-is:
- I'm not sure if adding "unified" (or "now supports AT91") all over
the place is the right thing to do. If the driver is selectable
when you configure for AT91, it should obviously work on AT91.
- The patch seems to do a bit too much all at once. The bug fix which
has already been fixed is one example, another is the clock cap
option -- we used to have a module parameter for the same purpose,
but Pierre (the MMC maintainer, who should probably be added to the
loop) had problems with it. If this feature was in a separate
patch, it could be rejected without blowing away the rest of the
driver.
- The AT91 platform parts should be separated from the rest since it
may need to go through a different maintainer.
Haavard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/