Re: [U-Boot] [PATCH v2 2/3] avr32: Use uncached() macro to get an address for SDRAM init
Detlev Zundel d...@denx.de wrote: Problem is that in order to make the CFI driver work on avr32, we need to change the map_physmem() macro to return the physical address unchanged. I see. And I presume you cannot tell the situation apart inside map_physmem? I don't think so. How do you propose we do that? The map_physmem() macro currently does exactly the same thing as the uncached() macro, and the unmap is a noop, but the next patch changes it in order to fix the CFI driver. If the next patch is applied without this patch being applied first, the SDRAM driver will do cached accesses during initialization, and that may cause the initialization to fail. Why not include a note to this extent into the git commit message? This would be a great help for other people to later understand why this change has been done the backward way that it was. The commit message already contains this: The paging system which is required to set up caching properties has not yet been initialized when the SDRAM is initialized. So when the map_physmem() function is converted to return the physical address unchanged, the SDRAM initialization will break on some boards. which is essentially the same thing, isn't it? Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 0/3] avr32: simple paging support
This series fixes a longstanding problem with the 'saveenv' command on ATNGW100. It also includes a trivial enhancement to the exception reporting which I found very useful during debugging. Hopefully this will make mainline u-boot useful again on AVR32. Without this fix, v2008.10 is the latest usable release. This is the second version of this series. The following changes have been made since v1: - Rebased onto testing/arm-reloc-and-cache-support - Dropped asm/unaligned.h fix because a similar patch has already been applied to mainline. - Fixed SDRAM initialization issue noticed on certain boards Haavard Skinnemoen (3): avr32: Print unrelocated PC on exception avr32: Use uncached() macro to get an address for SDRAM init avr32: Add simple paging support arch/avr32/cpu/at32ap700x/Makefile |2 +- arch/avr32/cpu/at32ap700x/mmu.c| 78 arch/avr32/cpu/exception.c |3 +- arch/avr32/cpu/start.S | 19 +++-- arch/avr32/include/asm/arch-at32ap700x/addrspace.h |5 +- arch/avr32/include/asm/arch-at32ap700x/mmu.h | 66 + arch/avr32/lib/board.c |4 + board/atmel/atngw100/atngw100.c| 19 - board/atmel/atstk1000/atstk1000.c | 19 - board/earthlcd/favr-32-ezkit/favr-32-ezkit.c | 19 - board/mimc/mimc200/mimc200.c | 24 +- board/miromico/hammerhead/hammerhead.c | 19 - include/configs/atngw100.h |3 + include/configs/atstk1002.h|3 + include/configs/atstk1003.h|3 + include/configs/atstk1004.h|3 + include/configs/atstk1006.h|3 + include/configs/favr-32-ezkit.h|3 + include/configs/hammerhead.h |3 + include/configs/mimc200.h |3 + 20 files changed, 274 insertions(+), 27 deletions(-) create mode 100644 arch/avr32/cpu/at32ap700x/mmu.c create mode 100644 arch/avr32/include/asm/arch-at32ap700x/mmu.h ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 2/3] avr32: Use uncached() macro to get an address for SDRAM init
The paging system which is required to set up caching properties has not yet been initialized when the SDRAM is initialized. So when the map_physmem() function is converted to return the physical address unchanged, the SDRAM initialization will break on some boards. The avr32-specific uncached() macro will return an address which will always cause uncached accessed to be made. Since this happens in the board code, using avr32-specific features should be ok, and will allow the SDRAM initialization to keep working. Signed-off-by: Haavard Skinnemoen haavard.skinnem...@atmel.com --- board/atmel/atngw100/atngw100.c |4 +--- board/atmel/atstk1000/atstk1000.c|4 +--- board/earthlcd/favr-32-ezkit/favr-32-ezkit.c |4 +--- board/mimc/mimc200/mimc200.c |4 +--- board/miromico/hammerhead/hammerhead.c |4 +--- 5 files changed, 5 insertions(+), 15 deletions(-) diff --git a/board/atmel/atngw100/atngw100.c b/board/atmel/atngw100/atngw100.c index 004d8da..4580f55 100644 --- a/board/atmel/atngw100/atngw100.c +++ b/board/atmel/atngw100/atngw100.c @@ -75,13 +75,11 @@ phys_size_t initdram(int board_type) unsigned long actual_size; void *sdram_base; - sdram_base = map_physmem(EBI_SDRAM_BASE, EBI_SDRAM_SIZE, MAP_NOCACHE); + sdram_base = uncached(EBI_SDRAM_BASE); expected_size = sdram_init(sdram_base, sdram_config); actual_size = get_ram_size(sdram_base, expected_size); - unmap_physmem(sdram_base, EBI_SDRAM_SIZE); - if (expected_size != actual_size) printf(Warning: Only %lu of %lu MiB SDRAM is working\n, actual_size 20, expected_size 20); diff --git a/board/atmel/atstk1000/atstk1000.c b/board/atmel/atstk1000/atstk1000.c index c36cb57..d91d594 100644 --- a/board/atmel/atstk1000/atstk1000.c +++ b/board/atmel/atstk1000/atstk1000.c @@ -97,13 +97,11 @@ phys_size_t initdram(int board_type) unsigned long actual_size; void *sdram_base; - sdram_base = map_physmem(EBI_SDRAM_BASE, EBI_SDRAM_SIZE, MAP_NOCACHE); + sdram_base = uncached(EBI_SDRAM_BASE); expected_size = sdram_init(sdram_base, sdram_config); actual_size = get_ram_size(sdram_base, expected_size); - unmap_physmem(sdram_base, EBI_SDRAM_SIZE); - if (expected_size != actual_size) printf(Warning: Only %lu of %lu MiB SDRAM is working\n, actual_size 20, expected_size 20); diff --git a/board/earthlcd/favr-32-ezkit/favr-32-ezkit.c b/board/earthlcd/favr-32-ezkit/favr-32-ezkit.c index 8af680f..d2843c9 100644 --- a/board/earthlcd/favr-32-ezkit/favr-32-ezkit.c +++ b/board/earthlcd/favr-32-ezkit/favr-32-ezkit.c @@ -68,13 +68,11 @@ phys_size_t initdram(int board_type) unsigned long actual_size; void *sdram_base; - sdram_base = map_physmem(EBI_SDRAM_BASE, EBI_SDRAM_SIZE, MAP_NOCACHE); + sdram_base = uncached(EBI_SDRAM_BASE); expected_size = sdram_init(sdram_base, sdram_config); actual_size = get_ram_size(sdram_base, expected_size); - unmap_physmem(sdram_base, EBI_SDRAM_SIZE); - if (expected_size != actual_size) printf(Warning: Only %lu of %lu MiB SDRAM is working\n, actual_size 20, expected_size 20); diff --git a/board/mimc/mimc200/mimc200.c b/board/mimc/mimc200/mimc200.c index cc0f137..9940669 100644 --- a/board/mimc/mimc200/mimc200.c +++ b/board/mimc/mimc200/mimc200.c @@ -153,13 +153,11 @@ phys_size_t initdram(int board_type) unsigned long actual_size; void *sdram_base; - sdram_base = map_physmem(EBI_SDRAM_BASE, EBI_SDRAM_SIZE, MAP_NOCACHE); + sdram_base = uncached(EBI_SDRAM_BASE); expected_size = sdram_init(sdram_base, sdram_config); actual_size = get_ram_size(sdram_base, expected_size); - unmap_physmem(sdram_base, EBI_SDRAM_SIZE); - if (expected_size != actual_size) printf(Warning: Only %lu of %lu MiB SDRAM is working\n, actual_size 20, expected_size 20); diff --git a/board/miromico/hammerhead/hammerhead.c b/board/miromico/hammerhead/hammerhead.c index 8b3e22c..5ab999e 100644 --- a/board/miromico/hammerhead/hammerhead.c +++ b/board/miromico/hammerhead/hammerhead.c @@ -80,13 +80,11 @@ phys_size_t initdram(int board_type) unsigned long actual_size; void *sdram_base; - sdram_base = map_physmem(EBI_SDRAM_BASE, EBI_SDRAM_SIZE, MAP_NOCACHE); + sdram_base = uncached(EBI_SDRAM_BASE); expected_size = sdram_init(sdram_base, sdram_config); actual_size = get_ram_size(sdram_base, expected_size); - unmap_physmem(sdram_base, EBI_SDRAM_SIZE); - if (expected_size != actual_size) printf(Warning: Only %lu of %lu MiB SDRAM is working\n, actual_size 20, expected_size 20); -- 1.7.0.4
[U-Boot] [PATCH v2 1/3] avr32: Print unrelocated PC on exception
In addition to the real PC value, also print the value of PC after subtracting the relocation offset. This value will match the address in the ELF file so it's much easier to figure out where things went wrong. Signed-off-by: Haavard Skinnemoen haavard.skinnem...@atmel.com --- arch/avr32/cpu/exception.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/arch/avr32/cpu/exception.c b/arch/avr32/cpu/exception.c index dc9c300..b21ef1f 100644 --- a/arch/avr32/cpu/exception.c +++ b/arch/avr32/cpu/exception.c @@ -59,7 +59,8 @@ void do_unknown_exception(unsigned int ecr, struct pt_regs *regs) { unsigned int mode; - printf(\n *** Unhandled exception %u at PC=0x%08lx\n, ecr, regs-pc); + printf(\n *** Unhandled exception %u at PC=0x%08lx [%08lx]\n, + ecr, regs-pc, regs-pc - gd-reloc_off); switch (ecr) { case ECR_BUS_ERROR_WRITE: -- 1.7.0.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 3/3] avr32: Add simple paging support
Use the MMU hardware to set up 1:1 mappings between physical and virtual addresses. This allows us to bypass the cache when accessing the flash without having to do any physical-to-virtual address mapping in the CFI driver. The virtual memory mappings are defined at compile time through a sorted array of virtual memory range objects. When a TLB miss exception happens, the exception handler does a binary search through the array until it finds a matching entry and loads it into the TLB. The u-boot image itself is covered by a fixed TLB entry which is never replaced. This makes the 'saveenv' command work again on ATNGW100 and other boards using the CFI driver, hopefully without breaking any rules. Signed-off-by: Haavard Skinnemoen haavard.skinnem...@atmel.com --- arch/avr32/cpu/at32ap700x/Makefile |2 +- arch/avr32/cpu/at32ap700x/mmu.c| 78 arch/avr32/cpu/start.S | 19 +++-- arch/avr32/include/asm/arch-at32ap700x/addrspace.h |5 +- arch/avr32/include/asm/arch-at32ap700x/mmu.h | 66 + arch/avr32/lib/board.c |4 + board/atmel/atngw100/atngw100.c| 15 board/atmel/atstk1000/atstk1000.c | 15 board/earthlcd/favr-32-ezkit/favr-32-ezkit.c | 15 board/mimc/mimc200/mimc200.c | 20 + board/miromico/hammerhead/hammerhead.c | 15 include/configs/atngw100.h |3 + include/configs/atstk1002.h|3 + include/configs/atstk1003.h|3 + include/configs/atstk1004.h|3 + include/configs/atstk1006.h|3 + include/configs/favr-32-ezkit.h|3 + include/configs/hammerhead.h |3 + include/configs/mimc200.h |3 + 19 files changed, 267 insertions(+), 11 deletions(-) create mode 100644 arch/avr32/cpu/at32ap700x/mmu.c create mode 100644 arch/avr32/include/asm/arch-at32ap700x/mmu.h diff --git a/arch/avr32/cpu/at32ap700x/Makefile b/arch/avr32/cpu/at32ap700x/Makefile index 46e6ef6..30ea925 100644 --- a/arch/avr32/cpu/at32ap700x/Makefile +++ b/arch/avr32/cpu/at32ap700x/Makefile @@ -24,7 +24,7 @@ include $(TOPDIR)/config.mk LIB:= $(obj)lib$(SOC).a -COBJS := portmux.o clk.o +COBJS := portmux.o clk.o mmu.o SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c) OBJS := $(addprefix $(obj),$(SOBJS) $(COBJS)) diff --git a/arch/avr32/cpu/at32ap700x/mmu.c b/arch/avr32/cpu/at32ap700x/mmu.c new file mode 100644 index 000..c3a1b93 --- /dev/null +++ b/arch/avr32/cpu/at32ap700x/mmu.c @@ -0,0 +1,78 @@ +#include common.h +#include asm/arch/mmu.h +#include asm/sysreg.h + +void mmu_init_r(unsigned long dest_addr) +{ + uintptr_t vmr_table_addr; + + /* Round monitor address down to the nearest page boundary */ + dest_addr = PAGE_ADDR_MASK; + + /* Initialize TLB entry 0 to cover the monitor, and lock it */ + sysreg_write(TLBEHI, dest_addr | SYSREG_BIT(TLBEHI_V)); + sysreg_write(TLBELO, dest_addr | MMU_VMR_CACHE_WRBACK); + sysreg_write(MMUCR, SYSREG_BF(DRP, 0) | SYSREG_BF(DLA, 1) + | SYSREG_BIT(MMUCR_S) | SYSREG_BIT(M)); + __builtin_tlbw(); + + /* +* Calculate the address of the VM range table in a PC-relative +* manner to make sure we hit the SDRAM and not the flash. +*/ + vmr_table_addr = (uintptr_t)mmu_vmr_table; + sysreg_write(PTBR, vmr_table_addr); + printf(VMR table @ 0x%08x\n, vmr_table_addr); + + /* Enable paging */ + sysreg_write(MMUCR, SYSREG_BF(DRP, 1) | SYSREG_BF(DLA, 1) + | SYSREG_BIT(MMUCR_S) | SYSREG_BIT(M) | SYSREG_BIT(E)); +} + +int mmu_handle_tlb_miss(void) +{ + const struct mmu_vm_range *vmr_table; + const struct mmu_vm_range *vmr; + unsigned int fault_pgno; + int first, last; + + fault_pgno = sysreg_read(TLBEAR) PAGE_SHIFT; + vmr_table = (const struct mmu_vm_range *)sysreg_read(PTBR); + + /* Do a binary search through the VM ranges */ + first = 0; + last = CONFIG_SYS_NR_VM_REGIONS; + while (first last) { + unsigned int start; + int middle; + + /* Pick the entry in the middle of the remaining range */ + middle = (first + last) 1; + vmr = vmr_table[middle]; + start = vmr-virt_pgno; + + /* Do the bisection thing */ + if (fault_pgno start) { + last = middle; + } else if (fault_pgno = (start + vmr-nr_pages)) { + first = middle + 1; + } else { + /* Got it; let's slam it into the TLB */ + uint32_t tlbelo
Re: [U-Boot] [PATCH v2 2/3] avr32: Use uncached() macro to get an address for SDRAM init
Detlev Zundel d...@denx.de wrote: So this patch replaces a construct which seems to be valid over all architectures by a construct which is only used in avr32, right? It also deletes the explicit statement that such a mapping is not needed any further. Problem is that in order to make the CFI driver work on avr32, we need to change the map_physmem() macro to return the physical address unchanged. Isn't this a step backward? Can't you put the functionality inside the map function and leave the unmap a noop? I agree it's a step backward, but since the previous flamewar didn't get us anywhere, I decided to go for a compromise this time. At least this small architecture-specific kludge is localized to architecture-specific code. The map_physmem() macro currently does exactly the same thing as the uncached() macro, and the unmap is a noop, but the next patch changes it in order to fix the CFI driver. If the next patch is applied without this patch being applied first, the SDRAM driver will do cached accesses during initialization, and that may cause the initialization to fail. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] ATMEL Custodians == /dev/null ??
Mike Frysinger vap...@gentoo.org wrote: On Sun, Aug 8, 2010 at 9:27 PM, Haavard Skinnemoen wrote: Mike Frysinger vap...@gentoo.org wrote: explicitly cc-ing the atmel guys just so there's no surprise when at91/avr32 have been handled over to someone else without their explicit ACK ... So...what exactly are you Cc'ing us on? read the thread That sort of brings us back to my original question, doesn't it? reads the archives Ok, so there are complaints about the non-responsiveness of some of Atmel's custodians and a wish for someone to take over. Fine by me, I guess, since I've already indicated I'm probably not the right person for the job anymore. I can't speak for the AT91 team, however. But it does seem kind of rude to just hand everything off without Cc'ing any of the maintainers in question. Perhaps they would respond more quickly if people actually send e-mail to them? Btw, I'm not ranting at you, Mike. Thanks for letting me know about the discussion, although I would have preferred a bit more context... For the record, the thread seems to be this one, but I seem to be unable to find the start of it... http://lists.denx.de/pipermail/u-boot/2010-August/074769.html Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] ATMEL Custodians == /dev/null ??
Wolfgang Denk w...@denx.de wrote: Dear Haavard Skinnemoen, In message 20100809132949.43c81...@hskinnemoen-d830 you wrote: But it does seem kind of rude to just hand everything off without Cc'ing any of the maintainers in question. Perhaps they would respond more quickly if people actually send e-mail to them? Rude? send e-mail to them? I mean rude _not_ to send e-mail to them. It's not that I'm going to hand off (note that nothing has happened yet) anything from any active custodian. It's not that I necessarily oppose such a hand-off either. First, I have poked them a number of times, both on and off list. I haven't received any such pokes from you in a long time. Second, they are subscribed to this list, and supposed to read the traffic. Especially if addresses directly in the Subject they should notice that, right? I used to be subscribed to a whole bunch of lists, but after hitting around 70,000 unread e-mail, I decided to unsubscribe from most of them, including u-boot and LKML. Of course, this is also the main reason why I wanted to resign as a custodian; I felt I hadn't been able to do a proper job for some time. But this makes it especially odd that I wasn't Cc'd on the discussion about custodianship. Third, there have been patches posted that clearly fall in their domain, and there is zero response: no comment, no activity in the custodian directory, no pull request, nothing. If I wasn't Cc'd, that would explain it. Of course, it's always best if maintainers follow all relevant mailing lists, but sometimes it's just not an option, not if you're working on several other projects besides u-boot. Finally, I have to admit that I have been a bit sceptic right from the beginning to assign custodianship to someone who has no track record as developer in the community. But obviously Atmel would be in the best positition to provide decent support for their chips. At least that was my hope. Atmel should definitely be in a good position for that, at least in theory. But the reality is that people need to do other things too, and it's difficult to justify spending a lot of time on the boot loader when there are more customer-focused tasks at hand. And then there's the whole cost/benefit thing. I've been arguing quite enthusiastically for the benefits of getting things upstream earlier, but I've lately started to realize that maybe we're not really getting all that much in return for the work we've been putting in. Especially when we're forced to implement a bloody VM subsystem just to fix a regression someone else introduced. I am seriously sick with the current situation. We have a number of AT91 and AVR32 related patches sitting there with nobody taking care of them. No life signs of any custodian or anybody else from Atmel. Again, not Cc'ing the relevant maintainers might be an explanation, though I'm not saying I'm sure it's the _right_ explanation. But then there are people available who are actively working with these chips, who post fixes and other patches, and who even volunteer to take the burden of maintaining the tree. Sure, if someone outside Atmel wants to take over the AVR32 tree, I'm all for it. I would really appreciate to be Cc'd on the discussion, however. And I can be available to help whoever takes over the custodianship if there's anything avr32-specific they can't figure out. Again, I'm not speaking for the AT91 team. Guess what I'll do? Continue to wait that some vendor wakes up? Not commenting on this beyond what I said above. Btw, I'm not ranting at you, Mike. Thanks for letting me know about the discussion, although I would have preferred a bit more context... For the record, the thread seems to be this one, but I seem to be unable to find the start of it... http://lists.denx.de/pipermail/u-boot/2010-August/074769.html Just have a look at the References: headers, and look up the first one in gmane - just append the message ID to http://mid.gmane.org/ == http://mid.gmane.org/4c50e124.2000...@emk-elektronik.de == http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/81812 Thanks, I'll have a look at those. Please forgive me for following the link on your mailserver ;-) Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] ATMEL Custodians == /dev/null ??
Wolfgang Denk w...@denx.de wrote: Dear Haavard Skinnemoen, In message 20100809181318.5ec2a...@hskinnemoen-d830 you wrote: First, I have poked them a number of times, both on and off list. I haven't received any such pokes from you in a long time. I'm not talking about you here. You have clearly indicated that you resign as custodian, which I have accepted. So why should I poke you? I didn't resign _that_ long ago. I used to be subscribed to a whole bunch of lists, but after hitting around 70,000 unread e-mail, I decided to unsubscribe from most of them, including u-boot and LKML. Of course, this is also the main reason why I wanted to resign as a custodian; I felt I hadn't been able to do a proper job for some time. But this makes it especially odd that I wasn't Cc'd on the discussion about custodianship. A custodian who is not subscried on the mailing list? How is this supposed to work? I have to admit that I never expected somebody would come up with such a concept. It actually works on Linux, where people know that they need to Cc the maintainer to get his attention. So you can actually maintain a dozen drivers across half a dozen subsystems without getting completely bogged down with e-mail. If you just drop a patch into the LKML firehose, it will most likely be ignored unless akpm picks it up and pokes the relevant maintainer. Third, there have been patches posted that clearly fall in their domain, and there is zero response: no comment, no activity in the custodian directory, no pull request, nothing. If I wasn't Cc'd, that would explain it. Of course, it's always best if It has never been a requirement to Cc: the custodian. It is up to the custodian to pick up the work that falls into hiw bailiwick, including for example global changes that happen to affect his architecture / subsystem. Of course this requires that you are subscribed. And that you actually *read* the list, at leats to the extend that you recognize certain buzzwords in the Subject: lines, like the name of the architure you feel responsible for. In other words, being a u-boot custodian takes a lot more work than being a Linux maintainer. Combine this with what I said before about it being difficult to justify spending a lot of time keeping the bootloader limping along, and it's not good news if you want more vendor involvement. maintainers follow all relevant mailing lists, but sometimes it's just not an option, not if you're working on several other projects besides u-boot. Your idea of working as a maintainer is very much different from mine, and from the actual requirements for the job. That seems to be the case, yes. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] ATMEL Custodians == /dev/null ??
Mike Frysinger vap...@gentoo.org wrote: explicitly cc-ing the atmel guys just so there's no surprise when at91/avr32 have been handled over to someone else without their explicit ACK ... So...what exactly are you Cc'ing us on? Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/3] avr32: Add simple paging support
Wolfgang Denk w...@denx.de wrote: Can you please try and rebase this code on top of Heiko's ARM rework patches, i. e. with cache and relocation support? See http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/81825/focus=82142 My intention is that after -rc1 has been released (i. e. when we have a next branch again), I will first apply the new environment code patches, and then, probably with a week delay or so, Heiko's ARM rework. Your stuff will then have to fit on top of this. Sure. The patches probably needs more testing and a refresh anyway...I just saw some issues with SDRAM initialization on some boards which need to be investigated. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/3] avr32: Add missing asm/unaligned.h header file
Simply include the generic version. We could optimize this a bit more, as unaligned 32-bit accesses are ok on AP7, but let's make it work first. Signed-off-by: Haavard Skinnemoen haavard.skinnem...@atmel.com --- arch/avr32/include/asm/unaligned.h |1 + 1 files changed, 1 insertions(+), 0 deletions(-) create mode 100644 arch/avr32/include/asm/unaligned.h diff --git a/arch/avr32/include/asm/unaligned.h b/arch/avr32/include/asm/unaligned.h new file mode 100644 index 000..6cecbbb --- /dev/null +++ b/arch/avr32/include/asm/unaligned.h @@ -0,0 +1 @@ +#include asm-generic/unaligned.h -- 1.7.0.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 2/3] avr32: Print unrelocated PC on exception
In addition to the real PC value, also print the value of PC after subtracting the relocation offset. This value will match the address in the ELF file so it's much easier to figure out where things went wrong. Signed-off-by: Haavard Skinnemoen haavard.skinnem...@atmel.com --- arch/avr32/cpu/exception.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/arch/avr32/cpu/exception.c b/arch/avr32/cpu/exception.c index dc9c300..b21ef1f 100644 --- a/arch/avr32/cpu/exception.c +++ b/arch/avr32/cpu/exception.c @@ -59,7 +59,8 @@ void do_unknown_exception(unsigned int ecr, struct pt_regs *regs) { unsigned int mode; - printf(\n *** Unhandled exception %u at PC=0x%08lx\n, ecr, regs-pc); + printf(\n *** Unhandled exception %u at PC=0x%08lx [%08lx]\n, + ecr, regs-pc, regs-pc - gd-reloc_off); switch (ecr) { case ECR_BUS_ERROR_WRITE: -- 1.7.0.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 0/3] avr32 fixes
This series fixes a trivial build issue, as well as a longstanding problem with the 'saveenv' command on ATNGW100. It also includes a trivial enhancement to the exception reporting which I found very useful during debugging. Hopefully this will make mainline u-boot useful again on AVR32. Without this fix, v2008.10 is the latest usable release. The patches are based on v2010.06, but it merges fine with the latest upstream master. The AVR32 master branch currently contains a workaround which I plan to revert if these patches are acceptable. Haavard Skinnemoen (3): avr32: Add missing asm/unaligned.h header file avr32: Print unrelocated PC on exception avr32: Add simple paging support arch/avr32/cpu/at32ap700x/Makefile |2 +- arch/avr32/cpu/at32ap700x/mmu.c| 80 arch/avr32/cpu/exception.c |3 +- arch/avr32/cpu/start.S | 19 +++-- arch/avr32/include/asm/arch-at32ap700x/addrspace.h |5 +- arch/avr32/include/asm/arch-at32ap700x/mmu.h | 66 arch/avr32/include/asm/unaligned.h |1 + arch/avr32/lib/board.c |3 + board/atmel/atngw100/atngw100.c| 15 include/configs/atngw100.h |3 + 10 files changed, 185 insertions(+), 12 deletions(-) create mode 100644 arch/avr32/cpu/at32ap700x/mmu.c create mode 100644 arch/avr32/include/asm/arch-at32ap700x/mmu.h create mode 100644 arch/avr32/include/asm/unaligned.h ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 3/3] avr32: Add simple paging support
Use the MMU hardware to set up 1:1 mappings between physical and virtual addresses. This allows us to bypass the cache when accessing the flash without having to do any physical-to-virtual address mapping in the CFI driver. The virtual memory mappings are defined at compile time through a sorted array of virtual memory range objects. When a TLB miss exception happens, the exception handler does a binary search through the array until it finds a matching entry and loads it into the TLB. The u-boot image itself is covered by a fixed TLB entry which is never replaced. This makes the 'saveenv' command work again on ATNGW100 and other boards using the CFI driver, hopefully without breaking any rules. Signed-off-by: Haavard Skinnemoen haavard.skinnem...@atmel.com --- arch/avr32/cpu/at32ap700x/Makefile |2 +- arch/avr32/cpu/at32ap700x/mmu.c| 78 arch/avr32/cpu/start.S | 19 +++-- arch/avr32/include/asm/arch-at32ap700x/addrspace.h |5 +- arch/avr32/include/asm/arch-at32ap700x/mmu.h | 66 + arch/avr32/lib/board.c |4 + board/atmel/atngw100/atngw100.c| 15 board/atmel/atstk1000/atstk1000.c | 15 board/earthlcd/favr-32-ezkit/favr-32-ezkit.c | 15 board/mimc/mimc200/mimc200.c | 20 + board/miromico/hammerhead/hammerhead.c | 15 include/configs/atngw100.h |3 + include/configs/atstk1002.h|3 + include/configs/atstk1003.h|3 + include/configs/atstk1004.h|3 + include/configs/atstk1006.h|3 + include/configs/favr-32-ezkit.h|3 + include/configs/hammerhead.h |3 + include/configs/mimc200.h |3 + 19 files changed, 267 insertions(+), 11 deletions(-) create mode 100644 arch/avr32/cpu/at32ap700x/mmu.c create mode 100644 arch/avr32/include/asm/arch-at32ap700x/mmu.h diff --git a/arch/avr32/cpu/at32ap700x/Makefile b/arch/avr32/cpu/at32ap700x/Makefile index 46e6ef6..30ea925 100644 --- a/arch/avr32/cpu/at32ap700x/Makefile +++ b/arch/avr32/cpu/at32ap700x/Makefile @@ -24,7 +24,7 @@ include $(TOPDIR)/config.mk LIB:= $(obj)lib$(SOC).a -COBJS := portmux.o clk.o +COBJS := portmux.o clk.o mmu.o SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c) OBJS := $(addprefix $(obj),$(SOBJS) $(COBJS)) diff --git a/arch/avr32/cpu/at32ap700x/mmu.c b/arch/avr32/cpu/at32ap700x/mmu.c new file mode 100644 index 000..c3a1b93 --- /dev/null +++ b/arch/avr32/cpu/at32ap700x/mmu.c @@ -0,0 +1,78 @@ +#include common.h +#include asm/arch/mmu.h +#include asm/sysreg.h + +void mmu_init_r(unsigned long dest_addr) +{ + uintptr_t vmr_table_addr; + + /* Round monitor address down to the nearest page boundary */ + dest_addr = PAGE_ADDR_MASK; + + /* Initialize TLB entry 0 to cover the monitor, and lock it */ + sysreg_write(TLBEHI, dest_addr | SYSREG_BIT(TLBEHI_V)); + sysreg_write(TLBELO, dest_addr | MMU_VMR_CACHE_WRBACK); + sysreg_write(MMUCR, SYSREG_BF(DRP, 0) | SYSREG_BF(DLA, 1) + | SYSREG_BIT(MMUCR_S) | SYSREG_BIT(M)); + __builtin_tlbw(); + + /* +* Calculate the address of the VM range table in a PC-relative +* manner to make sure we hit the SDRAM and not the flash. +*/ + vmr_table_addr = (uintptr_t)mmu_vmr_table; + sysreg_write(PTBR, vmr_table_addr); + printf(VMR table @ 0x%08x\n, vmr_table_addr); + + /* Enable paging */ + sysreg_write(MMUCR, SYSREG_BF(DRP, 1) | SYSREG_BF(DLA, 1) + | SYSREG_BIT(MMUCR_S) | SYSREG_BIT(M) | SYSREG_BIT(E)); +} + +int mmu_handle_tlb_miss(void) +{ + const struct mmu_vm_range *vmr_table; + const struct mmu_vm_range *vmr; + unsigned int fault_pgno; + int first, last; + + fault_pgno = sysreg_read(TLBEAR) PAGE_SHIFT; + vmr_table = (const struct mmu_vm_range *)sysreg_read(PTBR); + + /* Do a binary search through the VM ranges */ + first = 0; + last = CONFIG_SYS_NR_VM_REGIONS; + while (first last) { + unsigned int start; + int middle; + + /* Pick the entry in the middle of the remaining range */ + middle = (first + last) 1; + vmr = vmr_table[middle]; + start = vmr-virt_pgno; + + /* Do the bisection thing */ + if (fault_pgno start) { + last = middle; + } else if (fault_pgno = (start + vmr-nr_pages)) { + first = middle + 1; + } else { + /* Got it; let's slam it into the TLB */ + uint32_t tlbelo
Re: [U-Boot] [PATCH 0/3] avr32 fixes
Bas Mevissen ab...@basmevissen.nl wrote: On Mon, 2 Aug 2010 14:06:26 +0200, Haavard Skinnemoen haavard.skinnem...@atmel.com wrote: This series fixes a trivial build issue, as well as a longstanding problem with the 'saveenv' command on ATNGW100. Is that the same problem that makes flash erase actions fail with the error that the start or end address does not match a sector boundary? Yes, that's the one. Hopefully this will make mainline u-boot useful again on AVR32. Without this fix, v2008.10 is the latest usable release. I had a problem with this version as well when the environment was empty. It said the flash was 4Gig and kept crashing with some DMA error (NGW100 board). Ok, I wasn't aware of that problem. I don't think I've ever tried running u-boot with empty environment; in fact, I'm not even sure how you do that since it will fall back to a default environment if it can't find anything in flash. The patches are based on v2010.06, but it merges fine with the latest upstream master. The AVR32 master branch currently contains a workaround which I plan to revert if these patches are acceptable. I'm happy to test them on my board. What a coincidence with my (first) mailing list post from earlier today. That would be awesome. I don't really follow the mailing list very closely these days, so I didn't see your post. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] Resigning as the AVR32 custodian (was Re: AVR32 / ATNGW100 FLASH adressing issues)
Wolfgang Denk w...@denx.de wrote: I think that the issue should be fixed by making sure FLASH is detected at 0x and NOT at 0xa000, correct? This has been discussed in one of the longer and more heated discussions on that list; see thread starting here: http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/66904 Right...and since we never reached an agreement, and I still have no idea about how to bring the most popular AVR32 board into working shape, it seems kind of pointless for me to continue as the AVR32 custodian. In fact, I should probably have resigned a long time ago, but this kind of failure is not easy to admit. Please remove me from the list. I'm sorry it didn't work out. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Virtual addresses, u-boot, and the MMU
Becky Bruce becky.br...@freescale.com wrote: I'm not really deep enough in the implementation details and thus would appreciate comments for example from Becky and Stefan. In my opinion, turning on or off the cache on an address range should be implemented as outlined above, i. e. as an operation changing the caching properties of the address range. This makes sense to me. The disable function would need to flush the range from the cache, but that's the only difficulty I forsee. However, I dug up some AVR32 docs, and it looks like the whole dual cacheable/CI mapping thing may be architectural. The architecture seems to specify a virtual memory map for privileged state, and part of the VA range is not translated by the MMU, but has a default translation. There appear to be segments in the untranslated VA space that map to the same PA, one cacheable and the other not. Yes, that is correct. As Andrew pointed out, MIPS does essentially the same thing, and I _think_ some versions of SH have a similar setup as well. This makes it easy to set up uncached access on certain physical addresses without wasting any TLB entries. On the other hand, turning off the cache entirely is a quite complicated operation which involves accessing the memory-mapped dcache tag memory and marking everything as allocated and invalid. So I'd rather not do that, especially when there's a much easier way. Another alternative is to enable paging, which will allow fine-grained control over caching properties. It's probably not all that difficult to do, but it just seems so _pointless_. Also, what I don't get is that your architecture _also_ needs to remap physical to virtual addresses, but for a different reason, so what is wrong with having an API that can do both? In other words, the map_physmem() API can support the following three classes of architectures: - Architectures which don't need address remapping but do need to change caching properties: Just update the caching properties and return the physical address unchanged. - Architectures like AVR32 which change caching properties through the MMU: Return a new virtual address with the desired properties. - Architectures with PAVA: Update the caching properties if necessary and return a temporary virtual address corresponding to the given physical address, allowing the entire physical address range of the CPU to be made available to drivers utilizing this API. What is wrong with this API? Why are everyone so hell-bent on making it difficult for the last two classes of architectures? Did I get any or all of the scenarios above wrong? Using a completely different address range instead is a different thing, and not what I have in mind. I really dislike the idea of supporting alternate addresses in this context - even if this is what would be easiest to implement on some architectures. I agree, but, then, I'm coming from an architecture where doing this is bad joo-joo. Again, it looks like AVR may actually need this - hopefully someone who knows more about AVR will speak up here. I've said what I intend to say about this. I find it really hard to argue about opinions which are not backed by facts. The alternate addresses are there whether we decide use them or not, but not using them makes what should be a trivial operation really hard to do. Oh, and I'm really sorry about the tone I used a couple of days ago. I deliberately stopped posting for a few days after that...but I'm hoping we can all continue working towards a solution now. Btw, I do have a few suggestions on how to resolve this, but I'm not going to come forward with them as long as I'm being stonewalled on the VA/PA mapping issue. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Virtual addresses, u-boot, and the MMU
Mark Jackson mpfj-l...@mimc.co.uk wrote: The functions could also return (architecture dependant) a remapped address to be used, then we could support:- Right, and that is the important part: If I'm allowed to return a remapped address, this API will be trivial to implement on AVR32. If not, it will be quite difficult (and make everything larger and slower). Your idea is good; it's mostly similar to what map_physmem() does today, but perhaps the name is wrong, and I suppose it should probably flush the caches in addition to just remapping the address. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
Wolfgang Denk w...@denx.de wrote: Dear Haavard Skinnemoen, In message 20090831155327.62b58...@hskinnemoen-d830 you wrote: Possibly. But it means even more effort and even larger code, so I'm not exactly happy about it. Really? Sorry if I'm asking dumb questions - I don't know AVR32: is it true that stting up a non-1:1 mapping for the NOR flash (i. e. what you are doing now) is easier (less code) than setting up a 1:1 mapping? What exactly are the reasons for this? Like I explained in an earlier mail, the default setup includes a 1:1 mapping of the lowest addresses, but it's cacheable. The default setup also includes an uncached mapping of the same memory at a higher virtual address. Yes, it is much easier (and smaller) to use the default virtual memory layout than setting up paging (which involves several exception handlers). Indeed I am, and intentionally, because this is a different topic. If your system requires to set up the MMU to enable caching, then you are supposed to do it in a way compatible with the rest of the software design, i. e. as transparently as possible. None of the architectures I know resonably well have problems setting up a 1: 1 address mapping even when the MMU is on (but I have to admit that AVR32 is not among these architectures). I suspect quite a few other architectures run with caches disabled because it's not safe to run with caches enabled with the current software design. Well, usually we run with IC on and with DC off, usually because quite a number of drivers and other code do not use proper I/O accessors yet, and/or because it's easier and nobody really cares. For example on PowerPC, adding support for the data cache usually gives only a minimal performance boost. This may be different on other architectures. Ok, so the code is broken and nobody else cares? I suppose I could disable the DC (which is a bit complicated, but possible), but that would just add to the already high cost (in terms of both code size and performance) of using common code (i.e. the CFI driver), so I'm leaning towards a custom flash driver instead. Cache should not be relevant at all when defining a physcal or virtual memory map. Physical, no, but it's very common that the MMU defines caching properties (enabled/disabled, writeback/writethrough, etc.) Agreed. But it should be not so difficult to use the MMU to set up a 1:1 mapping if you have to set up the MMU at all - or is there any problems with that which I'm not aware of? Yes, there is: caching. Heh. If more platforms had broken this rule we would probably have become aware of these violations earlier, and stopped them doing such naughty things ;-) Seems like you think it's more important to follow arbitrary rules than writing code that works well. Keeping the code as simple as possible is not exactly an arbitrary rule. At least not for me. As simple as possible, but no simpler... Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
Becky Bruce becky.br...@freescale.com wrote: Sorry, guys, I'm still not clear on what's going on. Haavard, did you expect each call to flash_map to return a different VA each time (i.e. you're trying to do some sort of dynamic mapping), or are you just calling it to get the VA that corresponds to some PA, since VA != PA on these 2 boards? I'm calling it to get some VA which corresponds to a given PA with given caching properties. I don't think it's good design if the board code for every single board needs to know exactly which VA is going to be returned. But exactly what's wrong with hiding all this complexity inside map_physmem()? It was designed _exactly_ for this purpose -- to allow platforms with non-trivial mappings between VA and PA to do the necessary mapping when the driver requests it. Sorry, I don't know that code and it's users well enough to coment. Maybe Becky and/or Stefan can shed some light on this... The problem is that would mean the CFI driver would be treating start as a PA, while every other flash driver that I looked at treats it as a VA. That kind of inconsistency would be bad. Except that it's even more inconsistent now since lots of code elsewhere treats start as a PA, and the CFI driver was the only driver which did it mostly correctly. Plus, Wolfgang, Stefan, Kumar, and I discussed this on the list/IRC last november and agreed that it made sense for command-line foo (including the flash commands) to take a virtual address as the argument. Platforms that have a non-fixed memory map would need to provide a command-line interface to get a VA to use (since that's highly unusual and expected to remain so). I think that's an incredibly stupid idea, and it's really a shame that I didn't participate in the IRC meeting. It would have been so much easier if the command line only had to deal with PA and the platform had to remap things transparently if necessary. Now, if you need to access an arbitrary physical address through a script, you need to first call this set up a mapping command, record the value it prints out (presumably) and make sure not to interpret any error message as a virtual address, use it to perform the command, and then unmap it afterwards. So if anything is breaking the u-boot philosophy of simplicity it is this, and it does it in the worst possible way: it exposes all the complexity through the user interface! That PA==VA rule is pretty much the reason we're in this mess -- if Let's say, the fact that this rule has not been stricter enforced has caused that teh appearant problems were not discovered earlier. more platforms had broken this rule, perhaps more of the code would Heh. If more platforms had broken this rule we would probably have become aware of these violations earlier, and stopped them doing such naughty things ;-) Well, u-boot was supposed to be simple, and a VA=PA assumption ended up built into a lot of the code. Which isn't a problem until you need something different I had a lot of fun standing on my head trying to get 36-bit physical addressing on PowerPC working as a result of this. I suspect the next big u-boot problem will be the need for dynamic mappings, because we're assuming for the most part now that the board memory map is static. Exactly, and with the current approach, dynamic mappings will _never_ be possible because all the VA-to-PA mapping assumptions will be built into both the board code and the user interface! So I ask again: Why don't we just fix the broken code instead? We have the mechanisms available to make this work via the map_physmem() functions, we just need to use them. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
Becky Bruce becky.br...@freescale.com wrote: Becky: the fact that Haavard's code is breaking is not considered an indication of a deficiency in his code. I'm not calling the code deficient, just a bit inconsistent, and it wasn't clear to me why. When code uses the same value 1) by mapping it and 2) by dereferencing it directly, that's a bit strange. Why map it in some cases and in not others? I agree, that is inconsistent. However, you fixed the code which was actually doing it correctly...we should instead fix the code which is incorrectly using the physical address as a virtual one. While doing that, we could also consider dropping that hideous start array altogether and instead provide a handful of accessors for locating sector start addresses on the flash. How is that supposed to work when VA!=PA or when the VA can change? This is one of the reasons that it seemed to make sense to modify the driver as I did - it should now be able to work when VA!=PA as well as when we're 1:1. I could find no users that seemed to need the dynamic mapping. However, if anyone does need to *dynamically* map the flash in and out with a different VA each time, then we do need to do things a bit differently and we should look into a solution for that. I don't think anything needs a dynamic mapping right now, but if we're going to _ever_ support such systems in the future (and your 36-bit PA system is a likely candidate, isn't it?) we have to stop locking ourselves into a static mapping. Clearly, I'm not the expert on the CFI code, so when I published that patch I expected someone to smack me if I was being a moron :) I apologize for not doing so earlier ;-) Anyway, I have to take most of the blame for this situation...if I had paid attention and flagged the problem earlier, much of this could have been avoided. I'm hoping we can avoid too much pointing of fingers and work towards a reasonably future-proof solution which works well on all platforms. If the CFI driver kind of allowed for VAs before (but incompletely / incorrectly), then this dis not cause problems on any systems using a strict 1:1 mapping. Any changes to the code to correctly support other mappings must be done in a way that they (1) do not break and (2) do not add additional burdon on systems with a simple 1:1 mapping. Agreed, there shouldn't be any burden on those systems. Agree too, as long as those systems does not include common code which needs to run on all systems. Everything is treated as virtual unless it's being used for hardware setup. Thisis NOT correct. U-Boot usually does NOT use virtual addresses. Only very few systems do, and these must care not to disturb the majority of systems which do no need to differentiate between physical and virtual addresses. I'm not saying it *is* a VA as far as U-Boot knows, but that it is *treated* as one, as mentioned above. And this code was not expected to disturb the 1:1 case. Exposing VAs to the user interface and board code is IMO a very bad idea, however. And that is what's happening now -- instead of localizing the VAs to the CFI flash driver, it is now spread all over the place. A lot of code had been just using the PA as a VA, because things were always mapped 1-1. Not only were, but _are_ and _will_be_. Of course - that should continue to be the default case unless it needs to differ for some reason. Yes, and that's why the external interface (at least the command line and board configuration files) needs to use PA. Indeed I'm deeply trouble when log standing rules get silently bent and even broken. I agree 100% that 1:1 support should not be disturbed, since that's the default case. There was absolutely no intent here to bend any logsic ;-) standing rules, and nothing has been done here that should have any impact on 1:1 as far as I'm aware. Agree, 1:1 isn't the issue, it's two different systems with !1:1 which have started making incompatible changes to the CFI driver. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
Stefan Roese s...@denx.de wrote: On Tuesday 01 September 2009 10:57:52 Haavard Skinnemoen wrote: Well, usually we run with IC on and with DC off, usually because quite a number of drivers and other code do not use proper I/O accessors yet, and/or because it's easier and nobody really cares. For example on PowerPC, adding support for the data cache usually gives only a minimal performance boost. This may be different on other architectures. Ok, so the code is broken and nobody else cares? I wouldn't put it like this. The CFI driver assumes that the FLASH mapping is not cached. This makes perfect sense in my point of view. Yes, and it can do that safely because it specifically asks for an uncached address from map_physmem(). That part didn't actually change with the patch in question -- the problem is that the address returned by map_physmem() is now exposed through external interfaces -- it's not just an internal implementation detail anymore. I suppose I could disable the DC (which is a bit complicated, but possible), but that would just add to the already high cost (in terms of both code size and performance) of using common code (i.e. the CFI driver), so I'm leaning towards a custom flash driver instead. On some 440 platforms we configure the FLASH cached upon powerup, and disable the caching after relocating to SDRAM. So when the CFI driver is started, the FLASH is not cached any more, but we have the cache speedup upon relocation. That's what I want to do as well! And I can actually do that as long as the flash subsystem uses map_physmem() consistently. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
Wolfgang Denk w...@denx.de wrote: Dear Haavard Skinnemoen, Like I explained in an earlier mail, the default setup includes a 1:1 mapping of the lowest addresses, but it's cacheable. The default setup also includes an uncached mapping of the same memory at a higher virtual address. You mean you want to have the same memory area mapped _twice_, once cached and once (at another address) uncached? Yes. Well, this obviously cannot be done with a plain 1:1 mapping. But then: isn't that asking for trouble anyway? Or is there anything that prevents you for example reading stale cached data after the memory content has changed by accesses through the uncached mapping? There's nothing which prevents me from accessing a completely different address either -- I just need to make sure that I access the correct address, or things will blow up one way or another. The default virtual memory model makes it very easy to do uncached access to certain types of memory (i.e. flash) and cached access to others (i.e. SDRAM). It also makes it easy to run from cached flash memory at startup and switch to uncached access later (after flushing the cache, of course). Yes, it is much easier (and smaller) to use the default virtual memory layout than setting up paging (which involves several exception handlers). I don't see how paging comes into play here? If I can't use the default scheme described above (and you're pretty much saying I can't), I can define caching properties on a page-by-page basis, but that obviously requires paging to be enabled. Ok, so the code is broken and nobody else cares? Broken? You might call it a design decision. This is a boot loader, and simplicity of design and easy porting and board bring up are usually higher rated that sequeezing out the last few percent of performance. And that is EXACTLY why I'm trying to advocate: Keep the additional complexity (which can be kept to a minimum) localized to a handful of drivers, and don't change the command line interface or the board configuration interface. IIRC, on PowerPC the difference of running with DC enabled or not is only in the 10% range or so. But as a matter of principle, I don't want to reduce the performance _and_ increase the code size just because a driver that used to work got broken. I want to fix the driver instead! I suppose I could disable the DC (which is a bit complicated, but possible), but that would just add to the already high cost (in terms of both code size and performance) of using common code (i.e. the CFI driver), so I'm leaning towards a custom flash driver instead. If you want to run with data caches enabled by default, then it would probably make more sense if you invested efforts in extending the CFI driver to provide hooks / callbacks to (temporarily) switch of data cache for the memory range in question. This wouls allow you to run with DC enabled on the flash area, and still use the CFI driver. But that is exactly how map_physmem() works -- it allows the CFI driver (or any other driver) to request the caches to be bypassed for a given physical address range, possibly resulting in a different virtual address (though for backwards compatibility, all platforms except AVR32 simply return the physical address unchanged.) The problem is that this virtual address (which currently isn't dynamic, but it's easy to imagine a platform where it could be) is exposed to the rest of the world, thus breaking both existing board configurations and potentially any command-line scripts (and, apparently, the jffs2 driver though I'm not entirely sure how that happened.) Such changes will have it much easier to find their way into mainline than adding a proprietary flash driver. It certainly won't be a proprietary driver; if anything, it would be a variant of the driver currently used by ATSTK1000. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
Wolfgang Denk w...@denx.de wrote: Dear Haavard Skinnemoen, In message 20090901123829.55fcb...@hskinnemoen-d830 you wrote: And that is EXACTLY why I'm trying to advocate: Keep the additional complexity (which can be kept to a minimum) localized to a handful of drivers, and don't change the command line interface or the board configuration interface. We're not doing this. At least not intentionally. Good. Then let's please put a stop to the madness of exposing virtual addresses all over the place. The discussion we had was based on our knowledge about existing systems, and needs to also handle more complex situations like for example 32 bit systems with 36 bit physical addresses. As far as I understand, the only difference for such systems is that keeping 64-bit physical addresses around are a bit more costly than passing around 32-bit virtual pointers. But presumably those systems have enough memory to make it a non-issue... But as a matter of principle, I don't want to reduce the performance _and_ increase the code size just because a driver that used to work got broken. I want to fix the driver instead! Agreed - assuming it is possible without hurting the majority of other existing configurations. Yes, that is indeed possible, as evidenced by the fact that it used to work without hurting any other configurations. It took another special case to break it. If you want to run with data caches enabled by default, then it would probably make more sense if you invested efforts in extending the CFI driver to provide hooks / callbacks to (temporarily) switch of data cache for the memory range in question. This wouls allow you to run with DC enabled on the flash area, and still use the CFI driver. But that is exactly how map_physmem() works -- it allows the CFI driver (or any other driver) to request the caches to be bypassed for a given physical address range, possibly resulting in a different virtual address (though for backwards compatibility, all platforms except AVR32 simply return the physical address unchanged.) I have to admit that I have no idea how map_physmem() used to work or how it works now; at this point, I don;t care about implementation details, I try to focus only on the overall design. It works exactly the same way now as it used to work. The difference is that its return value (which is basically just an architecture-specific cookie, aka. virtual address) is exposed to a much larger part of the system. I think your double mapping is a hightly architecture-specific feature which I do not like at all, but if you are happy with it and it works for you I cannot and will not prevent it. Yes, it was always meant to be an architecture-specific thing. Though I think some MIPS- and SH-based processors do something very similar. But in order to utilize architecture-specific features, we need an architecture-independent abstraction and that's basically what map_physmem() is. But after discussions with Stefan Roese and Detlev Zundel it seems to me that map_physmem() is probably not the right approach (judging only from the name). We should not try to fix cache attributes by modifying addresses / address maps And why not? That's what Linux does, and it works wonderfully across 26 different architectures. Instead, we should really extend the CFI driver such that it does not matter if the addresses you are passing refer to cached or uncached memory, and then provide hooks in the CFI driver to allow for testing if cache is enabled, and switching cache off if it is. What's the advantage of such an approach? It sounds much more complicated from the driver's point of view as well. Detlev Zundel suggested to use this as a chance to come up with something like a cache API which then could be used by other drivers as well. My suggestion is to use map_physmem() and unmap_physmem(). It exists today, and it works, provided that we keep its usage internal to the driver and don't expose whatever architecture-specific values it returns. To me such an approach makes much more sense, as it is generic and can be used by other drivers and other architectures - even if it may come at the cost of more effort on your systems. In what way is it more generic? In what way can map/unmap_physmem() not be used with other drivers and architectures? Such changes will have it much easier to find their way into mainline than adding a proprietary flash driver. It certainly won't be a proprietary driver; if anything, it would be a variant of the driver currently used by ATSTK1000. Well, but board/atmel/atstk1000/flash.c _is_ a proprietary driver for some flash chips that seem to be CFI conformant at first glance. You would not get such a driver into mailine any more. So I guess dropping support for ATNGW100 is the only remaining option? At least STK1000 has a working flash driver
Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
Wolfgang Denk w...@denx.de wrote: Dear Haavard Skinnemoen, In message 20090901134257.59961...@hskinnemoen-d830 you wrote: complexity (which can be kept to a minimum) localized to a handful of drivers, and don't change the command line interface or the board configuration interface. We're not doing this. At least not intentionally. Good. Then let's please put a stop to the madness of exposing virtual addresses all over the place. But that's what we've been doing all the time. You just did not notice it because of the usual 1:1 mapping. Up until this commit, yes: commit 09ce9921a7d8b1ce764656b14b42217bbf4faa38 Author: Becky Bruce bec...@kernel.crashing.org Date: Mon Feb 2 16:34:51 2009 -0600 flash/cfi_flash: Use virtual sector start address, not phys after that, NGW100 support is broken because virtual addresses are no longer an implementation detail and are being exposed all over the place. On a 32 bit system with 36 bit physical addresses you cannot use a physical address when running the md command, for example. Yes you can, if you teach the md command to map it at a virtual address first. Again, map_physmem() makes this possible without changing the external interface in any way. we always assumed that the 32 bit VA we used matched 1:1 to a PA with the missing high bits set to 0, right? Yes, but how can you possibly access an arbitrary 36-bit address using that setup? The discussion we had was based on our knowledge about existing systems, and needs to also handle more complex situations like for example 32 bit systems with 36 bit physical addresses. As far as I understand, the only difference for such systems is that keeping 64-bit physical addresses around are a bit more costly than passing around 32-bit virtual pointers. But presumably those systems have enough memory to make it a non-issue... That's not true. There are 32 bit systems with bigger physical address spaces. Which part of what I said isn't true? The part about some systems might require 64-bit variables to store a physical address or that such variables take more storage than a 32-bit virtual address? And note that this is not a new thing. We have been doing this allt he time - just without ever explicitly mentioning it, because so far nobody ever complained about it. Doing what exactly? Limiting 36-bit PA systems to only use the lower 4GB of memory because VA must always equal PA come hell or high water? Agreed - assuming it is possible without hurting the majority of other existing configurations. Yes, that is indeed possible, as evidenced by the fact that it used to work without hurting any other configurations. It took another special case to break it. My understanding is that the special case is yours - using a non-1:1 mapping. But the other system must be a special case too -- otherwise it would work fine without commit 09ce9921a7d8b1ce764656b14b42217bbf4faa38. That commit does not make any difference whatsoever on 1:1 systems. If the other system isn't a special case, why don't we just revert that commit? discussions with Stefan Roese and Detlev Zundel it seems to me that map_physmem() is probably not the right approach (judging only from the name). We should not try to fix cache attributes by modifying addresses / address maps And why not? That's what Linux does, and it works wonderfully across 26 different architectures. We do not want to implement a full-fledged OS with virtual memory and on-demand paging and all that stuff in U-Boot. Then why are you forcing me to implement it for AVR32?!? Instead, we should really extend the CFI driver such that it does not matter if the addresses you are passing refer to cached or uncached memory, and then provide hooks in the CFI driver to allow for testing if cache is enabled, and switching cache off if it is. What's the advantage of such an approach? It sounds much more complicated from the driver's point of view as well. The advantage is that other drivers with similar needs can use it as well. But they can use map_physmem() today! It allows you to specify exactly what caching properties you need. The fact that it _may_ return a virtual address which is different from the physical one just allows more flexibility in how the architecture chooses to implement it! To me such an approach makes much more sense, as it is generic and can be used by other drivers and other architectures - even if it may come at the cost of more effort on your systems. In what way is it more generic? In what way can map/unmap_physmem() not be used with other drivers and architectures? On other architectures it is not possible to map the same memory area multiple times with different attributes. So what? They can just return the physical address unchanged. It's not _mandatory_ to remap anything... Remapping addresses
Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
Wolfgang Denk w...@denx.de wrote: Well, that was the part of me saying before: as long as it doesn't hurt others. We don't want to add that complexity to U-Boot as noone needs it. Right. I forgot that nobody actually needs this. Fuck it, I'm out. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
Thiago A. Corrêa thiago.cor...@gmail.com wrote: Hi, I don't want to intrude too much into the discussion, but I would like to offer a small bit of info Oh, I wish more people would intrude ;-) On Tue, Sep 1, 2009 at 10:23 AM, Haavard Skinnemoenhaavard.skinnem...@atmel.com wrote: It would be a good idea to clean up this board support, remove the proprietary flash driver and use the CFI driver instead. Patches are welcome. You must be joking. Replacing a working driver with a broken one? Why the hell would anyone do that? My custom board uses AT49BV640D. I wen't thru a lot of trouble getting u-boot to work with it. And one of my attempts was to use the working driver from stk1000 and it didn't work. Understandably since 640D uses the intel command set while the 6416 chip on STK1000 uses the AMD command set and has a couple of interesting bugs... Now, I still want to use common code as much as possible, but it's always been quite expensive in terms of code size, and it currently doesn't even work. While both of those should be possible to overcome, I'm getting incredibly frustrated that all my attempts at fixing it are being shot down by arcane, non-sensical rules which aren't even being enforced consistently. On the other hand, the CFI driver with the tripple revert that Haavard proposed did. I'm currently patching it like that so I can continue my development, while people with much more knowleadge than I have on u-boot code could fix the issue, but looks like I'm about to get orphan on u-boot and Atmel. Right...I'm beginning to doubt that anyone is familiar enough with the u-boot code, since everyone seems to have their own opinion about how things are supposed to work. To summarize, here are the possible ways to fix the problem as I see it: - Use virtual address in CONFIG_ENV_ADDR to conform with the way the CFI driver currently works. Rejected by Wolfgang because virtual addresses don't exist. - Fix the API and user interface breakage by reverting commit 09ce9921. Rejected because virtual addresses are used everywhere. - Fix it by using map_physmem() in a way that works for all architectures. Rejected because all other architectures than PPC are evil and need to be punished for doing things differently. - Introduce a custom flash driver for ATNGW100. Rejected because stupid principles are more important than making things work. So I don't really know where to proceed from here. I guess two additional options are forking the damn thing or creating a proprietary bootloader, but I don't really want to do either. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [GIT PULL] AVR32: NGW100 fix for 2009.08
Hans-Christian Egtvedt hans-christian.egtv...@atmel.com wrote: Yeah...I'm unsure myself. The system will boot, but the 'saveenv' command doesn't work...so while I really want to fix this issue _properly_, I'm not sure if there's enough time to do that before the next release. Did you test loading something from JFFS2? I have a bad feeling it might still be broken. Right. Does anyone have any clue about why it's broken? Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
On Fri, 28 Aug 2009 14:58:15 -0500 Becky Bruce becky.br...@freescale.com wrote: FYI, before the patch, the CFI driver was sometimes doing the map, but IIRC it was also abusing the physical address, treating it as a virtual address without mapping it. In that case, those places should have been fixed, no? The only way for that to work is when VA=PA (or, depending on what you were doing with the address, you just got lucky). The CFI driver was the outlier - all the other flash code was treating the start field as a VA already. So I don't think just reverting the patch is the answer. Except for everything _outside_ the flash code which still deals with physical addresses, like the environment stuff and JFFS2. The flash code takes those addresses and compares them with the virtual addresses in the start array, and things break. So...which config symbols are supposed to be virtual now, and how are you supposed to know how the virtual-to-physical mappings are set up in advance? Everything is treated as virtual unless it's being used for hardware setup. Exactly what constitutes hardware setup? If you use something to do memory accesses, it's virtual. Yes, but then the address should also be in a pointer, not an unsigned long which the flash 'start' array is. A lot of code had been just using the PA as a VA, because things were always mapped 1-1. Yes, there's lots of code which is broken in that respect... Can you point me at an example in your scenario of code that interacts with the flash? CONFIG_ENV_ADDR is used to store the environment in CFI flash. Reading the environment works OK-ish since the flash is accessible through a cacheable 1:1 mapping from virtual/physical address 0. However, when writing and erasing, the physical address stored in CONFIG_ENV_ADDR appears to be outside of the virtual sector addresses stored in the 'start' array, so the flash code throws an error. There are basically two ways to fix it: Either go back to using physical addresses in the flash API, or make CONFIG_ENV_ADDR virtual (and from what I hear, the jffs2 code needs a similar fix.) This patch does the latter, but it seems like it doesn't fix things completely, and Wolfgang didn't appear very happy about it. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
On Sat, 29 Aug 2009 13:39:02 +0200 Stefan Roese s...@denx.de wrote: I think too, that revering the patch in question is not the right solution for this problem in the current stage. But I have to admit that I don't have enough insight into your platform to come up with another good idea quickly. Yeah, I only meant to suggest it as one possible solution, with this patch being one other, less intrusive but also less complete, solution. And I'm not really even opposed to the patch in question, I just need to know how I should configure my systems from now on since all of a sudden, some addresses are supposed to be virtual while others are physical, and it doesn't seem to be documented anywhere what should be used where. So...which config symbols are supposed to be virtual now, and how are you supposed to know how the virtual-to-physical mappings are set up in advance? Everything is treated as virtual unless it's being used for hardware setup. If you use something to do memory accesses, it's virtual. A lot of code had been just using the PA as a VA, because things were always mapped 1-1. Correct. All those addresses to configure the CFI driver should be virtual addresses. That cannot possibly be true, as the start address of the flash is passed to physmem_map(). It is also an absolutely idiotic thing to do since on any CPU with a MMU, the virtual address can be anything, so locking all the configuration symbols to specific virtual addresses would effectively lock down the MMU configuration forever (and I imagine that would be a particularly nasty problem on hardware with more physical address bits than virtual.) Can you point me at an example in your scenario of code that interacts with the flash? Yes, please give us an example of your memory mapping (physical and virtual) on your failing platform. CFI-compatible flash at physical address 0. By default, all virtual addresses up to 2GB are mapped 1:1 with caching enabled (when paging is enabled, which it isn't in u-boot, this area is translated through the MMU.) Then, virtual address 0x8000..0x9fff is mapped to 0x0..0x1fff with caching enabled (supervisor-only and not translated even with paging enabled; this is where the Linux kernel is mapped). Then, virtual address 0xa000..0xbfff is mapped to 0x0..0x1fff with caching disabled -- this is where we want to access the flash and other external hardware. Then there's another paged region and finally a 512MB uncached, direct-mapped region for accessing internal peripherals. So the problem is that even though we may read from the flash using the physical address as virtual address, we cannot initialize, erase or write it using those addresses. Hence the need for map_physmem(). Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
On Sun, 30 Aug 2009 20:11:01 +0200 Wolfgang Denk w...@denx.de wrote: Dear Haavard Becky, In message 20090830173640.2af9c...@siona you wrote: The only way for that to work is when VA=PA (or, depending on what you were doing with the address, Well, VA==PA is the default setting for U-Boot that shall be used for all systems, unless it is really impossible on a board to implement an exception from that rule. While not impossible, following that rule on the NGW100 would require that I either disable the caches (which would result in a massive slowdown) or start using the MMU more actively to get the caching properties right. you just got lucky). The CFI driver was the outlier - all the other flash code was treating the start field as a VA already. So I don't think just reverting the patch is the answer. We did not even have any notion of VA's in U-Boot until very recently, and I call on everybody not to make U-Boot more complicated than necessary. I don't think any boards with PA==VA are affected. This issue is about two platforms with VA!=PA following different rules... In almost all cases RAM and NOR flash fit easily in the physical address space of the CPUs, and for the sake of this majority of systems it must be possible to access memory on such systems assuming a simple 1:1 mapping. You're ignoring cache issues. Any changes to the code to correctly support other mappings must be done in a way that they (1) do not break and (2) do not add additional burdon on systems with a simple 1:1 mapping. That was the idea when I introduced map_physmem() to the CFI driver a while back: the externally visible addresses were kept unchanged, while the remapping was done internally. Also, map_physmem() is a no-op on platforms with a simple 1:1 mapping. If you use something to do memory accesses, it's virtual. Unless you have a very special system you can rely on a strict 1:1 mapping. Technically, the addresses seen by the CPU are virtual. And I don't think systems with a cache should be considered 'very special' these days... There are basically two ways to fix it: Either go back to using physical addresses in the flash API, or make CONFIG_ENV_ADDR virtual I understand we cannot do that, because some systems need to map (NOR) flash to virtual addresses outside the physical address space. Ok, so the CFI driver shall consistently be able to use VAs - but on simple systems with a 1:1 mapping there shall be no need in the system configuration to spend any thoughts on this. But exactly what's wrong with hiding all this complexity inside map_physmem()? It was designed _exactly_ for this purpose -- to allow platforms with non-trivial mappings between VA and PA to do the necessary mapping when the driver requests it. (and from what I hear, the jffs2 code needs a similar fix.) This patch does the latter, but it seems like it doesn't fix things completely, and Wolfgang didn't appear very happy about it. Indeed I'm deeply trouble when log standing rules get silently bent and even broken. And I deeply regret using the CFI driver on NGW100...it took a lot of effort to get it mostly limping along, and then it got broken at the first opportunity. I should have stuck with the much smaller and more efficient board-specific driver I had to begin with. That PA==VA rule is pretty much the reason we're in this mess -- if more platforms had broken this rule, perhaps more of the code would have been written properly without lots of implicit conversion from PA to VA via ugly casts between unsigned long and all sorts of pointers. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [GIT PULL] AVR32: NGW100 fix for 2009.08
Hi Wolfgang, Please pull git://git.denx.de/u-boot-avr32.git master to receive the following fix for a fairly longstanding and annoying ATNGW100 bug. Haavard Skinnemoen (1): atngw100: Use virtual address in CONFIG_ENV_ADDR include/configs/atngw100.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
Ever since the CFI driver was rewritten to use virtual addresses, thus eliminating the whole point of the map_physmem() macro, ATNGW100 has been broken like this: U-Boot saveenv Saving Environment to Flash... Error: start and/or end address not on sector boundary So let's take a different approach and store the virtual address in CONFIG_ENV_ADDR. I personally believe that's rubbish and will break whenever we decide to change the virtual-to-physical mapping in any way, but it looks like it's the direction in which u-boot is currently moving, and it does fix the problem. Signed-off-by: Haavard Skinnemoen haavard.skinnem...@atmel.com --- include/configs/atngw100.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/include/configs/atngw100.h b/include/configs/atngw100.h index 4ed5514..9777ec0 100644 --- a/include/configs/atngw100.h +++ b/include/configs/atngw100.h @@ -155,7 +155,7 @@ #define CONFIG_ENV_IS_IN_FLASH 1 #define CONFIG_ENV_SIZE65536 -#define CONFIG_ENV_ADDR(CONFIG_SYS_FLASH_BASE + CONFIG_SYS_FLASH_SIZE - CONFIG_ENV_SIZE) +#define CONFIG_ENV_ADDR(0xa000 + CONFIG_SYS_FLASH_SIZE - CONFIG_ENV_SIZE) #define CONFIG_SYS_INIT_SP_ADDR(CONFIG_SYS_INTRAM_BASE + CONFIG_SYS_INTRAM_SIZE) -- 1.6.0.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
Mark Jackson mpfj-l...@mimc.co.uk wrote: Haavard Skinnemoen wrote: Ever since the CFI driver was rewritten to use virtual addresses, thus eliminating the whole point of the map_physmem() macro, ATNGW100 has been broken like this: How about other boards (like the MIMC200) ? Aren't *all* AVR32 boards affected in this way ? Possibly, but NGW100 is the only one which I've seen reports about. STK1000 is safe since it doesn't use the CFI driver. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
Mark Jackson mpfj-l...@mimc.co.uk wrote: Haavard Skinnemoen wrote: Possibly, but NGW100 is the only one which I've seen reports about. STK1000 is safe since it doesn't use the CFI driver. I did kinda report this in the thread JFFS2 scanning bug, and the triple-revert patch you posted on 26/05/09 16:58 appeared to fix it. Ah...so it breaks JFFS2 as well? I doubt that changing the environment address fixes that... Since this didn't change any board files (only the core CFI files) I guess I assumed this revert would work its way upstream and I wouldn't have to change anything. Hmm, yeah, maybe I should post the revert again. I have to admit I'm completely confused about how u-boot deals with virtual and physical addresses these days. It used to expose only physical addresses through external interfaces, but now it looks like it's a bit of both, and it's impossible to tell which goes where. Shall I just submit a patch to fix the mimc200 board in the same way as your NGW100 patch ? Yes, that will probably be a good idea if it has the same problem with saveenv. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
Wolfgang Denk w...@denx.de wrote: #define CONFIG_ENV_IS_IN_FLASH 1 #define CONFIG_ENV_SIZE65536 -#define CONFIG_ENV_ADDR(CONFIG_SYS_FLASH_BASE + CONFIG_SYS_FLASH_SIZE - CONFIG_ENV_SIZE) +#define CONFIG_ENV_ADDR(0xa000 + CONFIG_SYS_FLASH_SIZE - CONFIG_ENV_SIZE) This is definitely a change to the worse. Yes, I think so too. I feel a strong urge to NAK this. There must be a better way to fix the problems you are experiencing. Yes...the idea behind the map_physmem() macro was to do any physical-to-virtual address mapping inside the CFI flash driver and never expose anything but physical addresses to the outside world. This meant that the sector start addresses were expressed in terms of physical addresses, matching how things were wired up on the board, and the architecture was free to map those to any virtual address which is most suitable in terms of caching properties, etc. Now, however, the flash start addresses are mapped to virtual addresses at initialization time, so everything that wants to interact with the flash must known which address the architecture decided to map the flash at, which is not necessarily even possible to know in advance if it is being done dynamically through a hardware MMU. Reverting 09ce9921a7d8b1ce764656b14b42217bbf4faa38 would bring things back to the way they were, and fix both the saveenv problem and the jffs2 problem. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
Kumar Gala ga...@kernel.crashing.org wrote: Reverting 09ce9921a7d8b1ce764656b14b42217bbf4faa38 would bring things back to the way they were, and fix both the saveenv problem and the jffs2 problem. Such a revert would break other boards that now expect the new behavior (like all the 36-bit physical cfg for FSL boards) Right, I suspected that. So...which config symbols are supposed to be virtual now, and how are you supposed to know how the virtual-to-physical mappings are set up in advance? Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] asm-generic: Consolidate errno.h to asm-generic/errno.h
Michal Simek wrote: Hi Custodians, Do you have any problem with this asm-generic/errno.h patch? Patch is available in u-boot-microblaze.git asm-generic branch. Looks good to me too. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] U-book and GPLv3? (fwd)
Mike Frysinger wrote: Obviously the second item here will become void if vendor lockout of updates becomes common. So what will be left of the essential freedoms? I can study the code, I can modify it, but I am not allowed to run it. Excellent. and this is why i dislike the GPLv3. the GPLv2 was all about the source, so the conversation between developers and everyone else was you can take my source and modify it all you want, but i want to see the changes. sounds fair. GPLv3 (ignoring the fix for the loophole with web applications) adds *nothing* to this premise. instead, it's used as an ideological club such that the conversation is now i have all these ideas about how software should and shouldnt be utilized, so if you want to use my software, you too now have to subscribe to my way of thinking and you have to show me the changes. so what does moving from GPLv2 to GPLv3 gain us in terms of protections ? nothing. it does however allow us to restrict the people who want to use u- boot to using it in only ways we've blessed. that's plain wrong in my eyes and none of our business in the first place. Wow, I was just about to compose a mail summarizing my point of view when I realized you had done it already :-) While I think fighting for extensible and hackable hardware is good, I think a software license is the wrong way to go about it. Let's stick to the proven model of GPLv2: You can use my software if I get to use your improvements. Trying to impose restrictions on this model in order to fight a different battle against restricted hardware will only make the software less attractive and hurt us in the long run. I think it is not a coincidence that devices which can be updated with arbitrary firmware sells pretty good in the meantime. Who buys routers capable of running OpenWRT because of their original firmware? then let your wallet/politicians do the talking. i certainly do -- i avoid purchasing any music/games encumbered with DRM, or companies that employ such methods. but i'm above going around and forcing people to think the way i do with licenses. Exactly. Hardware manufacturers already seem to recognize that open hardware designs lead to better sales, and that has _nothing_ to do with GPLv3 (though it may or may not have something to do with the Defective By Design campaign.) These are only my personal opinions; I'm not speaking for Atmel as a whole. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] U-book and GPLv3? (fwd)
Wolfgang Denk wrote: In message 20090707135141.79798...@hskinnemoen-d830 you wrote: While I think fighting for extensible and hackable hardware is good, I think a software license is the wrong way to go about it. Let's stick to the proven model of GPLv2: You can use my software if I get to use your improvements. Trying to impose restrictions on this model in order The point is that GPLv2 results in situations where you cannot use and modify your own software any more because it is protected and any versions you build don't run. But this is a problem with the _hardware_, not the software. I think placing restrictions on the hardware design is way outside the scope of a software license. Even if the hardware is restricted this way, you can still take the software, modify it, and run it on a different, better piece of hardware. If you play your cards right, you might even come out with a healthy profit as people see that your product based on unrestricted hardware is simply _better_ (which is a term I think covers more free as well.) In my experience, the most popular AVR-based boards are the ones that not only allow the firmware to be replaced freely, but which actively encourage modification by making lots of signals available through expansion headers. This kind of hackability can never be enforced through any kind of software license. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] U-book and GPLv3? (fwd)
Wolfgang Denk wrote: I'm only talking about software (code and data) here. If I cannot change (or just rebuild) the (Free!) software any more because to actually run it I need some secret data (like a signature) then this is still a software problem. One that can be prevented by releasing the software under adequate licensing terms. The mechanism preventing reprogramming of the target device is not part of the software being licensed. So I just don't think it's reasonable for us to prevent the software from being used on such devices, even though I don't particularly like such restrictions either. This assuming GPLv3 actually does prevent such problems, of course. There seems to be a few loopholes in there, as others have pointed out (though I won't claim to fully understand it, which is another reason I'm not particularly fond of it.) Agreed. Hardware should be hackable, too :-) (which includes documentation where you don't have to sell your soul and sign NDAs that are plain evil). Absolutely. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH ... resent] Atmel LCD driver GUARDTIME fix
Jean-Christophe PLAGNIOL-VILLARD wrote: for at91 the GUARD_TIME is 1 and IIRC it's lcd specific You just contradicted yourself. The Guard time is the number of empty frames (with control signals enabled but no data) to wait before starting to send valid data to the display. Setting it slightly too high shouldn't make any difference. However, setting it slightly too low may cause strange failures every now and then. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH ... resent] Atmel LCD driver GUARDTIME fix
Jean-Christophe PLAGNIOL-VILLARD wrote: On 08:53 Tue 23 Jun , Haavard Skinnemoen wrote: Jean-Christophe PLAGNIOL-VILLARD wrote: for at91 the GUARD_TIME is 1 and IIRC it's lcd specific You just contradicted yourself. at91 boards Ok, I see. The Guard time is the number of empty frames (with control signals enabled but no data) to wait before starting to send valid data to the display. Setting it slightly too high shouldn't make any difference. However, setting it slightly too low may cause strange failures every now and then. for at91 boards it's 1 and I've never seen any problem, same in the kernel So I'll prefer to make it optionial and no hardcoded for all boards. Good point. How about if we turn it into a configuration symbol and default to 1 if it's unset? Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH ... resent] Atmel LCD driver GUARDTIME fix
Mark Jackson wrote: User-Agent: Thunderbird 2.0.0.21 (X11/20090409) (...) My patch has been mangled ... there's an extra space at the start of each unchanged patch line. Read about how to make Thunderbird behave here: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/email-clients.txt Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH ... resent] Atmel LCD driver GUARDTIME fix
On Mon, 22 Jun 2009 16:31:20 +0100 Mark Jackson mpfj-l...@mimc.co.uk wrote: Haavard Skinnemoen wrote: Mark Jackson wrote: User-Agent: Thunderbird 2.0.0.21 (X11/20090409) (...) My patch has been mangled ... there's an extra space at the start of each unchanged patch line. Read about how to make Thunderbird behave here: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/email-clients.txt But I've sent in patches before without any problems !! Weird. And as per my other mail on the topic, I posted to both the u-b...@denx and u-b...@avr32linux MLs, and only *one* of the incoming mails (u-b...@denx) was mangled. Hmm...the one I received through avr32linux.org looks mangled too; it has extra spaces before some of the lines. Any ideas ? My guess is that format=flowed is causing problems. From the patch: Content-Type: text/plain; charset=ISO-8859-1; format=flowed I wish people who implement e-mail software weren't completely insane. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] avr32/hsdramc: Move conditional compilation to Makefile
Jean-Christophe PLAGNIOL-VILLARD wrote: Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com Cc: Haavard Skinnemoen haavard.skinnem...@atmel.com Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 06/13] Add support for the AT91RM9200EK Board.
On Fri, 27 Mar 2009 23:30:19 +0100 Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com wrote: diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index 631b969..175d82a 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -1788,13 +1788,10 @@ static void flash_fixup_atmel(flash_info_t *info, struct cfi_qry *qry) /* AT49BV6416(T) list the erase regions in the wrong order. * However, the device ID is identical with the non-broken - * AT49BV642D since u-boot only reads the low byte (they - * differ in the high byte.) So leave out this fixup for now. + * AT49BV642D they differ in the high byte. */ -#if 0 if (info-device_id == 0xd6 || info-device_id == 0xd2) reverse_geometry = !reverse_geometry; -#endif Hmm...was this shortcoming actually fixed or does this change bugger up a large number of currently working boards? Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] macb: add set_hw_enetaddr support
Jean-Christophe PLAGNIOL-VILLARD wrote: Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com Cc: Ben Warren biggerbadder...@gmail.com Cc: Haavard Skinnemoen haavard.skinnem...@atmel.com Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 7/7] sf: stmicro: use common page timeout define
Mike Frysinger wrote: - /* Up to 2 seconds */ - ret = stmicro_wait_ready(flash, 2 * CONFIG_SYS_HZ); + ret = stmicro_wait_ready(flash, SPI_FLASH_PAGE_ERASE_TIMEOUT); 50 ms is an awful lot less than 2 seconds. Sure this is safe? Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 7/7] sf: stmicro: use common page timeout define
Mike Frysinger wrote: On Thursday 02 April 2009 07:20:13 Haavard Skinnemoen wrote: Mike Frysinger wrote: - /* Up to 2 seconds */ - ret = stmicro_wait_ready(flash, 2 * CONFIG_SYS_HZ); + ret = stmicro_wait_ready(flash, SPI_FLASH_PAGE_ERASE_TIMEOUT); 50 ms is an awful lot less than 2 seconds. Sure this is safe? the 2 sec mark was copied in all spi flash drivers based on the original port rather than referring to a datasheet, and it works on my Blackfin boards that have stmicro parts. Ok, that's good enough for me. personally i think the timeouts in the current spi flash common code is too low in general, so i'd propose we raise them. after all, the timeouts only matter in when something goes wrong, so they wouldnt be reached. and the low threshold seems like it makes the presumption of faster SPI bus speeds. i.e. how about raising the timeout values 50x or 100x ? I think that sounds like a good idea. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] sf: set common timeouts in seconds, not milliseconds
Mike Frysinger wrote: Since timeouts are only hit when there is a problem in the system, we don't want to prematurely timeout on a functioning setup. Thus having low timeouts (in milliseconds) doesn't gain us anything in the production case, but rather increases likely hood of causing problems where none otherwise exist. Signed-off-by: Mike Frysinger vap...@gentoo.org CC: Haavard Skinnemoen haavard.skinnem...@atmel.com Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] sf patches
Mike Frysinger wrote: since there doesnt seem to be a proper location for spi flash patches to accumulate, do you mind if i start up a branch to accumulate the current set ? No, please feel free to do that. i dont know how active you want to be with the sf subsystem ... or maybe you're like me; you dont care so long as it continues to work properly :). I'm like you in that respect :-) Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] sf: add driver for SST flashes
Mike Frysinger wrote: Signed-off-by: Mike Frysinger vap...@gentoo.org CC: Haavard Skinnemoen haavard.skinnem...@atmel.com Looks good to me. Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] sf: stmicro: drop redundant id read
Mike Frysinger wrote: The common SPI flash code reads the idcode and passes it down to the SPI flash driver, so there is no need to read it again ourselves. Signed-off-by: Mike Frysinger vap...@gentoo.org CC: Haavard Skinnemoen haavard.skinnem...@atmel.com CC: Jason McMullan mcmul...@netapp.com CC: TsiChung Liew tsi-chung.l...@freescale.com Looks reasonable, though it's probably best if someone more familiar with this particular driver Acks it as well. Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] SF: always read 5 bytes for the idcode
Mike Frysinger wrote: Some SPI flash drivers like to have extended id information available (like the spansion flash), so rather than making it re-issue the ID cmd to get at the last 2 bytes, have the common code read 5 bytes rather than just 3. This also matches the Linux behavior where it always reads 5 id bytes from all flashes. Signed-off-by: Mike Frysinger vap...@gentoo.org CC: Mingkai Hu mingkai...@freescale.com CC: Haavard Skinnemoen haavard.skinnem...@atmel.com Yes, that's much better. But perhaps you should pass the last two bytes to the debug() call a bit further down too? I guess it's not essential though, so Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] sf: drop DEBUG defines
Mike Frysinger wrote: Signed-off-by: Mike Frysinger vap...@gentoo.org CC: Haavard Skinnemoen haavard.skinnem...@atmel.com Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH for avr32/next] avr32: fix cacheflush.h location introducted by d8f2aa3298610b
Jean-Christophe PLAGNIOL-VILLARD wrote: Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com Applied, thanks Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [GIT PULL] AVR32 update
The following changes since commit ee1702d75a30d076139d1841383a1fa7220a0e11: Wolfgang Denk (1): Merge branch 'next' of ../next are available in the git repository at: git://git.denx.de/u-boot-avr32.git master Sorry about all the merges at the end...I missed a couple of merge windows, so I had to beat the tree back into shape from time to time. Gunnar Rangoy (1): AVR32: Make GPIO implmentation cpu dependent Haavard Skinnemoen (15): avr32: Update README avr32: data_bits should reflect the actual number of data bits avr32: refactor the portmux/gpio code avr32: Add gclk helper functions hammerhead: Use gclk helper functions avr32: Use board_postclk_init instead of gclk_init avr32: use board_early_init_r instead of board_init_info atstk1000: Convert to new-style makefile avr32: Add support for GPIO port mux Merge branch 'fixes' into cleanups Merge branch 'cleanups' into next Merge branch 'mimc200' into next Merge branch 'evk1100-prep' into next Merge branch 'mimc200' Merge branch 'evk1100-prep' Jean-Christophe PLAGNIOL-VILLARD (1): avr32: fix cacheflush.h location introducted by d8f2aa3298610b Mark Jackson (3): MIMC200 board now uses CONFIG_DISABLE_CONSOLE MIMC200: tidy GCLK init code Setup extra MIMC200 chip selects Olav Morken (3): AVR32: Make cacheflush cpu-dependent AVR32: Move addrspace.h to arch-directory, and move some functions from io.h to addrspace.h AVR32: Must add NOPs after disabling interrupts for AT32UC3A0512ES board/atmel/atngw100/atngw100.c| 18 +- board/atmel/atstk1000/Makefile |9 +- board/atmel/atstk1000/atstk1000.c | 15 +- board/atmel/atstk1000/flash.c |2 +- board/earthlcd/favr-32-ezkit/favr-32-ezkit.c | 13 +- board/earthlcd/favr-32-ezkit/flash.c |2 +- board/mimc/mimc200/mimc200.c | 117 --- board/miromico/hammerhead/hammerhead.c | 26 +-- cpu/at32ap/Makefile|3 +- cpu/at32ap/at32ap700x/Makefile |2 +- cpu/at32ap/at32ap700x/clk.c| 25 +++ cpu/at32ap/at32ap700x/gpio.c | 199 --- cpu/at32ap/at32ap700x/portmux.c| 204 +++ cpu/at32ap/cache.c |2 +- cpu/at32ap/cpu.c |3 - cpu/at32ap/pio.c | 116 --- cpu/at32ap/portmux-gpio.c | 107 ++ cpu/at32ap/portmux-pio.c | 92 + doc/README.AVR32 | 24 +-- doc/README.AVR32-port-muxing | 208 include/asm-avr32/addrspace.h | 46 - include/asm-avr32/arch-at32ap700x/addrspace.h | 84 .../asm-avr32/{ = arch-at32ap700x}/cacheflush.h |0 include/asm-avr32/arch-at32ap700x/clk.h| 101 +- include/asm-avr32/arch-at32ap700x/gpio-impl.h | 86 include/asm-avr32/arch-at32ap700x/gpio.h | 184 +- include/asm-avr32/arch-at32ap700x/portmux.h| 89 + include/asm-avr32/arch-common/portmux-gpio.h | 114 +++ include/asm-avr32/arch-common/portmux-pio.h| 138 + include/asm-avr32/dma-mapping.h|2 +- include/asm-avr32/initcalls.h |1 - include/asm-avr32/io.h | 39 + include/asm-avr32/sdram.h |4 +- include/configs/atngw100.h |2 +- include/configs/atstk1002.h|2 +- include/configs/atstk1003.h|2 +- include/configs/atstk1004.h|2 +- include/configs/atstk1006.h|2 +- include/configs/favr-32-ezkit.h|2 +- include/configs/hammerhead.h |2 +- include/configs/mimc200.h |8 +- lib_avr32/board.c | 14 ++- lib_avr32/bootm.c |2 +- lib_avr32/interrupts.c |7 + 44 files changed, 1388 insertions(+), 732 deletions(-) delete mode 100644 cpu/at32ap/at32ap700x/gpio.c create mode 100644 cpu/at32ap/at32ap700x/portmux.c delete mode 100644 cpu/at32ap/pio.c create mode 100644 cpu/at32ap/portmux-gpio.c create mode 100644 cpu/at32ap/portmux-pio.c create mode 100644 doc/README.AVR32-port-muxing delete mode 100644 include/asm-avr32/addrspace.h create mode 100644 include/asm-avr32/arch-at32ap700x/addrspace.h rename include/asm-avr32/{ = arch-at32ap700x}/cacheflush.h (100
Re: [U-Boot] [PATCH] Network AT91 AVR32: generic way of addressing USRIO register layout
Jean-Christophe PLAGNIOL-VILLARD wrote: On 09:58 Mon 23 Mar , Nicolas Ferre wrote: The MACB IP used by AVR32 AT91 have two different layout for USRIO register. We have to differentiate this in the driver code. No more cpu specific #ifdefs in driver: we manage a configuration variable. Signed-off-by: Nicolas Ferre nicolas.fe...@atmel.com Ack-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com Haavard is ok for you too? Sure. Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] CUSTODIANS: Urgent boarding call for flight 2009.03
Jean-Christophe PLAGNIOL-VILLARD wrote: Latest upstream master builds just fine here...though the last release may be broken because I pushed the fix too late. Ah, I see the avr32 master branch is still broken. I've fixed it by pulling from mainline. I've try the next branch and it's failled too Right...looks like one of the UC3 patches got mangled somehow. I'll have a look at that. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] CUSTODIANS: Urgent boarding call for flight 2009.03
Jean-Christophe PLAGNIOL-VILLARD wrote: make[1]: In file included from clk.c:24: /private/u-boot-arm/include/asm/io.h:129: error: conflicting types for 'virt_to_phys' /private/u-boot-arm/include/asm/io.h:80: error: previous definition of 'virt_to_phys' was here Haavard could you take a look? Latest upstream master builds just fine here...though the last release may be broken because I pushed the fix too late. Thanks a lot for keeping an eye on things. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1 V3] cmd_bdinfo: move implementation to arch instead of common
Jean-Christophe PLAGNIOL-VILLARD wrote: introduce two new weak functions board_bdinfo and cpu_bdinfo to allow board and cpu to print more information Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com Cc: Haavard Skinnemoen haavard.skinnem...@atmel.com Cc: Mike Frysinger vap...@gentoo.org --- rebase for-next Precedent version Ack-by: Mike Frysinger vap...@gentoo.org Ack-by: Haavard Skinnemoen haavard.skinnem...@atmel.com Aren't the Acked-by lines supposed to go above the '---' line? Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Status open patches: AVR32
Wolfgang Denk wrote: I have the following patches still marked as open in my list. Could you please have a look... Sure. Would those patches be acceptable now or should I hold them off until the next merge window? Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/9] AVR32: Make cacheflush cpu-dependent
Gunnar Rangoy wrote: From: Olav Morken olav...@gmail.com The AT32UC3A series of processors doesn't contain any cache, and issuing cache control instructions on those will cause an exception. This commit makes cacheflush.h arch-dependent in preparation for the AT32UC3A-support. Signed-off-by: Gunnar Rangoy gun...@rangoy.com Signed-off-by: Paul Driveklepp pauldrivekl...@gmail.com Signed-off-by: Olav Morken olav...@gmail.com Applied to evk1100-prep and merged into next. I'll send it upstream as soon as it's fine with Wolfgang. Btw, I had to rebuild my next branch since it's become a bit stale. Since all the non-merge commit IDs are the same, I hope it won't cause any problems -- please let me know if you see any weird merge issues. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 3/9] AVR32: Move addrspace.h to arch-directory, and move some functions from io.h to addrspace.h
Gunnar Rangoy wrote: From: Olav Morken olav...@gmail.com The AVR32A architecture (which AT32UC3A-series is based on) has a different memory layout than the AVR32B-architecture. This patch moves addrspace.h to an arch-dependent directory in preparation for AT32UC3A-support. It also moves some address-space manipulation functions from io.h to addrspace.h. Signed-off-by: Gunnar Rangoy gun...@rangoy.com Signed-off-by: Paul Driveklepp pauldrivekl...@gmail.com Signed-off-by: Olav Morken olav...@gmail.com Applied to evk1100-prep, thanks. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 4/9] AVR32: Make GPIO implmentation cpu dependent
Gunnar Rangoy wrote: There are some differences in the implementation of GPIO in the at32uc chip compared to the ap700x series. Signed-off-by: Gunnar Rangoy gun...@rangoy.com Signed-off-by: Paul Driveklepp pauldrivekl...@gmail.com Signed-off-by: Olav Morken olav...@gmail.com Applied to evk1100-prep, thanks. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 7/9] AVR32: Must add NOPs after disabling interrupts for AT32UC3A0512ES
Gunnar Rangoy wrote: From: Olav Morken olav...@gmail.com The AT32UC3A0512ES chip has a bug when disabling interrupts. As a workaround, two NOPs can be inserted. Signed-off-by: Gunnar Rangoy gun...@rangoy.com Signed-off-by: Paul Driveklepp pauldrivekl...@gmail.com Signed-off-by: Olav Morken olav...@gmail.com Applied to evk1100-prep, thanks. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1 v2] Setup extra MIMC200 chip selects
Mark Jackson wrote: Added code to setup the extra Flash and FRAM chip selects as used on the MIMC200 board. V2 moves the init code from the common cpu.c file into the board specific setup file. Signed-off-by: Mark Jackson m...@mimc.co.uk Applied to mimc200 after fixing up some whitespace damage, thanks. --- board/mimc/mimc200/mimc200.c | 14 ++ 1 files changed, 14 insertions(+), 0 deletions(-) diff --git a/board/mimc/mimc200/mimc200.c b/board/mimc/mimc200/mimc200.c index 8516dcb..423238b 100644 --- a/board/mimc/mimc200/mimc200.c +++ b/board/mimc/mimc200/mimc200.c @@ -29,6 +29,8 @@ #include asm/arch/hmatrix.h #include lcd.h +#include ../../../cpu/at32ap/hsmc3.h it would probably be better if this file was moved somewhere under include/asm-avr32, however. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3] Add 16bpp BMP support
Mark Jackson wrote: Haavard Skinnemoen wrote: Mark Jackson wrote: We do NOT want to do everything that is possible, but only what is reasonable. Exactly ... otherwise where do you stop ? JPG, GIF, TIFF, PNG, etc ? We're *only* meant to be showing a simply boot up image (not view lots of different sized photos or movies !!), in a very controlled environment (i.e. no user options ... just what the designers want / require). Why not do it even simpler? Drop BMP and generate an image matching the native format of the LCD controller at compile time :-) Not sure if you're serious, but that'd reduce some of the functionality I was expecting to use. Well, it was just a thought that struck me, so I'm not going to claim I considered all the pros and cons. My splashimage is stored in a particular, separate flash partition, and I'm intending to allow end-users to change the boot logo (via a userspace app ?) to customise / personalise the unit as they require (e.g. their own company logo). You can still do this if the userspace app knows how to generate an image in the correct format -- I'm not arguing against storing the image in a separate flash partition or anything like that. I just think it might be possible to reduce the run-time size and complexity of u-boot by being more strict about the image format. You could also add support for PNG, JPEG and any format you want to the userspace app -- this will probably be much easier than adding similar support to u-boot itself. Hard-coding the image would render this impossible. Of course. But hard-coding the image _format_ isn't the same thing. In a way, we're already using a hard-coded image format, but it's one that is easy to generate for the host, not one that's easy to display by the target. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3] Add 16bpp BMP support
Mark Jackson wrote: We do NOT want to do everything that is possible, but only what is reasonable. Exactly ... otherwise where do you stop ? JPG, GIF, TIFF, PNG, etc ? We're *only* meant to be showing a simply boot up image (not view lots of different sized photos or movies !!), in a very controlled environment (i.e. no user options ... just what the designers want / require). Why not do it even simpler? Drop BMP and generate an image matching the native format of the LCD controller at compile time :-) Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 5/9] AVR32: macb - Disable 100mbps if clock is slow
Ben Warren wrote: Jean-Christophe PLAGNIOL-VILLARD wrote: make sense so I'll put a 10Mpbs phy personnaly instead or a 10/100 that can be put in a 10 mode instead to avoid to manage it in the code I haven't shopped for PHYs in a while, but imagine it's probably hard to find one that's 10Mb only, and if so it's probably more expensive than 10/100 Especially when the board really is _supposed_ to do 100Mbps just fine. No need to disable autonegotiation -- you still want to select between half an full duplex, for example. But you'll need to limit the available options to the ones that actually work. I do not mean the autoneg but to specify to the phy, if possible, to never accept the 100Mps instead of do it in the code That's basically why the advertise register is r/w, so you can limit autonegotiation through software and don't have to allocate pins for strapping. Yeah, I thought the advertise register was the correct way of informing the phy about what modes you support... I suppose we could move the initialization of the advertise register to board code, but that's going to be a lot uglier since the board code would have to access the MACB hardware to drive the MDIO pins. Perhaps some boards don't need any initialization at all, but then again, isn't it safer to unconditionally write the correct value? And when we're going to write the register _anyway_, we might as well allow the board code to influence the value we're going to write. The more I think of this, the more I'm convinced we should have allowed that from the beginning. The current driver is making assumptions it shouldn't be making. That said, I kind of like Ben's suggestion -- it's a more general solution to all sorts of board/phy/chip/whatever limitations. As for a better name, how about CONFIG_MACB_ADVERTISE? why not I like it too. One of the common checkbox items, though: do any Atmel chips have more than one MACB, in which case this should be CONFIG_MACBx_ADVERTISE or something like that? Yes, AT32AP7000 has two MACBs, so that's probably a good idea. Better yet, make it CONFIG_MACB_ADVERTISE(macb) so that the board can distinguish between various instances if it has to, and the driver doesn't have to do a long sequence of #ifdefs to find the correct value. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 5/9] AVR32: macb - Disable 100mbps if clock is slow
Jean-Christophe PLAGNIOL-VILLARD wrote: On the EVK1100 board, the CPU (UC3A0512) is connected to the PHY via an RMII bus. This requires the CPU clock to be at least 50 MHz. Unfortunately, the chip on current EVK1100 boards may be unable to run at more than 50 MHz, and with the oscillator on the board, the closest frequency we can generate is 48 MHz. IMHO It's a HW design error to not use the MII Some people want to use the extra pins for other things... Unfortunately, there are quite a few boards with early engineering samples around, and they have various issues. The chips that are in production are capable of running fast enough to support RMII. This patch makes it possible to limit the macb to 10 MBit for this case. We are open for suggestions for other solutions. I guest you may need to disable the phy auto config mode and force him to be see as a 10Mbps phy evenif it's a 10/100 No need to disable autonegotiation -- you still want to select between half an full duplex, for example. But you'll need to limit the available options to the ones that actually work. That said, I kind of like Ben's suggestion -- it's a more general solution to all sorts of board/phy/chip/whatever limitations. As for a better name, how about CONFIG_MACB_ADVERTISE? Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] AVR32 and AT91 common drivers Maintaining
Jean-Christophe PLAGNIOL-VILLARD wrote: On 22:36 Sat 17 Jan , Jean-Christophe PLAGNIOL-VILLARD wrote: Hi all, AVR32 AT91 share a lot of IP as networking for example (macb) so it will the same in the u-boot drivers In consequence these common drivers will need ack from Haavard and I. Hi Haavard, I'd like to known how do you wish to work for this? Sounds good to me. I'd appreciate a Cc when something needs my attention, as I tend to fall behind on the mailing lists from time to time... Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [GIT PULL] avr32 fix for v2009.01
Hi Wolfgang, Please pull git://git.denx.de/u-boot-avr32.git master to receive the following build fix for avr32 (all boards are affected). Haavard Skinnemoen (2): avr32: Remove second definition of virt_to_phys() Merge branch 'fixes' include/asm-avr32/io.h |9 ++--- 1 files changed, 2 insertions(+), 7 deletions(-) diff --git a/include/asm-avr32/io.h b/include/asm-avr32/io.h index d22cd35..50967ac 100644 --- a/include/asm-avr32/io.h +++ b/include/asm-avr32/io.h @@ -76,12 +76,12 @@ extern void __readwrite_bug(const char *fn); #include asm/addrspace.h /* virt_to_phys will only work when address is in P1 or P2 */ -static __inline__ unsigned long virt_to_phys(volatile void *address) +static inline phys_addr_t virt_to_phys(volatile void *address) { return PHYSADDR(address); } -static __inline__ void * phys_to_virt(unsigned long address) +static inline void *phys_to_virt(phys_addr_t address) { return (void *)P1SEGADDR(address); } @@ -125,9 +125,4 @@ static inline void unmap_physmem(void *vaddr, unsigned long len) } -static inline phys_addr_t virt_to_phys(void * vaddr) -{ - return (phys_addr_t)(vaddr); -} - #endif /* __ASM_AVR32_IO_H */ ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 07/11] flash/cfi_flash: Use virtual sector start address, not phys
Kumar Gala wrote: On Dec 3, 2008, at 11:04 PM, Becky Bruce wrote: include/flash.h was commented to say that the address in flash_info-start was a physical address. However, from u-boot's point of view, and looking at most flash code, it makes more sense for this to be a virtual address. So I corrected the comment to indicate that this was a virtual address. The only flash driver that was actually treating the address as physical was the mtd/cfi_flash driver. However, this code was using it inconsistently as it actually directly dereferenced the start element, while it used map_physmem to get a virtual address in other places. I changed this driver so that the code which initializes the info-start field calls map_physmem to get a virtual address, eliminating the need for further map_physmem calls. The code is now consistent. The *only* place a physical address should be used is when defining the flash banks list that is used to initialize the flash_info struct. I have fixed the one platform that was impacted by this change (MPC8641D). Signed-off-by: Becky Bruce bec...@kernel.crashing.org --- drivers/mtd/cfi_flash.c | 53 + +-- include/configs/MPC8641HPCN.h |2 +- include/flash.h |2 +- 3 files changed, 26 insertions(+), 31 deletions(-) Stefan, Have you reviewed this. I'm not sure if remvoing the map_physmem() is the right answer because I'm assuming Haavard added them for a reason (AVR32?). Should we instead change info-start to be a phys_addr_t? There was definitely a reason: On AVR32, the caches are enabled by default, and I prefer that they stay that way since it makes everything faster. But when dealing with the flash, we must bypass the caches by using a different virtual address which maps to the same physical memory. From what I can tell from a quick scan, this patch doesn't seem to change that; all it does is push the map_physmem() call a bit up the stack so it gets called less often. That seems like a good thing to me. However, what I don't understand is if the 'start' array is really supposed to hold virtual addresses, why isn't it an array of pointers? Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 07/11] flash/cfi_flash: Use virtual sector start address, not phys
Kumar Gala wrote: As I look at this we really need to understand what Haavard was trying to get with: commit 12d30aa79779c2aa7a998bbae4c075f822a53004 Author: Haavard Skinnemoen hskinnem...@atmel.com Date: Thu Dec 13 12:56:34 2007 +0100 cfi_flash: Use map_physmem() and unmap_physmem() Use map_physmem() and unmap_physmem() to convert from physical to virtual addresses. This gives the arch a chance to provide an uncached mapping for flash accesses. Signed-off-by: Haavard Skinnemoen hskinnem...@atmel.com Since this only addresses the usage in cfi_flash.c but not in other users of flash_info (like commands) Yeah, there are probably tons of places that really should use map/unmap_physmem() but doesn't. I've been playing with the thought of enabling the Paging bit on AVR32 to support uncached mappings of all physical memory...this is likely to make all of those places very visible. My goal was to solve a very specific problem (make the cfi_flash driver work on AVR32 boards) by introducing a generic primitive which may be used to solve other similar problems in the future. I also think it conceptually makes sense to treat physical and virtual addresses as different entities, as they are fundamentally not the same thing on many architectures. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [ANNOUNCE] DATAFLASH DRIVER
Ulf Samuelsson wrote: ons 2009-01-07 klockan 21:56 +0100 skrev Jean-Christophe PLAGNIOL-VILLARD: Hi, The AT91 arch use a at45 driver design to be show as a parallel flash but it's a spi flash. Haavard add a new spi flash framework which support the dataflash so the current driver is mark as *depracated* and plan to be remove There is very limited support for the flash in the new spi driver. so most things you would like to do with a flash is not supported Yes, it is a bit minimal right now. But it should be fairly easy to extend once we know what the requirements are. Now, the current driver does work fairly well for my purposes, so I won't promise anything about when I can get around to extend the driver. But it shouldn't be too difficult to do for someone who has an itch to scratch. I would like to have capabilities to 1) print contents of the dataflash We should add a sf dump command similar to the existing nand dump command. 2) compare contents of the dataflash with SDRAM. This is one of the controversial points, I believe. 3) Automatically add a CRC when the dataflash is written based on an environment variable crc-check What kind of CRC and where should it be placed? 4) Verify that the CRC is correct. Same issue. 5) high granularity read/writes The dataflash can easily support byte write. Supported by the current driver, I believe. Erase still operates on full pages, however, and I don't think there's anything to do about that. 6) Support partitions for bootstrap environemnt u-boot kernel filesystem in the boot dataflash. additional dataflash should have other partitions. We should add a partitioning layer on top of the current interface. And even better solution would be to introduce a common flash interface for NOR, NAND, SPI, etc. flash and add a partitioning layer on top of that. 7) protection of partitions. As well as protection of raw sectors, perhaps? A partitioning layer could use such functionality to implement protection of partitions. I have my own patches for the memory commands to enable most of this but adding that to the cmd_mem driver was not accepted. Yes, as you probably know, I for one think memory mapping of serial devices is a bad idea. At that time it was promised that the new driver will not limit the functionality, and would only remove the use of direct addressing to the dataflash. There's nothing in the driver or its architecture which limits any functionality. If you need additional features, feel free to implement them, or convince someone else to do it. The whole storage concept is really not working when storage becomes larger than the SDRAM. How do you download an 128 MB image over the network to a machine with 64 MB SDRAM. - Major pain... You need to be able to TFTP to flash directly if you want to have an easy user interface. Or start an operating system which can do all of this much faster and with support for more protocols. That is likely to require another way of handling storage. and parameter parsing so you can do tftp mmc0:kernel linux-2.6.28 or tftp df3:fs rootfs.arm.ext2 Now, I kind of like that syntax, but I seem to recall it got shot down at some point too. The tftp can then write incoming packets to a caching driver and will delay fetching new packets when waiting for flash writes to complete. Whn your storage is handled by a DMA controller, you should be able to implement this nicely. Adding DMA support to the SPI driver shouldn't be a big deal, but I don't see what it has to do with the user interface. Also, caching sounds like something which comes dangerously close to crossing the line between boot loader and operating system. I don't think it fits well into the current u-boot architecture. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] spansion spi flash driver ?
Mike Frysinger wrote: in your original drivers/mtd/spi/spi_flash.c commit, you had this: #ifdef CONFIG_SPI_FLASH_SPANSION case 0x01: flash = spi_flash_probe_spansion(spi, idcode); break; #endif does that mean you have a spansion driver sitting around but it just wasnt merged ? if i dont have to write it from scratch, that'd be great :). No, I'm afraid I don't have a spansion driver. I don't know why I put it there...probably just to see what it would look like with more supported flash types, and then I forgot to remove it before submitting. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd_sf: rename speed to hz
Mike Frysinger wrote: The term hz is used everywhere else when talking about the frequency of the SPI bus, so have the sf command use it as well to stay consistent. It even presents itself as hz when showing user help. Signed-off-by: Mike Frysinger vap...@gentoo.org I like it. hz makes it perfectly clear what the value means. Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] spi flash: fix crash due to spi flash miscommunication
Mike Frysinger wrote: From: Brad Bozarth bfli...@yumbrad.com Higher spi flash layers expect to be given back a pointer that was malloced so that it can free the result, but the lower layers return a pointer that is in the middle of the malloced memory. Reorder the members of the lower spi structures so that things work out. Signed-off-by: Brad Bozarth bfli...@yumbrad.com Signed-off-by: Mike Frysinger vap...@gentoo.org CC: Haavard Skinnemoen haavard.skinnem...@atmel.com Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 1/4] Introduce virt_to_phys()
Kumar Gala wrote: /* virt_to_phys will only work when address is in P1 or P2 */ -static __inline__ unsigned long virt_to_phys(volatile void *address) +static inline phys_addr_t virt_to_phys(volatile void *address) { Is the volatile really needed? The problem is that the 'packet' parameter to struct eth_device.send() is volatile, and it propagates into this function. So if I remove the volatile, I have to either add ugly casts elsewhere or change the eth_device API... I have no idea why the network subsystem feels the need to enforce the use of volatile in all drivers, though... Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 1/4] Introduce virt_to_phys()
Kumar Gala wrote: Lets go w/volatile for now and worry about this post v2009.01 Sounds good to me. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 1/4] Introduce virt_to_phys()
Kumar Gala wrote: diff --git a/include/asm-avr32/io.h b/include/asm-avr32/io.h index 06e52b1..d22cd35 100644 --- a/include/asm-avr32/io.h +++ b/include/asm-avr32/io.h @@ -125,4 +125,9 @@ static inline void unmap_physmem(void *vaddr, unsigned long len) } +static inline phys_addr_t virt_to_phys(void * vaddr) +{ + return (phys_addr_t)(vaddr); +} + avr32 has already got one of those, so this breaks the build. I'm going to apply the patch below to my 'fixes' branch if it looks ok to you. Haavard From 92c78a3bbcb2ce508b4bf1c4a1e0940406a024bb Mon Sep 17 00:00:00 2001 From: Haavard Skinnemoen haavard.skinnem...@atmel.com Date: Wed, 17 Dec 2008 16:43:18 +0100 Subject: [PATCH] avr32: Remove second definition of virt_to_phys() The second definition introduced by 65e43a1063 conflicts with the existing one. Also, convert the existing definition to use phys_addr_t. The volatile qualifier is still needed due to brain damage elsewhere. Signed-off-by: Haavard Skinnemoen haavard.skinnem...@atmel.com --- include/asm-avr32/io.h |9 ++--- 1 files changed, 2 insertions(+), 7 deletions(-) diff --git a/include/asm-avr32/io.h b/include/asm-avr32/io.h index d22cd35..50967ac 100644 --- a/include/asm-avr32/io.h +++ b/include/asm-avr32/io.h @@ -76,12 +76,12 @@ extern void __readwrite_bug(const char *fn); #include asm/addrspace.h /* virt_to_phys will only work when address is in P1 or P2 */ -static __inline__ unsigned long virt_to_phys(volatile void *address) +static inline phys_addr_t virt_to_phys(volatile void *address) { return PHYSADDR(address); } -static __inline__ void * phys_to_virt(unsigned long address) +static inline void *phys_to_virt(phys_addr_t address) { return (void *)P1SEGADDR(address); } @@ -125,9 +125,4 @@ static inline void unmap_physmem(void *vaddr, unsigned long len) } -static inline phys_addr_t virt_to_phys(void * vaddr) -{ - return (phys_addr_t)(vaddr); -} - #endif /* __ASM_AVR32_IO_H */ -- 1.5.6.3 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] env_sf: support embedded environments
Mike Frysinger wrote: If both CONFIG_ENV_SECT_SIZE and CONFIG_ENV_SIZE are defined, and the sect size is larger than the env size, then it means the env is embedded in a block. So we have to save/restore the part of the sector which is not the environment. Previously, saving the environment in SPI flash in this setup would probably brick the board as the rest of the sector tends to contain actual U-Boot data/code. Signed-off-by: Mike Frysinger [EMAIL PROTECTED] Makes sense to me. Assuming this is how other types of flash work, Acked-by: Haavard Skinnemoen [EMAIL PROTECTED] ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 07/10] AVR32: Disable relocation of command table when on AT32UC3A for now
Wolfgang Denk [EMAIL PROTECTED] wrote: In message [EMAIL PROTECTED] you wrote: Due to a bug with the SDRAM-controller, running code from the SDRAM is impossible. This patch disables relocation of the command table on those chips. You are aware that this is dangerous, as it will probably prevent you rom using U-Boot to update itself in flash? It's only a temporary thing. A patch has already been posted to re-enable relocation when running from SRAM. Eventually, when chips without this erratum have been generally available for a while, relocation can be enabled unconditionally. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 09/10] AVR32: CPU support for AT32UC3A0xxx CPUs
Wolfgang Denk [EMAIL PROTECTED] wrote: Dear Olav Morken, In message [EMAIL PROTECTED] you wrote: This patch adds support for the AT32UC3A0xxx chips. Signed-off-by: Gunnar Rangoy [EMAIL PROTECTED] Signed-off-by: Paul Driveklepp [EMAIL PROTECTED] Signed-off-by: Olav Morken [EMAIL PROTECTED] Coding style violations: C++ comments, indentation not by TAB, too long lines, incorrect multiline comment style. ... +static inline unsigned long get_hsb_clk_rate(void) +{ + //TODO HSB is always the same as cpu-rate ---^^ + return MAIN_CLK_RATE CFG_CLKDIV_CPU; +} +static inline unsigned long get_pba_clk_rate(void) +{ + return MAIN_CLK_RATE CFG_CLKDIV_PBA; +} +static inline unsigned long get_pbb_clk_rate(void) +{ + return MAIN_CLK_RATE CFG_CLKDIV_PBB; +} + +/* Accessors for specific devices. More will be added as needed. */ +static inline unsigned long get_sdram_clk_rate(void) +{ + return get_hsb_clk_rate(); +} +#ifdef AT32UC3A0xxx_CHIP_HAS_USART +static inline unsigned long get_usart_clk_rate(unsigned int dev_id) +{ + return get_pba_clk_rate(); +} +#endif +#ifdef AT32UC3A0xxx_CHIP_HAS_MACB +static inline unsigned long get_macb_pclk_rate(unsigned int dev_id) +{ + return get_pbb_clk_rate(); +} +static inline unsigned long get_macb_hclk_rate(unsigned int dev_id) +{ + return get_hsb_clk_rate(); +} +#endif +#ifdef AT32UC3A0xxx_CHIP_HAS_SPI +static inline unsigned long get_spi_clk_rate(unsigned int dev_id) +{ + return get_pba_clk_rate(); +} +#endif Would it make sense to provide weak functions the get defined accordingly? A function which only calls another function looks stupid to me. This matches other AVR32 chips. I'm not sure how you intend to use weak functions here...the whole point of these things is to make drivers unaware of which bus a certain peripheral is connected to. I suppose the same thing could be done using aliases, but then you can't do it inline, which in turn means that the constant calculations won't be constant anymore, and the code gets larger and slower as a result. +#ifdef AT32UC3A0xxx_CHIP_HAS_USART +static inline void portmux_enable_usart0(unsigned long drive_strength) +{ + portmux_select_peripheral(PORTMUX_PORT_A, (1 0) | (1 1), + PORTMUX_FUNC_A, 0); +} + +static inline void portmux_enable_usart1(unsigned long drive_strength) +{ + portmux_select_peripheral(PORTMUX_PORT_A, (1 5) | (1 6), + PORTMUX_FUNC_A, 0); +} + +static inline void portmux_enable_usart2(unsigned long drive_strength) +{ + portmux_select_peripheral(PORTMUX_PORT_B, (1 29) | (1 30), + PORTMUX_FUNC_A, 0); +} + +static inline void portmux_enable_usart3(unsigned long drive_strength) +{ + portmux_select_peripheral(PORTMUX_PORT_B, (1 10) | (1 11), + PORTMUX_FUNC_B, 0); +} +#endif I don't like this either. Why not? I think it's pretty elegant -- the board code only needs to specify which USARTs to enable, it doesn't need to know which pins and functions it corresponds to. That said, it might be a better idea to make a single function with a switch () block. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] flash: Export flash_sector_size() function.
Piotr Ziecik [EMAIL PROTECTED] wrote: Export flash_sector_size() function from drivers/mtd/cfi_flash.c. Signed-off-by: Piotr Ziecik [EMAIL PROTECTED] Why? Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] cfi_flash: Make all flash access functions weak
Stefan Roese [EMAIL PROTECTED] wrote: This patch defines all flash access functions as weak so that they can be overridden by board specific versions. This will be used by the upcoming VCTH board support where the NOR FLASH unfortunately can't be accessed memory-mapped. Special accessor functions are needed here. To enable this weak functions you need to define CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS in your board config header. Otherwise the old default functions will be used resulting in smaller code. Signed-off-by: Stefan Roese [EMAIL PROTECTED] Acked-by: Haavard Skinnemoen [EMAIL PROTECTED] ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] weak functions versus conditional compile
Mike Frysinger [EMAIL PROTECTED] wrote: On Monday 17 November 2008 16:17:54 Graeme Russ wrote: Should I declare these functions as weak in the core i386 code and use a config #define to override or should I seperate the functions out into seperate source files and use conditional compilation? i wonder if weak functions that are always satisfied and used unconditionally result in larger code, or if this is only a problem for people whose linkers lack lazy relaxation support ? It's probably a problem with linkers that don't support --gc-sections, which is different from relaxation. Also, you definitely need to compile with -ffunction-sections, or the weak functions might end up in the same section as something linked unconditionally, which makes it impossible for the linker to remove them. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 09/10] AVR32: CPU support for AT32UC3A0xxx CPUs
Wolfgang Denk [EMAIL PROTECTED] wrote: +static inline unsigned long get_hsb_clk_rate(void) +{ + //TODO HSB is always the same as cpu-rate ---^^ + return MAIN_CLK_RATE CFG_CLKDIV_CPU; Simply removing this comment should be fine. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 00/10] AVR32: RFC/preview - support for ATEVK1100 evaluation board
Olav Morken [EMAIL PROTECTED] wrote: This is a patch series which adds support for the ATEVK1100 evaluation board[1], and the AT32UC3A0xxx[2] microcontrollers used on that board. The patch series is based on avr32/next. I've applied this and the other series you posted to the 'evk1100' branch in git://git.denx.de/u-boot-avr32.git evk1100 Please submit incremental patches addressing the comments you got during review. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 00/10] AVR32: RFC/preview - support for ATEVK1100 evaluation board
Wolfgang Denk [EMAIL PROTECTED] wrote: Dear Haavard Skinnemoen, In message [EMAIL PROTECTED] you wrote: I've applied this and the other series you posted to the 'evk1100' branch in git://git.denx.de/u-boot-avr32.git evk1100 Please submit incremental patches addressing the comments you got during review. Umm... but please do NOT merge this branch into your AVR32 repository for me to poll from. In mainline, I want to see consolidated patches only. That's the plan. I just thought I'd give them somewhere to live until they're ready. So a big fat warning to everyone: the evk1100 branch _will_ get rebased. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] cfi_flash: Make all flash access functions weak
Stefan Roese [EMAIL PROTECTED] wrote: Old version without weak aliases: textdata bss dec hex filename 280964 20232 50788 351984 55ef0 ./u-boot New version with weak aliases: textdata bss dec hex filename 280520 20232 50788 351540 55d34 ./u-boot So the difference is 444 bytes (with gcc 4.2.2). I have to admit that this is more than I thought. If necessary I could make this weak change conditionally of course. I just didn't want to pollute to the source with more #ifdef's. Looks like it's 444 bytes _smaller_ with weak functions...that's a bit surprising. From you comment, it sounds like it's really 444 bytes larger. Did you mix up the numbers? If it's really 444 bytes larger, I think that's a bit much for something as simple as memory accessors. Ideally, they should boil down to a single instruction each. Perhaps you could allow overriding them in the board header by doing something like #ifndef flash_write8 # #define flash_write8(value, addr) __raw_writeb(value, addr) ... in cfi_flash.c? Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] cfi_flash: Make all flash access functions weak
Stefan Roese [EMAIL PROTECTED] wrote: I could do it this way, sure. But how about this version: static void __flash_write8(u8 value, void *addr) { __raw_writeb(value, addr); } ... #ifdef CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS void flash_write8(u8 value, void *addr)__attribute__((weak, alias(__flash_write8))); ... #else #define flash_read8 __flash_read8 ... #endif We would still have the original accessor functions this way. And the resulting source code looks a little better to me (less #ifdef's). Hmm...1 #ifdef vs. 1 #ifdef...I'd say that's pretty much the same ;-) But sure, your way works too. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] avr32/bootm: remove unused variable 'ret'
Jean-Christophe PLAGNIOL-VILLARD [EMAIL PROTECTED] wrote: Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD [EMAIL PROTECTED] Acked-by: Haavard Skinnemoen [EMAIL PROTECTED] I'm a bit out of sync at the moment. Wolfgang, can you apply it directly? diff --git a/lib_avr32/bootm.c b/lib_avr32/bootm.c index 556e3ea..03ab8d1 100644 --- a/lib_avr32/bootm.c +++ b/lib_avr32/bootm.c @@ -176,7 +176,6 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) void(*theKernel)(int magic, void *tagtable); struct tag *params, *params_start; char*commandline = getenv(bootargs); - int ret; if ((flag != 0) (flag != BOOTM_STATE_OS_GO)) return 1; ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd_bdinfo: move implementation to arch instead of common
Jean-Christophe PLAGNIOL-VILLARD [EMAIL PROTECTED] wrote: This patch trades off the removal of most of the #ifdef ugly for a lot of duplication. Which is the lesser of two evils? Only 4 archs share actually the same code avr32, i386, mips and sh which actually I've plan to modify for sh soon And the avr32 code is mostly wrong. So it looks like the current situation is #ifdef mess _and_ duplication. This patch definitely improves things. The reason why the avr32 part is wrong is that I simply didn't notice that I had to do anything in there until quite recently. So IMO making this thing arch-specific will make it easier to get it right for new architectures (since you'll get a nice and friendly link error reminding you that you missed it.) Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/6] AT572D940HF-EB Support v2 (cpu folder)
Antonio R. Costa [EMAIL PROTECTED] wrote: As we agreed with Haavard I'll try to make a general SDHC patch for all Atmel platform soon. Note that there's a generic MMC framework in the works: http://www.nabble.com/-U-Boot---PATCH-00-10--Add-MMC-Framework-td20256840.html which we should probably consider using once it is finished. Of course, adding support for AT91 and SDHC to the atmel-mci driver is a good thing in itself, so it can happen earlier, but depending on the timing of things, there may be conflicts. So I highly recommend that you keep the list informed about what you're doing at that front, so that we can avoid stepping on each others' toes. If you have the time, I'm sure Andy will appreciate your comments on his patches too. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 04/10] Eliminated arch-specific mmc header requirement
Andy Fleming [EMAIL PROTECTED] wrote: diff --git a/include/asm-avr32/arch-at32ap700x/mmc.h b/include/asm-avr32/arch-at32ap700x/mmc.h deleted file mode 100644 index 9caba91..000 --- a/include/asm-avr32/arch-at32ap700x/mmc.h +++ /dev/null -struct mmc_cid { -struct mmc_csd -#define R1_ILLEGAL_COMMAND (1 22) -#define R1_APP_CMD (1 5) Where can those definitions be found now? The driver still needs them... Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 06/10] Add MMC Framework
Andy Fleming [EMAIL PROTECTED] wrote: Here's a new framework (based roughly off the linux one) for managing MMC controllers. It handles all of the standard SD/MMC transactions, leaving the host drivers to implement only what is necessary to deal with their specific hardware. This also hooks the infrastructure into the PowerPC board code (similar to how the ethernet infrastructure now hooks in) Some of this code was contributed by Dave Liu [EMAIL PROTECTED] Signed-off-by: Andy Fleming [EMAIL PROTECTED] Nice. I have a few comments, please see below. --- common/cmd_mmc.c | 122 +++ drivers/mmc/Makefile |1 + drivers/mmc/mmc.c| 926 ++ include/mmc.h| 174 +- lib_ppc/board.c |9 + 5 files changed, 1223 insertions(+), 9 deletions(-) create mode 100644 drivers/mmc/mmc.c diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index 21873d6..f27d7ef 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -25,6 +25,7 @@ #include command.h #include mmc.h +#ifndef CONFIG_GENERIC_MMC int do_mmc (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) { if (mmc_legacy_init (1) != 0) { @@ -39,3 +40,124 @@ U_BOOT_CMD( mmcinit - init mmc card\n, NULL ); +#endif /* !CONFIG_GENERIC_MMC */ + +static void print_mmcinfo(struct mmc *mmc) +{ + printf(Device: %s\n, mmc-name); + printf(Manufacturer ID: %x\n, mmc-cid[0] 24); + printf(OEM: %x\n, (mmc-cid[0] 8) 0x); + printf(Name: %c%c%c%c%c \n, mmc-cid[0] 0xff, + (mmc-cid[1] 24), (mmc-cid[1] 16) 0xff, + (mmc-cid[1] 8) 0xff, mmc-cid[1] 0xff); + + printf(Tran Speed: %d\n, mmc-tran_speed); + printf(Rd Block Len: %d\n, mmc-read_bl_len); + + printf(%s version %d.%d\n, IS_SD(mmc) ? SD : MMC, + (mmc-version 4) 0xf, mmc-version 0xf); + + printf(High Capacity: %s\n, mmc-high_capacity ? Yes : No); + printf(Capacity: %lld\n, mmc-capacity); + + printf(Bus Width: %d-bit\n, mmc-bus_width); +} + +int do_mmcinfo (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{ + struct mmc *mmc; + int dev_num; + + if (argc 2) + dev_num = 0; + else + dev_num = simple_strtoul(argv[1], NULL, 0); + + mmc = find_mmc_device(dev_num); + + if (mmc) { + mmc_init(mmc); + + print_mmcinfo(mmc); + } + + return 0; +} + +U_BOOT_CMD(mmcinfo, 2, 0, do_mmcinfo, mmcinfo dev num-- display MMC info\n, + NULL); + +int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{ + int rc = 0; + + switch (argc) { + case 0: + case 1: + case 3: + case 4: + printf(Usage:\n%s\n, cmdtp-usage); + return 1; + + case 2: + if (!strcmp(argv[1], list)) { + print_mmc_devices('\n'); + return 0; + } + return 1; + default: /* at least 5 args */ + if (strcmp(argv[1], read) == 0) { + int dev = simple_strtoul(argv[2], NULL, 10); + void *addr = (void *)simple_strtoul(argv[3], NULL, 16); + u32 cnt = simple_strtoul(argv[5], NULL, 16); + u32 n; + u32 blk = simple_strtoul(argv[4], NULL, 16); + struct mmc *mmc = find_mmc_device(dev); + + printf(\nMMC read: dev # %d, block # %d, count %d ... , + dev, blk, cnt); + + mmc_init(mmc); + + n = mmc-block_dev.block_read(dev, blk, cnt, addr); + + /* flush cache after read */ + flush_cache((ulong)addr, cnt * 512); //FIXME + + printf(%d blocks read: %s\n, + n, (n==cnt) ? OK : ERROR); + return (n == cnt) ? 0 : 1; + } else if (strcmp(argv[1], write) == 0) { + int dev = simple_strtoul(argv[2], NULL, 10); + void *addr = (void *)simple_strtoul(argv[3], NULL, 16); + u32 cnt = simple_strtoul(argv[5], NULL, 16); + u32 n; + struct mmc *mmc = find_mmc_device(dev); + + int blk = simple_strtoul(argv[4], NULL, 16); + + printf(\nMMC write: dev # %d, block # %d, count %d ... , + dev, blk, cnt); + + mmc_init(mmc); + + n = mmc-block_dev.block_write(dev, blk, cnt, addr); + + printf(%d blocks written: %s\n, + n, (n == cnt) ? OK : ERROR); + return (n == cnt) ? 0 : 1; + } else { + printf(Usage:\n%s\n,