Re: [U-Boot] [PATCH v2 2/3] avr32: Use uncached() macro to get an address for SDRAM init

2010-08-13 Thread Haavard Skinnemoen
Detlev Zundel d...@denx.de wrote:
  Problem is that in order to make the CFI driver work on avr32, we need
  to change the map_physmem() macro to return the physical address
  unchanged.
 
 I see.  And I presume you cannot tell the situation apart inside
 map_physmem?

I don't think so. How do you propose we do that?

  The map_physmem() macro currently does exactly the same thing as the
  uncached() macro, and the unmap is a noop, but the next patch changes
  it in order to fix the CFI driver. If the next patch is applied without
  this patch being applied first, the SDRAM driver will do cached
  accesses during initialization, and that may cause the initialization
  to fail.
 
 Why not include a note to this extent into the git commit message?  This
 would be a great help for other people to later understand why this
 change has been done the backward way that it was.

The commit message already contains this:

The paging system which is required to set up caching properties has not
yet been initialized when the SDRAM is initialized. So when the
map_physmem() function is converted to return the physical address
unchanged, the SDRAM initialization will break on some boards.

which is essentially the same thing, isn't it?

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2 0/3] avr32: simple paging support

2010-08-12 Thread Haavard Skinnemoen
This series fixes a longstanding problem with the 'saveenv' command on
ATNGW100.

It also includes a trivial enhancement to the exception reporting which
I found very useful during debugging.

Hopefully this will make mainline u-boot useful again on AVR32. Without
this fix, v2008.10 is the latest usable release.

This is the second version of this series. The following changes have
been made since v1:
  - Rebased onto testing/arm-reloc-and-cache-support
  - Dropped asm/unaligned.h fix because a similar patch has already been
applied to mainline.
  - Fixed SDRAM initialization issue noticed on certain boards

Haavard Skinnemoen (3):
  avr32: Print unrelocated PC on exception
  avr32: Use uncached() macro to get an address for SDRAM init
  avr32: Add simple paging support

 arch/avr32/cpu/at32ap700x/Makefile |2 +-
 arch/avr32/cpu/at32ap700x/mmu.c|   78 
 arch/avr32/cpu/exception.c |3 +-
 arch/avr32/cpu/start.S |   19 +++--
 arch/avr32/include/asm/arch-at32ap700x/addrspace.h |5 +-
 arch/avr32/include/asm/arch-at32ap700x/mmu.h   |   66 +
 arch/avr32/lib/board.c |4 +
 board/atmel/atngw100/atngw100.c|   19 -
 board/atmel/atstk1000/atstk1000.c  |   19 -
 board/earthlcd/favr-32-ezkit/favr-32-ezkit.c   |   19 -
 board/mimc/mimc200/mimc200.c   |   24 +-
 board/miromico/hammerhead/hammerhead.c |   19 -
 include/configs/atngw100.h |3 +
 include/configs/atstk1002.h|3 +
 include/configs/atstk1003.h|3 +
 include/configs/atstk1004.h|3 +
 include/configs/atstk1006.h|3 +
 include/configs/favr-32-ezkit.h|3 +
 include/configs/hammerhead.h   |3 +
 include/configs/mimc200.h  |3 +
 20 files changed, 274 insertions(+), 27 deletions(-)
 create mode 100644 arch/avr32/cpu/at32ap700x/mmu.c
 create mode 100644 arch/avr32/include/asm/arch-at32ap700x/mmu.h
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2 2/3] avr32: Use uncached() macro to get an address for SDRAM init

2010-08-12 Thread Haavard Skinnemoen
The paging system which is required to set up caching properties has not
yet been initialized when the SDRAM is initialized. So when the
map_physmem() function is converted to return the physical address
unchanged, the SDRAM initialization will break on some boards.

The avr32-specific uncached() macro will return an address which will
always cause uncached accessed to be made. Since this happens in the
board code, using avr32-specific features should be ok, and will allow
the SDRAM initialization to keep working.

Signed-off-by: Haavard Skinnemoen haavard.skinnem...@atmel.com
---
 board/atmel/atngw100/atngw100.c  |4 +---
 board/atmel/atstk1000/atstk1000.c|4 +---
 board/earthlcd/favr-32-ezkit/favr-32-ezkit.c |4 +---
 board/mimc/mimc200/mimc200.c |4 +---
 board/miromico/hammerhead/hammerhead.c   |4 +---
 5 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/board/atmel/atngw100/atngw100.c b/board/atmel/atngw100/atngw100.c
index 004d8da..4580f55 100644
--- a/board/atmel/atngw100/atngw100.c
+++ b/board/atmel/atngw100/atngw100.c
@@ -75,13 +75,11 @@ phys_size_t initdram(int board_type)
unsigned long actual_size;
void *sdram_base;
 
-   sdram_base = map_physmem(EBI_SDRAM_BASE, EBI_SDRAM_SIZE, MAP_NOCACHE);
+   sdram_base = uncached(EBI_SDRAM_BASE);
 
expected_size = sdram_init(sdram_base, sdram_config);
actual_size = get_ram_size(sdram_base, expected_size);
 
-   unmap_physmem(sdram_base, EBI_SDRAM_SIZE);
-
if (expected_size != actual_size)
printf(Warning: Only %lu of %lu MiB SDRAM is working\n,
actual_size  20, expected_size  20);
diff --git a/board/atmel/atstk1000/atstk1000.c 
b/board/atmel/atstk1000/atstk1000.c
index c36cb57..d91d594 100644
--- a/board/atmel/atstk1000/atstk1000.c
+++ b/board/atmel/atstk1000/atstk1000.c
@@ -97,13 +97,11 @@ phys_size_t initdram(int board_type)
unsigned long actual_size;
void *sdram_base;
 
-   sdram_base = map_physmem(EBI_SDRAM_BASE, EBI_SDRAM_SIZE, MAP_NOCACHE);
+   sdram_base = uncached(EBI_SDRAM_BASE);
 
expected_size = sdram_init(sdram_base, sdram_config);
actual_size = get_ram_size(sdram_base, expected_size);
 
-   unmap_physmem(sdram_base, EBI_SDRAM_SIZE);
-
if (expected_size != actual_size)
printf(Warning: Only %lu of %lu MiB SDRAM is working\n,
actual_size  20, expected_size  20);
diff --git a/board/earthlcd/favr-32-ezkit/favr-32-ezkit.c 
b/board/earthlcd/favr-32-ezkit/favr-32-ezkit.c
index 8af680f..d2843c9 100644
--- a/board/earthlcd/favr-32-ezkit/favr-32-ezkit.c
+++ b/board/earthlcd/favr-32-ezkit/favr-32-ezkit.c
@@ -68,13 +68,11 @@ phys_size_t initdram(int board_type)
unsigned long actual_size;
void *sdram_base;
 
-   sdram_base = map_physmem(EBI_SDRAM_BASE, EBI_SDRAM_SIZE, MAP_NOCACHE);
+   sdram_base = uncached(EBI_SDRAM_BASE);
 
expected_size = sdram_init(sdram_base, sdram_config);
actual_size = get_ram_size(sdram_base, expected_size);
 
-   unmap_physmem(sdram_base, EBI_SDRAM_SIZE);
-
if (expected_size != actual_size)
printf(Warning: Only %lu of %lu MiB SDRAM is working\n,
actual_size  20, expected_size  20);
diff --git a/board/mimc/mimc200/mimc200.c b/board/mimc/mimc200/mimc200.c
index cc0f137..9940669 100644
--- a/board/mimc/mimc200/mimc200.c
+++ b/board/mimc/mimc200/mimc200.c
@@ -153,13 +153,11 @@ phys_size_t initdram(int board_type)
unsigned long actual_size;
void *sdram_base;
 
-   sdram_base = map_physmem(EBI_SDRAM_BASE, EBI_SDRAM_SIZE, MAP_NOCACHE);
+   sdram_base = uncached(EBI_SDRAM_BASE);
 
expected_size = sdram_init(sdram_base, sdram_config);
actual_size = get_ram_size(sdram_base, expected_size);
 
-   unmap_physmem(sdram_base, EBI_SDRAM_SIZE);
-
if (expected_size != actual_size)
printf(Warning: Only %lu of %lu MiB SDRAM is working\n,
actual_size  20, expected_size  20);
diff --git a/board/miromico/hammerhead/hammerhead.c 
b/board/miromico/hammerhead/hammerhead.c
index 8b3e22c..5ab999e 100644
--- a/board/miromico/hammerhead/hammerhead.c
+++ b/board/miromico/hammerhead/hammerhead.c
@@ -80,13 +80,11 @@ phys_size_t initdram(int board_type)
unsigned long actual_size;
void *sdram_base;
 
-   sdram_base = map_physmem(EBI_SDRAM_BASE, EBI_SDRAM_SIZE, MAP_NOCACHE);
+   sdram_base = uncached(EBI_SDRAM_BASE);
 
expected_size = sdram_init(sdram_base, sdram_config);
actual_size = get_ram_size(sdram_base, expected_size);
 
-   unmap_physmem(sdram_base, EBI_SDRAM_SIZE);
-
if (expected_size != actual_size)
printf(Warning: Only %lu of %lu MiB SDRAM is working\n,
   actual_size  20, expected_size  20);
-- 
1.7.0.4

[U-Boot] [PATCH v2 1/3] avr32: Print unrelocated PC on exception

2010-08-12 Thread Haavard Skinnemoen
In addition to the real PC value, also print the value of PC after
subtracting the relocation offset. This value will match the address in
the ELF file so it's much easier to figure out where things went wrong.

Signed-off-by: Haavard Skinnemoen haavard.skinnem...@atmel.com
---
 arch/avr32/cpu/exception.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/avr32/cpu/exception.c b/arch/avr32/cpu/exception.c
index dc9c300..b21ef1f 100644
--- a/arch/avr32/cpu/exception.c
+++ b/arch/avr32/cpu/exception.c
@@ -59,7 +59,8 @@ void do_unknown_exception(unsigned int ecr, struct pt_regs 
*regs)
 {
unsigned int mode;
 
-   printf(\n *** Unhandled exception %u at PC=0x%08lx\n, ecr, regs-pc);
+   printf(\n *** Unhandled exception %u at PC=0x%08lx [%08lx]\n,
+   ecr, regs-pc, regs-pc - gd-reloc_off);
 
switch (ecr) {
case ECR_BUS_ERROR_WRITE:
-- 
1.7.0.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2 3/3] avr32: Add simple paging support

2010-08-12 Thread Haavard Skinnemoen
Use the MMU hardware to set up 1:1 mappings between physical and virtual
addresses. This allows us to bypass the cache when accessing the flash
without having to do any physical-to-virtual address mapping in the CFI
driver.

The virtual memory mappings are defined at compile time through a sorted
array of virtual memory range objects. When a TLB miss exception
happens, the exception handler does a binary search through the array
until it finds a matching entry and loads it into the TLB. The u-boot
image itself is covered by a fixed TLB entry which is never replaced.

This makes the 'saveenv' command work again on ATNGW100 and other boards
using the CFI driver, hopefully without breaking any rules.

Signed-off-by: Haavard Skinnemoen haavard.skinnem...@atmel.com
---
 arch/avr32/cpu/at32ap700x/Makefile |2 +-
 arch/avr32/cpu/at32ap700x/mmu.c|   78 
 arch/avr32/cpu/start.S |   19 +++--
 arch/avr32/include/asm/arch-at32ap700x/addrspace.h |5 +-
 arch/avr32/include/asm/arch-at32ap700x/mmu.h   |   66 +
 arch/avr32/lib/board.c |4 +
 board/atmel/atngw100/atngw100.c|   15 
 board/atmel/atstk1000/atstk1000.c  |   15 
 board/earthlcd/favr-32-ezkit/favr-32-ezkit.c   |   15 
 board/mimc/mimc200/mimc200.c   |   20 +
 board/miromico/hammerhead/hammerhead.c |   15 
 include/configs/atngw100.h |3 +
 include/configs/atstk1002.h|3 +
 include/configs/atstk1003.h|3 +
 include/configs/atstk1004.h|3 +
 include/configs/atstk1006.h|3 +
 include/configs/favr-32-ezkit.h|3 +
 include/configs/hammerhead.h   |3 +
 include/configs/mimc200.h  |3 +
 19 files changed, 267 insertions(+), 11 deletions(-)
 create mode 100644 arch/avr32/cpu/at32ap700x/mmu.c
 create mode 100644 arch/avr32/include/asm/arch-at32ap700x/mmu.h

diff --git a/arch/avr32/cpu/at32ap700x/Makefile 
b/arch/avr32/cpu/at32ap700x/Makefile
index 46e6ef6..30ea925 100644
--- a/arch/avr32/cpu/at32ap700x/Makefile
+++ b/arch/avr32/cpu/at32ap700x/Makefile
@@ -24,7 +24,7 @@ include $(TOPDIR)/config.mk
 
 LIB:= $(obj)lib$(SOC).a
 
-COBJS  := portmux.o clk.o
+COBJS  := portmux.o clk.o mmu.o
 SRCS   := $(SOBJS:.o=.S) $(COBJS:.o=.c)
 OBJS   := $(addprefix $(obj),$(SOBJS) $(COBJS))
 
diff --git a/arch/avr32/cpu/at32ap700x/mmu.c b/arch/avr32/cpu/at32ap700x/mmu.c
new file mode 100644
index 000..c3a1b93
--- /dev/null
+++ b/arch/avr32/cpu/at32ap700x/mmu.c
@@ -0,0 +1,78 @@
+#include common.h
+#include asm/arch/mmu.h
+#include asm/sysreg.h
+
+void mmu_init_r(unsigned long dest_addr)
+{
+   uintptr_t   vmr_table_addr;
+
+   /* Round monitor address down to the nearest page boundary */
+   dest_addr = PAGE_ADDR_MASK;
+
+   /* Initialize TLB entry 0 to cover the monitor, and lock it */
+   sysreg_write(TLBEHI, dest_addr | SYSREG_BIT(TLBEHI_V));
+   sysreg_write(TLBELO, dest_addr | MMU_VMR_CACHE_WRBACK);
+   sysreg_write(MMUCR, SYSREG_BF(DRP, 0) | SYSREG_BF(DLA, 1)
+   | SYSREG_BIT(MMUCR_S) | SYSREG_BIT(M));
+   __builtin_tlbw();
+
+   /*
+* Calculate the address of the VM range table in a PC-relative
+* manner to make sure we hit the SDRAM and not the flash.
+*/
+   vmr_table_addr = (uintptr_t)mmu_vmr_table;
+   sysreg_write(PTBR, vmr_table_addr);
+   printf(VMR table @ 0x%08x\n, vmr_table_addr);
+
+   /* Enable paging */
+   sysreg_write(MMUCR, SYSREG_BF(DRP, 1) | SYSREG_BF(DLA, 1)
+   | SYSREG_BIT(MMUCR_S) | SYSREG_BIT(M) | SYSREG_BIT(E));
+}
+
+int mmu_handle_tlb_miss(void)
+{
+   const struct mmu_vm_range *vmr_table;
+   const struct mmu_vm_range *vmr;
+   unsigned int fault_pgno;
+   int first, last;
+
+   fault_pgno = sysreg_read(TLBEAR)  PAGE_SHIFT;
+   vmr_table = (const struct mmu_vm_range *)sysreg_read(PTBR);
+
+   /* Do a binary search through the VM ranges */
+   first = 0;
+   last = CONFIG_SYS_NR_VM_REGIONS;
+   while (first  last) {
+   unsigned int start;
+   int middle;
+
+   /* Pick the entry in the middle of the remaining range */
+   middle = (first + last)  1;
+   vmr = vmr_table[middle];
+   start = vmr-virt_pgno;
+
+   /* Do the bisection thing */
+   if (fault_pgno  start) {
+   last = middle;
+   } else if (fault_pgno = (start + vmr-nr_pages)) {
+   first = middle + 1;
+   } else {
+   /* Got it; let's slam it into the TLB */
+   uint32_t tlbelo

Re: [U-Boot] [PATCH v2 2/3] avr32: Use uncached() macro to get an address for SDRAM init

2010-08-12 Thread Haavard Skinnemoen
Detlev Zundel d...@denx.de wrote:
 So this patch replaces a construct which seems to be valid over all
 architectures by a construct which is only used in avr32, right?  It
 also deletes the explicit statement that such a mapping is not needed
 any further.

Problem is that in order to make the CFI driver work on avr32, we need
to change the map_physmem() macro to return the physical address
unchanged.

 Isn't this a step backward?  Can't you put the functionality inside the
 map function and leave the unmap a noop?

I agree it's a step backward, but since the previous flamewar didn't
get us anywhere, I decided to go for a compromise this time. At least
this small architecture-specific kludge is localized to
architecture-specific code.

The map_physmem() macro currently does exactly the same thing as the
uncached() macro, and the unmap is a noop, but the next patch changes
it in order to fix the CFI driver. If the next patch is applied without
this patch being applied first, the SDRAM driver will do cached
accesses during initialization, and that may cause the initialization
to fail.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] ATMEL Custodians == /dev/null ??

2010-08-09 Thread Haavard Skinnemoen
Mike Frysinger vap...@gentoo.org wrote:
 On Sun, Aug 8, 2010 at 9:27 PM, Haavard Skinnemoen wrote:
  Mike Frysinger vap...@gentoo.org wrote:  
  explicitly cc-ing the atmel guys just so there's no surprise when 
  at91/avr32
  have been handled over to someone else without their explicit ACK ...  
 
  So...what exactly are you Cc'ing us on?  
 
 read the thread

That sort of brings us back to my original question, doesn't it?

reads the archives

Ok, so there are complaints about the non-responsiveness of some of
Atmel's custodians and a wish for someone to take over. Fine by me, I
guess, since I've already indicated I'm probably not the right person
for the job anymore. I can't speak for the AT91 team, however.

But it does seem kind of rude to just hand everything off without
Cc'ing any of the maintainers in question. Perhaps they would respond
more quickly if people actually send e-mail to them?

Btw, I'm not ranting at you, Mike. Thanks for letting me know about the
discussion, although I would have preferred a bit more context...

For the record, the thread seems to be this one, but I seem to be
unable to find the start of it...

http://lists.denx.de/pipermail/u-boot/2010-August/074769.html

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] ATMEL Custodians == /dev/null ??

2010-08-09 Thread Haavard Skinnemoen
Wolfgang Denk w...@denx.de wrote:
 Dear Haavard Skinnemoen,
 
 In message 20100809132949.43c81...@hskinnemoen-d830 you wrote:
 
  But it does seem kind of rude to just hand everything off without
  Cc'ing any of the maintainers in question. Perhaps they would respond
  more quickly if people actually send e-mail to them?
 
 Rude? send e-mail to them?

I mean rude _not_ to send e-mail to them.

 It's not that I'm going to hand off (note that nothing has happened
 yet) anything from any active custodian. 

It's not that I necessarily oppose such a hand-off either.

 First, I have poked them a number of times, both on and off list.

I haven't received any such pokes from you in a long time.

 Second, they are subscribed to this list, and supposed to read the
 traffic. Especially if addresses directly in the Subject they should
 notice that, right?

I used to be subscribed to a whole bunch of lists, but after hitting
around 70,000 unread e-mail, I decided to unsubscribe from most of
them, including u-boot and LKML.

Of course, this is also the main reason why I wanted to resign as a
custodian; I felt I hadn't been able to do a proper job for some time.
But this makes it especially odd that I wasn't Cc'd on the discussion
about custodianship.

 Third, there have been patches posted that clearly fall in their
 domain, and there is zero response: no comment, no activity in the
 custodian directory, no pull request, nothing.

If I wasn't Cc'd, that would explain it. Of course, it's always best if
maintainers follow all relevant mailing lists, but sometimes it's just
not an option, not if you're working on several other projects besides
u-boot.

 Finally, I have to admit that I have been a bit sceptic right from the
 beginning to assign custodianship to someone who has no track record
 as developer in the community. But obviously Atmel would be in the
 best positition to provide decent support for their chips. At least
 that was my hope.

Atmel should definitely be in a good position for that, at least in
theory. But the reality is that people need to do other things too, and
it's difficult to justify spending a lot of time on the boot loader
when there are more customer-focused tasks at hand.

And then there's the whole cost/benefit thing. I've been arguing quite
enthusiastically for the benefits of getting things upstream earlier,
but I've lately started to realize that maybe we're not really getting
all that much in return for the work we've been putting in. Especially
when we're forced to implement a bloody VM subsystem just to fix a
regression someone else introduced.

 I am seriously sick with the current situation. We have a number of
 AT91 and AVR32 related patches sitting there with nobody taking care
 of them. No life signs of any custodian or anybody else from Atmel.

Again, not Cc'ing the relevant maintainers might be an explanation,
though I'm not saying I'm sure it's the _right_ explanation.

 But then there are people available who are actively working with
 these chips, who post fixes and other patches, and who even volunteer
 to take the burden of maintaining the tree.

Sure, if someone outside Atmel wants to take over the AVR32 tree, I'm
all for it. I would really appreciate to be Cc'd on the discussion,
however. And I can be available to help whoever takes over the
custodianship if there's anything avr32-specific they can't figure out.

Again, I'm not speaking for the AT91 team.

 Guess what I'll do?  Continue to wait that some vendor wakes up?

Not commenting on this beyond what I said above.

  Btw, I'm not ranting at you, Mike. Thanks for letting me know about the
  discussion, although I would have preferred a bit more context...
  
  For the record, the thread seems to be this one, but I seem to be
  unable to find the start of it...
  
  http://lists.denx.de/pipermail/u-boot/2010-August/074769.html
 
 Just have a look at the References: headers, and look up the first
 one in gmane - just append the message ID to http://mid.gmane.org/
 
 == http://mid.gmane.org/4c50e124.2000...@emk-elektronik.de
 
 == http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/81812

Thanks, I'll have a look at those. Please forgive me for following the
link on your mailserver ;-)

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] ATMEL Custodians == /dev/null ??

