[U-Boot-Users] [PATCH] for Xilinx's ml300 board

[Andrew May]

On Wed, 2004-06-30 at 07:30, Peter Ryser wrote:
> >The whole xparameters.h file is really annoying. It makes the IPIF code
> >impossible to use on more than one board type without a recompile. That
> >may be OK for U-boot but it is horrible in the Linux kernel.
> >
> Unfortunately, we have not yet found a good way to store the hardware 
> information (let's call it CROM) as part of the bitstream in a way that 
> it becomes easily available to the software. The hardware system is very 
> flexible, i.e. the location where such information is stored may be 
> different for various boards (bitstreams).
>
> One solution might be to pass the location of the CROM as part of the 
> boot parameters to the Linux kernel or even go one step further and pass 
> the whole HW configuration as a boot parameter to the Linux kernel. A 
> similar technique might be doable for u-boot.

My problem isn't really that I want to get the entire config from
hardware, but with the way the xparamters.h file is compiled into
static arrays.
So the whole point of the doing the get the device by the ID is wasted.

Here is a walk through of the current code.

In xparameters.h the defines.
===================================
#define XPAR_XEMAC_NUM_INSTANCES 1
#define XPAR_OPB_ETHERNET_0_BASEADDR 0x60000000
#define XPAR_OPB_ETHERNET_0_HIGHADDR 0x60003FFF
#define XPAR_OPB_ETHERNET_0_DEVICE_ID 0
===================================
Then the eth_init() which calls this init
Result = XEmac_Initialize(&Emac, XPAR_EMAC_0_DEVICE_ID);
===================================
which calls this lookup
XEmac_LookupConfig(DeviceId);
===================================
and then the lookup uses the table
XEmac_Config *
XEmac_LookupConfig(u16 DeviceId)
{
	XEmac_Config *CfgPtr = NULL;
	int i;

	for (i = 0; i < XPAR_XEMAC_NUM_INSTANCES; i++) {
		if (XEmac_ConfigTable[i].DeviceId == DeviceId) {
			CfgPtr = &XEmac_ConfigTable[i];
			break;
		}
	}

	return CfgPtr;
}
===================================
and then in a seperate file the actual table is statically
defined with same values from the xparmeters.h

XEmac_Config XEmac_ConfigTable[] = {
	{
	 XPAR_OPB_ETHERNET_0_DEVICE_ID,
	 XPAR_OPB_ETHERNET_0_BASEADDR,
	 XPAR_OPB_ETHERNET_0_ERR_COUNT_EXIST,
	 XPAR_OPB_ETHERNET_0_DMA_PRESENT,
	 XPAR_OPB_ETHERNET_0_MII_EXIST}
};
===================================
Now for u-boot you could really argue the whole chain of calls is a
complete waste of space. You are just using one define to get a set
of them that are compiled into the code, when it would just be as easy
to use defines directly in the XEmac_Initialize() call without going
through the middle man/functions.

Again for u-boot this is probably overkill, but for Linux it would
really help to be able to use one image on more than one board easily.

A simple change that would go a long way to help is to have the
XEmac_ConfigTable be set at run time.
something like
XEmac_Config *XEmac_ConfigTable;

XEmac_LookupConfig(u16 DeviceId)
{
	if( XEmac_ConfigTable == NULL ){
		int bt;
		bt = get_board_type();/*Checks simple HW reg or env var*/
		if( bt == 2 ){
			XEmac_ConfigTable = XEmac_table_2mac;
		}else{
			XEmac_ConfigTable = XEmac_table_1mac;
		}
	}
	/*Fixup table to be null terminated or move the size into
	 *a wrapping struct instead of the constant.
	 */
	.....
}
Since I don't think it is really a u-boot issue we can try to take this
off to another list.