[U-Boot] [PATCH] MCI support for AT91 family processors.

[Albin Tonnerre]

On Mon, Aug 31, 2009 at 02:22:47PM +0300, Sami Kantoluoto wrote :
> On Sun, Aug 30, 2009 at 01:08:27AM +0200, Albin Tonnerre wrote:
> > On Sat, Aug 29, 2009 at 08:18:32PM +0300, Sami Kantoluoto wrote :
> > > Fixed to parse CSD correctly on little endian processors as gcc orders
> > > bitfields differently between big and little endian ones.
> > 
> > Please also see this patch, which will fix those bugs as weel, while switching
> > to the new GENRIC_MMC API:
> > http://lists.denx.de/pipermail/u-boot/2009-August/059456.html
> > I'd highly appreciate if you could test it, to get some feedback
> 
> Thanks, I'll test when I get some time later this week but I think (by 
> reading the patch so I probably missed something) it won't solve the CSD
> problem. The real reason of the "CSD problem" of course is that how the
> mmc_csd structure is defined (host byte order not taken in account or
> at least how gcc handles bitfields).
> 

drivers/mmc/mmc.c does not actually use the bitfield to parse the csd struct,
and got fixed a while back to work no matter what endianness you're using, so it
should solve it anyway.

> [snip]
> 
> > the configuration for the MCI controller, which should be done in the
> > at91sam*_devices.c files. See the attached patch (relying on the patch
> > mentionned above) that adds such support. It's not complete yet either: it
> > partially lacks support for models which have 2 MCI controllers, but
> > that's not the case of the 9G20 anyway (that's just a matter of putting
> > proper ifdefs before defining AT91_BASE_MCI).
> > If what I've done in this patch is not the way to go, I'd appreciate
> > guidance on how to do it correctly.
> 
> It seems ok to me except one comment about the interface:
> 
> > diff --git a/include/asm-arm/arch-at91/at91_common.h b/include/asm-arm/arch-at91/at91_common.h
> >
> > +void at91_mci0_hw_init(int);
> > +void at91_mci1_hw_init(int);
> 
> I'm just wondering if the bus width should be configurable. It's probably
> quite unusual to use just one data bit but not impossible though. So maybe
> the interface call could be:
> 
> void at91_mci0_hw_init(int bwForSlotA, bwForSlotB)
> 
> Where the bwForSlot<X> is either 0 (which means slot is not used), 1 or 4.

I guess I'll just use something similar to what the spi init function does (eg.
take only 1 argument, and use 1 << 0 for slot A/B, 1 << 1 for 1wire/4wires)

Thanks for your comments.

Regards,
-- 
Albin Tonnerre, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com