2010-08-09 Thread Haavard Skinnemoen
Wolfgang Denk w...@denx.de wrote:
 Dear Haavard Skinnemoen,
 
 In message 20100809181318.5ec2a...@hskinnemoen-d830 you wrote:
 
   First, I have poked them a number of times, both on and off list.
  
  I haven't received any such pokes from you in a long time.
 
 I'm not talking about you here. You have clearly indicated that you
 resign as custodian, which I have accepted.  So why should I poke you?

I didn't resign _that_ long ago.

  I used to be subscribed to a whole bunch of lists, but after hitting
  around 70,000 unread e-mail, I decided to unsubscribe from most of
  them, including u-boot and LKML.
  
  Of course, this is also the main reason why I wanted to resign as a
  custodian; I felt I hadn't been able to do a proper job for some time.
  But this makes it especially odd that I wasn't Cc'd on the discussion
  about custodianship.
 
 A custodian who is not subscried on the mailing list?  How is this
 supposed to work?  I have to admit that I never expected somebody
 would come up with such a concept.

It actually works on Linux, where people know that they need to Cc the
maintainer to get his attention. So you can actually maintain a dozen
drivers across half a dozen subsystems without getting completely
bogged down with e-mail. If you just drop a patch into the LKML
firehose, it will most likely be ignored unless akpm picks it up and
pokes the relevant maintainer.

   Third, there have been patches posted that clearly fall in their
   domain, and there is zero response: no comment, no activity in the
   custodian directory, no pull request, nothing.
  
  If I wasn't Cc'd, that would explain it. Of course, it's always best if
 
 It has never been a requirement to Cc: the custodian. It is up to the
 custodian to pick up the work that falls into hiw bailiwick,
 including for example global changes that happen to affect his
 architecture / subsystem. Of course this requires that you are
 subscribed. And that you actually *read* the list, at leats to the
 extend that you recognize certain buzzwords in the Subject: lines,
 like the name of the architure you feel responsible for.

In other words, being a u-boot custodian takes a lot more work than
being a Linux maintainer. Combine this with what I said before about it
being difficult to justify spending a lot of time keeping the
bootloader limping along, and it's not good news if you want more
vendor involvement.

  maintainers follow all relevant mailing lists, but sometimes it's just
  not an option, not if you're working on several other projects besides
  u-boot.
 
 Your idea of working as a maintainer is very much different from mine,
 and from the actual requirements for the job.

That seems to be the case, yes.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] ATMEL Custodians == /dev/null ??

2010-08-08 Thread Haavard Skinnemoen
Mike Frysinger vap...@gentoo.org wrote:
 explicitly cc-ing the atmel guys just so there's no surprise when at91/avr32 
 have been handled over to someone else without their explicit ACK ...

So...what exactly are you Cc'ing us on?

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 3/3] avr32: Add simple paging support

2010-08-08 Thread Haavard Skinnemoen
Wolfgang Denk w...@denx.de wrote:
 Can you please try and rebase this code on top of Heiko's ARM rework
 patches, i. e. with cache and relocation support?
 
 See
 http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/81825/focus=82142
 
 
 My intention is that after -rc1 has been released (i. e. when we have
 a next branch again), I will first apply the new environment code
 patches, and then, probably with a week delay or so, Heiko's ARM
 rework.  Your stuff will then have to fit on top of this.

Sure. The patches probably needs more testing and a refresh anyway...I
just saw some issues with SDRAM initialization on some boards which
need to be investigated.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 1/3] avr32: Add missing asm/unaligned.h header file

2010-08-02 Thread Haavard Skinnemoen
Simply include the generic version. We could optimize this a bit more,
as unaligned 32-bit accesses are ok on AP7, but let's make it work
first.

Signed-off-by: Haavard Skinnemoen haavard.skinnem...@atmel.com
---
 arch/avr32/include/asm/unaligned.h |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
 create mode 100644 arch/avr32/include/asm/unaligned.h

diff --git a/arch/avr32/include/asm/unaligned.h 
b/arch/avr32/include/asm/unaligned.h
new file mode 100644
index 000..6cecbbb
--- /dev/null
+++ b/arch/avr32/include/asm/unaligned.h
@@ -0,0 +1 @@
+#include asm-generic/unaligned.h
-- 
1.7.0.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 2/3] avr32: Print unrelocated PC on exception

2010-08-02 Thread Haavard Skinnemoen
In addition to the real PC value, also print the value of PC after
subtracting the relocation offset. This value will match the address in
the ELF file so it's much easier to figure out where things went wrong.

Signed-off-by: Haavard Skinnemoen haavard.skinnem...@atmel.com
---
 arch/avr32/cpu/exception.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/avr32/cpu/exception.c b/arch/avr32/cpu/exception.c
index dc9c300..b21ef1f 100644
--- a/arch/avr32/cpu/exception.c
+++ b/arch/avr32/cpu/exception.c
@@ -59,7 +59,8 @@ void do_unknown_exception(unsigned int ecr, struct pt_regs 
*regs)
 {
unsigned int mode;
 
-   printf(\n *** Unhandled exception %u at PC=0x%08lx\n, ecr, regs-pc);
+   printf(\n *** Unhandled exception %u at PC=0x%08lx [%08lx]\n,
+   ecr, regs-pc, regs-pc - gd-reloc_off);
 
switch (ecr) {
case ECR_BUS_ERROR_WRITE:
-- 
1.7.0.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 0/3] avr32 fixes

2010-08-02 Thread Haavard Skinnemoen
This series fixes a trivial build issue, as well as a longstanding
problem with the 'saveenv' command on ATNGW100.

It also includes a trivial enhancement to the exception reporting which
I found very useful during debugging.

Hopefully this will make mainline u-boot useful again on AVR32. Without
this fix, v2008.10 is the latest usable release.

The patches are based on v2010.06, but it merges fine with the latest
upstream master. The AVR32 master branch currently contains a workaround
which I plan to revert if these patches are acceptable.

Haavard Skinnemoen (3):
  avr32: Add missing asm/unaligned.h header file
  avr32: Print unrelocated PC on exception
  avr32: Add simple paging support

 arch/avr32/cpu/at32ap700x/Makefile |2 +-
 arch/avr32/cpu/at32ap700x/mmu.c|   80 
 arch/avr32/cpu/exception.c |3 +-
 arch/avr32/cpu/start.S |   19 +++--
 arch/avr32/include/asm/arch-at32ap700x/addrspace.h |5 +-
 arch/avr32/include/asm/arch-at32ap700x/mmu.h   |   66 
 arch/avr32/include/asm/unaligned.h |1 +
 arch/avr32/lib/board.c |3 +
 board/atmel/atngw100/atngw100.c|   15 
 include/configs/atngw100.h |3 +
 10 files changed, 185 insertions(+), 12 deletions(-)
 create mode 100644 arch/avr32/cpu/at32ap700x/mmu.c
 create mode 100644 arch/avr32/include/asm/arch-at32ap700x/mmu.h
 create mode 100644 arch/avr32/include/asm/unaligned.h
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 3/3] avr32: Add simple paging support

2010-08-02 Thread Haavard Skinnemoen
Use the MMU hardware to set up 1:1 mappings between physical and virtual
addresses. This allows us to bypass the cache when accessing the flash
without having to do any physical-to-virtual address mapping in the CFI
driver.

The virtual memory mappings are defined at compile time through a sorted
array of virtual memory range objects. When a TLB miss exception
happens, the exception handler does a binary search through the array
until it finds a matching entry and loads it into the TLB. The u-boot
image itself is covered by a fixed TLB entry which is never replaced.

This makes the 'saveenv' command work again on ATNGW100 and other boards
using the CFI driver, hopefully without breaking any rules.

Signed-off-by: Haavard Skinnemoen haavard.skinnem...@atmel.com
---
 arch/avr32/cpu/at32ap700x/Makefile |2 +-
 arch/avr32/cpu/at32ap700x/mmu.c|   78 
 arch/avr32/cpu/start.S |   19 +++--
 arch/avr32/include/asm/arch-at32ap700x/addrspace.h |5 +-
 arch/avr32/include/asm/arch-at32ap700x/mmu.h   |   66 +
 arch/avr32/lib/board.c |4 +
 board/atmel/atngw100/atngw100.c|   15 
 board/atmel/atstk1000/atstk1000.c  |   15 
 board/earthlcd/favr-32-ezkit/favr-32-ezkit.c   |   15 
 board/mimc/mimc200/mimc200.c   |   20 +
 board/miromico/hammerhead/hammerhead.c |   15 
 include/configs/atngw100.h |3 +
 include/configs/atstk1002.h|3 +
 include/configs/atstk1003.h|3 +
 include/configs/atstk1004.h|3 +
 include/configs/atstk1006.h|3 +
 include/configs/favr-32-ezkit.h|3 +
 include/configs/hammerhead.h   |3 +
 include/configs/mimc200.h  |3 +
 19 files changed, 267 insertions(+), 11 deletions(-)
 create mode 100644 arch/avr32/cpu/at32ap700x/mmu.c
 create mode 100644 arch/avr32/include/asm/arch-at32ap700x/mmu.h

diff --git a/arch/avr32/cpu/at32ap700x/Makefile 
b/arch/avr32/cpu/at32ap700x/Makefile
index 46e6ef6..30ea925 100644
--- a/arch/avr32/cpu/at32ap700x/Makefile
+++ b/arch/avr32/cpu/at32ap700x/Makefile
@@ -24,7 +24,7 @@ include $(TOPDIR)/config.mk
 
 LIB:= $(obj)lib$(SOC).a
 
-COBJS  := portmux.o clk.o
+COBJS  := portmux.o clk.o mmu.o
 SRCS   := $(SOBJS:.o=.S) $(COBJS:.o=.c)
 OBJS   := $(addprefix $(obj),$(SOBJS) $(COBJS))
 
