Re: [U-Boot] [PATCH] MCI support for AT91 family processors.
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).
[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.
-sk
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,
On Mon, Aug 31, 2009 at 01:39:26PM +0200, Albin Tonnerre wrote:
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.
Ok. Like I said (in other post) I was using the older u-boot version before so I'm not too familiar with current one (and wasn't actually with the old one either ;-). I'll try to do my homework better next time!
-sk
On Mon, Aug 31, 2009 at 01:39:26PM +0200, Albin Tonnerre wrote:
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.
I just tested the patch (+ your at91_mci patch + my new patches) and this seems to work just find.
Thanks!
Best Regards,
-sk
participants (2)
-
Albin Tonnerre -
Sami Kantoluoto