Re: [U-Boot] [PATCH 4/4 V3] PXA: Adapt Voipac PXA270 to OneNAND SPL
> On 11/04/2011 03:07 PM, Marek Vasut wrote: > >> For now, until we decide to do something SPL-wide, call it what you > >> want. > > > > Well basically from what I see, you'd like me to do the NAND/OneNAND > > merge. As I already said, that's way out of the scope of this patchset > > so I'm not doing that. > > Oh, I didn't mean to imply that was within the scope of this patch. I > was just describing the background behind why I'm frustrated with the > OneNAND situation, and this looked like an extension of the "onenand is > a totally different thing" model, in an independent context. > > > Either way, are you OK with current state of the patch? > > I won't block it. > > -Scott Ok, I'll queue this patchset and submit to Albert when I re-add the PXA250 support to start.S (done already, but I need to test it). ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/4 V3] PXA: Adapt Voipac PXA270 to OneNAND SPL
> On 11/04/2011 03:07 PM, Marek Vasut wrote: > >> For now, until we decide to do something SPL-wide, call it what you > >> want. > > > > Well basically from what I see, you'd like me to do the NAND/OneNAND > > merge. As I already said, that's way out of the scope of this patchset > > so I'm not doing that. > > Oh, I didn't mean to imply that was within the scope of this patch. I > was just describing the background behind why I'm frustrated with the > OneNAND situation, and this looked like an extension of the "onenand is > a totally different thing" model, in an independent context. Well I suppose this is rooted in Linux. It's different subsys there too. > > > Either way, are you OK with current state of the patch? > > I won't block it. That's what I meant. > > -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/4 V3] PXA: Adapt Voipac PXA270 to OneNAND SPL
On 11/04/2011 03:07 PM, Marek Vasut wrote: >> For now, until we decide to do something SPL-wide, call it what you want. >> > > Well basically from what I see, you'd like me to do the NAND/OneNAND merge. > As I > already said, that's way out of the scope of this patchset so I'm not doing > that. Oh, I didn't mean to imply that was within the scope of this patch. I was just describing the background behind why I'm frustrated with the OneNAND situation, and this looked like an extension of the "onenand is a totally different thing" model, in an independent context. > Either way, are you OK with current state of the patch? I won't block it. -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/4 V3] PXA: Adapt Voipac PXA270 to OneNAND SPL
> On 11/03/2011 07:55 PM, Marek Vasut wrote: > >> On 11/03/2011 04:52 PM, Marek Vasut wrote: > >> Why do we want to separate them? What is the fundamental difference > >> between OneNAND, and a high-level NAND controller such as fsl_elbc? > > > > Honestly, I'm not the author of the subsystem, but please check the > > documentation. The way we retrieve data from onenand is different to > > NAND. > > What documentation? How is it different? There are substantial > differences in how we "retrieve data" between drivers that use the NAND > subsystem. Surely you've seen that in the mxs_nand driver. :-) > > >> Maybe there would be some differences on init if we can't produce > >> "normal" ID data, but that doesn't justify duplicating the whole > >> subsystem. > > > > Where do you see such duplication? cmd_onenand ? > > cmd_onenand and env_onenand are the most irritating, since they're at a > layer that really shouldn't care about the differences -- we should > probably have a plain "mtd" command instead, for most of the functionality. > > There's also onenand_bbt.c -- what are the hardware-based differences in > how the bad block table is managed? > > nand_base.c/onenand_base.c are less clear. Obviously much of what is in > onenand_base.c would be in the controller driver if it used the NAND > subsystem. But it looks like some of it is duplication. > > >> Why should the code that just wants to use an API to move data around > >> need to care which it is? Why should there be behavioral differences > >> that aren't rooted in the actual hardware? Another approach might be to > >> use MTD as the common interface, but factor out common code into > >> libraries that drivers can use, and avoid the main nand_base.c code even > >> for things like fsl_elbc. > > > > I think you're mistaken here. OneNAND != NAND. > > Well, last I tried I couldn't find any public documentation, so all I > have to go on is the code, some marketing-type info, and asking > questions of people that appear to know more about OneNAND than I do. :-) > > From what I can see, it looks like NAND with an integrated controller > that exposes an unusual command set, but still for the most part > provides the same operations. Several of our existing NAND-subsystem > drivers have to fake the command set for the generic layer as well. > Initialization/identification might be a problem area that current > drivers don't have to deal with, though. Actually integrating OneNAND > with NAND would likely involve an already-needed restructuring of the > subsystem. > > If the answer really is that it makes sense to consider OneNAND to be a > totally different thing from NAND, then it's outside my jurisdiction as > NAND custodian -- which is fine with me. Frankly, even if it does make > sense to merge them, I'd rather not be custodian of the OneNAND stuff > unless someone is actually willing to do the merge. I don't have access > to hardware or documentation, and it's an entirely separate codebase. > People just started sending me the patches a few years back. > > >> This is not a new complaint -- I've asked for this before but nobody's > >> put the time into sorting out the mess (and I have neither time nor > >> hardware nor documentation). The SPL load_image function is a simple > >> enough interface to start with, though. :-) > > > > Well, it seems what you are proposing is way beyond the scope of this > > patchset. > > > >> In fact, it should probably just be spl_load_image() with whatever boot > >> source has been configured into this SPL build. > > > > What if you have two boot sources? > > Traditionally SPL has been small and purpose-built to do exactly one > thing -- so we decide at compile-time things that we might otherwise > decide at runtime. > > If there's a requirement for multiple boot sources decided at runtime, > then we'll obviously need a runtime mechanism. -- but it seems a bit > hackish to why does it matter whether it's two different types of > device, or two of the same type of device (possibly with different > controller types)? If the answer is that, for example, NAND versus USB > versus MMC is a likely use case, but two different NANDs is not likely, > is NAND versus OneNAND any more likely? > > Maybe spl_load_image should be a function pointer that board code sets, > with each implementation being distinctly named (in which case > nand_spl_load_image would become nand_spl_simple_load_image, unless we > move it to nand_spl_load.c and make nand_read_page a function pointer). > If needed to save a few bytes, we could use #defines to eliminate the > function pointers in a single-target SPL build. > > For now, until we decide to do something SPL-wide, call it what you want. > Well basically from what I see, you'd like me to do the NAND/OneNAND merge. As I already said, that's way out of the scope of this patchset so I'm not doing that. Either way, are you OK with current state of the patch? M _
Re: [U-Boot] [PATCH 4/4 V3] PXA: Adapt Voipac PXA270 to OneNAND SPL
On 11/03/2011 07:55 PM, Marek Vasut wrote: >> On 11/03/2011 04:52 PM, Marek Vasut wrote: >> Why do we want to separate them? What is the fundamental difference >> between OneNAND, and a high-level NAND controller such as fsl_elbc? > > Honestly, I'm not the author of the subsystem, but please check the > documentation. The way we retrieve data from onenand is different to NAND. What documentation? How is it different? There are substantial differences in how we "retrieve data" between drivers that use the NAND subsystem. Surely you've seen that in the mxs_nand driver. :-) >> Maybe there would be some differences on init if we can't produce >> "normal" ID data, but that doesn't justify duplicating the whole subsystem. > > Where do you see such duplication? cmd_onenand ? cmd_onenand and env_onenand are the most irritating, since they're at a layer that really shouldn't care about the differences -- we should probably have a plain "mtd" command instead, for most of the functionality. There's also onenand_bbt.c -- what are the hardware-based differences in how the bad block table is managed? nand_base.c/onenand_base.c are less clear. Obviously much of what is in onenand_base.c would be in the controller driver if it used the NAND subsystem. But it looks like some of it is duplication. >> Why should the code that just wants to use an API to move data around >> need to care which it is? Why should there be behavioral differences >> that aren't rooted in the actual hardware? Another approach might be to >> use MTD as the common interface, but factor out common code into >> libraries that drivers can use, and avoid the main nand_base.c code even >> for things like fsl_elbc. > > I think you're mistaken here. OneNAND != NAND. Well, last I tried I couldn't find any public documentation, so all I have to go on is the code, some marketing-type info, and asking questions of people that appear to know more about OneNAND than I do. :-) >From what I can see, it looks like NAND with an integrated controller that exposes an unusual command set, but still for the most part provides the same operations. Several of our existing NAND-subsystem drivers have to fake the command set for the generic layer as well. Initialization/identification might be a problem area that current drivers don't have to deal with, though. Actually integrating OneNAND with NAND would likely involve an already-needed restructuring of the subsystem. If the answer really is that it makes sense to consider OneNAND to be a totally different thing from NAND, then it's outside my jurisdiction as NAND custodian -- which is fine with me. Frankly, even if it does make sense to merge them, I'd rather not be custodian of the OneNAND stuff unless someone is actually willing to do the merge. I don't have access to hardware or documentation, and it's an entirely separate codebase. People just started sending me the patches a few years back. >> This is not a new complaint -- I've asked for this before but nobody's >> put the time into sorting out the mess (and I have neither time nor >> hardware nor documentation). The SPL load_image function is a simple >> enough interface to start with, though. :-) > > Well, it seems what you are proposing is way beyond the scope of this > patchset. >> >> In fact, it should probably just be spl_load_image() with whatever boot >> source has been configured into this SPL build. > > What if you have two boot sources? Traditionally SPL has been small and purpose-built to do exactly one thing -- so we decide at compile-time things that we might otherwise decide at runtime. If there's a requirement for multiple boot sources decided at runtime, then we'll obviously need a runtime mechanism. -- but it seems a bit hackish to why does it matter whether it's two different types of device, or two of the same type of device (possibly with different controller types)? If the answer is that, for example, NAND versus USB versus MMC is a likely use case, but two different NANDs is not likely, is NAND versus OneNAND any more likely? Maybe spl_load_image should be a function pointer that board code sets, with each implementation being distinctly named (in which case nand_spl_load_image would become nand_spl_simple_load_image, unless we move it to nand_spl_load.c and make nand_read_page a function pointer). If needed to save a few bytes, we could use #defines to eliminate the function pointers in a single-target SPL build. For now, until we decide to do something SPL-wide, call it what you want. -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/4 V3] PXA: Adapt Voipac PXA270 to OneNAND SPL
> On 11/03/2011 04:52 PM, Marek Vasut wrote: > >> On 11/02/2011 08:56 PM, Marek Vasut wrote: > >>> + onenand_spl_load_image(CONFIG_SPL_ONENAND_LOAD_ADDR, > >>> + CONFIG_SPL_ONENAND_LOAD_SIZE, > >>> + (void *)CONFIG_SYS_TEXT_BASE); > >> > >> If we make it "nand_spl_load_image", and make the #defines conform, we > >> can have the same code call the function for nand and onenand. I don't > >> see any reason why onenand is a completely different subsystem in > >> general, rather than just another NAND driver. The NAND subsystem's > >> driver interface is lower level than it should be, but that affects > >> other NAND controllers as well (such as fsl_elbc). > >> > >> Switching to the generic nand_boot() in > >> http://patchwork.ozlabs.org/patch/123219/ would get you the ability to > >> load the environment during the SPL. > >> > >> -Scott > > > > I don't think I understand. Why do you want to mix onenand and nand ? > > Why do we want to separate them? What is the fundamental difference > between OneNAND, and a high-level NAND controller such as fsl_elbc? Honestly, I'm not the author of the subsystem, but please check the documentation. The way we retrieve data from onenand is different to NAND. > Maybe there would be some differences on init if we can't produce > "normal" ID data, but that doesn't justify duplicating the whole subsystem. Where do you see such duplication? cmd_onenand ? > > Why should the code that just wants to use an API to move data around > need to care which it is? Why should there be behavioral differences > that aren't rooted in the actual hardware? Another approach might be to > use MTD as the common interface, but factor out common code into > libraries that drivers can use, and avoid the main nand_base.c code even > for things like fsl_elbc. I think you're mistaken here. OneNAND != NAND. > > This is not a new complaint -- I've asked for this before but nobody's > put the time into sorting out the mess (and I have neither time nor > hardware nor documentation). The SPL load_image function is a simple > enough interface to start with, though. :-) Well, it seems what you are proposing is way beyond the scope of this patchset. > > In fact, it should probably just be spl_load_image() with whatever boot > source has been configured into this SPL build. What if you have two boot sources? > > > Also, will your approach still allow me to squeeze the important code > > into the first 1kb for the initial copying of SPL? > > If you can't fit the common load sequence in, then of course don't use > it, but there's no need for the function name to be different. > > -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/4 V3] PXA: Adapt Voipac PXA270 to OneNAND SPL
On 11/03/2011 04:52 PM, Marek Vasut wrote: >> On 11/02/2011 08:56 PM, Marek Vasut wrote: >>> + onenand_spl_load_image(CONFIG_SPL_ONENAND_LOAD_ADDR, >>> + CONFIG_SPL_ONENAND_LOAD_SIZE, >>> + (void *)CONFIG_SYS_TEXT_BASE); >> >> If we make it "nand_spl_load_image", and make the #defines conform, we >> can have the same code call the function for nand and onenand. I don't >> see any reason why onenand is a completely different subsystem in >> general, rather than just another NAND driver. The NAND subsystem's >> driver interface is lower level than it should be, but that affects >> other NAND controllers as well (such as fsl_elbc). >> >> Switching to the generic nand_boot() in >> http://patchwork.ozlabs.org/patch/123219/ would get you the ability to >> load the environment during the SPL. >> >> -Scott > > I don't think I understand. Why do you want to mix onenand and nand ? Why do we want to separate them? What is the fundamental difference between OneNAND, and a high-level NAND controller such as fsl_elbc? Maybe there would be some differences on init if we can't produce "normal" ID data, but that doesn't justify duplicating the whole subsystem. Why should the code that just wants to use an API to move data around need to care which it is? Why should there be behavioral differences that aren't rooted in the actual hardware? Another approach might be to use MTD as the common interface, but factor out common code into libraries that drivers can use, and avoid the main nand_base.c code even for things like fsl_elbc. This is not a new complaint -- I've asked for this before but nobody's put the time into sorting out the mess (and I have neither time nor hardware nor documentation). The SPL load_image function is a simple enough interface to start with, though. :-) In fact, it should probably just be spl_load_image() with whatever boot source has been configured into this SPL build. > Also, will your approach still allow me to squeeze the important code into > the first 1kb > for the initial copying of SPL? If you can't fit the common load sequence in, then of course don't use it, but there's no need for the function name to be different. -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/4 V3] PXA: Adapt Voipac PXA270 to OneNAND SPL
> On 11/02/2011 08:56 PM, Marek Vasut wrote: > > + onenand_spl_load_image(CONFIG_SPL_ONENAND_LOAD_ADDR, > > + CONFIG_SPL_ONENAND_LOAD_SIZE, > > + (void *)CONFIG_SYS_TEXT_BASE); > > If we make it "nand_spl_load_image", and make the #defines conform, we > can have the same code call the function for nand and onenand. I don't > see any reason why onenand is a completely different subsystem in > general, rather than just another NAND driver. The NAND subsystem's > driver interface is lower level than it should be, but that affects > other NAND controllers as well (such as fsl_elbc). > > Switching to the generic nand_boot() in > http://patchwork.ozlabs.org/patch/123219/ would get you the ability to > load the environment during the SPL. > > -Scott I don't think I understand. Why do you want to mix onenand and nand ? Also, will your approach still allow me to squeeze the important code into the first 1kb for the initial copying of SPL? ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/4 V3] PXA: Adapt Voipac PXA270 to OneNAND SPL
On 11/02/2011 08:56 PM, Marek Vasut wrote: > + onenand_spl_load_image(CONFIG_SPL_ONENAND_LOAD_ADDR, > + CONFIG_SPL_ONENAND_LOAD_SIZE, > + (void *)CONFIG_SYS_TEXT_BASE); If we make it "nand_spl_load_image", and make the #defines conform, we can have the same code call the function for nand and onenand. I don't see any reason why onenand is a completely different subsystem in general, rather than just another NAND driver. The NAND subsystem's driver interface is lower level than it should be, but that affects other NAND controllers as well (such as fsl_elbc). Switching to the generic nand_boot() in http://patchwork.ozlabs.org/patch/123219/ would get you the ability to load the environment during the SPL. -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 4/4 V3] PXA: Adapt Voipac PXA270 to OneNAND SPL
Signed-off-by: Marek Vasut
Cc: Albert ARIBAUD
---
board/vpac270/Makefile |4 ++
board/vpac270/onenand.c | 66 ++
board/vpac270/u-boot-spl.lds | 92 ++
board/vpac270/vpac270.c |2 +
include/configs/vpac270.h| 25 +--
5 files changed, 184 insertions(+), 5 deletions(-)
create mode 100644 board/vpac270/onenand.c
create mode 100644 board/vpac270/u-boot-spl.lds
V2: Add missing u-boot-spl.lds, convert bitshifts to division,
convert to spl_onenand_load_image()
V3: Drop inlines, update linker script, update to conform new onenand_spl.
diff --git a/board/vpac270/Makefile b/board/vpac270/Makefile
index b5c60fd..5967055 100644
--- a/board/vpac270/Makefile
+++ b/board/vpac270/Makefile
@@ -23,7 +23,11 @@ include $(TOPDIR)/config.mk
LIB= $(obj)lib$(BOARD).o
+ifndef CONFIG_SPL_BUILD
COBJS := vpac270.o
+else
+COBJS := onenand.o
+endif
SRCS := $(COBJS:.o=.c)
OBJS := $(addprefix $(obj),$(COBJS))
diff --git a/board/vpac270/onenand.c b/board/vpac270/onenand.c
new file mode 100644
index 000..6a0a37b
--- /dev/null
+++ b/board/vpac270/onenand.c
@@ -0,0 +1,66 @@
+/*
+ * Voipac PXA270 OneNAND SPL
+ *
+ * Copyright (C) 2011 Marek Vasut
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include
+#include
+#include
+#include
+
+extern void pxa_dram_init(void);
+
+void board_init_f(unsigned long unused)
+{
+ extern uint32_t _end;
+ uint32_t tmp;
+
+ asm volatile("mov %0, pc" : "=r"(tmp));
+ tmp >>= 24;
+
+ /* The code runs from OneNAND RAM, copy SPL to SRAM and execute it. */
+ if (tmp == 0) {
+ tmp = (uint32_t)&_end - CONFIG_SPL_TEXT_BASE;
+ onenand_spl_load_image(0, tmp, (void *)CONFIG_SPL_TEXT_BASE);
+ asm volatile("mov pc, %0" : : "r"(CONFIG_SPL_TEXT_BASE));
+ }
+
+ /* Hereby, the code runs from (S)RAM, copy U-Boot and execute it. */
+ arch_cpu_init();
+ pxa_dram_init();
+ onenand_spl_load_image(CONFIG_SPL_ONENAND_LOAD_ADDR,
+ CONFIG_SPL_ONENAND_LOAD_SIZE,
+ (void *)CONFIG_SYS_TEXT_BASE);
+ asm volatile("mov pc, %0" : : "r"(CONFIG_SYS_TEXT_BASE));
+
+ for (;;)
+ ;
+}
+
+void __attribute__((noreturn)) hang(void)
+{
+ for (;;)
+ ;
+}
+
+void icache_disable(void) {}
+void dcache_disable(void) {}
diff --git a/board/vpac270/u-boot-spl.lds b/board/vpac270/u-boot-spl.lds
new file mode 100644
index 000..1958c2f
--- /dev/null
+++ b/board/vpac270/u-boot-spl.lds
@@ -0,0 +1,92 @@
+/*
+ * Copyright (C) 2011 Marek Vasut
+ * on behalf of DENX Software Engineering GmbH
+ *
+ * January 2004 - Changed to support H4 device
+ * Copyright (c) 2004-2008 Texas Instruments
+ *
+ * (C) Copyright 2002
+ * Gary Jennejohn, DENX Software Engineering,
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
+OUTPUT_ARCH(arm)
+ENTRY(_start)
+SECTIONS
+{
+ . = CONFIG_SPL_TEXT_BASE;
+ .text.0 :
+ {
+ arch/arm/cpu/pxa/start.o(.text*)
+ board/vpac270/libvpac270.o (.text*)
+ drivers/mtd/onenand/libonenand.o(.text*)
+ }
+
+
+ /* Start of the rest of the SPL */
+ . = CONFIG_SPL_TEXT_BASE + 0x800;
+
+ .text.1 :
+