diff --git a/arch/avr32/cpu/at32ap700x/mmu.c b/arch/avr32/cpu/at32ap700x/mmu.c
new file mode 100644
index 000..c3a1b93
--- /dev/null
+++ b/arch/avr32/cpu/at32ap700x/mmu.c
@@ -0,0 +1,78 @@
+#include common.h
+#include asm/arch/mmu.h
+#include asm/sysreg.h
+
+void mmu_init_r(unsigned long dest_addr)
+{
+   uintptr_t   vmr_table_addr;
+
+   /* Round monitor address down to the nearest page boundary */
+   dest_addr = PAGE_ADDR_MASK;
+
+   /* Initialize TLB entry 0 to cover the monitor, and lock it */
+   sysreg_write(TLBEHI, dest_addr | SYSREG_BIT(TLBEHI_V));
+   sysreg_write(TLBELO, dest_addr | MMU_VMR_CACHE_WRBACK);
+   sysreg_write(MMUCR, SYSREG_BF(DRP, 0) | SYSREG_BF(DLA, 1)
+   | SYSREG_BIT(MMUCR_S) | SYSREG_BIT(M));
+   __builtin_tlbw();
+
+   /*
+* Calculate the address of the VM range table in a PC-relative
+* manner to make sure we hit the SDRAM and not the flash.
+*/
+   vmr_table_addr = (uintptr_t)mmu_vmr_table;
+   sysreg_write(PTBR, vmr_table_addr);
+   printf(VMR table @ 0x%08x\n, vmr_table_addr);
+
+   /* Enable paging */
+   sysreg_write(MMUCR, SYSREG_BF(DRP, 1) | SYSREG_BF(DLA, 1)
+   | SYSREG_BIT(MMUCR_S) | SYSREG_BIT(M) | SYSREG_BIT(E));
+}
+
+int mmu_handle_tlb_miss(void)
+{
+   const struct mmu_vm_range *vmr_table;
+   const struct mmu_vm_range *vmr;
+   unsigned int fault_pgno;
+   int first, last;
+
+   fault_pgno = sysreg_read(TLBEAR)  PAGE_SHIFT;
+   vmr_table = (const struct mmu_vm_range *)sysreg_read(PTBR);
+
+   /* Do a binary search through the VM ranges */
+   first = 0;
+   last = CONFIG_SYS_NR_VM_REGIONS;
+   while (first  last) {
+   unsigned int start;
+   int middle;
+
+   /* Pick the entry in the middle of the remaining range */
+   middle = (first + last)  1;
+   vmr = vmr_table[middle];
+   start = vmr-virt_pgno;
+
+   /* Do the bisection thing */
+   if (fault_pgno  start) {
+   last = middle;
+   } else if (fault_pgno = (start + vmr-nr_pages)) {
+   first = middle + 1;
+   } else {
+   /* Got it; let's slam it into the TLB */
+   uint32_t tlbelo

Re: [U-Boot] [PATCH 0/3] avr32 fixes

2010-08-02 Thread Haavard Skinnemoen
Bas Mevissen ab...@basmevissen.nl wrote:
 On Mon,  2 Aug 2010 14:06:26 +0200, Haavard Skinnemoen
 haavard.skinnem...@atmel.com wrote:
  This series fixes a trivial build issue, as well as a longstanding
  problem with the 'saveenv' command on ATNGW100.
  
 
 Is that the same problem that makes flash erase actions fail with the
 error that the start or end address does not match a sector boundary?

Yes, that's the one.

  Hopefully this will make mainline u-boot useful again on AVR32. Without
  this fix, v2008.10 is the latest usable release.
 
 
 I had a problem with this version as well when the environment was empty.
 It said the flash was 4Gig and kept crashing with some DMA error (NGW100
 board).

Ok, I wasn't aware of that problem. I don't think I've ever tried
running u-boot with empty environment; in fact, I'm not even sure how
you do that since it will fall back to a default environment if it
can't find anything in flash.

  The patches are based on v2010.06, but it merges fine with the latest
  upstream master. The AVR32 master branch currently contains a workaround
  which I plan to revert if these patches are acceptable.
 
 I'm happy to test them on my board. What a coincidence with my (first)
 mailing list post from earlier today.

That would be awesome. I don't really follow the mailing list very
closely these days, so I didn't see your post.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] Resigning as the AVR32 custodian (was Re: AVR32 / ATNGW100 FLASH adressing issues)

2010-06-01 Thread Haavard Skinnemoen
Wolfgang Denk w...@denx.de wrote:
  I think that the issue should be fixed by making sure FLASH is detected 
  at 0x and NOT at 0xa000, correct?  
 
 This has been discussed in one of the longer and more heated
 discussions on that list; see thread starting here:
 http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/66904

Right...and since we never reached an agreement, and I still have no
idea about how to bring the most popular AVR32 board into working
shape, it seems kind of pointless for me to continue as the AVR32
custodian. In fact, I should probably have resigned a long time ago,
but this kind of failure is not easy to admit.

Please remove me from the list. I'm sorry it didn't work out.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] Virtual addresses, u-boot, and the MMU

2009-09-04 Thread Haavard Skinnemoen
Becky Bruce becky.br...@freescale.com wrote:
  I'm not really deep enough in the implementation details and thus
  would appreciate comments for example from Becky and Stefan. In my
  opinion, turning on or off the cache on an address range should be
  implemented as outlined above, i. e. as an operation changing the
  caching properties of the address range.  
 
 This makes sense to me.  The disable function would need to flush the  
 range from the cache, but that's the only difficulty I forsee.   
 However, I dug up some AVR32 docs, and it looks like the whole dual  
 cacheable/CI mapping thing may be architectural.  The architecture  
 seems to specify a virtual memory map for privileged state, and part  
 of the VA range is not translated by the MMU, but has a default  
 translation.  There appear to be segments in the untranslated VA space  
 that map to the same PA, one cacheable and the other not.

Yes, that is correct. As Andrew pointed out, MIPS does essentially the
same thing, and I _think_ some versions of SH have a similar setup as
well. This makes it easy to set up uncached access on certain physical
addresses without wasting any TLB entries.

On the other hand, turning off the cache entirely is a quite
complicated operation which involves accessing the memory-mapped dcache
tag memory and marking everything as allocated and invalid. So I'd
rather not do that, especially when there's a much easier way.

Another alternative is to enable paging, which will allow fine-grained
control over caching properties. It's probably not all that difficult
to do, but it just seems so _pointless_.

Also, what I don't get is that your architecture _also_ needs to remap
physical to virtual addresses, but for a different reason, so what is
wrong with having an API that can do both?

In other words, the map_physmem() API can support the following three
classes of architectures:
  - Architectures which don't need address remapping but do need to
change caching properties: Just update the caching properties and
return the physical address unchanged.
  - Architectures like AVR32 which change caching properties through
the MMU: Return a new virtual address with the desired properties.
  - Architectures with PAVA: Update the caching properties if
necessary and return a temporary virtual address corresponding to
the given physical address, allowing the entire physical address
range of the CPU to be made available to drivers utilizing this API.

What is wrong with this API? Why are everyone so hell-bent on making it
difficult for the last two classes of architectures? Did I get any or
all of the scenarios above wrong?

  Using a completely different address range instead is a different
  thing, and not what I have in mind. I really dislike the idea of
  supporting alternate addresses in this context - even if this is
  what would be easiest to implement on some architectures.  
 
 I agree, but, then, I'm coming from an architecture where doing this  
 is bad joo-joo.  Again, it looks like AVR may actually need this -  
 hopefully someone who knows more about AVR will speak up here.

I've said what I intend to say about this. I find it really hard to
argue about opinions which are not backed by facts. The alternate
addresses are there whether we decide use them or not, but not using
them makes what should be a trivial operation really hard to do.

Oh, and I'm really sorry about the tone I used a couple of days ago. I
deliberately stopped posting for a few days after that...but I'm hoping
we can all continue working towards a solution now.

Btw, I do have a few suggestions on how to resolve this, but I'm not
going to come forward with them as long as I'm being stonewalled on the
VA/PA mapping issue.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] Virtual addresses, u-boot, and the MMU

2009-09-04 Thread Haavard Skinnemoen
Mark Jackson mpfj-l...@mimc.co.uk wrote:
 The functions could also return (architecture dependant) a remapped
 address to be used, then we could support:-

Right, and that is the important part: If I'm allowed to return a
remapped address, this API will be trivial to implement on AVR32. If
not, it will be quite difficult (and make everything larger and slower).

Your idea is good; it's mostly similar to what map_physmem() does
today, but perhaps the name is wrong, and I suppose it should probably
flush the caches in addition to just remapping the address.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR

2009-09-01 Thread Haavard Skinnemoen
Wolfgang Denk w...@denx.de wrote:
 Dear Haavard Skinnemoen,
 
 In message 20090831155327.62b58...@hskinnemoen-d830 you wrote:
 
  Possibly. But it means even more effort and even larger code, so I'm
  not exactly happy about it.
 
 Really? Sorry if I'm asking dumb questions - I don't know  AVR32:  is
 it  true  that  stting  up a non-1:1 mapping for the NOR flash (i. e.
 what you are doing now) is easier (less code) than setting up  a  1:1
 mapping? What exactly are the reasons for this?

Like I explained in an earlier mail, the default setup includes a 1:1
mapping of the lowest addresses, but it's cacheable. The default setup
also includes an uncached mapping of the same memory at a higher
virtual address.

Yes, it is much easier (and smaller) to use the default virtual memory
layout than setting up paging (which involves several exception
handlers).

   Indeed I am, and intentionally, because this is a different topic. If
   your system requires to set up the MMU to enable  caching,  then  you
   are  supposed  to  do  it  in  a  way compatible with the rest of the
   software design, i. e. as transparently  as  possible.  None  of  the
   architectures  I  know resonably well have problems setting up a 1: 1
   address mapping even when the MMU is on (but I  have  to  admit  that
   AVR32 is not among these architectures).
  
  I suspect quite a few other architectures run with caches disabled
  because it's not safe to run with caches enabled with the current
  software design.
 
 Well, usually we run with IC on and  with  DC  off,  usually  because
 quite  a  number  of  drivers  and  other  code do not use proper I/O
 accessors yet, and/or because it's easier and  nobody  really  cares.
 For  example  on  PowerPC,  adding support for the data cache usually
 gives only a minimal performance boost.  This  may  be  different  on
 other architectures.

Ok, so the code is broken and nobody else cares?

I suppose I could disable the DC (which is a bit complicated, but
possible), but that would just add to the already high cost (in terms
of both code size and performance) of using common code (i.e. the CFI
driver), so I'm leaning towards a custom flash driver instead.

   Cache should not be relevant  at  all  when  defining  a  physcal  or
   virtual memory map.
  
  Physical, no, but it's very common that the MMU defines caching
  properties (enabled/disabled, writeback/writethrough, etc.)
 
 Agreed. But it should be not so difficult to use the MMU to set up  a
 1:1  mapping  if  you have to set up the MMU at all - or is there any
 problems with that which I'm not aware of?

Yes, there is: caching.

   Heh. If more  platforms had broken this rule we would probably have
   become aware of these violations earlier, and stopped them doing such
   naughty things ;-)
  
  Seems like you think it's more important to follow arbitrary rules than
  writing code that works well.
 
 Keeping the code as simple as possible is not exactly an arbitrary
 rule. At least not for me.

As simple as possible, but no simpler...

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR

2009-09-01 Thread Haavard Skinnemoen
Becky Bruce becky.br...@freescale.com wrote:
 Sorry, guys, I'm still not clear on what's going on. Haavard, did you  
 expect each call to flash_map to return a different VA each time (i.e.  
 you're trying to do some sort of dynamic mapping), or are you just  
 calling it to get the VA that corresponds to some PA, since VA != PA  
 on these 2 boards?

I'm calling it to get some VA which corresponds to a given PA with
given caching properties. I don't think it's good design if the board
code for every single board needs to know exactly which VA is going to
be returned.

  But exactly what's wrong with hiding all this complexity inside
  map_physmem()? It was designed _exactly_ for this purpose -- to allow
  platforms with non-trivial mappings between VA and PA to do the
  necessary mapping when the driver requests it.
 
  Sorry, I don't know that code and it's users well enough to coment.
  Maybe Becky and/or Stefan can shed some light on this...
 
 The problem is that would mean the CFI driver would be treating start  
 as a PA, while every other flash driver that I looked at treats it as  
 a VA.  That kind of inconsistency would be bad.

Except that it's even more inconsistent now since lots of code
elsewhere treats start as a PA, and the CFI driver was the only driver
which did it mostly correctly.

 Plus, Wolfgang,  
 Stefan, Kumar, and I discussed this on the list/IRC last november and  
 agreed that it made sense for command-line foo (including the flash  
 commands)  to take a virtual address as the argument.  Platforms that  
 have a non-fixed memory map would need to provide a command-line  
 interface to get a VA to use (since that's highly unusual and expected  
 to remain so).

I think that's an incredibly stupid idea, and it's really a shame that
I didn't participate in the IRC meeting.

It would have been so much easier if the command line only had to deal
with PA and the platform had to remap things transparently if
necessary. Now, if you need to access an arbitrary physical address
through a script, you need to first call this set up a mapping
command, record the value it prints out (presumably) and make sure not
to interpret any error message as a virtual address, use it to perform
the command, and then unmap it afterwards.

So if anything is breaking the u-boot philosophy of simplicity it is
this, and it does it in the worst possible way: it exposes all the
complexity through the user interface!

  That PA==VA rule is pretty much the reason we're in this mess -- if
 
  Let's say, the fact that this rule has not been stricter enforced has
  caused that teh appearant problems were not discovered earlier.
 
  more platforms had broken this rule, perhaps more of the code would
 
  Heh. If more  platforms had broken this rule we would probably have
  become aware of these violations earlier, and stopped them doing such
  naughty things ;-)
 
 Well, u-boot was supposed to be simple, and a VA=PA assumption ended  
 up built into a lot of the code.  Which isn't a problem until you need  
 something different  I had a lot of fun standing on my head trying  
 to get 36-bit physical addressing on PowerPC working as a result of  
 this.   I suspect the next big u-boot problem will be the need for  
 dynamic mappings, because we're assuming for the most part now that  
 the board memory map is static.

Exactly, and with the current approach, dynamic mappings will _never_
be possible because all the VA-to-PA mapping assumptions will be built
into both the board code and the user interface!

So I ask again: Why don't we just fix the broken code instead? We have
the mechanisms available to make this work via the map_physmem()
functions, we just need to use them.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR

2009-09-01 Thread Haavard Skinnemoen
Becky Bruce becky.br...@freescale.com wrote:
  Becky: the fact that Haavard's code is breaking is not considered an
  indication of a deficiency in his code.  
 
 I'm not calling the code deficient, just a bit inconsistent, and it  
 wasn't clear to me why.  When code uses the same value 1) by mapping  
 it and 2) by dereferencing it directly, that's a bit strange.  Why map  
 it in some cases and in not others?

I agree, that is inconsistent. However, you fixed the code which was
actually doing it correctly...we should instead fix the code which is
incorrectly using the physical address as a virtual one.

While doing that, we could also consider dropping that hideous start
array altogether and instead provide a handful of accessors for
locating sector start addresses on the flash.

  How is that supposed to work when  
 VA!=PA or when the VA can change? This is one of the reasons that it  
 seemed to make sense to modify the driver as I did - it should now be  
 able to work when VA!=PA as well as when we're 1:1.  I could find no  
 users that seemed to need the dynamic mapping.  However, if anyone  
 does need to *dynamically* map the flash in and out with a different  
 VA each time, then we do need to do things a bit differently and we  
 should look into a solution for that.

I don't think anything needs a dynamic mapping right now, but if we're
going to _ever_ support such systems in the future (and your 36-bit PA
system is a likely candidate, isn't it?) we have to stop locking
ourselves into a static mapping.

  Clearly, I'm not the expert on  
 the CFI code, so when I published that patch I expected someone to  
 smack me if I was being a moron :)

I apologize for not doing so earlier ;-)

Anyway, I have to take most of the blame for this situation...if I had
paid attention and flagged the problem earlier, much of this could have
been avoided.

I'm hoping we can avoid too much pointing of fingers and work towards a
reasonably future-proof solution which works well on all platforms.

  If the CFI driver kind of allowed for VAs before (but incompletely /
  incorrectly), then this dis not cause problems on any systems using a
  strict 1:1 mapping.
 
  Any changes to the code to correctly support other mappings must be
  done in a way that they (1) do not break and (2) do not add additional
  burdon on systems with a simple 1:1 mapping.  
 
 Agreed, there shouldn't be any burden on those systems.

Agree too, as long as those systems does not include common code which
needs to run on all systems.

  Everything is treated as virtual unless it's being used for hardware
  setup.  
 
  Thisis NOT correct. U-Boot usually does NOT use virtual addresses.
  Only very few systems do, and these must care not to disturb the
  majority of systems which do no need to differentiate between
  physical and virtual addresses.  
 
 I'm not saying it *is* a VA as far as U-Boot knows, but that it is  
 *treated* as one, as mentioned above.   And this code was not expected  
 to disturb the 1:1 case.

Exposing VAs to the user interface and board code is IMO a very bad
idea, however. And that is what's happening now -- instead of
localizing the VAs to the CFI flash driver, it is now spread all over
the place.

  A
  lot of code had been just using the PA as a VA, because things were
  always mapped 1-1.  
 
  Not only were, but _are_ and _will_be_.  
 
 Of course - that should continue to be the default case unless it  
 needs to differ for some reason.

Yes, and that's why the external interface (at least the command line
and board configuration files) needs to use PA.

  Indeed I'm deeply trouble when log standing rules get silently bent
  and even broken.  
 
 I agree 100% that 1:1 support should not be disturbed, since that's  
 the default case.   There was absolutely no intent here to bend any  
 logsic ;-) standing rules, and nothing has been done here that  
 should have any impact on 1:1 as far as I'm aware.

Agree, 1:1 isn't the issue, it's two different systems with !1:1 which
have started making incompatible changes to the CFI driver.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR

2009-09-01 Thread Haavard Skinnemoen
Stefan Roese s...@denx.de wrote:
 On Tuesday 01 September 2009 10:57:52 Haavard Skinnemoen wrote:
   Well, usually we run with IC on and  with  DC  off,  usually  because
   quite  a  number  of  drivers  and  other  code do not use proper I/O
   accessors yet, and/or because it's easier and  nobody  really  cares.
   For  example  on  PowerPC,  adding support for the data cache usually
   gives only a minimal performance boost.  This  may  be  different  on
   other architectures.
 
  Ok, so the code is broken and nobody else cares?
 
 I wouldn't put it like this. The CFI driver assumes that the FLASH mapping is 
 not cached. This makes perfect sense in my point of view.

Yes, and it can do that safely because it specifically asks for an
uncached address from map_physmem(). That part didn't actually change
with the patch in question -- the problem is that the address returned
by map_physmem() is now exposed through external interfaces -- it's not
just an internal implementation detail anymore.

  I suppose I could disable the DC (which is a bit complicated, but
  possible), but that would just add to the already high cost (in terms
  of both code size and performance) of using common code (i.e. the CFI
  driver), so I'm leaning towards a custom flash driver instead.
 
 On some 440 platforms we configure the FLASH cached upon powerup, and disable 
 the caching after relocating to SDRAM. So when the CFI driver is started, the 
 FLASH is not cached any more, but we have the cache speedup upon relocation.

