On 30/12/10 18:20, Andreas Färber wrote:
Hi Andreas,
This does not really address my comments...
Ah okay - I was still hoping that no news was good news and I admit I wanted something in place to unblock the SPARC32 OFMEM patchset.
+/* Private functions for mapping between physical/virtual addresses */ +inline phys_addr_t +va2pa(unsigned long va) +{
- if (va >= OF_CODE_START && va < OF_CODE_START + OF_CODE_SIZE) {
The latter will overflow on 32-bit ppc, and (va < 0) == 0 so that branch would never be taken. Is there no compiler warning?
- return (phys_addr_t)get_rom_base() - OF_CODE_START + va;
Again, what's the reason for duplicating this calculation?
- } else {
- return (phys_addr_t)va;
- }
+}
Only because I was in two minds with regards to whether to keep pa2va and va2pa as "symmetrical" as possible. I'm happy to go with whatever you think is best.
What about the following trivial implementation instead?
phys_addr_t va2pa(unsigned long va) { ucell mode; return ea_to_phys(va, &mode); // calls ofmem_translate() internally for the 'else' case }
Yes - that seems to work in my tests here.
+inline unsigned long +pa2va(phys_addr_t pa) +{
- if ((pa - get_rom_base() + OF_CODE_START >= OF_CODE_START) &&
- (pa - get_rom_base() + OF_CODE_START < OF_CODE_START + OF_CODE_SIZE))
- return (unsigned long)pa - get_rom_base() + OF_CODE_START;
- else
- return (unsigned long)pa;
+}
Trailing whitespace?
Missing braces for QEMU style.
Ooops my bad. Does the attached patch look better?
ATB,
Mark.