[U-Boot-Users] [PATCH] PCMCIA card support for PXA/WEPEP250

[Leon Kukovec]

Hello Wolfgang,

On Sun, 2003-12-07 at 20:07, Wolfgang Denk wrote: 
> Dear Leon,
> 
> in message <Pine.CYG.4.55.0311201012400.1212 at sniper.ultra.si> you wrote:
> > 
> > Note that I have changed TEXT_ADDRESS for WEPEP250 board to start at
> > 0xA1000000. If my understanding is correct, Linux kernel will use the memory
> > that was occupied by U-Boot code while running in RAM. Tweaking can be done
> > later.
> 
> Why do you think such a change (which does waste memory while running
> U-Boot) is necessary?

I have changed TEXT_ADDRESS to 0xA1FB0000 now.

By default it was set to 0xA1FE0000, which caused BSS to end above
0xA1FFFFFF when features such as PCMCIA, IDE and FAT were enabled. With
TEXT_ADDRESS 0xA1FB0000 BSS ends at 0xA1FF9950.

> * Please do not add trailing white space to your source files.

Removed.

> * Please do not use names like "convert_string_in_big_endian_to_little_endian()";
>   such code is impossible to read.
> 
> * I don't want to see code like "convert_string_in_big_endian_to_little_endian()"
>   in common/cmd_ide.c

Function renamed to ident_big2little. Function is still inside cmd_ide.c
since it is only used in ide_ident. I suggest, since this is a generic
string manipulation function, that the ident_big2little is renamed to
string_big2little and moved inside lib_generic/string.c, but let me do
this as a second stage please. Any better function name suggestions are
welcome.

> * The implementation of "convert_string_in_big_endian_to_little_endian()"
>   seems broken to me. The use of two_byte_p and complicated shift ops
>   is not necessary when simple operations of character pointers  will
>   do,  too.  Also,  using  a  "ushort *" might cause problems on some
>   boards when the string buffer is not alligned to an even address.

Reimplemented it using char pointer.

> * "#if defined(__LITTLE_ENDIAN) && defined(CONFIG_WEPEP250)"  -  this
>   code seems broken to me. Either this is something that is necessary
>   on  all  little-endian  systems, or only for the WEPEP250 board, or
>   for all boards using this processor.

Removed defined(CONFIG_WEPEP250).

> * I see no good reason to include cpu/pxa/generic.c - if  you  really
>   want  to  use  get_lclk_frequency_10khz()  you can copy this single
>   source line; otherwise please clean up cpu/pxa/generic.c - the  use
>   of "printk()" etc. is not acceptable.

Moved _only_ get_lclk_frequency_10khz inside cpu/pxa/cpu.c.

> * You add/modify a number of architecture specific header  files  and
>   some  source  files.  Can you guarantee that these modifications do
>   not break support for other ARM boards? Are all these modifications
>   really needed? To me it seems  only  a  small  subset  is  actually
>   necessary.  I  don't  want  to  blow  up U-Boot if we can avoid it.
>   Please clean up to use only the really necessary stuff.

I have built the image for all ARM boards using "MAKEALL arm" have added
the missing include/asm-arm/arch-arm720t and dummy io.h and hardware.h
where they were required. Since I can not verify for all other boards
except wepep250 I'm asking people to try the patch and see if it works
for them and respond.

File list breakdown:

board/wepep250/pcmcia.c should not have any issues with other boards.
common/cmd_ide.c	possible issues with BIG_ENDIAN and or 			LITTLE_ENDIAN
common/cmd_pcmcia.c	possible issue with PPC boards, #ifdef 			CONFIG_PPC
cpu/pxa/cpu.c		possible issue with boards equipped with PXA. 			However
nobody except wepep250 			callsget_lclk_frequency_10khz
cpu/pxa/pcmcia.c	possible issue with boards equipped with PXA and
			PCMCIA defined. However apropriate ifdefs were 			used.
include/asm-arm/arch-arm720t/hardware.h
include/asm-arm/arch-arm720t/io.h
include/asm-arm/arch-arm920t/hardware.h
include/asm-arm/arch-arm920t/io.h
include/asm-arm/arch-arm925t/hardware.h
include/asm-arm/arch-arm925t/io.h
include/asm-arm/arch-arm926ejs/hardware.h
include/asm-arm/arch-arm926ejs/io.h
include/asm-arm/arch-at91rm9200/hardware.h
include/asm-arm/arch-at91rm9200/io.h
include/asm-arm/arch-ixp/hardware.h
include/asm-arm/arch-ixp/io.h
include/asm-arm/arch-sa1100/io.h
include/asm-arm/arch-sa1100/hardware.h

			Dummy header files that I have found in Linux 			kernel and not in
U-Boot. Since I have added
			include/asm-arm/io.h which includes 			<asm/arch/io.h> the dummy
headers also had to be
			added in order to avoid hacks.

include/asm-arm/assembler.h
			possible issues with ASM files including 			assembler.h. Currently
only
			lib_arm/io-readsw-arm4.S and
			lib_arm/io-writesw-arm4.S use that header file.
include/asm-arm/proc-armv/assembler.h
			LOADREGS and RETINST macros are used in 			lib_arm/io-readsw and
io-writesw ASM files.
include/pcmcia.h	added some defines for PCMCIA, no possible 			issues
include/pcmcia/pxa.h	PXA specific PCMCIA defines and inlines,
			possible issues with PXA boards.
include/linux/linkage.h possible issue with MIPS since
			asm-mips/mipsregs.h includes it.

Other files are not critical.

If there are still issues, let me know.

PS: this patch is against U-Boot 1.0

--
Best Regards,
	Leon.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: u-boot-1.0-pxa-pcmcia.patch.bz2
Type: application/x-bzip
Size: 12796 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20031210/7ff32087/attachment.bin