That's what I want to do as well! And I can actually do that as long as
the flash subsystem uses map_physmem() consistently.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR

2009-09-01 Thread Haavard Skinnemoen
Wolfgang Denk w...@denx.de wrote:
 Dear Haavard Skinnemoen,
  Like I explained in an earlier mail, the default setup includes a 1:1
  mapping of the lowest addresses, but it's cacheable. The default setup
  also includes an uncached mapping of the same memory at a higher
  virtual address.
 
 You mean you want to have the same memory area mapped _twice_, once
 cached and once (at another address) uncached?

Yes.

 Well, this obviously cannot be done with a plain 1:1 mapping. But
 then: isn't that asking for trouble anyway? Or is there anything that
 prevents you for example reading stale cached data after the memory
 content has changed by accesses through the uncached mapping?

There's nothing which prevents me from accessing a completely different
address either -- I just need to make sure that I access the correct
address, or things will blow up one way or another.

The default virtual memory model makes it very easy to do uncached
access to certain types of memory (i.e. flash) and cached access to
others (i.e. SDRAM). It also makes it easy to run from cached flash
memory at startup and switch to uncached access later (after flushing
the cache, of course).

  Yes, it is much easier (and smaller) to use the default virtual memory
  layout than setting up paging (which involves several exception
  handlers).
 
 I don't see how paging comes into play here?

If I can't use the default scheme described above (and you're pretty
much saying I can't), I can define caching properties on a page-by-page
basis, but that obviously requires paging to be enabled.

  Ok, so the code is broken and nobody else cares?
 
 Broken? You might call it a design decision. This is a boot loader,
 and simplicity of design and easy porting and board bring up are
 usually higher rated that sequeezing out the last few percent of
 performance.

And that is EXACTLY why I'm trying to advocate: Keep the additional
complexity (which can be kept to a minimum) localized to a handful of
drivers, and don't change the command line interface or the board
configuration interface.

 IIRC, on PowerPC the difference of running with DC
 enabled or not is only in the 10% range or so.

But as a matter of principle, I don't want to reduce the performance
_and_ increase the code size just because a driver that used to work
got broken. I want to fix the driver instead!

  I suppose I could disable the DC (which is a bit complicated, but
  possible), but that would just add to the already high cost (in terms
  of both code size and performance) of using common code (i.e. the CFI
  driver), so I'm leaning towards a custom flash driver instead.
 
 If you want to run with data caches enabled by default, then it would
 probably make more sense if you invested efforts in extending the CFI
 driver to provide hooks / callbacks to (temporarily) switch of data
 cache for the memory range in question. This wouls allow you to run
 with DC enabled on the flash area, and still use the CFI driver.

But that is exactly how map_physmem() works -- it allows the CFI driver
(or any other driver) to request the caches to be bypassed for a given
physical address range, possibly resulting in a different virtual
address (though for backwards compatibility, all platforms except AVR32
simply return the physical address unchanged.)

The problem is that this virtual address (which currently isn't
dynamic, but it's easy to imagine a platform where it could be) is
exposed to the rest of the world, thus breaking both existing board
configurations and potentially any command-line scripts (and,
apparently, the jffs2 driver though I'm not entirely sure how that
happened.)

 Such changes will have it much easier to find their way into mainline
 than adding a proprietary flash driver.

It certainly won't be a proprietary driver; if anything, it would be a
variant of the driver currently used by ATSTK1000.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR

2009-09-01 Thread Haavard Skinnemoen
Wolfgang Denk w...@denx.de wrote:
 Dear Haavard Skinnemoen,
 
 In message 20090901123829.55fcb...@hskinnemoen-d830 you wrote:
 
  And that is EXACTLY why I'm trying to advocate: Keep the additional
  complexity (which can be kept to a minimum) localized to a handful of
  drivers, and don't change the command line interface or the board
  configuration interface.
 
 We're not doing this. At least not intentionally.

Good. Then let's please put a stop to the madness of exposing virtual
addresses all over the place.

 The discussion we had was based on our knowledge about existing
 systems, and needs to also handle more complex situations like for
 example 32 bit systems with 36 bit physical addresses.

As far as I understand, the only difference for such systems is that
keeping 64-bit physical addresses around are a bit more costly than
passing around 32-bit virtual pointers. But presumably those systems
have enough memory to make it a non-issue...

  But as a matter of principle, I don't want to reduce the performance
  _and_ increase the code size just because a driver that used to work
  got broken. I want to fix the driver instead!
 
 Agreed - assuming it is possible without hurting the majority of
 other existing configurations.

Yes, that is indeed possible, as evidenced by the fact that it used to
work without hurting any other configurations. It took another special
case to break it.

   If you want to run with data caches enabled by default, then it would
   probably make more sense if you invested efforts in extending the CFI
   driver to provide hooks / callbacks to (temporarily) switch of data
   cache for the memory range in question. This wouls allow you to run
   with DC enabled on the flash area, and still use the CFI driver.
  
  But that is exactly how map_physmem() works -- it allows the CFI driver
  (or any other driver) to request the caches to be bypassed for a given
  physical address range, possibly resulting in a different virtual
  address (though for backwards compatibility, all platforms except AVR32
  simply return the physical address unchanged.)
 
 I have to admit that I have no idea how map_physmem() used to work or
 how it works now; at this point, I don;t care about implementation
 details, I try to focus only on the overall design.

It works exactly the same way now as it used to work. The difference is
that its return value (which is basically just an architecture-specific
cookie, aka. virtual address) is exposed to a much larger part of the
system.

 I think  your  double  mapping  is  a  hightly  architecture-specific
 feature  which I do not like at all, but if you are happy with it and
 it works for you  I  cannot  and  will  not  prevent  it.

Yes, it was always meant to be an architecture-specific thing. Though I
think some MIPS- and SH-based processors do something very similar.

But in order to utilize architecture-specific features, we need an
architecture-independent abstraction and that's basically what
map_physmem() is.

  But  after
 discussions  with  Stefan Roese and Detlev Zundel it seems to me that
 map_physmem() is probably not the right approach (judging  only  from
 the  name).  We  should  not try to fix cache attributes by modifying
 addresses / address maps

And why not? That's what Linux does, and it works wonderfully across 26
different architectures.

 Instead, we should really extend the CFI driver such that it does not
 matter if the addresses you are passing refer to cached or uncached
 memory, and then provide hooks in the CFI driver to allow for testing
 if cache is enabled, and switching cache off if it is.

What's the advantage of such an approach? It sounds much more
complicated from the driver's point of view as well.

 Detlev Zundel suggested to use this as  a  chance  to  come  up  with
 something  like a cache API which then could be used by other drivers
 as well.

My suggestion is to use map_physmem() and unmap_physmem(). It exists
today, and it works, provided that we keep its usage internal to the
driver and don't expose whatever architecture-specific values it
returns.

 To me such an approach makes much more sense, as it  is  generic  and
 can be used by other drivers and other architectures - even if it may
 come at the cost of more effort on your systems.

In what way is it more generic? In what way can map/unmap_physmem() not
be used with other drivers and architectures?

   Such changes will have it much easier to find their way into mainline
   than adding a proprietary flash driver.
  
  It certainly won't be a proprietary driver; if anything, it would be a
  variant of the driver currently used by ATSTK1000.
 
 Well, but board/atmel/atstk1000/flash.c _is_ a proprietary driver for
 some flash chips that seem to be CFI conformant at first glance.  You
 would not get such a driver into mailine any more. 

So I guess dropping support for ATNGW100 is the only remaining option?
At least STK1000 has a working flash driver

Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR

2009-09-01 Thread Haavard Skinnemoen
Wolfgang Denk w...@denx.de wrote:
 Dear Haavard Skinnemoen,
 
 In message 20090901134257.59961...@hskinnemoen-d830 you wrote:
 
complexity (which can be kept to a minimum) localized to a handful of
drivers, and don't change the command line interface or the board
configuration interface.
   
   We're not doing this. At least not intentionally.
  
  Good. Then let's please put a stop to the madness of exposing virtual
  addresses all over the place.
 
 But that's what we've been doing all the time. You just did not notice
 it because of the usual 1:1 mapping.

Up until this commit, yes:

commit 09ce9921a7d8b1ce764656b14b42217bbf4faa38
Author: Becky Bruce bec...@kernel.crashing.org
Date:   Mon Feb 2 16:34:51 2009 -0600

flash/cfi_flash: Use virtual sector start address, not phys

after that, NGW100 support is broken because virtual addresses are no
longer an implementation detail and are being exposed all over the
place.

 On a 32 bit system with 36 bit physical addresses you cannot use a
 physical address when running the md command, for example.

Yes you can, if you teach the md command to map it at a virtual
address first. Again, map_physmem() makes this possible without
changing the external interface in any way.

 we
 always assumed that the 32 bit VA we used matched 1:1 to a PA with
 the missing high bits set to 0, right?

Yes, but how can you possibly access an arbitrary 36-bit address using
that setup?

   The discussion we had was based on our knowledge about existing
   systems, and needs to also handle more complex situations like for
   example 32 bit systems with 36 bit physical addresses.
  
  As far as I understand, the only difference for such systems is that
  keeping 64-bit physical addresses around are a bit more costly than
  passing around 32-bit virtual pointers. But presumably those systems
  have enough memory to make it a non-issue...
 
 That's not true. There are 32 bit systems with bigger physical
 address spaces. 

Which part of what I said isn't true? The part about some systems might
require 64-bit variables to store a physical address or that such
variables take more storage than a 32-bit virtual address?

 And note that this is not a new thing. We have been doing this allt
 he time - just without ever explicitly mentioning it, because so far
 nobody ever complained about it.

Doing what exactly? Limiting 36-bit PA systems to only use the lower
4GB of memory because VA must always equal PA come hell or high water?

   Agreed - assuming it is possible without hurting the majority of
   other existing configurations.
  
  Yes, that is indeed possible, as evidenced by the fact that it used to
  work without hurting any other configurations. It took another special
  case to break it.
 
 My understanding is that the special case is yours - using a non-1:1
 mapping.

But the other system must be a special case too -- otherwise it would
work fine without commit 09ce9921a7d8b1ce764656b14b42217bbf4faa38. That
commit does not make any difference whatsoever on 1:1 systems.

If the other system isn't a special case, why don't we just revert that
commit?

   discussions  with  Stefan Roese and Detlev Zundel it seems to me that
   map_physmem() is probably not the right approach (judging  only  from
   the  name).  We  should  not try to fix cache attributes by modifying
   addresses / address maps
  
  And why not? That's what Linux does, and it works wonderfully across 26
  different architectures.
 
 We do not want to implement a full-fledged OS with virtual memory and
 on-demand paging and all that stuff in U-Boot.

Then why are you forcing me to implement it for AVR32?!?

   Instead, we should really extend the CFI driver such that it does not
   matter if the addresses you are passing refer to cached or uncached
   memory, and then provide hooks in the CFI driver to allow for testing
   if cache is enabled, and switching cache off if it is.
  
  What's the advantage of such an approach? It sounds much more
  complicated from the driver's point of view as well.
 
 The advantage is that other drivers with similar needs can use it as
 well.

But they can use map_physmem() today! It allows you to specify exactly
what caching properties you need. The fact that it _may_ return a
virtual address which is different from the physical one just allows
more flexibility in how the architecture chooses to implement it!

   To me such an approach makes much more sense, as it  is  generic  and
   can be used by other drivers and other architectures - even if it may
   come at the cost of more effort on your systems.
  
  In what way is it more generic? In what way can map/unmap_physmem() not
  be used with other drivers and architectures?
 
 On other architectures it is not possible to map the same memory area
 multiple times with different attributes.

So what? They can just return the physical address unchanged. It's not
_mandatory_ to remap anything...

  Remapping  addresses

Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR

2009-09-01 Thread Haavard Skinnemoen
Wolfgang Denk w...@denx.de wrote:
 Well, that was the part of me saying before: as long as it doesn't
 hurt others. We don't want to add that complexity to U-Boot as noone
 needs it.

Right. I forgot that nobody actually needs this.

Fuck it, I'm out.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR

2009-09-01 Thread Haavard Skinnemoen
Thiago A. Corrêa thiago.cor...@gmail.com wrote:
 Hi,
 
I don't want to intrude too much into the discussion, but I would
 like to offer a small bit of info

Oh, I wish more people would intrude ;-)

 On Tue, Sep 1, 2009 at 10:23 AM, Haavard
 Skinnemoenhaavard.skinnem...@atmel.com wrote:
  It would be a good idea to clean up this board  support,  remove  the
  proprietary  flash driver and use the CFI driver instead. Patches are
  welcome.
 
  You must be joking. Replacing a working driver with a broken one? Why
  the hell would anyone do that?
 
 
 My custom board uses AT49BV640D. I wen't thru a lot of trouble getting
 u-boot to work with it. And one of my attempts was to use the working
 driver from stk1000 and it didn't work.

Understandably since 640D uses the intel command set while the 6416
chip on STK1000 uses the AMD command set and has a couple of
interesting bugs...

Now, I still want to use common code as much as possible, but it's
always been quite expensive in terms of code size, and it currently
doesn't even work. While both of those should be possible to overcome,
I'm getting incredibly frustrated that all my attempts at fixing it are
being shot down by arcane, non-sensical rules which aren't even being
enforced consistently.

 On the other hand, the CFI
 driver with the tripple revert that Haavard proposed did. I'm
 currently patching it like that so I can continue my development,
 while people with much more knowleadge than I have on u-boot code
 could fix the issue, but looks like I'm about to get orphan on u-boot
 and Atmel.

Right...I'm beginning to doubt that anyone is familiar enough with the
u-boot code, since everyone seems to have their own opinion about how
things are supposed to work.

To summarize, here are the possible ways to fix the problem as I see it:
  - Use virtual address in CONFIG_ENV_ADDR to conform with the way the
CFI driver currently works. Rejected by Wolfgang because virtual
addresses don't exist.
  - Fix the API and user interface breakage by reverting commit
09ce9921. Rejected because virtual addresses are used everywhere.
  - Fix it by using map_physmem() in a way that works for all
architectures. Rejected because all other architectures than PPC
are evil and need to be punished for doing things differently.
  - Introduce a custom flash driver for ATNGW100. Rejected because
stupid principles are more important than making things work.

So I don't really know where to proceed from here. I guess two
additional options are forking the damn thing or creating a proprietary
bootloader, but I don't really want to do either.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [GIT PULL] AVR32: NGW100 fix for 2009.08

2009-08-31 Thread Haavard Skinnemoen
Hans-Christian Egtvedt hans-christian.egtv...@atmel.com wrote:
  Yeah...I'm unsure myself. The system will boot, but the 'saveenv'
  command doesn't work...so while I really want to fix this issue
  _properly_, I'm not sure if there's enough time to do that before the
  next release.

 
 Did you test loading something from JFFS2? I have a bad feeling it
 might still be broken.

Right. Does anyone have any clue about why it's broken?

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR

2009-08-30 Thread Haavard Skinnemoen
On Fri, 28 Aug 2009 14:58:15 -0500
Becky Bruce becky.br...@freescale.com wrote:
 FYI, before the patch, the CFI driver was sometimes doing the map,
 but IIRC it was also abusing the physical address, treating it as
 a virtual address without mapping it.

In that case, those places should have been fixed, no?

  The only way for that to work
 is when VA=PA (or, depending on what you were doing with the address,
 you just got lucky).  The CFI driver was the outlier - all the other
 flash code was treating the start field as a VA already.  So I don't
 think just  reverting the patch is the answer.

Except for everything _outside_ the flash code which still deals with
physical addresses, like the environment stuff and JFFS2. The flash
code takes those addresses and compares them with the virtual addresses
in the start array, and things break.

  So...which config symbols are supposed to be virtual now, and how
  are you supposed to know how the virtual-to-physical mappings are
  set up in
  advance?
 
 Everything is treated as virtual unless it's being used for hardware  
 setup.

Exactly what constitutes hardware setup?

  If you use something to do memory accesses, it's virtual.

Yes, but then the address should also be in a pointer, not an unsigned
long which the flash 'start' array is.

  A  
 lot of code had been just using the PA as a VA, because things were  
 always mapped 1-1.

Yes, there's lots of code which is broken in that respect...

 Can you point me at an example in your scenario of code that
 interacts with the flash?

CONFIG_ENV_ADDR is used to store the environment in CFI flash. Reading
the environment works OK-ish since the flash is accessible through a
cacheable 1:1 mapping from virtual/physical address 0. However, when
writing and erasing, the physical address stored in CONFIG_ENV_ADDR
appears to be outside of the virtual sector addresses stored in the
'start' array, so the flash code throws an error.

There are basically two ways to fix it: Either go back to using
physical addresses in the flash API, or make CONFIG_ENV_ADDR virtual
(and from what I hear, the jffs2 code needs a similar fix.) This patch
does the latter, but it seems like it doesn't fix things
completely, and Wolfgang didn't appear very happy about it.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR

2009-08-30 Thread Haavard Skinnemoen
On Sat, 29 Aug 2009 13:39:02 +0200
Stefan Roese s...@denx.de wrote:
 I think too, that revering the patch in question is not the right
 solution for this problem in the current stage. But I have to admit
 that I don't have enough insight into your platform to come up with
 another good idea quickly.

