[U-Boot-Users] [PATCH] support for CF/IDE on PCMCIA for PXA25X

Hi, I tried this with some different CFs with FAT file-systems on them. There is a README in the patch itself that goes in documentation directory.
Bye,

Dear Christian,
in message 1080814283.1850.24.camel@absolute you wrote:
Hi, I tried this with some different CFs with FAT file-systems on them. There is a README in the patch itself that goes in documentation directory.
Added, thanks.
But next time please provide a proper CHANGELOG entry, and stick to the coding style and patch rules, for example:
...
diff -u -b -B -N -r -X dodiff.not u-boot.orig/common/cmd_pcmcia.c u-boot/common/cmd_pcmcia.c --- u-boot.orig/common/cmd_pcmcia.c 2004-03-25 20:29:40.000000000 +0100 +++ u-boot/common/cmd_pcmcia.c 2004-04-01 11:49:15.594458134 +0200 @@ -48,8 +48,6 @@
- They are maximum 64KByte each...
*/
-/* #define DEBUG 1 */
/*
- PCMCIA support
*/
Don't change stuff that you don't understand and that doesn't hurt you.
+static int hardware_enable (int slot) +{
- return 0; /* No hardware to enable */
+}
... etc. ...
+int pcmcia_on (void) +{
- unsigned int reg_arr[] = {
- 0x48000028, CFG_MCMEM0_VAL,
- 0x4800002c, CFG_MCMEM1_VAL,
- 0x48000030, CFG_MCATT0_VAL,
- 0x48000034, CFG_MCATT1_VAL,
- 0x48000038, CFG_MCIO0_VAL,
- 0x4800003c, CFG_MCIO1_VAL,
- 0,0
- };
- int i, rc;
+#ifdef CONFIG_EXADRON1
- int cardDetect;
- volatile unsigned int *v_pBCRReg = (volatile unsigned int *) 0x08000000;
+#endif
- debug("%s\n", __FUNCTION__);
... etc.
Please use standard indentation!!! Don't add trailing white space! Don't use C++ comments.
diff -u -b -B -N -r -X dodiff.not u-boot.orig/include/fat.h u-boot/include/fat.h --- u-boot.orig/include/fat.h 2004-02-23 20:31:01.000000000 +0100 +++ u-boot/include/fat.h 2004-03-31 15:15:46.000000000 +0200 @@ -210,4 +210,11 @@ const char *file_getfsname(int idx); int fat_register_device(block_dev_desc_t *dev_desc, int part_no);
+#ifdef CONFIG_PXA250 +#undef FAT2CPU16 +#define FAT2CPU16(x) x +#undef FAT2CPU32 +#define FAT2CPU32(x) x +#endif
#endif /* _FAT_H_ */
I added this as is, but I ask you to provide an additional patch to clean this up. If the definitions of FAT2CPU16 and FAT2CPU32 are wrong they must be changed where they are defined. Otherwise you may run into problems, for example your code relies heavily on a certain order of the #include statements. This must be fixed ASAP.
Best regards,
Wolfgang Denk

On Fri, 2004-04-16 at 00:37, Wolfgang Denk wrote:
diff -u -b -B -N -r -X dodiff.not u-boot.orig/include/fat.h u-boot/include/fat.h --- u-boot.orig/include/fat.h 2004-02-23 20:31:01.000000000 +0100 +++ u-boot/include/fat.h 2004-03-31 15:15:46.000000000 +0200 @@ -210,4 +210,11 @@ const char *file_getfsname(int idx); int fat_register_device(block_dev_desc_t *dev_desc, int part_no);
+#ifdef CONFIG_PXA250 +#undef FAT2CPU16 +#define FAT2CPU16(x) x +#undef FAT2CPU32 +#define FAT2CPU32(x) x +#endif
#endif /* _FAT_H_ */
Hi, sorry for this I'll look at it ASAP. I realized, like you said, that there is a deeper problem with endianess when dealing with PXA endianess. For example I had to do a byte swap when adapting the driver for a custom board flash. Guess there is some assuption on big-endian somewhere.
Thanks, Bye!

On Fri, 2004-04-16 at 08:56, Christian Pell wrote:
Hi, sorry for this I'll look at it ASAP. I realized, like you said, that there is a deeper problem with endianess when dealing with PXA endianess. For example I had to do a byte swap when adapting the driver for a custom board flash. Guess there is some assuption on big-endian somewhere.
Hi,
attached it's the patch that clears my early mistake. The include files that deals with endianess wasn't included in fat.h.
Regards,

In message 1082358087.1850.10.camel@absolute you wrote:
attached it's the patch that clears my early mistake. The include files that deals with endianess wasn't included in fat.h.
Added, thanks.
Best regards,
Wolfgang Denk

We just got this working on our custom hardware. Thanks for your excellent work.
One comment about the README. We had to
#define CONFIG_IDE_PCMCIA 1
before the ide interface would hook in. Should this be added to the readme?
Best Regards,
Dave
On Thursday 01 April 2004 02:11, Christian Pell wrote:
Hi, I tried this with some different CFs with FAT file-systems on them. There is a README in the patch itself that goes in documentation directory.
Bye,

In message 200404192158.35662.dave@minervatech.net you wrote:
One comment about the README. We had to
#define CONFIG_IDE_PCMCIA 1
before the ide interface would hook in. Should this be added to the readme?
I think this is a good idea. Can you please submit a patch?
Thanks in advance.
Best regards,
Wolfgang Denk

Dear Christian,
about a year ago,
in message 1080814283.1850.24.camel@absolute you wrote:
Hi, I tried this with some different CFs with FAT file-systems on them. There is a README in the patch itself that goes in documentation directory.
...
- while (reg_arr[i])
- ( (volatile unsigned int *) reg_arr[i++]) |= reg_arr[i++];
This construct is illegal; the operation on "i" is undefined.
Can you please explain what the code is supposed to do, and/or submit a fix?
Thanks.
Best regards,
Wolfgang Denk

Dear Christian,
a long, long time ago (Thu, 01 Apr 2004), in message 1080814283.1850.24.camel@absolute you wrote:
Hi, I tried this with some different CFs with FAT file-systems on them. There is a README in the patch itself that goes in documentation directory.
Your patch to add PCMCIA support for PXA systems contains a pretty dubious construct:
- unsigned int reg_arr[] = {
- 0x48000028, CFG_MCMEM0_VAL,
- 0x4800002c, CFG_MCMEM1_VAL,
- 0x48000030, CFG_MCATT0_VAL,
- 0x48000034, CFG_MCATT1_VAL,
- 0x48000038, CFG_MCIO0_VAL,
- 0x4800003c, CFG_MCIO1_VAL,
...
- i = 0;
- while (reg_arr[i])
- ( (volatile unsigned int *) reg_arr[i++]) |= reg_arr[i++];
The compiler complains:
cmd_pcmcia.c:343: warning: operation on `i' may be undefined
and he is obviously right.
Can you please provide a patch to fix this?
Thanks.
Best regards,
Wolfgang Denk
participants (4)
-
Christian Pell
-
Christian Pell
-
David Miles
-
Wolfgang Denk