Skip to contentSkip to conversation

support for CF/IDE on PCMCIA for PXA25X

PATCH New Christian Pell 4 participants 9 messages Apr 1, 2004 – Aug 13, 2005

lore
Share & tools
lore
#0 lore raw

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 
#1 lore raw

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.c u-boot/common/cmd_pcmcia.c

common/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.h u-boot/include/fat.h

include/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 -0
 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

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
#2 lore raw

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.h u-boot/include/fat.h

include/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]>
#3 lore raw

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 
#4 lore raw

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]>
#5 lore raw

PATCH 1/1 support for CF/IDE on PCMCIA for PXA25X

In reply to Christian Pell

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

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,

#6 lore raw

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_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

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_
#7 lore raw

PATCH 1/1 support for CF/IDE on PCMCIA for PXA25X

In reply to Christian Pell

Dear 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. White
#8 lore raw

PATCH 1/1 support for CF/IDE on PCMCIA for PXA25X

In reply to Christian Pell

Dear 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?