Yeah, I only meant to suggest it as one possible solution, with this
patch being one other, less intrusive but also less complete, solution.

And I'm not really even opposed to the patch in question, I just need
to know how I should configure my systems from now on since all of a
sudden, some addresses are supposed to be virtual while others are
physical, and it doesn't seem to be documented anywhere what should be
used where.

   So...which config symbols are supposed to be virtual now, and how
   are you supposed to know how the virtual-to-physical mappings are
   set up in
   advance?
 
  Everything is treated as virtual unless it's being used for hardware
  setup.  If you use something to do memory accesses, it's virtual.  A
  lot of code had been just using the PA as a VA, because things were
  always mapped 1-1.
 
 Correct. All those addresses to configure the CFI driver should be
 virtual addresses.

That cannot possibly be true, as the start address of the flash is
passed to physmem_map().

It is also an absolutely idiotic thing to do since on any CPU with
a MMU, the virtual address can be anything, so locking all the
configuration symbols to specific virtual addresses would effectively
lock down the MMU configuration forever (and I imagine that would be a
particularly nasty problem on hardware with more physical address bits
than virtual.)

  Can you point me at an example in your scenario of code that
  interacts with the flash?
 
 Yes, please give us an example of your memory mapping (physical and
 virtual) on your failing platform.

CFI-compatible flash at physical address 0. By default, all virtual
addresses up to 2GB are mapped 1:1 with caching enabled (when paging
is enabled, which it isn't in u-boot, this area is translated through
the MMU.) Then, virtual address 0x8000..0x9fff is mapped to
0x0..0x1fff with caching enabled (supervisor-only and not translated
even with paging enabled; this is where the Linux kernel is mapped).
Then, virtual address 0xa000..0xbfff is mapped to
0x0..0x1fff with caching disabled -- this is where we want to access
the flash and other external hardware. Then there's another paged
region and finally a 512MB uncached, direct-mapped region for accessing
internal peripherals.

So the problem is that even though we may read from the flash using the
physical address as virtual address, we cannot initialize, erase or
write it using those addresses. Hence the need for map_physmem().

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR

2009-08-30 Thread Haavard Skinnemoen
On Sun, 30 Aug 2009 20:11:01 +0200
Wolfgang Denk w...@denx.de wrote:

 Dear Haavard  Becky,
 
 In message 20090830173640.2af9c...@siona you wrote:
 
The only way for that to work
   is when VA=PA (or, depending on what you were doing with the
   address,
 
 Well, VA==PA is the default setting for U-Boot that shall be used for
 all systems, unless it is really impossible on a board to implement an
 exception from that rule.

While not impossible, following that rule on the NGW100 would require
that I either disable the caches (which would result in a massive
slowdown) or start using the MMU more actively to get the caching
properties right.

   you just got lucky).  The CFI driver was the outlier - all the
   other flash code was treating the start field as a VA already.
   So I don't think just  reverting the patch is the answer.
 
 We did not even have any notion of VA's in U-Boot until very
 recently, and I call on everybody not to make U-Boot more complicated
 than necessary.

I don't think any boards with PA==VA are affected. This issue is about
two platforms with VA!=PA following different rules...

 In almost all cases RAM and NOR flash fit easily in the physical
 address space of the CPUs, and for the sake of this majority of
 systems it must be possible to access memory on such systems assuming
 a simple 1:1 mapping.

You're ignoring cache issues.

 Any changes to the code to correctly support other mappings must be
 done in a way that they (1) do not break and (2) do not add additional
 burdon on systems with a simple 1:1 mapping.

That was the idea when I introduced map_physmem() to the CFI driver a
while back: the externally visible addresses were kept unchanged, while
the remapping was done internally. Also, map_physmem() is a no-op on
platforms with a simple 1:1 mapping.

If you use something to do memory accesses, it's virtual.
 
 Unless you have a very special system you can rely on a strict 1:1
 mapping.

Technically, the addresses seen by the CPU are virtual. And I don't
think systems with a cache should be considered 'very special' these
days...

  There are basically two ways to fix it: Either go back to using
  physical addresses in the flash API, or make CONFIG_ENV_ADDR virtual
 
 I understand we cannot do that, because some systems need to map (NOR)
 flash to virtual addresses outside the physical address space. Ok, so
 the CFI driver shall consistently be able to use VAs - but on simple
 systems with a 1:1 mapping there shall be no need in the system
 configuration to spend any thoughts on this.

But exactly what's wrong with hiding all this complexity inside
map_physmem()? It was designed _exactly_ for this purpose -- to allow
platforms with non-trivial mappings between VA and PA to do the
necessary mapping when the driver requests it.

  (and from what I hear, the jffs2 code needs a similar fix.) This
  patch does the latter, but it seems like it doesn't fix things
  completely, and Wolfgang didn't appear very happy about it.
 
 Indeed I'm deeply trouble when log standing rules get silently bent
 and even broken.

And I deeply regret using the CFI driver on NGW100...it took a lot of
effort to get it mostly limping along, and then it got broken at the
first opportunity. I should have stuck with the much smaller and more
efficient board-specific driver I had to begin with.

That PA==VA rule is pretty much the reason we're in this mess -- if
more platforms had broken this rule, perhaps more of the code would
have been written properly without lots of implicit conversion from PA
to VA via ugly casts between unsigned long and all sorts of pointers.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [GIT PULL] AVR32: NGW100 fix for 2009.08

2009-08-28 Thread Haavard Skinnemoen
Hi Wolfgang,

Please pull

  git://git.denx.de/u-boot-avr32.git master

to receive the following fix for a fairly longstanding and annoying
ATNGW100 bug.

Haavard Skinnemoen (1):
  atngw100: Use virtual address in CONFIG_ENV_ADDR

 include/configs/atngw100.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR

2009-08-28 Thread Haavard Skinnemoen
Ever since the CFI driver was rewritten to use virtual addresses, thus
eliminating the whole point of the map_physmem() macro, ATNGW100 has
been broken like this:

U-Boot saveenv
Saving Environment to Flash...
Error: start and/or end address not on sector boundary

So let's take a different approach and store the virtual address in
CONFIG_ENV_ADDR. I personally believe that's rubbish and will break
whenever we decide to change the virtual-to-physical mapping in any way,
but it looks like it's the direction in which u-boot is currently
moving, and it does fix the problem.

Signed-off-by: Haavard Skinnemoen haavard.skinnem...@atmel.com
---
 include/configs/atngw100.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/configs/atngw100.h b/include/configs/atngw100.h
index 4ed5514..9777ec0 100644
--- a/include/configs/atngw100.h
+++ b/include/configs/atngw100.h
@@ -155,7 +155,7 @@
 
 #define CONFIG_ENV_IS_IN_FLASH 1
 #define CONFIG_ENV_SIZE65536
-#define CONFIG_ENV_ADDR(CONFIG_SYS_FLASH_BASE + 
CONFIG_SYS_FLASH_SIZE - CONFIG_ENV_SIZE)
+#define CONFIG_ENV_ADDR(0xa000 + 
CONFIG_SYS_FLASH_SIZE - CONFIG_ENV_SIZE)
 
 #define CONFIG_SYS_INIT_SP_ADDR(CONFIG_SYS_INTRAM_BASE + 
CONFIG_SYS_INTRAM_SIZE)
 
-- 
1.6.0.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR

2009-08-28 Thread Haavard Skinnemoen
Mark Jackson mpfj-l...@mimc.co.uk wrote:
 Haavard Skinnemoen wrote:
  Ever since the CFI driver was rewritten to use virtual addresses, thus
  eliminating the whole point of the map_physmem() macro, ATNGW100 has
  been broken like this:  
 
 How about other boards (like the MIMC200) ?
 
 Aren't *all* AVR32 boards affected in this way ?

Possibly, but NGW100 is the only one which I've seen reports about.
STK1000 is safe since it doesn't use the CFI driver.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR

2009-08-28 Thread Haavard Skinnemoen
Mark Jackson mpfj-l...@mimc.co.uk wrote:
 Haavard Skinnemoen wrote:
  Possibly, but NGW100 is the only one which I've seen reports about.
  STK1000 is safe since it doesn't use the CFI driver.
 
 I did kinda report this in the thread JFFS2 scanning bug, and
 the triple-revert patch you posted on 26/05/09 16:58 appeared
 to fix it.

Ah...so it breaks JFFS2 as well? I doubt that changing the environment
address fixes that...

 Since this didn't change any board files (only the core CFI files)
 I guess I assumed this revert would work its way upstream and I
 wouldn't have to change anything.

Hmm, yeah, maybe I should post the revert again.

I have to admit I'm completely confused about how u-boot deals with
virtual and physical addresses these days. It used to expose only
physical addresses through external interfaces, but now it looks like
it's a bit of both, and it's impossible to tell which goes where.

 Shall I just submit a patch to fix the mimc200 board in the same way
 as your NGW100 patch ?

Yes, that will probably be a good idea if it has the same problem with
saveenv.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR

2009-08-28 Thread Haavard Skinnemoen
Wolfgang Denk w...@denx.de wrote:
   #define CONFIG_ENV_IS_IN_FLASH 1
   #define CONFIG_ENV_SIZE65536
  -#define CONFIG_ENV_ADDR(CONFIG_SYS_FLASH_BASE + 
  CONFIG_SYS_FLASH_SIZE - CONFIG_ENV_SIZE)
  +#define CONFIG_ENV_ADDR(0xa000 + 
  CONFIG_SYS_FLASH_SIZE - CONFIG_ENV_SIZE)  
 
 This is definitely a change to the worse.

Yes, I think so too.

 I feel a strong urge to NAK this. There must be a better way to fix
 the problems you are experiencing.

Yes...the idea behind the map_physmem() macro was to do any
physical-to-virtual address mapping inside the CFI flash driver and
never expose anything but physical addresses to the outside world. This
meant that the sector start addresses were expressed in terms of
physical addresses, matching how things were wired up on the board, and
the architecture was free to map those to any virtual address which is
most suitable in terms of caching properties, etc.

Now, however, the flash start addresses are mapped to virtual addresses
at initialization time, so everything that wants to interact with the
flash must known which address the architecture decided to map the
flash at, which is not necessarily even possible to know in advance if
it is being done dynamically through a hardware MMU.

Reverting 09ce9921a7d8b1ce764656b14b42217bbf4faa38 would bring things
back to the way they were, and fix both the saveenv problem and the
jffs2 problem.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR

2009-08-28 Thread Haavard Skinnemoen
Kumar Gala ga...@kernel.crashing.org wrote:
  Reverting 09ce9921a7d8b1ce764656b14b42217bbf4faa38 would bring things
  back to the way they were, and fix both the saveenv problem and the
  jffs2 problem.  
 
 Such a revert would break other boards that now expect the new  
 behavior (like all the 36-bit physical cfg for FSL boards)

Right, I suspected that.

So...which config symbols are supposed to be virtual now, and how are
you supposed to know how the virtual-to-physical mappings are set up in
advance?

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] asm-generic: Consolidate errno.h to asm-generic/errno.h

2009-07-09 Thread Haavard Skinnemoen
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)

2009-07-07 Thread Haavard Skinnemoen
Mike Frysinger wrote:
  Obviously the second item here will become void if vendor lockout of
  updates becomes common.  So what will be left of the essential freedoms?
  I can study the code, I can modify it, but I am not allowed to run it.
  Excellent.  
 
 and this is why i dislike the GPLv3.  the GPLv2 was all about the source, so 
 the conversation between developers and everyone else was you can take my 
 source and modify it all you want, but i want to see the changes.  sounds 
 fair.
 
 GPLv3 (ignoring the fix for the loophole with web applications) adds 
 *nothing* 
 to this premise.  instead, it's used as an ideological club such that the 
 conversation is now i have all these ideas about how software should and 
 shouldnt be utilized, so if you want to use my software, you too now have to 
 subscribe to my way of thinking and you have to show me the changes.
 
 so what does moving from GPLv2 to GPLv3 gain us in terms of protections ?  
 nothing.  it does however allow us to restrict the people who want to use u-
 boot to using it in only ways we've blessed.  that's plain wrong in my eyes 
 and none of our business in the first place.

Wow, I was just about to compose a mail summarizing my point of view
when I realized you had done it already :-)

While I think fighting for extensible and hackable hardware is good,
I think a software license is the wrong way to go about it. Let's stick
to the proven model of GPLv2: You can use my software if I get to use
your improvements. Trying to impose restrictions on this model in order
to fight a different battle against restricted hardware will only make
the software less attractive and hurt us in the long run.

  I think it is not a coincidence that devices which can be updated with
  arbitrary firmware sells pretty good in the meantime.   Who buys routers
  capable of running OpenWRT because of their original firmware?  
 
 then let your wallet/politicians do the talking.  i certainly do -- i avoid 
 purchasing any music/games encumbered with DRM, or companies that employ such 
 methods.  but i'm above going around and forcing people to think the way i do 
 with licenses.

Exactly. Hardware manufacturers already seem to recognize that open
hardware designs lead to better sales, and that has _nothing_ to do
with GPLv3 (though it may or may not have something to do with the
Defective By Design campaign.)

These are only my personal opinions; I'm not speaking for Atmel as a
whole.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] U-book and GPLv3? (fwd)

2009-07-07 Thread Haavard Skinnemoen
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)

2009-07-07 Thread Haavard Skinnemoen
Wolfgang Denk wrote:
 I'm only talking about software (code and data)  here.  If  I  cannot
 change  (or  just  rebuild)  the (Free!) software any more because to
 actually run it I need some secret data (like a signature) then  this
 is  still  a software problem. One that can be prevented by releasing
 the software under adequate licensing terms.

The mechanism preventing reprogramming of the target device is not part
of the software being licensed. So I just don't think it's reasonable
for us to prevent the software from being used on such devices, even
though I don't particularly like such restrictions either.

This assuming GPLv3 actually does prevent such problems, of course.
There seems to be a few loopholes in there, as others have pointed out
(though I won't claim to fully understand it, which is another reason
I'm not particularly fond of it.)

 Agreed.  Hardware  should  be  hackable,  too  :-)  (which   includes
 documentation  where  you  don't have to sell your soul and sign NDAs
 that are plain evil).

Absolutely.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH ... resent] Atmel LCD driver GUARDTIME fix

2009-06-23 Thread Haavard Skinnemoen
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

2009-06-23 Thread Haavard Skinnemoen
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

2009-06-22 Thread Haavard Skinnemoen
Mark Jackson wrote:
 User-Agent: Thunderbird 2.0.0.21 (X11/20090409)

(...)

 My patch has been mangled ... there's an extra space at the start of each 
 unchanged patch line.

Read about how to make Thunderbird behave here:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/email-clients.txt

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH ... resent] Atmel LCD driver GUARDTIME fix

2009-06-22 Thread Haavard Skinnemoen
On Mon, 22 Jun 2009 16:31:20 +0100
Mark Jackson mpfj-l...@mimc.co.uk wrote:

 Haavard Skinnemoen wrote:
  Mark Jackson wrote:
  User-Agent: Thunderbird 2.0.0.21 (X11/20090409)
  
  (...)
  
  My patch has been mangled ... there's an extra space at the start
  of each unchanged patch line.
  
  Read about how to make Thunderbird behave here:
  
  http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/email-clients.txt
 
 But I've sent in patches before without any problems !!

Weird.

 And as per my other mail on the topic, I posted to both the
 u-b...@denx and u-b...@avr32linux MLs, and only *one* of the incoming
 mails (u-b...@denx) was mangled.

Hmm...the one I received through avr32linux.org looks mangled too; it
has extra spaces before some of the lines.

 Any ideas ?

My guess is that format=flowed is causing problems. From the patch:

Content-Type: text/plain; charset=ISO-8859-1; format=flowed

I wish people who implement e-mail software weren't completely insane.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 1/1] avr32/hsdramc: Move conditional compilation to Makefile

2009-06-13 Thread Haavard Skinnemoen
Jean-Christophe PLAGNIOL-VILLARD wrote:
 Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com
 Cc: Haavard Skinnemoen haavard.skinnem...@atmel.com

Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 06/13] Add support for the AT91RM9200EK Board.

2009-05-26 Thread Haavard Skinnemoen
On Fri, 27 Mar 2009 23:30:19 +0100
Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com wrote:

 diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
 index 631b969..175d82a 100644
 --- a/drivers/mtd/cfi_flash.c
 +++ b/drivers/mtd/cfi_flash.c
 @@ -1788,13 +1788,10 @@ static void flash_fixup_atmel(flash_info_t
 *info, struct cfi_qry *qry) 
   /* AT49BV6416(T) list the erase regions in the wrong order.
* However, the device ID is identical with the non-broken
 -  * AT49BV642D since u-boot only reads the low byte (they
 -  * differ in the high byte.) So leave out this fixup for now.
 +  * AT49BV642D they differ in the high byte.
*/
 -#if 0
   if (info-device_id == 0xd6 || info-device_id == 0xd2)
   reverse_geometry = !reverse_geometry;
 -#endif

