Re: [U-Boot] [PATCH v2 2/3] avr32: Use uncached() macro to get an address for SDRAM init
Detlev Zundel 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
Re: [U-Boot] [PATCH v2 2/3] avr32: Use uncached() macro to get an address for SDRAM init
Detlev Zundel 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
[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
---
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
+#include
+#include
+
+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
[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
---
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 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
---
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(&quo
[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
Re: [U-Boot] ATMEL Custodians == /dev/null ??
Wolfgang Denk 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 ??
Wolfgang Denk 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 ??
Mike Frysinger wrote: > On Sun, Aug 8, 2010 at 9:27 PM, Haavard Skinnemoen wrote: > > Mike Frysinger 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? 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] [PATCH 3/3] avr32: Add simple paging support
Wolfgang Denk 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
Re: [U-Boot] ATMEL Custodians == /dev/null ??
Mike Frysinger 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 0/3] avr32 fixes
Bas Mevissen wrote: > On Mon, 2 Aug 2010 14:06:26 +0200, Haavard Skinnemoen > 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] [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
---
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
+#include
+#include
+
+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
[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 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
---
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 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 --- 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 -- 1.7.0.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Resigning as the AVR32 custodian (was Re: AVR32 / ATNGW100 FLASH adressing issues)
[resending because the mailing list was accidentally dropped from Cc] Reinhard Meyer wrote: > I would be willing to help solve the issue/dispute. Thanks. > Correct me if I am wrong: > > 1. The issue seems to be the requirement/wish by Wolfgang Denk that > virtual and physical addresses of flash should be 1:1 mapped? Correct. > 2. CFI Driver for parallel flash uses virtual adresses why? Any address accessed by the CPU is by definition virtual. > To be > compatible with serial flash? Or to generally avoid caching problems - > assuming every hardware can disable cache using the mmu? Various architectures provide different ways of managing the cache. Some (e.g. AVR32) do it via the MMU, others provide other mechanisms. This is why I added a generic function for mapping a physical address to a virtual address given a set of desired cache properties, and changed the CFI driver to use it. This worked very well, until the CFI driver was changed to do address calculations on virtual addresses instead of physical addresses. The proposed solution by Wolfgang is to toss that interface and instead introduce a "cache control" interface which is basically identical except that it doesn't return a virtual address. Since AVR32 uses the MMU to control caching properties, such an interface would be utterly useless. > 3. The default translation on AP700x is a000->, non cached. The default translations are as follows: - -> , cached - 8000 -> , cached - a000 -> , uncached - e000 -> e000, uncached > 4. With some effort that could be changed to ->? The > effort would be to setup page tables in RAM? With some effort, anything is possible. However, adding full-blown paging support to a boot loader seems just wrong. For a generic solution, it's not enough to just load the necessary pages into the TLB; you'll have to implement a TLB miss exception handler to support the case where the TLB isn't large enough to hold all the mappings at once. > 5. The current u-Boot works fine with the default translation, provided > the environment sector is used with its virtual address AND commands > involving the flash (protect, erase, cp) are done using the virtual > addresses. Yes, but ironically enough, this used to work in terms of physical addresses. The problems with exposing virtual addresses through the user interface is that they might change in the future. > 6. That can be achieved by defining the ENV_ADDR accordingly OR by > changing the load/saveenv functionality to have the address translated first Yes, but the translation used to be localized in the CFI driver. Introducing physical-to-virtual mappings into all parts of the system _except_ the CFI driver seems like a huge step backwards. > 7. What issues do exist with the jffs2 file system in that context? I don't know. 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 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
Mark Jackson 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] Virtual addresses, u-boot, and the MMU
Becky Bruce 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 PA>VA: 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] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
Thiago A. Corrêa 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 > Skinnemoen 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] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
Wolfgang Denk 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
Wolfgang Denk 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 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
Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
Wolfgang Denk 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 propri
Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
Wolfgang Denk 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
Stefan Roese 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
Becky Bruce 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 > "log 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
Becky Bruce 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
Wolfgang Denk 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
Wolfgang Denk wrote: > Dear Haavard Skinnemoen, > > In message <20090830224218.10f14...@siona> you wrote: > > > > > 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. > > OK, so this would be the plan for a clean fix, then? Possibly. But it means even more effort and even larger code, so I'm not exactly happy about it. > > > 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. > > 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. > > 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... > > 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.) > > 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 ;-) Seems like you think it's more important to follow arbitrary rules than writing code that works well. > As it seems, this requires some more discussion, i. e. a clean fix > within the next couple of days is unlikely. To me that means that it > makes no sense to offer to delay the release of v2009.08 by a week or > so, as we'd still not be ready then. I will therefore proceed as > planned, putting up with the fact that some (two, IIRC?) AVR32 boards > are broken in this release. [we had a very long testing phase, and the > problem is not exactly new, so it should have been detected (and > fixed) long before.] Yes, I agree. 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 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] [GIT PULL] AVR32: NGW100 fix for 2009.08
On Sun, 30 Aug 2009 20:30:59 +0200 Wolfgang Denk wrote: > Dear Haavard Skinnemoen, > > In message <20090828105050.0ddb0...@hskinnemoen-d830> you wrote: > > > > 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 > > I'm unsure what to do about that. I understand this patch is needed to > get the system working at all, untill we find a permanent solution. > Right? 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. > So you want me to apply this even though we agree it's not a nice > chance? If the 2009.08 release is around the corner, I would appreciate if you applied it before making the release. 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 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
Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR
On Sat, 29 Aug 2009 13:39:02 +0200 Stefan Roese 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 Fri, 28 Aug 2009 14:58:15 -0500 Becky Bruce 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
Kumar Gala 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] atngw100: Use virtual address in CONFIG_ENV_ADDR
Wolfgang Denk 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
Mark Jackson 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
Mark Jackson 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
[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 --- 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
[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
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)
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] 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)
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] [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
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
On Mon, 22 Jun 2009 16:31:20 +0100 Mark Jackson 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 ... 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 1/1] avr32/hsdramc: Move conditional compilation to Makefile
Jean-Christophe PLAGNIOL-VILLARD wrote: > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD > Cc: Haavard Skinnemoen Acked-by: Haavard Skinnemoen ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC] initcall mechanism introduction
Wolfgang Denk wrote: > > component X is always supposed to come before component Y, that can be > > done with different levels of initcalls, or just by arranging the > > makefiles appropriately (with a comment warning people not to change it). > > The problem is that there is no such fix order. It is board dependent. In other words, what we have right now is the worst of both worlds: - There are shitloads of #ifdefs in the arch code to cope with different board requirements. - There are huge differences between each architecture's init sequence which makes it hard to write generic code elsewhere, and hard to maintain each architecture since the init sequence needs to be changed when new boards add new features, and there's no "standard" way of doing it so the chances of getting it right to begin with are very slim. Also, the chances of getting this mess cleaned up is very slim since you need to touch all architectures in order to change something. IMO, introducing a common init sequence for all architectures would be an important step on the way to make this code somewhat maintainable, no matter how messy the initial result ends up being. Or perhaps a better first step would be to clean up the ppc code since it's what new architectures tend to copy, and it's just such an insane mess right now. Haavard ___ 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 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 > Cc: Ben Warren > Cc: Haavard Skinnemoen Acked-by: Haavard Skinnemoen ___ 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 > CC: Haavard Skinnemoen Acked-by: Haavard Skinnemoen ___ 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 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] 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: 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 > CC: Mingkai Hu > CC: Haavard Skinnemoen 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 ___ 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 > CC: Haavard Skinnemoen > CC: Jason McMullan > CC: TsiChung Liew Looks reasonable, though it's probably best if someone more familiar with this particular driver Acks it as well. Acked-by: Haavard Skinnemoen ___ 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 > CC: Haavard Skinnemoen Looks good to me. Acked-by: Haavard Skinnemoen ___ 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 > CC: Haavard Skinnemoen Acked-by: Haavard Skinnemoen ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
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 > Ack-by: Jean-Christophe PLAGNIOL-VILLARD > > Haavard is ok for you too? Sure. Acked-by: Haavard Skinnemoen ___ 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-
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 Applied, thanks 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: > > 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 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 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 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 > #include > > +#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 1/1] MIMC200: tidy GCLK init code
Mark Jackson wrote: > Change the MIMC200 startup code to use the built-in (rather than > hard-coded) funtions for setting up gclk outputs. > > We'll also move the code to the new, more-appropriate > board_postclk_init() routine. > > Signed-off-by: Mark Jackson Applied to mimc200 and merged into next, 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 > > The AT32UC3A0512ES chip has a bug when disabling interrupts. As a > workaround, two NOPs can be inserted. > > Signed-off-by: Gunnar Rangoy > Signed-off-by: Paul Driveklepp > Signed-off-by: Olav Morken 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 > Signed-off-by: Paul Driveklepp > Signed-off-by: Olav Morken 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 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 > > 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 > Signed-off-by: Paul Driveklepp > Signed-off-by: Olav Morken 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 2/9] AVR32: Make cacheflush cpu-dependent
Gunnar Rangoy wrote: > From: Olav Morken > > 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 > Signed-off-by: Paul Driveklepp > Signed-off-by: Olav Morken 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] 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 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 > Cc: Haavard Skinnemoen > Cc: Mike Frysinger > --- > rebase for-next > Precedent version > Ack-by: Mike Frysinger > Ack-by: Haavard Skinnemoen 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] [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 8/9] AVR32: CPU support for AT32UC3A0xxx CPUs
Olav Morken wrote: > On Mon, Jan 26, 2009 at 21:03, Wolfgang Denk wrote: > > Dear =?ISO-8859-1?Q?Gunnar_Rang=F8y?=, > > > > In message > > you wrote: > [] > >> > > >> > It would be nice if you used readable names instead of all these > >> > magic hardcoded constants. > >> > >> Each bit in the masks represents a single pin on the microcontroller. > >> The readable names would therefore become something like: > >> #define EBI_PINS_PORT0 (0x0003C000 | 0x1E00) > >> #define EBI_PINS_PORT1 (0x0010 | 0x7C00 | ...) > > > > Um, no. You didn't get it. You use magic numbers again. > > > > That should be some > > > > #define FOO_PIN_XXX 0x0003C000 > > #define FOO_PIN_YYY 0x1E00 > > #define BAR_PIN_XXX 0x0010 > > #DEFINE BAR_PIN_YYY 0x7C00 > > > > in one place and > > > > #define EBI_PINS_PORT0 (FOO_PIN_XXX | FOO_PIN_YYY) > > #define EBI_PINS_PORT1 ((BAR_PIN_XXX| BAR_PIN_YYY) > > > > elsewhere, using readable names for the defines. > > Sorry, but the bitwise or's in the code are a bit misleading. The only > function of them is to make it more readable in the presence of the > datasheet for the microcontroller. Each single bit in the bitmask is > one pin on the microcontroller. In this case, it would become something > like: > #define EBI_PIN_NCS0(1<<14) > #define EBI_PIN_ADDR20 (1<<15) > #define EBI_PIN_ADDR21 (1<<16) > #define EBI_PIN_ADDR22 (1<<17) > () > #define EBI_PINS_PORT0 (EBI_PIN_NCS0 | EBI_PIN_ADDR20 | EBI_PIN_ADDR21 \ > | EBI_PIN_ADDR22 | ()) I don't think that would be much of an improvement. It would make the code much larger, and the pin definitions themselves would still need some magic number...and you'll need to know which GPIO port they belong to in order to actually use them. > >> I'm not certain that that would make it much more readable. The > >> bitmasks are more or less based on reading the datasheet and counting > >> which pins are used. > > > > This is exactly what should NOT be necessary to read and understand > > the code. > > [] > > But in this case, this is code which should never be changed without > looking at the datasheet, and probably schematics for the board in > question. Exactly. At some point, you need code which encapsulates the definitions in the data sheet, and that's the whole purpose of these functions. > > This is simply not acceptable. > > > >> Maybe adding a comment before the code would be just as useful? > > > > No, a comment can't fix this, especially as you just have to wait a > > few months to see the comment and the code getting out of sync. > > > > Please fix the code. > > How about something like this: > /* > * These bitmasks represents the pins used by the EBI on this board. > * Please refer to the datasheet for the UC3A-series microcontrollers > * for a description of the individual pins. > */ > #define EBI_PINS_PORT0 0x1E03C000 > #define EBI_PINS_PORT1 0xE0037C10 > () > > And in the portmux-configuration-function: > portmux_select_peripheral(PORTMUX_PORT(0), EBI_PINS_PORT0, > PORTMUX_FUNC_C, 0); > () > > When looking at this code I can see that we need a way for each > individual board to change the bitmasks, since the same pin in some > cases can be routed out to several places (depending on the board). > Maybe a CONFIG_UC3A0xxx_EBI_PINS_PORT0 option or something like that > could be added... I think you need to actually use the parameters passed to that function in order to set up the correct address, data and chip select lines. Now, the alternative mappings for some of the pins are tricky...but you could perhaps add a few more flags to cover those possibilities... Once you do that, the whole function becomes a matter of shifting bits into position, and I very much doubt a bunch of defines can make that sort of code any clearer. Think of this function as the executable equivalent of a pin definition. > >> As most of the needed functionality is embedded in the microcontroller, > >> there are very few external peripherals used by U-Boot. Apart from > >> external memory, and oscillator, and level-shifters for the serial-port, > >> there is only the ethernet PHY, and that one shouldn't need a reset. > > > > Famous last words. What if exactrly the PHY is stuck and needs a > > reset? > > The only reset we can do on the PHY is a software reset, by sending a > reset command over the (R)MII bus, and I don't believe that the generic > chip code is the place to do that. If it should be done, I believe it > should be done by the macb-driver after the reset. This would allow it > to recover even if the microcontroller wasn't reset by the > reset-command, but for example by a watchdog timer. I suppose this might be a good idea. But I haven't heard of any problems with other boards because of this...and wasn't there a generic PHY layer in the works anyway? > > Hmmm... "apart from external memory" ... does externam memory also > >
Re: [U-Boot] [PATCH v2 5/9] AVR32: macb - Disable 100mbps if clock is slow
Olav Morken wrote: > > Yes, AP7000 have two Ethernet MACs. And if I got this right you want to > > make a generic config about it, so then I guess it should open up for > > having more than one MAC. > > OK, how about adding a CONFIG_MACB_ADVERTISE(id)-option, where id is > the id of the MACB (passed to the macb_eth_initialize-function). This > makes it possible to add this without touching anything but the > macb-driver (i.e. without changing the macb_eth_initialize-prototype). > > In the config-files, one could then have: > #define CONFIG_MACB_ADVERTISE(id) ( \ > (id == 0) ? ( \ > ADVERTISE_ALL | ADVERTISE_CSMA \ > ) : ( \ > ADVERTISE_CSMA | ADVERTISE_10HALF | \ > ADVERTISE_10FULL\ > )) > > Or in the simple (and probably mose usual case (only one set of options > advertised): > #define CONFIG_MACB_ADVERTISE(id) ( \ > (ADVERTISE_CSMA | ADVERTISE_10HALF | ADVERTISE_10FULL) > > > This would require saving the id to the macb_device struct. If this is > unacceptable, it could be changed to using the regs-offset instead of > the id. > > Any thoughts about this? Sounds good to me. The board decides the id, so it makes sense to pass it back to the board code. 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
/* 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: > As I look at this we really need to understand what Haavard was trying > to get with: > > commit 12d30aa79779c2aa7a998bbae4c075f822a53004 > Author: Haavard Skinnemoen > 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 > > 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] [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 > > --- > > 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] [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] [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 I like it. "hz" makes it perfectly clear what the value means. Acked-by: Haavard Skinnemoen ___ 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] spi flash: fix crash due to spi flash miscommunication
Mike Frysinger wrote: > From: Brad Bozarth > > 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 > Signed-off-by: Mike Frysinger > CC: Haavard Skinnemoen Acked-by: Haavard Skinnemoen ___ 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:
> > /* 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:
> 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
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
---
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
/* 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 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 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] flash: Export flash_sector_size() function.
Piotr Zięcik <[EMAIL PROTECTED]> wrote: > Wednesday 19 November 2008 14:51:01 Haavard Skinnemoen napisał(a): > > > Export flash_sector_size() function from drivers/mtd/cfi_flash.c. > > > > Why? > > This function is used by cfi-mtd driver, which I have posted yesterday > with other patches around UBI and MTD (see "[U-Boot] Export CFI Flash to the > MTD and support it in UBI"). Ok, can you please add that information to the patch description? 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] 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 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] [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 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
