On 19.02.2014, at 00:21, Scott Wood <scottw...@freescale.com> wrote:
> On Tue, 2014-02-11 at 01:10 +0100, Alexander Graf wrote:
>> - if (memsize)
>> - print_size(memsize, " left unmapped\n");
>> + if (size)
>> + print_size(size, " left unmapped\n");
>> +}
>
> The print_size should move to the caller, with some way to pass back the
> amout left unmapped. Non-RAM callers would treat a non-zero unmapped
> value as an error.
>
>> +unsigned int
>> +setup_ddr_tlbs_phys(phys_addr_t p_addr, unsigned int memsize_in_meg)
>> +{
>> + unsigned int ram_tlb_address = (unsigned int)CONFIG_SYS_DDR_SDRAM_BASE;
>> + u64 memsize = (u64)memsize_in_meg << 20;
>> +
>> + memsize = min(memsize, CONFIG_MAX_MEM_MAPPED);
>> + tlb_map_range(ram_tlb_address, p_addr, memsize, true);
>> return memsize_in_meg;
>> }
>
> Here you seem to be hiding the message for RAM.
It could still fail if we're running out of TLB entries, no?
> York, are you OK with just removing the message altogether, and having
> tlb_map_range return a normal error code if it can't map everything
> (with DDR size reduced in advance as above)?
How about we just change the return value of tlb_map_range to uint64_t and
return size? That way we can 1:1 move the print code out of the function into
the RAM map code and IO callers can just call assert(r != 0).
>
>> diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
>> index cadaeef..5493c51 100644
>> --- a/arch/powerpc/include/asm/mmu.h
>> +++ b/arch/powerpc/include/asm/mmu.h
>> @@ -509,6 +509,9 @@ extern void print_tlbcam(void);
>> extern unsigned int setup_ddr_tlbs(unsigned int memsize_in_meg);
>> extern void clear_ddr_tlbs(unsigned int memsize_in_meg);
>>
>> +extern void tlb_map_range(ulong v_addr, phys_addr_t p_addr, uint64_t size,
>> + bool is_ram);
>
> bool arguments tend to be hard to read at call sites -- flags (or enum)
> with something like MAP_RAM/MAP_IO would be nicer (I'm not insisting,
> though).
Good point.
Alex
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