Hmm...was this shortcoming actually fixed or does this change bugger up
a large number of currently working boards?

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 2/4] macb: add set_hw_enetaddr support

2009-05-11 Thread Haavard Skinnemoen
Jean-Christophe PLAGNIOL-VILLARD wrote:
 Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com
 Cc: Ben Warren biggerbadder...@gmail.com
 Cc: Haavard Skinnemoen haavard.skinnem...@atmel.com

Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 7/7] sf: stmicro: use common page timeout define

2009-04-02 Thread Haavard Skinnemoen
Mike Frysinger wrote:
 - /* Up to 2 seconds */
 - ret = stmicro_wait_ready(flash, 2 * CONFIG_SYS_HZ);
 + ret = stmicro_wait_ready(flash, SPI_FLASH_PAGE_ERASE_TIMEOUT);

50 ms is an awful lot less than 2 seconds. Sure this is safe?

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 7/7] sf: stmicro: use common page timeout define

2009-04-02 Thread Haavard Skinnemoen
Mike Frysinger wrote:
 On Thursday 02 April 2009 07:20:13 Haavard Skinnemoen wrote:
  Mike Frysinger wrote:
   - /* Up to 2 seconds */
   - ret = stmicro_wait_ready(flash, 2 * CONFIG_SYS_HZ);
   + ret = stmicro_wait_ready(flash, SPI_FLASH_PAGE_ERASE_TIMEOUT);
 
  50 ms is an awful lot less than 2 seconds. Sure this is safe?
 
 the 2 sec mark was copied in all spi flash drivers based on the original port 
 rather than referring to a datasheet, and it works on my Blackfin boards that 
 have stmicro parts.

Ok, that's good enough for me.

 personally i think the timeouts in the current spi flash common code is too 
 low in general, so i'd propose we raise them.  after all, the timeouts only 
 matter in when something goes wrong, so they wouldnt be reached.  and the low 
 threshold seems like it makes the presumption of faster SPI bus speeds.
 
 i.e. how about raising the timeout values 50x or 100x ?

I think that sounds like a good idea.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] sf: set common timeouts in seconds, not milliseconds

2009-04-02 Thread Haavard Skinnemoen
Mike Frysinger wrote:
 Since timeouts are only hit when there is a problem in the system, we
 don't want to prematurely timeout on a functioning setup.  Thus having
 low timeouts (in milliseconds) doesn't gain us anything in the production
 case, but rather increases likely hood of causing problems where none
 otherwise exist.
 
 Signed-off-by: Mike Frysinger vap...@gentoo.org
 CC: Haavard Skinnemoen haavard.skinnem...@atmel.com

Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] sf patches

2009-03-31 Thread Haavard Skinnemoen
Mike Frysinger wrote:
 since there doesnt seem to be a proper location for spi flash patches to 
 accumulate, do you mind if i start up a branch to accumulate the current set 
 ?  

No, please feel free to do that.

 i dont know how active you want to be with the sf subsystem ... or maybe 
 you're like me; you dont care so long as it continues to work properly :).

I'm like you in that respect :-)

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] sf: add driver for SST flashes

2009-03-29 Thread Haavard Skinnemoen
Mike Frysinger wrote:
 Signed-off-by: Mike Frysinger vap...@gentoo.org
 CC: Haavard Skinnemoen haavard.skinnem...@atmel.com

Looks good to me.

Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] sf: stmicro: drop redundant id read

2009-03-29 Thread Haavard Skinnemoen
Mike Frysinger wrote:
 The common SPI flash code reads the idcode and passes it down to the SPI
 flash driver, so there is no need to read it again ourselves.
 
 Signed-off-by: Mike Frysinger vap...@gentoo.org
 CC: Haavard Skinnemoen haavard.skinnem...@atmel.com
 CC: Jason McMullan mcmul...@netapp.com
 CC: TsiChung Liew tsi-chung.l...@freescale.com

Looks reasonable, though it's probably best if someone more familiar
with this particular driver Acks it as well.

Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] SF: always read 5 bytes for the idcode

2009-03-29 Thread Haavard Skinnemoen
Mike Frysinger wrote:
 Some SPI flash drivers like to have extended id information available
 (like the spansion flash), so rather than making it re-issue the ID cmd
 to get at the last 2 bytes, have the common code read 5 bytes rather than
 just 3.  This also matches the Linux behavior where it always reads 5 id
 bytes from all flashes.
 
 Signed-off-by: Mike Frysinger vap...@gentoo.org
 CC: Mingkai Hu mingkai...@freescale.com
 CC: Haavard Skinnemoen haavard.skinnem...@atmel.com

Yes, that's much better. But perhaps you should pass the last two bytes
to the debug() call a bit further down too?

I guess it's not essential though, so

Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] sf: drop DEBUG defines

2009-03-24 Thread Haavard Skinnemoen
Mike Frysinger wrote:
 Signed-off-by: Mike Frysinger vap...@gentoo.org
 CC: Haavard Skinnemoen haavard.skinnem...@atmel.com

Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH for avr32/next] avr32: fix cacheflush.h location introducted by d8f2aa3298610b

2009-03-23 Thread Haavard Skinnemoen
Jean-Christophe PLAGNIOL-VILLARD wrote:
 Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com

Applied, thanks

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [GIT PULL] AVR32 update

2009-03-23 Thread Haavard Skinnemoen
The following changes since commit ee1702d75a30d076139d1841383a1fa7220a0e11:
  Wolfgang Denk (1):
Merge branch 'next' of ../next

are available in the git repository at:

  git://git.denx.de/u-boot-avr32.git master

Sorry about all the merges at the end...I missed a couple of merge
windows, so I had to beat the tree back into shape from time to time.

Gunnar Rangoy (1):
  AVR32: Make GPIO implmentation cpu dependent

Haavard Skinnemoen (15):
  avr32: Update README
  avr32: data_bits should reflect the actual number of data bits
  avr32: refactor the portmux/gpio code
  avr32: Add gclk helper functions
  hammerhead: Use gclk helper functions
  avr32: Use board_postclk_init instead of gclk_init
  avr32: use board_early_init_r instead of board_init_info
  atstk1000: Convert to new-style makefile
  avr32: Add support for GPIO port mux
  Merge branch 'fixes' into cleanups
  Merge branch 'cleanups' into next
  Merge branch 'mimc200' into next
  Merge branch 'evk1100-prep' into next
  Merge branch 'mimc200'
  Merge branch 'evk1100-prep'

Jean-Christophe PLAGNIOL-VILLARD (1):
  avr32: fix cacheflush.h location introducted by d8f2aa3298610b

Mark Jackson (3):
  MIMC200 board now uses CONFIG_DISABLE_CONSOLE
  MIMC200: tidy GCLK init code
  Setup extra MIMC200 chip selects

Olav Morken (3):
  AVR32: Make cacheflush cpu-dependent
  AVR32: Move addrspace.h to arch-directory, and move some functions from 
io.h to addrspace.h
  AVR32: Must add NOPs after disabling interrupts for AT32UC3A0512ES

 board/atmel/atngw100/atngw100.c|   18 +-
 board/atmel/atstk1000/Makefile |9 +-
 board/atmel/atstk1000/atstk1000.c  |   15 +-
 board/atmel/atstk1000/flash.c  |2 +-
 board/earthlcd/favr-32-ezkit/favr-32-ezkit.c   |   13 +-
 board/earthlcd/favr-32-ezkit/flash.c   |2 +-
 board/mimc/mimc200/mimc200.c   |  117 ---
 board/miromico/hammerhead/hammerhead.c |   26 +--
 cpu/at32ap/Makefile|3 +-
 cpu/at32ap/at32ap700x/Makefile |2 +-
 cpu/at32ap/at32ap700x/clk.c|   25 +++
 cpu/at32ap/at32ap700x/gpio.c   |  199 ---
 cpu/at32ap/at32ap700x/portmux.c|  204 +++
 cpu/at32ap/cache.c |2 +-
 cpu/at32ap/cpu.c   |3 -
 cpu/at32ap/pio.c   |  116 ---
 cpu/at32ap/portmux-gpio.c  |  107 ++
 cpu/at32ap/portmux-pio.c   |   92 +
 doc/README.AVR32   |   24 +--
 doc/README.AVR32-port-muxing   |  208 
 include/asm-avr32/addrspace.h  |   46 -
 include/asm-avr32/arch-at32ap700x/addrspace.h  |   84 
 .../asm-avr32/{ = arch-at32ap700x}/cacheflush.h   |0
 include/asm-avr32/arch-at32ap700x/clk.h|  101 +-
 include/asm-avr32/arch-at32ap700x/gpio-impl.h  |   86 
 include/asm-avr32/arch-at32ap700x/gpio.h   |  184 +-
 include/asm-avr32/arch-at32ap700x/portmux.h|   89 +
 include/asm-avr32/arch-common/portmux-gpio.h   |  114 +++
 include/asm-avr32/arch-common/portmux-pio.h|  138 +
 include/asm-avr32/dma-mapping.h|2 +-
 include/asm-avr32/initcalls.h  |1 -
 include/asm-avr32/io.h |   39 +
 include/asm-avr32/sdram.h  |4 +-
 include/configs/atngw100.h |2 +-
 include/configs/atstk1002.h|2 +-
 include/configs/atstk1003.h|2 +-
 include/configs/atstk1004.h|2 +-
 include/configs/atstk1006.h|2 +-
 include/configs/favr-32-ezkit.h|2 +-
 include/configs/hammerhead.h   |2 +-
 include/configs/mimc200.h  |8 +-
 lib_avr32/board.c  |   14 ++-
 lib_avr32/bootm.c  |2 +-
 lib_avr32/interrupts.c |7 +
 44 files changed, 1388 insertions(+), 732 deletions(-)
 delete mode 100644 cpu/at32ap/at32ap700x/gpio.c
 create mode 100644 cpu/at32ap/at32ap700x/portmux.c
 delete mode 100644 cpu/at32ap/pio.c
 create mode 100644 cpu/at32ap/portmux-gpio.c
 create mode 100644 cpu/at32ap/portmux-pio.c
 create mode 100644 doc/README.AVR32-port-muxing
 delete mode 100644 include/asm-avr32/addrspace.h
 create mode 100644 include/asm-avr32/arch-at32ap700x/addrspace.h
 rename include/asm-avr32/{ = arch-at32ap700x}/cacheflush.h (100

Re: [U-Boot] [PATCH] Network AT91 AVR32: generic way of addressing USRIO register layout

2009-03-23 Thread Haavard Skinnemoen
Jean-Christophe PLAGNIOL-VILLARD wrote:
 On 09:58 Mon 23 Mar , Nicolas Ferre wrote:
  The MACB IP used by AVR32  AT91 have two different layout for USRIO
  register. We have to differentiate this in the driver code.
  No more cpu specific #ifdefs in driver: we manage a
  configuration variable.
 
  Signed-off-by: Nicolas Ferre nicolas.fe...@atmel.com  
 Ack-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com
 
 Haavard is ok for you too?

Sure.

Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] CUSTODIANS: Urgent boarding call for flight 2009.03

2009-03-20 Thread Haavard Skinnemoen
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

2009-03-19 Thread Haavard Skinnemoen
Jean-Christophe PLAGNIOL-VILLARD wrote:
 make[1]: In file included from clk.c:24:
 /private/u-boot-arm/include/asm/io.h:129: error: conflicting types for
 'virt_to_phys'
 /private/u-boot-arm/include/asm/io.h:80: error: previous definition of
 'virt_to_phys' was here
 
 Haavard could you take a look?

Latest upstream master builds just fine here...though the last release
may be broken because I pushed the fix too late.

Thanks a lot for keeping an eye on things.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 1/1 V3] cmd_bdinfo: move implementation to arch instead of common

2009-02-23 Thread Haavard Skinnemoen
Jean-Christophe PLAGNIOL-VILLARD wrote:
 introduce two new weak functions board_bdinfo and cpu_bdinfo to allow
 board and cpu to print more information
 
 Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com
 Cc: Haavard Skinnemoen haavard.skinnem...@atmel.com
 Cc: Mike Frysinger vap...@gentoo.org
 ---
 rebase for-next
 Precedent version
 Ack-by: Mike Frysinger vap...@gentoo.org
 Ack-by: Haavard Skinnemoen haavard.skinnem...@atmel.com

Aren't the Acked-by lines supposed to go above the '---' line?

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] Status open patches: AVR32

2009-02-23 Thread Haavard Skinnemoen
Wolfgang Denk wrote:
 I have the following patches still marked as open in my list. Could
 you please have a look... 

Sure. Would those patches be acceptable now or should I hold them off
until the next merge window?

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/9] AVR32: Make cacheflush cpu-dependent

2009-02-23 Thread Haavard Skinnemoen
Gunnar Rangoy wrote:
 From: Olav Morken olav...@gmail.com
 
 The AT32UC3A series of processors doesn't contain any cache, and issuing
 cache control instructions on those will cause an exception. This commit
 makes cacheflush.h arch-dependent in preparation for the AT32UC3A-support.
 
 Signed-off-by: Gunnar Rangoy gun...@rangoy.com
 Signed-off-by: Paul Driveklepp pauldrivekl...@gmail.com
 Signed-off-by: Olav Morken olav...@gmail.com

Applied to evk1100-prep and merged into next. I'll send it upstream as
soon as it's fine with Wolfgang.

Btw, I had to rebuild my next branch since it's become a bit stale.
Since all the non-merge commit IDs are the same, I hope it won't cause
any problems -- please let me know if you see any weird merge issues.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 3/9] AVR32: Move addrspace.h to arch-directory, and move some functions from io.h to addrspace.h

2009-02-23 Thread Haavard Skinnemoen
Gunnar Rangoy wrote:
 From: Olav Morken olav...@gmail.com
 
 The AVR32A architecture (which AT32UC3A-series is based on) has a
 different memory layout than the AVR32B-architecture. This patch moves
 addrspace.h to an arch-dependent directory in preparation for
 AT32UC3A-support. It also moves some address-space manipulation
 functions from io.h to addrspace.h.
 
 Signed-off-by: Gunnar Rangoy gun...@rangoy.com
 Signed-off-by: Paul Driveklepp pauldrivekl...@gmail.com
 Signed-off-by: Olav Morken olav...@gmail.com

Applied to evk1100-prep, thanks.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 4/9] AVR32: Make GPIO implmentation cpu dependent

2009-02-23 Thread Haavard Skinnemoen
Gunnar Rangoy wrote:
 There are some differences in the implementation of GPIO in the
 at32uc chip compared to the ap700x series.
 
 Signed-off-by: Gunnar Rangoy gun...@rangoy.com
 Signed-off-by: Paul Driveklepp pauldrivekl...@gmail.com
 Signed-off-by: Olav Morken olav...@gmail.com

Applied to evk1100-prep, thanks.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 7/9] AVR32: Must add NOPs after disabling interrupts for AT32UC3A0512ES

2009-02-23 Thread Haavard Skinnemoen
Gunnar Rangoy wrote:
 From: Olav Morken olav...@gmail.com
 
 The AT32UC3A0512ES chip has a bug when disabling interrupts. As a
 workaround, two NOPs can be inserted.
 
 Signed-off-by: Gunnar Rangoy gun...@rangoy.com
 Signed-off-by: Paul Driveklepp pauldrivekl...@gmail.com
 Signed-off-by: Olav Morken olav...@gmail.com

Applied to evk1100-prep, thanks.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot



Re: [U-Boot] [PATCH 1/1 v2] Setup extra MIMC200 chip selects

2009-02-23 Thread Haavard Skinnemoen
Mark Jackson wrote:
 Added code to setup the extra Flash and FRAM chip selects as used on the 
 MIMC200 board.
 
 V2 moves the init code from the common cpu.c file into the board specific 
 setup file.
 
 Signed-off-by: Mark Jackson m...@mimc.co.uk

Applied to mimc200 after fixing up some whitespace damage, thanks.

 ---
   board/mimc/mimc200/mimc200.c |   14 ++
   1 files changed, 14 insertions(+), 0 deletions(-)
 
 diff --git a/board/mimc/mimc200/mimc200.c b/board/mimc/mimc200/mimc200.c
 index 8516dcb..423238b 100644
 --- a/board/mimc/mimc200/mimc200.c
 +++ b/board/mimc/mimc200/mimc200.c
 @@ -29,6 +29,8 @@
   #include asm/arch/hmatrix.h
   #include lcd.h
 
 +#include ../../../cpu/at32ap/hsmc3.h

it would probably be better if this file was moved somewhere under
include/asm-avr32, however.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3] Add 16bpp BMP support

2009-02-23 Thread Haavard Skinnemoen
Mark Jackson wrote:
 Haavard Skinnemoen wrote:
  Mark Jackson wrote:  
  We do NOT want to do everything that is possible, but only what is
  reasonable.
  Exactly ... otherwise where do you stop ?  JPG, GIF, TIFF, PNG, etc ? 
  We're *only* meant to be showing a simply boot up image (not view lots 
  of different sized photos or movies !!), in a very controlled 
  environment (i.e. no user options ... just what the designers want / 
  require).  
  
  Why not do it even simpler? Drop BMP and generate an image matching the
  native format of the LCD controller at compile time :-)  
 
 Not sure if you're serious, but that'd reduce some of the functionality I was 
 expecting to use.

