Udi,

I am convinced now it is incorrect. A minimal fix is to make the code look like this:

/* Check the BAR type and set our address mask */ if (bar_response & PCI_BASE_ADDRESS_SPACE) { bar_size = ~(bar_response & PCI_BASE_ADDRESS_IO_MASK) + 1;

/* round up region base address to a multiple of size */ io = ((io - 1) | (bar_size - 1)) + 1; bar_value = io; /* compute new base address for region */ io = io + bar_size; } else { if ( (bar_response & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64) found_mem64 = 1;

bar_size = ~(bar_response & PCI_BASE_ADDRESS_MEM_MASK) + 1; /* round up region base address to multiple of size */ mem = ((mem - 1) | (bar_size - 1)) + 1; bar_value = mem; /* compute new region base address */ mem = mem + bar_size; }

The ORing just rounds up the current base for the region to be a multiple of the size. The ORing was in the wrong place and there was no increment of the region base to beyond the allocated area. It worked with cards with single BARs and suitable region bases.

This is not optimal, but it is useable for simple cases.

Thanks for the comments,

Don

Udi Finkelstein wrote:

I've looked at the code you posted, and it looks indeed incorrect. (I didn't look into the entire file in u-boot, so take this with a grain of salt). However, and optimal PCI alolcator must sort the BAR size in decreasing orders if it wants to pack the BARs into an area as small as possible. If you have BAR0 size 4K and BAR1 size 256M and BAR2 size 4K, then allocating BAR0 from (for example) 0xe0000000 would yield BAR0 range of 0xe0000000-0xe0000fff, BAR1 would be mapped to 0xf0000000 - 0xffffffff, and BAR2 would not fit! An optimal allocator must start from the biggest BARs first:

BAR1 would fit at 0xe0000000-0xefffffff BAR2 would fit at 0xf0000000-0xf0ffffff BAR0 would fit at 0xf1000000-0xf1000fff

Udi

Donald White wrote:

...

Rune Torgersen wrote:

...

...

Subject: [U-Boot-Users] Please explain this code in drivers/pci.c

...

The value of bar_response is 0xffff8008 which looks good. This gives bar

size of

...

0x8000 which also looks good. The value of mem is initially 0x90000000.

So,

...

(mem - 1) = 0x8fffffff and (bar_size - 1) = 0x7fff). When these are or'd

one gets

...

0x8fffffff and adding 1 gives 0x90000000. This result is that the card is

given 4

...

address bases all at 0x90000000.

If the card wants 4 different memory regiuons, I assume it has 4 BAR registers? Each BAR must have the correct base address, so you have to add the size of the last region to the memory pointer before writing the address to the next BAR

I am not sure if you are trying to clarify the question or provide an answer. My complaint is that the code does not do what you say. I believe the code should provide four unique values for the four BARs, but it does not. Each gets the same value because the ORing logic does not succeed in doing a new base = old base + size.

The logic envoked by CONFIG_PCI_PNP gets the calculation right. If someone is familiar with the code and can confirm that it is not behaving as intended, then I will do a patch. The ORing logic looks to me to be copied from the Linux kernel where the allocation starts at the top of the address range and proceeds downward, but maybe it is an attempt to force alignment. I don't know.

Don

---

This SF.net email is sponsored by: ObjectStore. If flattening out C++ or Java code to make your application fit in a relational database is painful, don't do it! Check out ObjectStore. Now part of Progress Software. http://www.objectstore.net/sourceforge _______________________________________________ U-Boot-Users mailing list U-Boot-Users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/u-boot-users