support for CF/IDE on PCMCIA for PXA25X
PATCH 1/1 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,
Signature · 10 lines
Christian Pellegrin <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]> -------------- next part -------------- A non-text attachment was scrubbed... Name: uboot-PXA_CF Type: text/x-patch Size: 11884 bytes Desc: not available Url : http://lists.denx.de/pipermail/u-boot/attachments/20040401/2bd2ee1c/attachment.bin
PATCH 1/1 support for CF/IDE on PCMCIA for PXA25X
Dear Christian,
Quoted 4 lines · 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:
...
common/cmd_pcmcia.c line 53
Quoted 12 lines
diff -u -b -B -N -r -X dodiff.not
u-boot.orig/common/cmd_pcmcia.cu-boot/common/cmd_pcmcia.ccommon/cmd_pcmcia.c +0
-2@@ -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.
common/cmd_pcmcia.c
Quoted 4 lines
common/cmd_pcmcia.c +4
-0+static int hardware_enable (int slot) +{ +return 0 ;/* No hardware to enable */ +}
...
etc.
...
Quoted 9 lines
common/cmd_pcmcia.c +9
-0+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 ,
common/cmd_pcmcia.c
Quoted 10 lines
common/cmd_pcmcia.c +10
-0+ + 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.
Quoted 5 lines
diff -u -b -B -N -r -X dodiff.not
u-boot.orig/include/fat.hu-boot/include/fat.hinclude/fat.h +0
-0@@ -210,4 +210,11 @@const char *file_getfsname (int idx);
include/fat.h line 220
Quoted 10 lines
include/fat.h +7
-0int 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
Signature · 4 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd at denx.de "They that can give up essential liberty to obtain a little temporary saftey deserve neither liberty not saftey." - Benjamin Franklin, 1759
PATCH 1/1 support for CF/IDE on PCMCIA for PXA25X
Quoted 8 lines · Wolfgang Denk
Quoted 8 lines
diff -u -b -B -N -r -X dodiff.not
u-boot.orig/include/fat.hu-boot/include/fat.hinclude/fat.h +0
-0@@ -210,4 +210,11 @@const char *file_getfsname (int idx);int fat_register_device (block_dev_desc_t *dev_desc,int part_no);
Quoted 9 lines
include/fat.h line 220
Quoted 8 lines
include/fat.h +7
-0+#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!
Signature · 3 lines
Christian Pellegrin <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>
PATCH 1/1 support for CF/IDE on PCMCIA for PXA25X
Quoted 7 lines · Christian Pell
Quoted 1 line
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,
Signature · 10 lines
Christian Pellegrin <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]> -------------- next part -------------- A non-text attachment was scrubbed... Name: denx.patch Type: text/x-patch Size: 772 bytes Desc: not available Url : http://lists.denx.de/pipermail/u-boot/attachments/20040419/66840e66/attachment.bin
PATCH 1/1 support for CF/IDE on PCMCIA for PXA25X
Quoted 3 lines · 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
Signature · 5 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux
Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd at denx.de
C++ was an interesting and valuable experiment, but we've learned its
lessons and it's time to move on.
- Peter Curran in <[email protected]>PATCH 1/1 support for CF/IDE on PCMCIA for PXA25X
In reply to Christian PellWe 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
Quoted 5 lines · Christian Pell
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,
PATCH 1/1 support for CF/IDE on PCMCIA for PXA25X
Quoted 6 lines · In message <[email protected]> you wrote:
One comment about the README. We had to
#define
CONFIG_IDE_PCMCIA1before 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
Signature · 4 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd at denx.de People are always a lot more complicated than you think. It's very important to remember that. - Terry Pratchett, _Truckers_
PATCH 1/1 support for CF/IDE on PCMCIA for PXA25X
In reply to Christian PellDear Christian,
about a year ago,
Quoted 4 lines · 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.
...
Review
Quoted 2 lines
(quoted diff) +2
-0+ 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
Signature · 5 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The most difficult thing in the world is to know how to do a thing
and to watch someone else doing it wrong, without commenting.
-- T.H. WhitePATCH 1/1 support for CF/IDE on PCMCIA for PXA25X
In reply to Christian PellDear Christian,
a long, long time ago (Thu, 01 Apr 2004), in message
Quoted 4 lines · <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:
Quoted 7 lines
(quoted diff) +7
-0+ 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,
...
Review
Quoted 3 lines
(quoted diff) +3
-0+ 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
Signature · 3 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de How can you tell when sour cream goes bad?