Well, it was just a thought that struck me, so I'm not going to claim I
considered all the pros and cons.

 My splashimage is stored in a particular, separate flash partition, and I'm 
 intending to allow end-users to change the boot logo (via a userspace app ?) 
 to customise / personalise the unit as they require (e.g. their own company 
 logo).

You can still do this if the userspace app knows how to generate an
image in the correct format -- I'm not arguing against storing the
image in a separate flash partition or anything like that. I just think
it might be possible to reduce the run-time size and complexity of
u-boot by being more strict about the image format.

You could also add support for PNG, JPEG and any format you want to the
userspace app -- this will probably be much easier than adding similar
support to u-boot itself.

 Hard-coding the image would render this impossible.

Of course. But hard-coding the image _format_ isn't the same thing. In
a way, we're already using a hard-coded image format, but it's one that
is easy to generate for the host, not one that's easy to display by the
target.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3] Add 16bpp BMP support

2009-02-02 Thread Haavard Skinnemoen
Mark Jackson wrote:
  We do NOT want to do everything that is possible, but only what is
  reasonable.  
 
 Exactly ... otherwise where do you stop ?  JPG, GIF, TIFF, PNG, etc ? 
 We're *only* meant to be showing a simply boot up image (not view lots 
 of different sized photos or movies !!), in a very controlled 
 environment (i.e. no user options ... just what the designers want / 
 require).

Why not do it even simpler? Drop BMP and generate an image matching the
native format of the LCD controller at compile time :-)

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 5/9] AVR32: macb - Disable 100mbps if clock is slow

2009-01-29 Thread Haavard Skinnemoen
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

2009-01-28 Thread Haavard Skinnemoen
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

2009-01-23 Thread Haavard Skinnemoen
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

2009-01-22 Thread Haavard Skinnemoen
Hi Wolfgang,

Please pull

  git://git.denx.de/u-boot-avr32.git master

to receive the following build fix for avr32 (all boards are affected).

Haavard Skinnemoen (2):
  avr32: Remove second definition of virt_to_phys()
  Merge branch 'fixes'

 include/asm-avr32/io.h |9 ++---
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/include/asm-avr32/io.h b/include/asm-avr32/io.h
index d22cd35..50967ac 100644
--- a/include/asm-avr32/io.h
+++ b/include/asm-avr32/io.h
@@ -76,12 +76,12 @@ extern void __readwrite_bug(const char *fn);
 #include asm/addrspace.h
 
 /* virt_to_phys will only work when address is in P1 or P2 */
-static __inline__ unsigned long virt_to_phys(volatile void *address)
+static inline phys_addr_t virt_to_phys(volatile void *address)
 {
return PHYSADDR(address);
 }
 
-static __inline__ void * phys_to_virt(unsigned long address)
+static inline void *phys_to_virt(phys_addr_t address)
 {
return (void *)P1SEGADDR(address);
 }
@@ -125,9 +125,4 @@ static inline void unmap_physmem(void *vaddr, unsigned long 
len)
 
 }
 
-static inline phys_addr_t virt_to_phys(void * vaddr)
-{
-   return (phys_addr_t)(vaddr);
-}
-
 #endif /* __ASM_AVR32_IO_H */
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 07/11] flash/cfi_flash: Use virtual sector start address, not phys

2009-01-14 Thread Haavard Skinnemoen
Kumar Gala wrote:
 
 On Dec 3, 2008, at 11:04 PM, Becky Bruce wrote:
 
  include/flash.h was commented to say that the address in
  flash_info-start was a physical address.  However, from u-boot's
  point of view, and looking at most flash code, it makes more
  sense for this to be a virtual address.  So I corrected the
  comment to indicate that this was a virtual address.
 
  The only flash driver that was actually treating the address
  as physical was the mtd/cfi_flash driver.  However, this code
  was using it inconsistently as it actually directly dereferenced
  the start element, while it used map_physmem to get a
  virtual address in other places.  I changed this driver so
  that the code which initializes the info-start field calls
  map_physmem to get a virtual address, eliminating the need for
  further map_physmem calls.  The code is now consistent.
 
  The *only* place a physical address should be used is when defining  
  the
  flash banks list that is used to initialize the flash_info struct.  I
  have fixed the one platform that was impacted by this change
  (MPC8641D).
 
  Signed-off-by: Becky Bruce bec...@kernel.crashing.org
  ---
  drivers/mtd/cfi_flash.c   |   53 + 
  +--
  include/configs/MPC8641HPCN.h |2 +-
  include/flash.h   |2 +-
  3 files changed, 26 insertions(+), 31 deletions(-)
 
 Stefan,
 
 Have you reviewed this.  I'm not sure if remvoing the map_physmem() is  
 the right answer because I'm assuming Haavard added them for a reason  
 (AVR32?). Should we instead change info-start to be a phys_addr_t?

There was definitely a reason: On AVR32, the caches are enabled by
default, and I prefer that they stay that way since it makes everything
faster. But when dealing with the flash, we must bypass the caches by
using a different virtual address which maps to the same physical
memory.

From what I can tell from a quick scan, this patch doesn't seem to
change that; all it does is push the map_physmem() call a bit up the
stack so it gets called less often. That seems like a good thing to me.

However, what I don't understand is if the 'start' array is really
supposed to hold virtual addresses, why isn't it an array of pointers?

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 07/11] flash/cfi_flash: Use virtual sector start address, not phys

2009-01-14 Thread Haavard Skinnemoen
Kumar Gala wrote:
 As I look at this we really need to understand what Haavard was trying  
 to get with:
 
 commit 12d30aa79779c2aa7a998bbae4c075f822a53004
 Author: Haavard Skinnemoen hskinnem...@atmel.com
 Date:   Thu Dec 13 12:56:34 2007 +0100
 
  cfi_flash: Use map_physmem() and unmap_physmem()
 
  Use map_physmem() and unmap_physmem() to convert from physical to
  virtual addresses. This gives the arch a chance to provide an  
 uncached
  mapping for flash accesses.
 
  Signed-off-by: Haavard Skinnemoen hskinnem...@atmel.com
 
 Since this only addresses the usage in cfi_flash.c but not in other  
 users of flash_info (like commands)

Yeah, there are probably tons of places that really should use
map/unmap_physmem() but doesn't. I've been playing with the thought of
enabling the Paging bit on AVR32 to support uncached mappings of all
physical memory...this is likely to make all of those places very
visible.

My goal was to solve a very specific problem (make the cfi_flash driver
work on AVR32 boards) by introducing a generic primitive which may be
used to solve other similar problems in the future.

I also think it conceptually makes sense to treat physical and virtual
addresses as different entities, as they are fundamentally not the same
thing on many architectures.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [ANNOUNCE] DATAFLASH DRIVER

2009-01-08 Thread Haavard Skinnemoen
Ulf Samuelsson wrote:
 ons 2009-01-07 klockan 21:56 +0100 skrev Jean-Christophe
 PLAGNIOL-VILLARD:
  Hi,
  
  The AT91 arch use a at45 driver design to be show as a parallel flash
  but it's a spi flash.
  
  Haavard add a new spi flash framework which support the dataflash
  so the current driver is mark as *depracated* and plan to be remove
 
 There is very limited support for the flash in the new spi driver.
 so most things you would like to do with a flash is not supported

Yes, it is a bit minimal right now. But it should be fairly easy to
extend once we know what the requirements are.

Now, the current driver does work fairly well for my purposes, so I
won't promise anything about when I can get around to extend the
driver. But it shouldn't be too difficult to do for someone who has an
itch to scratch.

 I would like to have capabilities to
 
 1) print contents of the dataflash

We should add a sf dump command similar to the existing nand dump
command.

 2) compare contents of the dataflash with SDRAM.

This is one of the controversial points, I believe.

 3) Automatically add a CRC when the dataflash is written
   based on an environment variable crc-check

What kind of CRC and where should it be placed?

 4) Verify that the CRC is correct.

Same issue.

 5) high granularity read/writes
   The dataflash can easily support byte write.

Supported by the current driver, I believe. Erase still operates on full
pages, however, and I don't think there's anything to do about that.

 6) Support partitions for 
   bootstrap
   environemnt
   u-boot
   kernel
   filesystem
in the boot dataflash.
additional dataflash should have other partitions.

We should add a partitioning layer on top of the current interface.

And even better solution would be to introduce a common flash interface
for NOR, NAND, SPI, etc. flash and add a partitioning layer on top of
that.

 7) protection of partitions.

As well as protection of raw sectors, perhaps? A partitioning layer
could use such functionality to implement protection of partitions.

 I have my own patches for the memory commands
 to enable most of this but adding that to the
 cmd_mem driver was not accepted.

Yes, as you probably know, I for one think memory mapping of serial
devices is a bad idea.

 At that time it was promised that the new driver
 will not limit the functionality, and would
 only remove the use of direct addressing to the dataflash.

There's nothing in the driver or its architecture which limits any
functionality. If you need additional features, feel free to implement
them, or convince someone else to do it.

 The whole storage concept is really not working
 when storage becomes larger than the SDRAM.
 
 How do you download an 128 MB image over the network
 to a machine with 64 MB SDRAM. - Major pain...
 You need to be able to TFTP to flash directly
 if you want to have an easy user interface.

Or start an operating system which can do all of this much faster and
with support for more protocols.

 That is likely to require another way of handling storage.
 and parameter parsing so you can do
 tftp mmc0:kernel linux-2.6.28
 or
 tftp df3:fs rootfs.arm.ext2

Now, I kind of like that syntax, but I seem to recall it got shot down
at some point too.

 The tftp can then write incoming packets to a caching driver
 and will delay fetching new packets when waiting for flash
 writes  to complete. Whn your storage is handled
 by a DMA controller, you should be able to implement
 this nicely.

Adding DMA support to the SPI driver shouldn't be a big deal, but I
don't see what it has to do with the user interface.

Also, caching sounds like something which comes dangerously close to
crossing the line between boot loader and operating system. I don't
think it fits well into the current u-boot architecture.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] spansion spi flash driver ?

2009-01-06 Thread Haavard Skinnemoen
Mike Frysinger wrote:
 in your original drivers/mtd/spi/spi_flash.c commit, you had this:
 #ifdef CONFIG_SPI_FLASH_SPANSION
 case 0x01:
 flash = spi_flash_probe_spansion(spi, idcode);
 break;
 #endif
 
 does that mean you have a spansion driver sitting around but it just wasnt 
 merged ?  if i dont have to write it from scratch, that'd be great :).

No, I'm afraid I don't have a spansion driver. I don't know why I put it
there...probably just to see what it would look like with more
supported flash types, and then I forgot to remove it before submitting.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] cmd_sf: rename speed to hz

2009-01-06 Thread Haavard Skinnemoen
Mike Frysinger wrote:
 The term hz is used everywhere else when talking about the frequency of
 the SPI bus, so have the sf command use it as well to stay consistent.  It
 even presents itself as hz when showing user help.
 
 Signed-off-by: Mike Frysinger vap...@gentoo.org

I like it. hz makes it perfectly clear what the value means.

Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] spi flash: fix crash due to spi flash miscommunication

2009-01-05 Thread Haavard Skinnemoen
Mike Frysinger wrote:
 From: Brad Bozarth bfli...@yumbrad.com
 
 Higher spi flash layers expect to be given back a pointer that was
 malloced so that it can free the result, but the lower layers return a
 pointer that is in the middle of the malloced memory.  Reorder the members
 of the lower spi structures so that things work out.
 
 Signed-off-by: Brad Bozarth bfli...@yumbrad.com
 Signed-off-by: Mike Frysinger vap...@gentoo.org
 CC: Haavard Skinnemoen haavard.skinnem...@atmel.com

Acked-by: Haavard Skinnemoen haavard.skinnem...@atmel.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v4 1/4] Introduce virt_to_phys()

2008-12-17 Thread Haavard Skinnemoen
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()

2008-12-17 Thread Haavard Skinnemoen
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()

2008-12-17 Thread Haavard Skinnemoen
Kumar Gala wrote:
 diff --git a/include/asm-avr32/io.h b/include/asm-avr32/io.h
 index 06e52b1..d22cd35 100644
 --- a/include/asm-avr32/io.h
 +++ b/include/asm-avr32/io.h
 @@ -125,4 +125,9 @@ static inline void unmap_physmem(void *vaddr, unsigned 
 long len)
  
  }
  
 +static inline phys_addr_t virt_to_phys(void * vaddr)
 +{
 + return (phys_addr_t)(vaddr);
 +}
 +

avr32 has already got one of those, so this breaks the build.

I'm going to apply the patch below to my 'fixes' branch if it looks ok
to you.

Haavard

From 92c78a3bbcb2ce508b4bf1c4a1e0940406a024bb Mon Sep 17 00:00:00 2001
From: Haavard Skinnemoen haavard.skinnem...@atmel.com
Date: Wed, 17 Dec 2008 16:43:18 +0100
Subject: [PATCH] avr32: Remove second definition of virt_to_phys()

The second definition introduced by 65e43a1063 conflicts with the
existing one.

Also, convert the existing definition to use phys_addr_t. The volatile
qualifier is still needed due to brain damage elsewhere.

Signed-off-by: Haavard Skinnemoen haavard.skinnem...@atmel.com
---
 include/asm-avr32/io.h |9 ++---
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/include/asm-avr32/io.h b/include/asm-avr32/io.h
index d22cd35..50967ac 100644
--- a/include/asm-avr32/io.h
+++ b/include/asm-avr32/io.h
@@ -76,12 +76,12 @@ extern void __readwrite_bug(const char *fn);
 #include asm/addrspace.h
 
 /* virt_to_phys will only work when address is in P1 or P2 */
-static __inline__ unsigned long virt_to_phys(volatile void *address)
+static inline phys_addr_t virt_to_phys(volatile void *address)
 {
return PHYSADDR(address);
 }
 
-static __inline__ void * phys_to_virt(unsigned long address)
+static inline void *phys_to_virt(phys_addr_t address)
 {
return (void *)P1SEGADDR(address);
 }
@@ -125,9 +125,4 @@ static inline void unmap_physmem(void *vaddr, unsigned long 
len)
 
 }
 
-static inline phys_addr_t virt_to_phys(void * vaddr)
-{
-   return (phys_addr_t)(vaddr);
-}
-
 #endif /* __ASM_AVR32_IO_H */
-- 
1.5.6.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] env_sf: support embedded environments

2008-12-11 Thread Haavard Skinnemoen
Mike Frysinger wrote:
 If both CONFIG_ENV_SECT_SIZE and CONFIG_ENV_SIZE are defined, and the sect
 size is larger than the env size, then it means the env is embedded in a
 block.  So we have to save/restore the part of the sector which is not the
 environment.  Previously, saving the environment in SPI flash in this
 setup would probably brick the board as the rest of the sector tends to
 contain actual U-Boot data/code.
 
 Signed-off-by: Mike Frysinger [EMAIL PROTECTED]

Makes sense to me. Assuming this is how other types of flash work,

Acked-by: Haavard Skinnemoen [EMAIL PROTECTED]
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 07/10] AVR32: Disable relocation of command table when on AT32UC3A for now

2008-11-19 Thread Haavard Skinnemoen
Wolfgang Denk [EMAIL PROTECTED] wrote:
 In message [EMAIL PROTECTED] you wrote:
  Due to a bug with the SDRAM-controller, running code from the SDRAM is
  impossible. This patch disables relocation of the command table on those
  chips.  
 
 You are aware that this is dangerous, as it will probably prevent you
 rom using U-Boot to update itself in flash?

It's only a temporary thing. A patch has already been posted to
re-enable relocation when running from SRAM.

Eventually, when chips without this erratum have been generally
available for a while, relocation can be enabled unconditionally.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 09/10] AVR32: CPU support for AT32UC3A0xxx CPUs

