| Message ID | 1366344655-8535-2-git-send-email-marex@denx.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
Dear Marek Vasut, On Friday, April 19, 2013 6:10:51 AM, Marek Vasut wrote: > Add support for generic NAND SPL via the SPL framework into the > mxc_nand_spl driver. This is basically just a simple rename and > publication of the already implemented functions. To avoid the > old function which are used with the nand_spl/ stuff getting in > the way of NAND SPL framework, the macro CONFIG_SPL_NAND_LEGACY > was introduced and two remaining legacy boards were adjusted. > These board need to be either fixed or removed in the long run, > but I don't have either. > > Also make sure the requested payload is aligned to full pages, > otherwise this simple driver fails to load the last page. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> > Cc: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> > Cc: Fabio Estevam <fabio.estevam@freescale.com> > Cc: Scott Wood <scottwood@freescale.com> > Cc: Stefano Babic <sbabic@denx.de> > Cc: Tom Rini <trini@ti.com> > --- > drivers/mtd/nand/mxc_nand_spl.c | 13 ++++++++++--- > include/configs/mx31pdk.h | 1 + > include/configs/tx25.h | 1 + > 3 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/mtd/nand/mxc_nand_spl.c > b/drivers/mtd/nand/mxc_nand_spl.c > index 09f23c3..8ff03c9 100644 > --- a/drivers/mtd/nand/mxc_nand_spl.c > +++ b/drivers/mtd/nand/mxc_nand_spl.c > @@ -290,7 +290,7 @@ static int is_badblock(int pagenumber) > return 0; > } > > -static int nand_load(unsigned int from, unsigned int size, unsigned char > *buf) > +int nand_spl_load_image(uint32_t from, unsigned int size, void *buf) > { > int i; > unsigned int page; > @@ -303,6 +303,7 @@ static int nand_load(unsigned int from, unsigned int > size, unsigned char *buf) > page = from / CONFIG_SYS_NAND_PAGE_SIZE; > i = 0; > > + size = roundup(size, CONFIG_SYS_NAND_PAGE_SIZE); > while (i < size / CONFIG_SYS_NAND_PAGE_SIZE) { > if (nfc_read_page(page, buf) < 0) > return -1; > @@ -332,6 +333,7 @@ static int nand_load(unsigned int from, unsigned int > size, unsigned char *buf) > return 0; > } > > +#ifdef CONFIG_SPL_NAND_SUPPORT_LEGACY > /* > * The main entry for NAND booting. It's necessary that SDRAM is already > * configured and available since this code loads the main U-Boot image > @@ -345,8 +347,9 @@ void nand_boot(void) > * CONFIG_SYS_NAND_U_BOOT_OFFS and CONFIG_SYS_NAND_U_BOOT_SIZE must > * be aligned to full pages > */ > - if (!nand_load(CONFIG_SYS_NAND_U_BOOT_OFFS, CONFIG_SYS_NAND_U_BOOT_SIZE, > - (uchar *)CONFIG_SYS_NAND_U_BOOT_DST)) { > + if (!nand_spl_load_image(CONFIG_SYS_NAND_U_BOOT_OFFS, > + CONFIG_SYS_NAND_U_BOOT_SIZE, > + (uchar *)CONFIG_SYS_NAND_U_BOOT_DST)) { > /* Copy from NAND successful, start U-boot */ > uboot = (void *)CONFIG_SYS_NAND_U_BOOT_START; > uboot(); > @@ -364,3 +367,7 @@ void hang(void) > /* Loop forever */ > while (1) ; > } > +#endif > + > +void nand_init(void) {} > +void nand_deselect(void) {} > diff --git a/include/configs/mx31pdk.h b/include/configs/mx31pdk.h > index 1754595..217552e 100644 > --- a/include/configs/mx31pdk.h > +++ b/include/configs/mx31pdk.h > @@ -50,6 +50,7 @@ > #define CONFIG_SPL_LDSCRIPT "arch/$(ARCH)/cpu/u-boot-spl.lds" > #define CONFIG_SPL_MAX_SIZE 2048 > #define CONFIG_SPL_NAND_SUPPORT > +#define CONFIG_SPL_NAND_SUPPORT_LEGACY > > #define CONFIG_SPL_TEXT_BASE 0x87dc0000 > #define CONFIG_SYS_TEXT_BASE 0x87e00000 > diff --git a/include/configs/tx25.h b/include/configs/tx25.h > index e72f8f6..7c362d0 100644 > --- a/include/configs/tx25.h > +++ b/include/configs/tx25.h > @@ -37,6 +37,7 @@ > #define CONFIG_SPL_LDSCRIPT "arch/$(ARCH)/cpu/u-boot-spl.lds" > #define CONFIG_SPL_MAX_SIZE 2048 > #define CONFIG_SPL_NAND_SUPPORT > +#define CONFIG_SPL_NAND_SUPPORT_LEGACY > > #define CONFIG_SPL_TEXT_BASE 0x810c0000 > #define CONFIG_SYS_TEXT_BASE 0x81200000 > -- > 1.7.11.7 This is not about legacy vs. non-legacy. This is about basic vs. more featured SPL because of SPL size constraints. So what about dropping CONFIG_SPL_NAND_SUPPORT_LEGACY and testing for CONFIG_SPL_FRAMEWORK definition instead? Best regards, Benoît
On Friday, April 19, 2013 10:38:48 AM, Benoît Thébaudeau wrote: > Dear Marek Vasut, > > On Friday, April 19, 2013 6:10:51 AM, Marek Vasut wrote: > > Add support for generic NAND SPL via the SPL framework into the > > mxc_nand_spl driver. This is basically just a simple rename and > > publication of the already implemented functions. To avoid the > > old function which are used with the nand_spl/ stuff getting in > > the way of NAND SPL framework, the macro CONFIG_SPL_NAND_LEGACY > > was introduced and two remaining legacy boards were adjusted. > > These board need to be either fixed or removed in the long run, > > but I don't have either. > > > > Also make sure the requested payload is aligned to full pages, > > otherwise this simple driver fails to load the last page. > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> > > Cc: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> > > Cc: Fabio Estevam <fabio.estevam@freescale.com> > > Cc: Scott Wood <scottwood@freescale.com> > > Cc: Stefano Babic <sbabic@denx.de> > > Cc: Tom Rini <trini@ti.com> > > --- > > drivers/mtd/nand/mxc_nand_spl.c | 13 ++++++++++--- > > include/configs/mx31pdk.h | 1 + > > include/configs/tx25.h | 1 + > > 3 files changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/mtd/nand/mxc_nand_spl.c > > b/drivers/mtd/nand/mxc_nand_spl.c > > index 09f23c3..8ff03c9 100644 > > --- a/drivers/mtd/nand/mxc_nand_spl.c > > +++ b/drivers/mtd/nand/mxc_nand_spl.c > > @@ -290,7 +290,7 @@ static int is_badblock(int pagenumber) > > return 0; > > } > > > > -static int nand_load(unsigned int from, unsigned int size, unsigned char > > *buf) > > +int nand_spl_load_image(uint32_t from, unsigned int size, void *buf) > > { > > int i; > > unsigned int page; > > @@ -303,6 +303,7 @@ static int nand_load(unsigned int from, unsigned int > > size, unsigned char *buf) > > page = from / CONFIG_SYS_NAND_PAGE_SIZE; > > i = 0; > > > > + size = roundup(size, CONFIG_SYS_NAND_PAGE_SIZE); > > while (i < size / CONFIG_SYS_NAND_PAGE_SIZE) { > > if (nfc_read_page(page, buf) < 0) > > return -1; > > @@ -332,6 +333,7 @@ static int nand_load(unsigned int from, unsigned int > > size, unsigned char *buf) > > return 0; > > } > > > > +#ifdef CONFIG_SPL_NAND_SUPPORT_LEGACY > > /* > > * The main entry for NAND booting. It's necessary that SDRAM is already > > * configured and available since this code loads the main U-Boot image > > @@ -345,8 +347,9 @@ void nand_boot(void) > > * CONFIG_SYS_NAND_U_BOOT_OFFS and CONFIG_SYS_NAND_U_BOOT_SIZE must > > * be aligned to full pages > > */ > > - if (!nand_load(CONFIG_SYS_NAND_U_BOOT_OFFS, CONFIG_SYS_NAND_U_BOOT_SIZE, > > - (uchar *)CONFIG_SYS_NAND_U_BOOT_DST)) { > > + if (!nand_spl_load_image(CONFIG_SYS_NAND_U_BOOT_OFFS, > > + CONFIG_SYS_NAND_U_BOOT_SIZE, > > + (uchar *)CONFIG_SYS_NAND_U_BOOT_DST)) { > > /* Copy from NAND successful, start U-boot */ > > uboot = (void *)CONFIG_SYS_NAND_U_BOOT_START; > > uboot(); > > @@ -364,3 +367,7 @@ void hang(void) > > /* Loop forever */ > > while (1) ; > > } > > +#endif > > + > > +void nand_init(void) {} > > +void nand_deselect(void) {} > > diff --git a/include/configs/mx31pdk.h b/include/configs/mx31pdk.h > > index 1754595..217552e 100644 > > --- a/include/configs/mx31pdk.h > > +++ b/include/configs/mx31pdk.h > > @@ -50,6 +50,7 @@ > > #define CONFIG_SPL_LDSCRIPT "arch/$(ARCH)/cpu/u-boot-spl.lds" > > #define CONFIG_SPL_MAX_SIZE 2048 > > #define CONFIG_SPL_NAND_SUPPORT > > +#define CONFIG_SPL_NAND_SUPPORT_LEGACY > > > > #define CONFIG_SPL_TEXT_BASE 0x87dc0000 > > #define CONFIG_SYS_TEXT_BASE 0x87e00000 > > diff --git a/include/configs/tx25.h b/include/configs/tx25.h > > index e72f8f6..7c362d0 100644 > > --- a/include/configs/tx25.h > > +++ b/include/configs/tx25.h > > @@ -37,6 +37,7 @@ > > #define CONFIG_SPL_LDSCRIPT "arch/$(ARCH)/cpu/u-boot-spl.lds" > > #define CONFIG_SPL_MAX_SIZE 2048 > > #define CONFIG_SPL_NAND_SUPPORT > > +#define CONFIG_SPL_NAND_SUPPORT_LEGACY > > > > #define CONFIG_SPL_TEXT_BASE 0x810c0000 > > #define CONFIG_SYS_TEXT_BASE 0x81200000 > > -- > > 1.7.11.7 > > This is not about legacy vs. non-legacy. This is about basic vs. more > featured > SPL because of SPL size constraints. So what about dropping > CONFIG_SPL_NAND_SUPPORT_LEGACY and testing for CONFIG_SPL_FRAMEWORK > definition > instead? Also, I don't see any nand_init() (called by spl_nand_load_image()) for m53evk. Best regards, Benoît
Dear Benoît Thébaudeau, > On Friday, April 19, 2013 10:38:48 AM, Benoît Thébaudeau wrote: > > Dear Marek Vasut, > > > > On Friday, April 19, 2013 6:10:51 AM, Marek Vasut wrote: > > > Add support for generic NAND SPL via the SPL framework into the > > > mxc_nand_spl driver. This is basically just a simple rename and > > > publication of the already implemented functions. To avoid the > > > old function which are used with the nand_spl/ stuff getting in > > > the way of NAND SPL framework, the macro CONFIG_SPL_NAND_LEGACY > > > was introduced and two remaining legacy boards were adjusted. > > > These board need to be either fixed or removed in the long run, > > > but I don't have either. > > > > > > Also make sure the requested payload is aligned to full pages, > > > otherwise this simple driver fails to load the last page. > > > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > > Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> > > > Cc: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> > > > Cc: Fabio Estevam <fabio.estevam@freescale.com> > > > Cc: Scott Wood <scottwood@freescale.com> > > > Cc: Stefano Babic <sbabic@denx.de> > > > Cc: Tom Rini <trini@ti.com> > > > --- > > > > > > drivers/mtd/nand/mxc_nand_spl.c | 13 ++++++++++--- > > > include/configs/mx31pdk.h | 1 + > > > include/configs/tx25.h | 1 + > > > 3 files changed, 12 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/mtd/nand/mxc_nand_spl.c > > > b/drivers/mtd/nand/mxc_nand_spl.c > > > index 09f23c3..8ff03c9 100644 > > > --- a/drivers/mtd/nand/mxc_nand_spl.c > > > +++ b/drivers/mtd/nand/mxc_nand_spl.c > > > @@ -290,7 +290,7 @@ static int is_badblock(int pagenumber) > > > > > > return 0; > > > > > > } > > > > > > -static int nand_load(unsigned int from, unsigned int size, unsigned > > > char *buf) > > > +int nand_spl_load_image(uint32_t from, unsigned int size, void *buf) > > > > > > { > > > > > > int i; > > > unsigned int page; > > > > > > @@ -303,6 +303,7 @@ static int nand_load(unsigned int from, unsigned > > > int size, unsigned char *buf) > > > > > > page = from / CONFIG_SYS_NAND_PAGE_SIZE; > > > i = 0; > > > > > > + size = roundup(size, CONFIG_SYS_NAND_PAGE_SIZE); > > > > > > while (i < size / CONFIG_SYS_NAND_PAGE_SIZE) { > > > > > > if (nfc_read_page(page, buf) < 0) > > > > > > return -1; > > > > > > @@ -332,6 +333,7 @@ static int nand_load(unsigned int from, unsigned > > > int size, unsigned char *buf) > > > > > > return 0; > > > > > > } > > > > > > +#ifdef CONFIG_SPL_NAND_SUPPORT_LEGACY > > > > > > /* > > > > > > * The main entry for NAND booting. It's necessary that SDRAM is > > > already * configured and available since this code loads the main > > > U-Boot image > > > > > > @@ -345,8 +347,9 @@ void nand_boot(void) > > > > > > * CONFIG_SYS_NAND_U_BOOT_OFFS and CONFIG_SYS_NAND_U_BOOT_SIZE must > > > * be aligned to full pages > > > */ > > > > > > - if (!nand_load(CONFIG_SYS_NAND_U_BOOT_OFFS, > > > CONFIG_SYS_NAND_U_BOOT_SIZE, - (uchar > > > *)CONFIG_SYS_NAND_U_BOOT_DST)) { > > > + if (!nand_spl_load_image(CONFIG_SYS_NAND_U_BOOT_OFFS, > > > + CONFIG_SYS_NAND_U_BOOT_SIZE, > > > + (uchar *)CONFIG_SYS_NAND_U_BOOT_DST)) { > > > > > > /* Copy from NAND successful, start U-boot */ > > > uboot = (void *)CONFIG_SYS_NAND_U_BOOT_START; > > > uboot(); > > > > > > @@ -364,3 +367,7 @@ void hang(void) > > > > > > /* Loop forever */ > > > while (1) ; > > > > > > } > > > > > > +#endif > > > + > > > +void nand_init(void) {} > > > +void nand_deselect(void) {} > > > diff --git a/include/configs/mx31pdk.h b/include/configs/mx31pdk.h > > > index 1754595..217552e 100644 > > > --- a/include/configs/mx31pdk.h > > > +++ b/include/configs/mx31pdk.h > > > @@ -50,6 +50,7 @@ > > > > > > #define CONFIG_SPL_LDSCRIPT "arch/$(ARCH)/cpu/u-boot-spl.lds" > > > #define CONFIG_SPL_MAX_SIZE 2048 > > > #define CONFIG_SPL_NAND_SUPPORT > > > > > > +#define CONFIG_SPL_NAND_SUPPORT_LEGACY > > > > > > #define CONFIG_SPL_TEXT_BASE 0x87dc0000 > > > #define CONFIG_SYS_TEXT_BASE 0x87e00000 > > > > > > diff --git a/include/configs/tx25.h b/include/configs/tx25.h > > > index e72f8f6..7c362d0 100644 > > > --- a/include/configs/tx25.h > > > +++ b/include/configs/tx25.h > > > @@ -37,6 +37,7 @@ > > > > > > #define CONFIG_SPL_LDSCRIPT "arch/$(ARCH)/cpu/u-boot- spl.lds" > > > #define CONFIG_SPL_MAX_SIZE 2048 > > > #define CONFIG_SPL_NAND_SUPPORT > > > > > > +#define CONFIG_SPL_NAND_SUPPORT_LEGACY > > > > > > #define CONFIG_SPL_TEXT_BASE 0x810c0000 > > > #define CONFIG_SYS_TEXT_BASE 0x81200000 > > > > > > -- > > > 1.7.11.7 > > > > This is not about legacy vs. non-legacy. This is about basic vs. more > > featured > > SPL because of SPL size constraints. So what about dropping > > CONFIG_SPL_NAND_SUPPORT_LEGACY and testing for CONFIG_SPL_FRAMEWORK > > definition > > instead? I was thinking about that, but the symbol is unrelated to NAND. I still think it's either a matter of fixing for new SPL or removing those two boards. The nand_spl/ stuff shall be removed ASAP. > Also, I don't see any nand_init() (called by spl_nand_load_image()) for > m53evk. What do you mean? Best regards, Marek Vasut
Dear Marek Vasut, On Friday, April 19, 2013 1:14:16 PM, Marek Vasut wrote: > Dear Benoît Thébaudeau, > > > On Friday, April 19, 2013 10:38:48 AM, Benoît Thébaudeau wrote: > > > Dear Marek Vasut, > > > > > > On Friday, April 19, 2013 6:10:51 AM, Marek Vasut wrote: > > > > Add support for generic NAND SPL via the SPL framework into the > > > > mxc_nand_spl driver. This is basically just a simple rename and > > > > publication of the already implemented functions. To avoid the > > > > old function which are used with the nand_spl/ stuff getting in > > > > the way of NAND SPL framework, the macro CONFIG_SPL_NAND_LEGACY > > > > was introduced and two remaining legacy boards were adjusted. > > > > These board need to be either fixed or removed in the long run, > > > > but I don't have either. > > > > > > > > Also make sure the requested payload is aligned to full pages, > > > > otherwise this simple driver fails to load the last page. > > > > > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > > > Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> > > > > Cc: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> > > > > Cc: Fabio Estevam <fabio.estevam@freescale.com> > > > > Cc: Scott Wood <scottwood@freescale.com> > > > > Cc: Stefano Babic <sbabic@denx.de> > > > > Cc: Tom Rini <trini@ti.com> > > > > --- > > > > > > > > drivers/mtd/nand/mxc_nand_spl.c | 13 ++++++++++--- > > > > include/configs/mx31pdk.h | 1 + > > > > include/configs/tx25.h | 1 + > > > > 3 files changed, 12 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/mtd/nand/mxc_nand_spl.c > > > > b/drivers/mtd/nand/mxc_nand_spl.c > > > > index 09f23c3..8ff03c9 100644 > > > > --- a/drivers/mtd/nand/mxc_nand_spl.c > > > > +++ b/drivers/mtd/nand/mxc_nand_spl.c > > > > @@ -290,7 +290,7 @@ static int is_badblock(int pagenumber) > > > > > > > > return 0; > > > > > > > > } > > > > > > > > -static int nand_load(unsigned int from, unsigned int size, unsigned > > > > char *buf) > > > > +int nand_spl_load_image(uint32_t from, unsigned int size, void *buf) > > > > > > > > { > > > > > > > > int i; > > > > unsigned int page; > > > > > > > > @@ -303,6 +303,7 @@ static int nand_load(unsigned int from, unsigned > > > > int size, unsigned char *buf) > > > > > > > > page = from / CONFIG_SYS_NAND_PAGE_SIZE; > > > > i = 0; > > > > > > > > + size = roundup(size, CONFIG_SYS_NAND_PAGE_SIZE); > > > > > > > > while (i < size / CONFIG_SYS_NAND_PAGE_SIZE) { > > > > > > > > if (nfc_read_page(page, buf) < 0) > > > > > > > > return -1; > > > > > > > > @@ -332,6 +333,7 @@ static int nand_load(unsigned int from, unsigned > > > > int size, unsigned char *buf) > > > > > > > > return 0; > > > > > > > > } > > > > > > > > +#ifdef CONFIG_SPL_NAND_SUPPORT_LEGACY > > > > > > > > /* > > > > > > > > * The main entry for NAND booting. It's necessary that SDRAM is > > > > already * configured and available since this code loads the main > > > > U-Boot image > > > > > > > > @@ -345,8 +347,9 @@ void nand_boot(void) > > > > > > > > * CONFIG_SYS_NAND_U_BOOT_OFFS and CONFIG_SYS_NAND_U_BOOT_SIZE must > > > > * be aligned to full pages > > > > */ > > > > > > > > - if (!nand_load(CONFIG_SYS_NAND_U_BOOT_OFFS, > > > > CONFIG_SYS_NAND_U_BOOT_SIZE, - (uchar > > > > *)CONFIG_SYS_NAND_U_BOOT_DST)) { > > > > + if (!nand_spl_load_image(CONFIG_SYS_NAND_U_BOOT_OFFS, > > > > + CONFIG_SYS_NAND_U_BOOT_SIZE, > > > > + (uchar *)CONFIG_SYS_NAND_U_BOOT_DST)) { > > > > > > > > /* Copy from NAND successful, start U-boot */ > > > > uboot = (void *)CONFIG_SYS_NAND_U_BOOT_START; > > > > uboot(); > > > > > > > > @@ -364,3 +367,7 @@ void hang(void) > > > > > > > > /* Loop forever */ > > > > while (1) ; > > > > > > > > } > > > > > > > > +#endif > > > > + > > > > +void nand_init(void) {} > > > > +void nand_deselect(void) {} > > > > diff --git a/include/configs/mx31pdk.h b/include/configs/mx31pdk.h > > > > index 1754595..217552e 100644 > > > > --- a/include/configs/mx31pdk.h > > > > +++ b/include/configs/mx31pdk.h > > > > @@ -50,6 +50,7 @@ > > > > > > > > #define CONFIG_SPL_LDSCRIPT "arch/$(ARCH)/cpu/u-boot-spl.lds" > > > > #define CONFIG_SPL_MAX_SIZE 2048 > > > > #define CONFIG_SPL_NAND_SUPPORT > > > > > > > > +#define CONFIG_SPL_NAND_SUPPORT_LEGACY > > > > > > > > #define CONFIG_SPL_TEXT_BASE 0x87dc0000 > > > > #define CONFIG_SYS_TEXT_BASE 0x87e00000 > > > > > > > > diff --git a/include/configs/tx25.h b/include/configs/tx25.h > > > > index e72f8f6..7c362d0 100644 > > > > --- a/include/configs/tx25.h > > > > +++ b/include/configs/tx25.h > > > > @@ -37,6 +37,7 @@ > > > > > > > > #define CONFIG_SPL_LDSCRIPT "arch/$(ARCH)/cpu/u-boot- > spl.lds" > > > > #define CONFIG_SPL_MAX_SIZE 2048 > > > > #define CONFIG_SPL_NAND_SUPPORT > > > > > > > > +#define CONFIG_SPL_NAND_SUPPORT_LEGACY > > > > > > > > #define CONFIG_SPL_TEXT_BASE 0x810c0000 > > > > #define CONFIG_SYS_TEXT_BASE 0x81200000 > > > > > > > > -- > > > > 1.7.11.7 > > > > > > This is not about legacy vs. non-legacy. This is about basic vs. more > > > featured > > > SPL because of SPL size constraints. So what about dropping > > > CONFIG_SPL_NAND_SUPPORT_LEGACY and testing for CONFIG_SPL_FRAMEWORK > > > definition > > > instead? > > I was thinking about that, but the symbol is unrelated to NAND. Well, it's CONFIG_SPL_FRAMEWORK + CONFIG_SPL_NAND_SUPPORT that defines the other implementation, and CONFIG_SPL_NAND_SUPPORT triggers the build of mxc_nand_spl.c for SPL, so the common point is CONFIG_SPL_FRAMEWORK. > I still think > it's either a matter of fixing for new SPL or removing those two boards. The > nand_spl/ stuff shall be removed ASAP. Removing those boards is not a solution. Is it really about "new" SPL? The generic SPL is enabled by CONFIG_SPL, and CONFIG_SPL_FRAMEWORK is a sub-option. If the generic SPL rules imposed to use the SPL framework functions, there would be no such sub-option. So it looks like these boards are complying to the new SPL rules. We could see if using CONFIG_SPL_FRAMEWORK would still allow the SPL to fit in 2 kiB in order to drop this function, but if it does not fit, the new SPL rules should still make it possible to have a solution for any board having SPL size constraints. > > Also, I don't see any nand_init() (called by spl_nand_load_image()) for > > m53evk. > > What do you mean? I mean that I don't identify the implementation of nand_init() used in the context of the m53evk since the builds of drivers/mtd/nand/nand.c and drivers/mtd/nand/nand_spl_simple.c do not seem to be enabled for this board, and I don't see another definition. Best regards, Benoît
On 2013-04-19 06:10:51 (+0200), Marek Vasut <marex@denx.de> wrote: > To avoid the old function which are used with the nand_spl/ stuff > getting in the way of NAND SPL framework, the macro > CONFIG_SPL_NAND_LEGACY was introduced and two remaining legacy > boards were adjusted. These board need to be either fixed or > removed in the long run, but I don't have either. It sounds like "fixing" these boards is mainly a matter of confirming that a configuration for them based around CONFIG_SPL_FRAMEWORK will fit in 2K. If so, I don't think there is any reason to keep legacy support around. > +void nand_init(void) {} > +void nand_deselect(void) {} Couldn't you just rename nfc_nand_init() to nand_init()? - Philip
Dear Benoît Thébaudeau, > Dear Marek Vasut, > > On Friday, April 19, 2013 1:14:16 PM, Marek Vasut wrote: > > Dear Benoît Thébaudeau, > > > > > On Friday, April 19, 2013 10:38:48 AM, Benoît Thébaudeau wrote: > > > > Dear Marek Vasut, > > > > > > > > On Friday, April 19, 2013 6:10:51 AM, Marek Vasut wrote: > > > > > Add support for generic NAND SPL via the SPL framework into the > > > > > mxc_nand_spl driver. This is basically just a simple rename and > > > > > publication of the already implemented functions. To avoid the > > > > > old function which are used with the nand_spl/ stuff getting in > > > > > the way of NAND SPL framework, the macro CONFIG_SPL_NAND_LEGACY > > > > > was introduced and two remaining legacy boards were adjusted. > > > > > These board need to be either fixed or removed in the long run, > > > > > but I don't have either. > > > > > > > > > > Also make sure the requested payload is aligned to full pages, > > > > > otherwise this simple driver fails to load the last page. > > > > > > > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > > > > Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> > > > > > Cc: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> > > > > > Cc: Fabio Estevam <fabio.estevam@freescale.com> > > > > > Cc: Scott Wood <scottwood@freescale.com> > > > > > Cc: Stefano Babic <sbabic@denx.de> > > > > > Cc: Tom Rini <trini@ti.com> > > > > > --- > > > > > > > > > > drivers/mtd/nand/mxc_nand_spl.c | 13 ++++++++++--- > > > > > include/configs/mx31pdk.h | 1 + > > > > > include/configs/tx25.h | 1 + > > > > > 3 files changed, 12 insertions(+), 3 deletions(-) > > > > > > > > > > diff --git a/drivers/mtd/nand/mxc_nand_spl.c > > > > > b/drivers/mtd/nand/mxc_nand_spl.c > > > > > index 09f23c3..8ff03c9 100644 > > > > > --- a/drivers/mtd/nand/mxc_nand_spl.c > > > > > +++ b/drivers/mtd/nand/mxc_nand_spl.c > > > > > @@ -290,7 +290,7 @@ static int is_badblock(int pagenumber) > > > > > > > > > > return 0; > > > > > > > > > > } > > > > > > > > > > -static int nand_load(unsigned int from, unsigned int size, > > > > > unsigned char *buf) > > > > > +int nand_spl_load_image(uint32_t from, unsigned int size, void > > > > > *buf) > > > > > > > > > > { > > > > > > > > > > int i; > > > > > unsigned int page; > > > > > > > > > > @@ -303,6 +303,7 @@ static int nand_load(unsigned int from, > > > > > unsigned int size, unsigned char *buf) > > > > > > > > > > page = from / CONFIG_SYS_NAND_PAGE_SIZE; > > > > > i = 0; > > > > > > > > > > + size = roundup(size, CONFIG_SYS_NAND_PAGE_SIZE); > > > > > > > > > > while (i < size / CONFIG_SYS_NAND_PAGE_SIZE) { > > > > > > > > > > if (nfc_read_page(page, buf) < 0) > > > > > > > > > > return -1; > > > > > > > > > > @@ -332,6 +333,7 @@ static int nand_load(unsigned int from, > > > > > unsigned int size, unsigned char *buf) > > > > > > > > > > return 0; > > > > > > > > > > } > > > > > > > > > > +#ifdef CONFIG_SPL_NAND_SUPPORT_LEGACY > > > > > > > > > > /* > > > > > > > > > > * The main entry for NAND booting. It's necessary that SDRAM is > > > > > already * configured and available since this code loads the main > > > > > U-Boot image > > > > > > > > > > @@ -345,8 +347,9 @@ void nand_boot(void) > > > > > > > > > > * CONFIG_SYS_NAND_U_BOOT_OFFS and CONFIG_SYS_NAND_U_BOOT_SIZE > > > > > must * be aligned to full pages > > > > > */ > > > > > > > > > > - if (!nand_load(CONFIG_SYS_NAND_U_BOOT_OFFS, > > > > > CONFIG_SYS_NAND_U_BOOT_SIZE, - (uchar > > > > > *)CONFIG_SYS_NAND_U_BOOT_DST)) { > > > > > + if (!nand_spl_load_image(CONFIG_SYS_NAND_U_BOOT_OFFS, > > > > > + CONFIG_SYS_NAND_U_BOOT_SIZE, > > > > > + (uchar *)CONFIG_SYS_NAND_U_BOOT_DST)) { > > > > > > > > > > /* Copy from NAND successful, start U-boot */ > > > > > uboot = (void *)CONFIG_SYS_NAND_U_BOOT_START; > > > > > uboot(); > > > > > > > > > > @@ -364,3 +367,7 @@ void hang(void) > > > > > > > > > > /* Loop forever */ > > > > > while (1) ; > > > > > > > > > > } > > > > > > > > > > +#endif > > > > > + > > > > > +void nand_init(void) {} > > > > > +void nand_deselect(void) {} > > > > > diff --git a/include/configs/mx31pdk.h b/include/configs/mx31pdk.h > > > > > index 1754595..217552e 100644 > > > > > --- a/include/configs/mx31pdk.h > > > > > +++ b/include/configs/mx31pdk.h > > > > > @@ -50,6 +50,7 @@ > > > > > > > > > > #define CONFIG_SPL_LDSCRIPT "arch/$(ARCH)/cpu/u-boot-spl.lds" > > > > > #define CONFIG_SPL_MAX_SIZE 2048 > > > > > #define CONFIG_SPL_NAND_SUPPORT > > > > > > > > > > +#define CONFIG_SPL_NAND_SUPPORT_LEGACY > > > > > > > > > > #define CONFIG_SPL_TEXT_BASE 0x87dc0000 > > > > > #define CONFIG_SYS_TEXT_BASE 0x87e00000 > > > > > > > > > > diff --git a/include/configs/tx25.h b/include/configs/tx25.h > > > > > index e72f8f6..7c362d0 100644 > > > > > --- a/include/configs/tx25.h > > > > > +++ b/include/configs/tx25.h > > > > > @@ -37,6 +37,7 @@ > > > > > > > > > > #define CONFIG_SPL_LDSCRIPT "arch/$(ARCH)/cpu/u-boot- > > > > spl.lds" > > > > > > > #define CONFIG_SPL_MAX_SIZE 2048 > > > > > #define CONFIG_SPL_NAND_SUPPORT > > > > > > > > > > +#define CONFIG_SPL_NAND_SUPPORT_LEGACY > > > > > > > > > > #define CONFIG_SPL_TEXT_BASE 0x810c0000 > > > > > #define CONFIG_SYS_TEXT_BASE 0x81200000 > > > > > > > > > > -- > > > > > 1.7.11.7 > > > > > > > > This is not about legacy vs. non-legacy. This is about basic vs. more > > > > featured > > > > SPL because of SPL size constraints. So what about dropping > > > > CONFIG_SPL_NAND_SUPPORT_LEGACY and testing for CONFIG_SPL_FRAMEWORK > > > > definition > > > > instead? > > > > I was thinking about that, but the symbol is unrelated to NAND. > > Well, it's CONFIG_SPL_FRAMEWORK + CONFIG_SPL_NAND_SUPPORT that defines the > other implementation, and CONFIG_SPL_NAND_SUPPORT triggers the build of > mxc_nand_spl.c for SPL, so the common point is CONFIG_SPL_FRAMEWORK. > > > I still think > > it's either a matter of fixing for new SPL or removing those two boards. > > The nand_spl/ stuff shall be removed ASAP. > > Removing those boards is not a solution. > > Is it really about "new" SPL? The generic SPL is enabled by CONFIG_SPL, and > CONFIG_SPL_FRAMEWORK is a sub-option. If the generic SPL rules imposed to > use the SPL framework functions, there would be no such sub-option. So it > looks like these boards are complying to the new SPL rules. > > We could see if using CONFIG_SPL_FRAMEWORK would still allow the SPL to fit > in 2 kiB in order to drop this function, but if it does not fit, the new > SPL rules should still make it possible to have a solution for any board > having SPL size constraints. Rather than this, the other option would be to make whatever calls nand_boot() compatible with the SPL frameworks' implementation of spl_nand, so we don't need different function calls. > > > Also, I don't see any nand_init() (called by spl_nand_load_image()) for > > > m53evk. > > > > What do you mean? > > I mean that I don't identify the implementation of nand_init() used in the > context of the m53evk since the builds of drivers/mtd/nand/nand.c and > drivers/mtd/nand/nand_spl_simple.c do not seem to be enabled for this > board, and I don't see another definition. It's there, as you said somewhere in the thread below ;-)
Dear Philip Paeps, > On 2013-04-19 06:10:51 (+0200), Marek Vasut <marex@denx.de> wrote: > > To avoid the old function which are used with the nand_spl/ stuff > > getting in the way of NAND SPL framework, the macro > > CONFIG_SPL_NAND_LEGACY was introduced and two remaining legacy > > boards were adjusted. These board need to be either fixed or > > removed in the long run, but I don't have either. > > It sounds like "fixing" these boards is mainly a matter of confirming > that a configuration for them based around CONFIG_SPL_FRAMEWORK will fit > in 2K. If so, I don't think there is any reason to keep legacy support > around. These boards are too limited, so we can do nothing about them but to keep this hack in. > > +void nand_init(void) {} > > +void nand_deselect(void) {} > > Couldn't you just rename nfc_nand_init() to nand_init()? What good would that do? Best regards, Marek Vasut
Dear Marek Vasut, On Friday, April 19, 2013 7:06:39 PM, Marek Vasut wrote: > Subject: Re: [PATCH 2/6] nand: Add SPL_NAND support to mxc_nand_spl > > Dear Benoît Thébaudeau, > > > Dear Marek Vasut, > > > > On Friday, April 19, 2013 1:14:16 PM, Marek Vasut wrote: > > > Dear Benoît Thébaudeau, > > > > > > > On Friday, April 19, 2013 10:38:48 AM, Benoît Thébaudeau wrote: > > > > > Dear Marek Vasut, > > > > > > > > > > On Friday, April 19, 2013 6:10:51 AM, Marek Vasut wrote: > > > > > > Add support for generic NAND SPL via the SPL framework into the > > > > > > mxc_nand_spl driver. This is basically just a simple rename and > > > > > > publication of the already implemented functions. To avoid the > > > > > > old function which are used with the nand_spl/ stuff getting in > > > > > > the way of NAND SPL framework, the macro CONFIG_SPL_NAND_LEGACY > > > > > > was introduced and two remaining legacy boards were adjusted. > > > > > > These board need to be either fixed or removed in the long run, > > > > > > but I don't have either. > > > > > > > > > > > > Also make sure the requested payload is aligned to full pages, > > > > > > otherwise this simple driver fails to load the last page. > > > > > > > > > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > > > > > Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> > > > > > > Cc: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> > > > > > > Cc: Fabio Estevam <fabio.estevam@freescale.com> > > > > > > Cc: Scott Wood <scottwood@freescale.com> > > > > > > Cc: Stefano Babic <sbabic@denx.de> > > > > > > Cc: Tom Rini <trini@ti.com> > > > > > > --- > > > > > > > > > > > > drivers/mtd/nand/mxc_nand_spl.c | 13 ++++++++++--- > > > > > > include/configs/mx31pdk.h | 1 + > > > > > > include/configs/tx25.h | 1 + > > > > > > 3 files changed, 12 insertions(+), 3 deletions(-) > > > > > > > > > > > > diff --git a/drivers/mtd/nand/mxc_nand_spl.c > > > > > > b/drivers/mtd/nand/mxc_nand_spl.c > > > > > > index 09f23c3..8ff03c9 100644 > > > > > > --- a/drivers/mtd/nand/mxc_nand_spl.c > > > > > > +++ b/drivers/mtd/nand/mxc_nand_spl.c > > > > > > @@ -290,7 +290,7 @@ static int is_badblock(int pagenumber) > > > > > > > > > > > > return 0; > > > > > > > > > > > > } > > > > > > > > > > > > -static int nand_load(unsigned int from, unsigned int size, > > > > > > unsigned char *buf) > > > > > > +int nand_spl_load_image(uint32_t from, unsigned int size, void > > > > > > *buf) > > > > > > > > > > > > { > > > > > > > > > > > > int i; > > > > > > unsigned int page; > > > > > > > > > > > > @@ -303,6 +303,7 @@ static int nand_load(unsigned int from, > > > > > > unsigned int size, unsigned char *buf) > > > > > > > > > > > > page = from / CONFIG_SYS_NAND_PAGE_SIZE; > > > > > > i = 0; > > > > > > > > > > > > + size = roundup(size, CONFIG_SYS_NAND_PAGE_SIZE); > > > > > > > > > > > > while (i < size / CONFIG_SYS_NAND_PAGE_SIZE) { > > > > > > > > > > > > if (nfc_read_page(page, buf) < 0) > > > > > > > > > > > > return -1; > > > > > > > > > > > > @@ -332,6 +333,7 @@ static int nand_load(unsigned int from, > > > > > > unsigned int size, unsigned char *buf) > > > > > > > > > > > > return 0; > > > > > > > > > > > > } > > > > > > > > > > > > +#ifdef CONFIG_SPL_NAND_SUPPORT_LEGACY > > > > > > > > > > > > /* > > > > > > > > > > > > * The main entry for NAND booting. It's necessary that SDRAM is > > > > > > already * configured and available since this code loads the main > > > > > > U-Boot image > > > > > > > > > > > > @@ -345,8 +347,9 @@ void nand_boot(void) > > > > > > > > > > > > * CONFIG_SYS_NAND_U_BOOT_OFFS and CONFIG_SYS_NAND_U_BOOT_SIZE > > > > > > must * be aligned to full pages > > > > > > */ > > > > > > > > > > > > - if (!nand_load(CONFIG_SYS_NAND_U_BOOT_OFFS, > > > > > > CONFIG_SYS_NAND_U_BOOT_SIZE, - (uchar > > > > > > *)CONFIG_SYS_NAND_U_BOOT_DST)) { > > > > > > + if (!nand_spl_load_image(CONFIG_SYS_NAND_U_BOOT_OFFS, > > > > > > + CONFIG_SYS_NAND_U_BOOT_SIZE, > > > > > > + (uchar *)CONFIG_SYS_NAND_U_BOOT_DST)) { > > > > > > > > > > > > /* Copy from NAND successful, start U-boot */ > > > > > > uboot = (void *)CONFIG_SYS_NAND_U_BOOT_START; > > > > > > uboot(); > > > > > > > > > > > > @@ -364,3 +367,7 @@ void hang(void) > > > > > > > > > > > > /* Loop forever */ > > > > > > while (1) ; > > > > > > > > > > > > } > > > > > > > > > > > > +#endif > > > > > > + > > > > > > +void nand_init(void) {} > > > > > > +void nand_deselect(void) {} > > > > > > diff --git a/include/configs/mx31pdk.h b/include/configs/mx31pdk.h > > > > > > index 1754595..217552e 100644 > > > > > > --- a/include/configs/mx31pdk.h > > > > > > +++ b/include/configs/mx31pdk.h > > > > > > @@ -50,6 +50,7 @@ > > > > > > > > > > > > #define CONFIG_SPL_LDSCRIPT "arch/$(ARCH)/cpu/u-boot-spl.lds" > > > > > > #define CONFIG_SPL_MAX_SIZE 2048 > > > > > > #define CONFIG_SPL_NAND_SUPPORT > > > > > > > > > > > > +#define CONFIG_SPL_NAND_SUPPORT_LEGACY > > > > > > > > > > > > #define CONFIG_SPL_TEXT_BASE 0x87dc0000 > > > > > > #define CONFIG_SYS_TEXT_BASE 0x87e00000 > > > > > > > > > > > > diff --git a/include/configs/tx25.h b/include/configs/tx25.h > > > > > > index e72f8f6..7c362d0 100644 > > > > > > --- a/include/configs/tx25.h > > > > > > +++ b/include/configs/tx25.h > > > > > > @@ -37,6 +37,7 @@ > > > > > > > > > > > > #define CONFIG_SPL_LDSCRIPT "arch/$(ARCH)/cpu/u-boot- > > > > > > spl.lds" > > > > > > > > > #define CONFIG_SPL_MAX_SIZE 2048 > > > > > > #define CONFIG_SPL_NAND_SUPPORT > > > > > > > > > > > > +#define CONFIG_SPL_NAND_SUPPORT_LEGACY > > > > > > > > > > > > #define CONFIG_SPL_TEXT_BASE 0x810c0000 > > > > > > #define CONFIG_SYS_TEXT_BASE 0x81200000 > > > > > > > > > > > > -- > > > > > > 1.7.11.7 > > > > > > > > > > This is not about legacy vs. non-legacy. This is about basic vs. more > > > > > featured > > > > > SPL because of SPL size constraints. So what about dropping > > > > > CONFIG_SPL_NAND_SUPPORT_LEGACY and testing for CONFIG_SPL_FRAMEWORK > > > > > definition > > > > > instead? > > > > > > I was thinking about that, but the symbol is unrelated to NAND. > > > > Well, it's CONFIG_SPL_FRAMEWORK + CONFIG_SPL_NAND_SUPPORT that defines the > > other implementation, and CONFIG_SPL_NAND_SUPPORT triggers the build of > > mxc_nand_spl.c for SPL, so the common point is CONFIG_SPL_FRAMEWORK. > > > > > I still think > > > it's either a matter of fixing for new SPL or removing those two boards. > > > The nand_spl/ stuff shall be removed ASAP. > > > > Removing those boards is not a solution. > > > > Is it really about "new" SPL? The generic SPL is enabled by CONFIG_SPL, and > > CONFIG_SPL_FRAMEWORK is a sub-option. If the generic SPL rules imposed to > > use the SPL framework functions, there would be no such sub-option. So it > > looks like these boards are complying to the new SPL rules. > > > > We could see if using CONFIG_SPL_FRAMEWORK would still allow the SPL to fit > > in 2 kiB in order to drop this function, but if it does not fit, the new > > SPL rules should still make it possible to have a solution for any board > > having SPL size constraints. > > Rather than this, the other option would be to make whatever calls > nand_boot() > compatible with the SPL frameworks' implementation of spl_nand, so we don't > need > different function calls. If by that you mean renaming and reorganizing functions without using the main SPL framework, sure. For hang(), this has already been done by Andreas' pending series, and for nand_boot(), Tom's plan seems to be to merge the various existing implementations, and beyond that it would be possible to make the calls look more like the SPL framework's as you suggest. Best regards, Benoît
Dear Benoît Thébaudeau, > Dear Marek Vasut, > > On Friday, April 19, 2013 7:06:39 PM, Marek Vasut wrote: > > Subject: Re: [PATCH 2/6] nand: Add SPL_NAND support to mxc_nand_spl > > > > Dear Benoît Thébaudeau, > > > > > Dear Marek Vasut, > > > > > > On Friday, April 19, 2013 1:14:16 PM, Marek Vasut wrote: > > > > Dear Benoît Thébaudeau, > > > > > > > > > On Friday, April 19, 2013 10:38:48 AM, Benoît Thébaudeau wrote: > > > > > > Dear Marek Vasut, > > > > > > > > > > > > On Friday, April 19, 2013 6:10:51 AM, Marek Vasut wrote: > > > > > > > Add support for generic NAND SPL via the SPL framework into the > > > > > > > mxc_nand_spl driver. This is basically just a simple rename and > > > > > > > publication of the already implemented functions. To avoid the > > > > > > > old function which are used with the nand_spl/ stuff getting in > > > > > > > the way of NAND SPL framework, the macro CONFIG_SPL_NAND_LEGACY > > > > > > > was introduced and two remaining legacy boards were adjusted. > > > > > > > These board need to be either fixed or removed in the long run, > > > > > > > but I don't have either. > > > > > > > > > > > > > > Also make sure the requested payload is aligned to full pages, > > > > > > > otherwise this simple driver fails to load the last page. > > > > > > > > > > > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > > > > > > Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> > > > > > > > Cc: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> > > > > > > > Cc: Fabio Estevam <fabio.estevam@freescale.com> > > > > > > > Cc: Scott Wood <scottwood@freescale.com> > > > > > > > Cc: Stefano Babic <sbabic@denx.de> > > > > > > > Cc: Tom Rini <trini@ti.com> > > > > > > > --- > > > > > > > > > > > > > > drivers/mtd/nand/mxc_nand_spl.c | 13 ++++++++++--- > > > > > > > include/configs/mx31pdk.h | 1 + > > > > > > > include/configs/tx25.h | 1 + > > > > > > > 3 files changed, 12 insertions(+), 3 deletions(-) > > > > > > > > > > > > > > diff --git a/drivers/mtd/nand/mxc_nand_spl.c > > > > > > > b/drivers/mtd/nand/mxc_nand_spl.c > > > > > > > index 09f23c3..8ff03c9 100644 > > > > > > > --- a/drivers/mtd/nand/mxc_nand_spl.c > > > > > > > +++ b/drivers/mtd/nand/mxc_nand_spl.c > > > > > > > @@ -290,7 +290,7 @@ static int is_badblock(int pagenumber) > > > > > > > > > > > > > > return 0; > > > > > > > > > > > > > > } > > > > > > > > > > > > > > -static int nand_load(unsigned int from, unsigned int size, > > > > > > > unsigned char *buf) > > > > > > > +int nand_spl_load_image(uint32_t from, unsigned int size, void > > > > > > > *buf) > > > > > > > > > > > > > > { > > > > > > > > > > > > > > int i; > > > > > > > unsigned int page; > > > > > > > > > > > > > > @@ -303,6 +303,7 @@ static int nand_load(unsigned int from, > > > > > > > unsigned int size, unsigned char *buf) > > > > > > > > > > > > > > page = from / CONFIG_SYS_NAND_PAGE_SIZE; > > > > > > > i = 0; > > > > > > > > > > > > > > + size = roundup(size, CONFIG_SYS_NAND_PAGE_SIZE); > > > > > > > > > > > > > > while (i < size / CONFIG_SYS_NAND_PAGE_SIZE) { > > > > > > > > > > > > > > if (nfc_read_page(page, buf) < 0) > > > > > > > > > > > > > > return -1; > > > > > > > > > > > > > > @@ -332,6 +333,7 @@ static int nand_load(unsigned int from, > > > > > > > unsigned int size, unsigned char *buf) > > > > > > > > > > > > > > return 0; > > > > > > > > > > > > > > } > > > > > > > > > > > > > > +#ifdef CONFIG_SPL_NAND_SUPPORT_LEGACY > > > > > > > > > > > > > > /* > > > > > > > > > > > > > > * The main entry for NAND booting. It's necessary that SDRAM > > > > > > > is already * configured and available since this code loads > > > > > > > the main U-Boot image > > > > > > > > > > > > > > @@ -345,8 +347,9 @@ void nand_boot(void) > > > > > > > > > > > > > > * CONFIG_SYS_NAND_U_BOOT_OFFS and > > > > > > > CONFIG_SYS_NAND_U_BOOT_SIZE must * be aligned to full pages > > > > > > > */ > > > > > > > > > > > > > > - if (!nand_load(CONFIG_SYS_NAND_U_BOOT_OFFS, > > > > > > > CONFIG_SYS_NAND_U_BOOT_SIZE, - (uchar > > > > > > > *)CONFIG_SYS_NAND_U_BOOT_DST)) { > > > > > > > + if (!nand_spl_load_image(CONFIG_SYS_NAND_U_BOOT_OFFS, > > > > > > > + CONFIG_SYS_NAND_U_BOOT_SIZE, > > > > > > > + (uchar *)CONFIG_SYS_NAND_U_BOOT_DST)) { > > > > > > > > > > > > > > /* Copy from NAND successful, start U-boot */ > > > > > > > uboot = (void *)CONFIG_SYS_NAND_U_BOOT_START; > > > > > > > uboot(); > > > > > > > > > > > > > > @@ -364,3 +367,7 @@ void hang(void) > > > > > > > > > > > > > > /* Loop forever */ > > > > > > > while (1) ; > > > > > > > > > > > > > > } > > > > > > > > > > > > > > +#endif > > > > > > > + > > > > > > > +void nand_init(void) {} > > > > > > > +void nand_deselect(void) {} > > > > > > > diff --git a/include/configs/mx31pdk.h > > > > > > > b/include/configs/mx31pdk.h index 1754595..217552e 100644 > > > > > > > --- a/include/configs/mx31pdk.h > > > > > > > +++ b/include/configs/mx31pdk.h > > > > > > > @@ -50,6 +50,7 @@ > > > > > > > > > > > > > > #define CONFIG_SPL_LDSCRIPT "arch/$(ARCH)/cpu/u-boot- spl.lds" > > > > > > > #define CONFIG_SPL_MAX_SIZE 2048 > > > > > > > #define CONFIG_SPL_NAND_SUPPORT > > > > > > > > > > > > > > +#define CONFIG_SPL_NAND_SUPPORT_LEGACY > > > > > > > > > > > > > > #define CONFIG_SPL_TEXT_BASE 0x87dc0000 > > > > > > > #define CONFIG_SYS_TEXT_BASE 0x87e00000 > > > > > > > > > > > > > > diff --git a/include/configs/tx25.h b/include/configs/tx25.h > > > > > > > index e72f8f6..7c362d0 100644 > > > > > > > --- a/include/configs/tx25.h > > > > > > > +++ b/include/configs/tx25.h > > > > > > > @@ -37,6 +37,7 @@ > > > > > > > > > > > > > > #define CONFIG_SPL_LDSCRIPT "arch/$(ARCH)/cpu/u- boot- > > > > > > > > spl.lds" > > > > > > > > > > > #define CONFIG_SPL_MAX_SIZE 2048 > > > > > > > #define CONFIG_SPL_NAND_SUPPORT > > > > > > > > > > > > > > +#define CONFIG_SPL_NAND_SUPPORT_LEGACY > > > > > > > > > > > > > > #define CONFIG_SPL_TEXT_BASE 0x810c0000 > > > > > > > #define CONFIG_SYS_TEXT_BASE 0x81200000 > > > > > > > > > > > > > > -- > > > > > > > 1.7.11.7 > > > > > > > > > > > > This is not about legacy vs. non-legacy. This is about basic vs. > > > > > > more featured > > > > > > SPL because of SPL size constraints. So what about dropping > > > > > > CONFIG_SPL_NAND_SUPPORT_LEGACY and testing for > > > > > > CONFIG_SPL_FRAMEWORK definition > > > > > > instead? > > > > > > > > I was thinking about that, but the symbol is unrelated to NAND. > > > > > > Well, it's CONFIG_SPL_FRAMEWORK + CONFIG_SPL_NAND_SUPPORT that defines > > > the other implementation, and CONFIG_SPL_NAND_SUPPORT triggers the > > > build of mxc_nand_spl.c for SPL, so the common point is > > > CONFIG_SPL_FRAMEWORK. > > > > > > > I still think > > > > it's either a matter of fixing for new SPL or removing those two > > > > boards. The nand_spl/ stuff shall be removed ASAP. > > > > > > Removing those boards is not a solution. > > > > > > Is it really about "new" SPL? The generic SPL is enabled by CONFIG_SPL, > > > and CONFIG_SPL_FRAMEWORK is a sub-option. If the generic SPL rules > > > imposed to use the SPL framework functions, there would be no such > > > sub-option. So it looks like these boards are complying to the new SPL > > > rules. > > > > > > We could see if using CONFIG_SPL_FRAMEWORK would still allow the SPL to > > > fit in 2 kiB in order to drop this function, but if it does not fit, > > > the new SPL rules should still make it possible to have a solution for > > > any board having SPL size constraints. > > > > Rather than this, the other option would be to make whatever calls > > nand_boot() > > compatible with the SPL frameworks' implementation of spl_nand, so we > > don't need > > different function calls. > > If by that you mean renaming and reorganizing functions without using the > main SPL framework, sure. For hang(), this has already been done by > Andreas' pending series, and for nand_boot(), Tom's plan seems to be to > merge the various existing implementations, and beyond that it would be > possible to make the calls look more like the SPL framework's as you > suggest. OK, makes sense.
diff --git a/drivers/mtd/nand/mxc_nand_spl.c b/drivers/mtd/nand/mxc_nand_spl.c index 09f23c3..8ff03c9 100644 --- a/drivers/mtd/nand/mxc_nand_spl.c +++ b/drivers/mtd/nand/mxc_nand_spl.c @@ -290,7 +290,7 @@ static int is_badblock(int pagenumber) return 0; } -static int nand_load(unsigned int from, unsigned int size, unsigned char *buf) +int nand_spl_load_image(uint32_t from, unsigned int size, void *buf) { int i; unsigned int page; @@ -303,6 +303,7 @@ static int nand_load(unsigned int from, unsigned int size, unsigned char *buf) page = from / CONFIG_SYS_NAND_PAGE_SIZE; i = 0; + size = roundup(size, CONFIG_SYS_NAND_PAGE_SIZE); while (i < size / CONFIG_SYS_NAND_PAGE_SIZE) { if (nfc_read_page(page, buf) < 0) return -1; @@ -332,6 +333,7 @@ static int nand_load(unsigned int from, unsigned int size, unsigned char *buf) return 0; } +#ifdef CONFIG_SPL_NAND_SUPPORT_LEGACY /* * The main entry for NAND booting. It's necessary that SDRAM is already * configured and available since this code loads the main U-Boot image @@ -345,8 +347,9 @@ void nand_boot(void) * CONFIG_SYS_NAND_U_BOOT_OFFS and CONFIG_SYS_NAND_U_BOOT_SIZE must * be aligned to full pages */ - if (!nand_load(CONFIG_SYS_NAND_U_BOOT_OFFS, CONFIG_SYS_NAND_U_BOOT_SIZE, - (uchar *)CONFIG_SYS_NAND_U_BOOT_DST)) { + if (!nand_spl_load_image(CONFIG_SYS_NAND_U_BOOT_OFFS, + CONFIG_SYS_NAND_U_BOOT_SIZE, + (uchar *)CONFIG_SYS_NAND_U_BOOT_DST)) { /* Copy from NAND successful, start U-boot */ uboot = (void *)CONFIG_SYS_NAND_U_BOOT_START; uboot(); @@ -364,3 +367,7 @@ void hang(void) /* Loop forever */ while (1) ; } +#endif + +void nand_init(void) {} +void nand_deselect(void) {} diff --git a/include/configs/mx31pdk.h b/include/configs/mx31pdk.h index 1754595..217552e 100644 --- a/include/configs/mx31pdk.h +++ b/include/configs/mx31pdk.h @@ -50,6 +50,7 @@ #define CONFIG_SPL_LDSCRIPT "arch/$(ARCH)/cpu/u-boot-spl.lds" #define CONFIG_SPL_MAX_SIZE 2048 #define CONFIG_SPL_NAND_SUPPORT +#define CONFIG_SPL_NAND_SUPPORT_LEGACY #define CONFIG_SPL_TEXT_BASE 0x87dc0000 #define CONFIG_SYS_TEXT_BASE 0x87e00000 diff --git a/include/configs/tx25.h b/include/configs/tx25.h index e72f8f6..7c362d0 100644 --- a/include/configs/tx25.h +++ b/include/configs/tx25.h @@ -37,6 +37,7 @@ #define CONFIG_SPL_LDSCRIPT "arch/$(ARCH)/cpu/u-boot-spl.lds" #define CONFIG_SPL_MAX_SIZE 2048 #define CONFIG_SPL_NAND_SUPPORT +#define CONFIG_SPL_NAND_SUPPORT_LEGACY #define CONFIG_SPL_TEXT_BASE 0x810c0000 #define CONFIG_SYS_TEXT_BASE 0x81200000
Add support for generic NAND SPL via the SPL framework into the mxc_nand_spl driver. This is basically just a simple rename and publication of the already implemented functions. To avoid the old function which are used with the nand_spl/ stuff getting in the way of NAND SPL framework, the macro CONFIG_SPL_NAND_LEGACY was introduced and two remaining legacy boards were adjusted. These board need to be either fixed or removed in the long run, but I don't have either. Also make sure the requested payload is aligned to full pages, otherwise this simple driver fails to load the last page. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> Cc: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> Cc: Fabio Estevam <fabio.estevam@freescale.com> Cc: Scott Wood <scottwood@freescale.com> Cc: Stefano Babic <sbabic@denx.de> Cc: Tom Rini <trini@ti.com> --- drivers/mtd/nand/mxc_nand_spl.c | 13 ++++++++++--- include/configs/mx31pdk.h | 1 + include/configs/tx25.h | 1 + 3 files changed, 12 insertions(+), 3 deletions(-)