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

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

[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,

-- 
Christian Pellegrin 
<chri@ascensit.com>, <c.pellegrin@eurotech.it>, 
<chri@infis.univ.ts.it>, <c.pellegrin@exadron.com>
-------------- 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 

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

[Wolfgang Denk]

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

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

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

[Christian Pell]

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!

-- 
Christian Pellegrin 
<chri@ascensit.com>, <c.pellegrin@eurotech.it>, 
<chri@infis.univ.ts.it>, <c.pellegrin@exadron.com>

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

[Christian Pell]

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,

-- 
Christian Pellegrin 
<chri@ascensit.com>, <c.pellegrin@eurotech.it>, 
<chri@infis.univ.ts.it>, <c.pellegrin@exadron.com>
-------------- 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 

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

[David Miles]

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,

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

[Wolfgang Denk]

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

-- 
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 <DCqM4z.BxB@isgtec.com>

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

[Wolfgang Denk]

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

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

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

[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

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

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

[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

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