2008-11-19 Thread Haavard Skinnemoen
Wolfgang Denk [EMAIL PROTECTED] wrote:
 Dear Olav Morken,
 
 In message [EMAIL PROTECTED] you wrote:
  This patch adds support for the AT32UC3A0xxx chips.
  
  Signed-off-by: Gunnar Rangoy [EMAIL PROTECTED]
  Signed-off-by: Paul Driveklepp [EMAIL PROTECTED]
  Signed-off-by: Olav Morken [EMAIL PROTECTED]
 
 Coding style violations: C++ comments, indentation not by TAB, too
 long lines, incorrect multiline comment style.
 
 ...
  +static inline unsigned long get_hsb_clk_rate(void)
  +{
  +   //TODO HSB is always the same as cpu-rate
 ---^^
  +   return MAIN_CLK_RATE  CFG_CLKDIV_CPU;
  +}
  +static inline unsigned long get_pba_clk_rate(void)
  +{
  +   return MAIN_CLK_RATE  CFG_CLKDIV_PBA;
  +}
  +static inline unsigned long get_pbb_clk_rate(void)
  +{
  +   return MAIN_CLK_RATE  CFG_CLKDIV_PBB;
  +}
  +
  +/* Accessors for specific devices. More will be added as needed. */
  +static inline unsigned long get_sdram_clk_rate(void)
  +{
  +   return get_hsb_clk_rate();
  +}
  +#ifdef AT32UC3A0xxx_CHIP_HAS_USART
  +static inline unsigned long get_usart_clk_rate(unsigned int dev_id)
  +{
  +   return get_pba_clk_rate();
  +}
  +#endif
  +#ifdef AT32UC3A0xxx_CHIP_HAS_MACB
  +static inline unsigned long get_macb_pclk_rate(unsigned int dev_id)
  +{
  +   return get_pbb_clk_rate();
  +}
  +static inline unsigned long get_macb_hclk_rate(unsigned int dev_id)
  +{
  +   return get_hsb_clk_rate();
  +}
  +#endif
  +#ifdef AT32UC3A0xxx_CHIP_HAS_SPI
  +static inline unsigned long get_spi_clk_rate(unsigned int dev_id)
  +{
  +   return get_pba_clk_rate();
  +}
  +#endif
 
 Would it make  sense  to  provide  weak  functions  the  get  defined
 accordingly?  A  function  which  only  calls  another function looks
 stupid to me.

This matches other AVR32 chips. I'm not sure how you intend to use weak
functions here...the whole point of these things is to make drivers
unaware of which bus a certain peripheral is connected to.

I suppose the same thing could be done using aliases, but then you
can't do it inline, which in turn means that the constant calculations
won't be constant anymore, and the code gets larger and slower as a
result.

  +#ifdef AT32UC3A0xxx_CHIP_HAS_USART
  +static inline void portmux_enable_usart0(unsigned long drive_strength)
  +{
  +   portmux_select_peripheral(PORTMUX_PORT_A, (1  0) | (1  1),
  +   PORTMUX_FUNC_A, 0);
  +}
  +
  +static inline void portmux_enable_usart1(unsigned long drive_strength)
  +{
  +   portmux_select_peripheral(PORTMUX_PORT_A, (1  5) | (1  6),
  +   PORTMUX_FUNC_A, 0);
  +}
  +
  +static inline void portmux_enable_usart2(unsigned long drive_strength)
  +{
  +   portmux_select_peripheral(PORTMUX_PORT_B, (1  29) | (1  30),
  +   PORTMUX_FUNC_A, 0);
  +}
  +
  +static inline void portmux_enable_usart3(unsigned long drive_strength)
  +{
  +   portmux_select_peripheral(PORTMUX_PORT_B, (1  10) | (1  11),
  +   PORTMUX_FUNC_B, 0);
  +}
  +#endif
 
 I don't like this either.

Why not? I think it's pretty elegant -- the board code only needs to
specify which USARTs to enable, it doesn't need to know which pins and
functions it corresponds to.

That said, it might be a better idea to make a single function with a
switch () block.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] flash: Export flash_sector_size() function.

2008-11-19 Thread Haavard Skinnemoen
Piotr Ziecik [EMAIL PROTECTED] wrote:
 Export flash_sector_size() function from drivers/mtd/cfi_flash.c.
 
 Signed-off-by: Piotr Ziecik [EMAIL PROTECTED]

Why?

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2] cfi_flash: Make all flash access functions weak

2008-11-19 Thread Haavard Skinnemoen
Stefan Roese [EMAIL PROTECTED] wrote:
 This patch defines all flash access functions as weak so that
 they can be overridden by board specific versions.
 
 This will be used by the upcoming VCTH board support where the NOR
 FLASH unfortunately can't be accessed memory-mapped. Special
 accessor functions are needed here.
 
 To enable this weak functions you need to define
 CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS in your board config header.
 Otherwise the old default functions will be used resulting
 in smaller code.
 
 Signed-off-by: Stefan Roese [EMAIL PROTECTED]

Acked-by: Haavard Skinnemoen [EMAIL PROTECTED]
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] weak functions versus conditional compile

2008-11-19 Thread Haavard Skinnemoen
Mike Frysinger [EMAIL PROTECTED] wrote:
 On Monday 17 November 2008 16:17:54 Graeme Russ wrote:
  Should I declare these functions as weak in the core i386 code and use a
  config #define to override or should I seperate the functions out into
  seperate source files and use conditional compilation?
 
 i wonder if weak functions that are always satisfied and used unconditionally 
 result in larger code, or if this is only a problem for people whose linkers 
 lack lazy relaxation support ?

It's probably a problem with linkers that don't support --gc-sections,
which is different from relaxation. Also, you definitely need to
compile with -ffunction-sections, or the weak functions might end up in
the same section as something linked unconditionally, which makes it
impossible for the linker to remove them.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 09/10] AVR32: CPU support for AT32UC3A0xxx CPUs

2008-11-19 Thread Haavard Skinnemoen
Wolfgang Denk [EMAIL PROTECTED] wrote:
  +static inline unsigned long get_hsb_clk_rate(void)
  +{
  +   //TODO HSB is always the same as cpu-rate  
 ---^^
  +   return MAIN_CLK_RATE  CFG_CLKDIV_CPU;

Simply removing this comment should be fine.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 00/10] AVR32: RFC/preview - support for ATEVK1100 evaluation board

2008-11-19 Thread Haavard Skinnemoen
Olav Morken [EMAIL PROTECTED] wrote:
 This is a patch series which adds support for the ATEVK1100 evaluation
 board[1], and the AT32UC3A0xxx[2] microcontrollers used on that board.
 The patch series is based on avr32/next.

I've applied this and the other series you posted to the 'evk1100'
branch in

  git://git.denx.de/u-boot-avr32.git evk1100

Please submit incremental patches addressing the comments you got
during review.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 00/10] AVR32: RFC/preview - support for ATEVK1100 evaluation board

2008-11-19 Thread Haavard Skinnemoen
Wolfgang Denk [EMAIL PROTECTED] wrote:
 Dear Haavard Skinnemoen,
 
 In message [EMAIL PROTECTED] you wrote:
 
  I've applied this and the other series you posted to the 'evk1100'
  branch in
  
git://git.denx.de/u-boot-avr32.git evk1100
  
  Please submit incremental patches addressing the comments you got
  during review.  
 
 Umm... but please do NOT merge this branch into your AVR32 repository
 for me to poll from. In mainline, I want to see consolidated  patches
 only.

That's the plan. I just thought I'd give them somewhere to live until
they're ready.

So a big fat warning to everyone: the evk1100 branch _will_ get rebased.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] cfi_flash: Make all flash access functions weak

2008-11-17 Thread Haavard Skinnemoen
Stefan Roese [EMAIL PROTECTED] wrote:
 Old version without weak aliases:
textdata bss dec hex filename
  280964   20232   50788  351984   55ef0 ./u-boot
 
 New version with weak aliases:
textdata bss dec hex filename
  280520   20232   50788  351540   55d34 ./u-boot
 
 So the difference is 444 bytes (with gcc 4.2.2). I have to admit that this is 
 more than I thought. If necessary I could make this weak change conditionally 
 of course. I just didn't want to pollute to the source with more #ifdef's.

Looks like it's 444 bytes _smaller_ with weak functions...that's a bit
surprising. From you comment, it sounds like it's really 444 bytes
larger. Did you mix up the numbers?

If it's really 444 bytes larger, I think that's a bit much for
something as simple as memory accessors. Ideally, they should boil down
to a single instruction each.

Perhaps you could allow overriding them in the board header by doing
something like

#ifndef flash_write8
# #define flash_write8(value, addr) __raw_writeb(value, addr)
...

in cfi_flash.c?

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] cfi_flash: Make all flash access functions weak

2008-11-17 Thread Haavard Skinnemoen
Stefan Roese [EMAIL PROTECTED] wrote:
 I could do it this way, sure. But how about this version:
 
 static void __flash_write8(u8 value, void *addr)
 {
   __raw_writeb(value, addr);
 }
 ...
 
 #ifdef CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS
 void flash_write8(u8 value, void *addr)__attribute__((weak, 
 alias(__flash_write8)));
 ...
 #else
 #define flash_read8   __flash_read8
 ...
 #endif
 
 We would still have the original accessor functions this way. And the
 resulting source code looks a little better to me (less #ifdef's).

Hmm...1 #ifdef vs. 1 #ifdef...I'd say that's pretty much the same ;-)

But sure, your way works too.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] avr32/bootm: remove unused variable 'ret'

2008-11-13 Thread Haavard Skinnemoen
Jean-Christophe PLAGNIOL-VILLARD [EMAIL PROTECTED] wrote:
 Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD [EMAIL PROTECTED]

Acked-by: Haavard Skinnemoen [EMAIL PROTECTED]

I'm a bit out of sync at the moment. Wolfgang, can you apply it
directly?

 diff --git a/lib_avr32/bootm.c b/lib_avr32/bootm.c
 index 556e3ea..03ab8d1 100644
 --- a/lib_avr32/bootm.c
 +++ b/lib_avr32/bootm.c
 @@ -176,7 +176,6 @@ int do_bootm_linux(int flag, int argc, char *argv[], 
 bootm_headers_t *images)
   void(*theKernel)(int magic, void *tagtable);
   struct  tag *params, *params_start;
   char*commandline = getenv(bootargs);
 - int ret;
  
   if ((flag != 0)  (flag != BOOTM_STATE_OS_GO))
   return 1;
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] cmd_bdinfo: move implementation to arch instead of common

2008-11-13 Thread Haavard Skinnemoen
Jean-Christophe PLAGNIOL-VILLARD [EMAIL PROTECTED] wrote:
 This patch trades off the removal of most of the #ifdef ugly for  
  a lot of duplication.  Which is the lesser of two evils?  
 Only 4 archs share actually the same code avr32, i386, mips and sh
 which actually I've plan to modify for sh soon

And the avr32 code is mostly wrong. So it looks like the current
situation is #ifdef mess _and_ duplication. This patch definitely
improves things.

The reason why the avr32 part is wrong is that I simply didn't notice
that I had to do anything in there until quite recently. So IMO making
this thing arch-specific will make it easier to get it right for new
architectures (since you'll get a nice and friendly link error
reminding you that you missed it.)

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 2/6] AT572D940HF-EB Support v2 (cpu folder)

2008-11-07 Thread Haavard Skinnemoen
Antonio R. Costa [EMAIL PROTECTED] wrote:

 As we agreed with Haavard I'll try to make a general SDHC patch for all
 Atmel platform soon.

Note that there's a generic MMC framework in the works:

http://www.nabble.com/-U-Boot---PATCH-00-10--Add-MMC-Framework-td20256840.html

which we should probably consider using once it is finished.

Of course, adding support for AT91 and SDHC to the atmel-mci driver is
a good thing in itself, so it can happen earlier, but depending on the
timing of things, there may be conflicts.

So I highly recommend that you keep the list informed about what you're
doing at that front, so that we can avoid stepping on each others' toes.

If you have the time, I'm sure Andy will appreciate your comments on
his patches too.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 04/10] Eliminated arch-specific mmc header requirement

2008-11-04 Thread Haavard Skinnemoen
Andy Fleming [EMAIL PROTECTED] wrote:
 diff --git a/include/asm-avr32/arch-at32ap700x/mmc.h 
 b/include/asm-avr32/arch-at32ap700x/mmc.h
 deleted file mode 100644
 index 9caba91..000
 --- a/include/asm-avr32/arch-at32ap700x/mmc.h
 +++ /dev/null

 -struct mmc_cid {

 -struct mmc_csd

 -#define R1_ILLEGAL_COMMAND   (1  22)
 -#define R1_APP_CMD   (1  5)

Where can those definitions be found now? The driver still needs them...

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 06/10] Add MMC Framework

2008-11-04 Thread Haavard Skinnemoen
Andy Fleming [EMAIL PROTECTED] wrote:
 Here's a new framework (based roughly off the linux one) for managing
 MMC controllers.  It handles all of the standard SD/MMC transactions,
 leaving the host drivers to implement only what is necessary to
 deal with their specific hardware.
 
 This also hooks the infrastructure into the PowerPC board code
 (similar to how the ethernet infrastructure now hooks in)
 
 Some of this code was contributed by Dave Liu [EMAIL PROTECTED]
 
 Signed-off-by: Andy Fleming [EMAIL PROTECTED]

Nice. I have a few comments, please see below.

 ---
  common/cmd_mmc.c |  122 +++
  drivers/mmc/Makefile |1 +
  drivers/mmc/mmc.c|  926 
 ++
  include/mmc.h|  174 +-
  lib_ppc/board.c  |9 +
  5 files changed, 1223 insertions(+), 9 deletions(-)
  create mode 100644 drivers/mmc/mmc.c
 
 diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
 index 21873d6..f27d7ef 100644
 --- a/common/cmd_mmc.c
 +++ b/common/cmd_mmc.c
 @@ -25,6 +25,7 @@
  #include command.h
  #include mmc.h
  
 +#ifndef CONFIG_GENERIC_MMC
  int do_mmc (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
  {
   if (mmc_legacy_init (1) != 0) {
 @@ -39,3 +40,124 @@ U_BOOT_CMD(
   mmcinit - init mmc card\n,
   NULL
  );
 +#endif /* !CONFIG_GENERIC_MMC */
 +
 +static void print_mmcinfo(struct mmc *mmc)
 +{
 + printf(Device: %s\n, mmc-name);
 + printf(Manufacturer ID: %x\n, mmc-cid[0]  24);
 + printf(OEM: %x\n, (mmc-cid[0]  8)  0x);
 + printf(Name: %c%c%c%c%c \n, mmc-cid[0]  0xff,
 + (mmc-cid[1]  24), (mmc-cid[1]  16)  0xff,
 + (mmc-cid[1]  8)  0xff, mmc-cid[1]  0xff);
 +
 + printf(Tran Speed: %d\n, mmc-tran_speed);
 + printf(Rd Block Len: %d\n, mmc-read_bl_len);
 +
 + printf(%s version %d.%d\n, IS_SD(mmc) ? SD : MMC,
 + (mmc-version  4)  0xf, mmc-version  0xf);
 +
 + printf(High Capacity: %s\n, mmc-high_capacity ? Yes : No);
 + printf(Capacity: %lld\n, mmc-capacity);
 +
 + printf(Bus Width: %d-bit\n, mmc-bus_width);
 +}
 +
 +int do_mmcinfo (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 +{
 + struct mmc *mmc;
 + int dev_num;
 +
 + if (argc  2)
 + dev_num = 0;
 + else
 + dev_num = simple_strtoul(argv[1], NULL, 0);
 +
 + mmc = find_mmc_device(dev_num);
 +
 + if (mmc) {
 + mmc_init(mmc);
 +
 + print_mmcinfo(mmc);
 + }
 +
 + return 0;
 +}
 +
 +U_BOOT_CMD(mmcinfo, 2, 0, do_mmcinfo, mmcinfo dev num-- display MMC 
 info\n,
 + NULL);
 +
 +int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 +{
 + int rc = 0;
 +
 + switch (argc) {
 + case 0:
 + case 1:
 + case 3:
 + case 4:
 + printf(Usage:\n%s\n, cmdtp-usage);
 + return 1;
 +
 + case 2:
 + if (!strcmp(argv[1], list)) {
 + print_mmc_devices('\n');
 + return 0;
 + }
 + return 1;
 + default: /* at least 5 args */
 + if (strcmp(argv[1], read) == 0) {
 + int dev = simple_strtoul(argv[2], NULL, 10);
 + void *addr = (void *)simple_strtoul(argv[3], NULL, 16);
 + u32 cnt = simple_strtoul(argv[5], NULL, 16);
 + u32 n;
 + u32 blk = simple_strtoul(argv[4], NULL, 16);
 + struct mmc *mmc = find_mmc_device(dev);
 +
 + printf(\nMMC read: dev # %d, block # %d, count %d ... 
 ,
 + dev, blk, cnt);
 +
 + mmc_init(mmc);
 +
 + n = mmc-block_dev.block_read(dev, blk, cnt, addr);
 +
 + /* flush cache after read */
 + flush_cache((ulong)addr, cnt * 512); //FIXME
 +
 + printf(%d blocks read: %s\n,
 + n, (n==cnt) ? OK : ERROR);
 + return (n == cnt) ? 0 : 1;
 + } else if (strcmp(argv[1], write) == 0) {
 + int dev = simple_strtoul(argv[2], NULL, 10);
 + void *addr = (void *)simple_strtoul(argv[3], NULL, 16);
 + u32 cnt = simple_strtoul(argv[5], NULL, 16);
 + u32 n;
 + struct mmc *mmc = find_mmc_device(dev);
 +
 + int blk = simple_strtoul(argv[4], NULL, 16);
 +
 + printf(\nMMC write: dev # %d, block # %d, count %d ... 
 ,
 + dev, blk, cnt);
 +
 + mmc_init(mmc);
 +
 + n = mmc-block_dev.block_write(dev, blk, cnt, addr);
 +
 + printf(%d blocks written: %s\n,
 + n, (n == cnt) ? OK : ERROR);
 + return (n == cnt) ? 0 : 1;
 + } else {
 + printf(Usage:\n%s\n, 

  1   2   >