[U-Boot] Using spi_alloc_slave() from SPL

Hi,
I am trying to use spi_flash_probe() inside SPL on a custom mx6 board.
The idea is to read some parameters from the SPI NOR flash and configure the DDR accordingly.
This is similar to what gw_ventana_spl.c does, but it reads from i2c eeprom instead of SPI NOR.
Here are the changes just to illustrate the problem:
--- a/board/freescale/mx6sabresd/mx6sabresd.c +++ b/board/freescale/mx6sabresd/mx6sabresd.c @@ -692,6 +692,19 @@ int checkboard(void) #ifdef CONFIG_SPL_BUILD #include <spl.h> #include <libfdt.h> +#include <spi_flash.h> +#include <spi.h> + +static int read_spi_flash(void) +{ + struct spi_flash *spi; + char buf[2]; + + spi = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS, + CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE); + return spi_flash_read(spi, 0, 2, buf); +}
const struct mx6dq_iomux_ddr_regs mx6_ddr_ioregs = { .dram_sdclk_0 = 0x00020030, @@ -837,6 +850,8 @@ void board_init_f(ulong dummy) /* UART clocks enabled and gd valid - init serial console */ preloader_console_init();
+ read_spi_flash(); + /* DDR initialization */ spl_dram_init();
diff --git a/include/configs/mx6sabresd.h b/include/configs/mx6sabresd.h index 41162ca..f5dfaf7 100644 --- a/include/configs/mx6sabresd.h +++ b/include/configs/mx6sabresd.h @@ -12,6 +12,10 @@ #ifdef CONFIG_SPL #define CONFIG_SPL_LIBCOMMON_SUPPORT #define CONFIG_SPL_MMC_SUPPORT +#define CONFIG_SPL_SPI_SUPPORT +#define CONFIG_SPL_SPI_FLASH_SUPPORT +#define CONFIG_SYS_SPI_U_BOOT_OFFS (64 * 1024) +#define CONFIG_SPL_SPI_LOAD #include "imx6_spl.h" #endif
@@ -35,6 +39,12 @@ #define CONFIG_SYS_MMC_ENV_DEV 1 /* SDHC3 */ #endif
+#define CONFIG_ENV_SECT_SIZE (64 * 1024) +#define CONFIG_ENV_SPI_BUS CONFIG_SF_DEFAULT_BUS +#define CONFIG_ENV_SPI_CS CONFIG_SF_DEFAULT_CS +#define CONFIG_ENV_SPI_MODE CONFIG_SF_DEFAULT_MODE +#define CONFIG_ENV_SPI_MAX_HZ CONFIG_SF_DEFAULT_SPEED + #define CONFIG_CMD_PCI #ifdef CONFIG_CMD_PCI #define CONFIG_PCI

On Thursday, August 06, 2015 at 03:25:22 PM, Fabio Estevam wrote:
Hi,
I am trying to use spi_flash_probe() inside SPL on a custom mx6 board.
The idea is to read some parameters from the SPI NOR flash and configure the DDR accordingly.
This is similar to what gw_ventana_spl.c does, but it reads from i2c eeprom instead of SPI NOR.
Here are the changes just to illustrate the problem:
I understand that you need to call spi_flash_probe() in board_init_f() at which point you still have no malloc() area available, so it fails with -ENOMEM or something like that, correct ?
What you can probably try is to define CONFIG_SYS_MALLOC_F_LEN and do the following before doing spi_flash_probe():
static u8 array[128] __aligned(32);
gd->malloc_limit = CONFIG_SYS_MALLOC_F_LEN; gd->malloc_ptr = array;
This might work, but is nasty.
Best regards, Marek Vasut

Hi Fabio,
On 06/08/2015 15:25, Fabio Estevam wrote:
Hi,
I am trying to use spi_flash_probe() inside SPL on a custom mx6 board.
The idea is to read some parameters from the SPI NOR flash and configure the DDR accordingly.
This is similar to what gw_ventana_spl.c does, but it reads from i2c eeprom instead of SPI NOR.
Here are the changes just to illustrate the problem:
--- a/board/freescale/mx6sabresd/mx6sabresd.c +++ b/board/freescale/mx6sabresd/mx6sabresd.c @@ -692,6 +692,19 @@ int checkboard(void) #ifdef CONFIG_SPL_BUILD #include <spl.h> #include <libfdt.h> +#include <spi_flash.h> +#include <spi.h>
+static int read_spi_flash(void) +{
struct spi_flash *spi;
char buf[2];
spi = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);
return spi_flash_read(spi, 0, 2, buf);
+}
const struct mx6dq_iomux_ddr_regs mx6_ddr_ioregs = { .dram_sdclk_0 = 0x00020030, @@ -837,6 +850,8 @@ void board_init_f(ulong dummy) /* UART clocks enabled and gd valid - init serial console */ preloader_console_init();
read_spi_flash();
/* DDR initialization */ spl_dram_init();
diff --git a/include/configs/mx6sabresd.h b/include/configs/mx6sabresd.h index 41162ca..f5dfaf7 100644 --- a/include/configs/mx6sabresd.h +++ b/include/configs/mx6sabresd.h @@ -12,6 +12,10 @@ #ifdef CONFIG_SPL #define CONFIG_SPL_LIBCOMMON_SUPPORT #define CONFIG_SPL_MMC_SUPPORT +#define CONFIG_SPL_SPI_SUPPORT +#define CONFIG_SPL_SPI_FLASH_SUPPORT +#define CONFIG_SYS_SPI_U_BOOT_OFFS (64 * 1024) +#define CONFIG_SPL_SPI_LOAD #include "imx6_spl.h" #endif
@@ -35,6 +39,12 @@ #define CONFIG_SYS_MMC_ENV_DEV 1 /* SDHC3 */ #endif
+#define CONFIG_ENV_SECT_SIZE (64 * 1024) +#define CONFIG_ENV_SPI_BUS CONFIG_SF_DEFAULT_BUS +#define CONFIG_ENV_SPI_CS CONFIG_SF_DEFAULT_CS +#define CONFIG_ENV_SPI_MODE CONFIG_SF_DEFAULT_MODE +#define CONFIG_ENV_SPI_MAX_HZ CONFIG_SF_DEFAULT_SPEED
#define CONFIG_CMD_PCI #ifdef CONFIG_CMD_PCI #define CONFIG_PCI
The when I run it:
U-Boot SPL 2015.07-08202-g6dcdca1-dirty (Aug 06 2015 - 10:18:33) mxc_spi: SPI Slave not allocated !
spi_flash_probe() ---> spi_setup_slave() ----> spi_alloc_slave() which fails on mxc_spi.c
Right - if I remember well, spi_alloc requires a full functional malloc, and then fails.
As read_spi_flash() is called prior to the DDR initialization, spi_alloc_slave() fails.
Is there a way to avoid calling spi_alloc_slave() in the SPL case?
Any ideas on how to fix this?
There is the possibility to set a malloc area inside SPL:
CONFIG_SYS_SPL_MALLOC_START CONFIG_SYS_SPL_MALLOC_SIZE
you do not need a lot of space, and you can try to put it inside the IRAM.
This should guarantee that spi_alloc_slave() works.
Best regards, Stefano

Hi Stefano and Marek,
Thanks for the suggestions.
On Thu, Aug 6, 2015 at 10:38 AM, Stefano Babic sbabic@denx.de wrote:
There is the possibility to set a malloc area inside SPL:
CONFIG_SYS_SPL_MALLOC_START CONFIG_SYS_SPL_MALLOC_SIZE you do not need a lot of space, and you can try to put it inside the IRAM.
This should guarantee that spi_alloc_slave() works.
So I tried moving them to the internal RAM:
--- a/include/configs/imx6_spl.h +++ b/include/configs/imx6_spl.h @@ -70,8 +70,8 @@ #else #define CONFIG_SPL_BSS_START_ADDR 0x18200000 #define CONFIG_SPL_BSS_MAX_SIZE 0x100000 /* 1 MB */ -#define CONFIG_SYS_SPL_MALLOC_START 0x18300000 -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x3200000 /* 50 MB */ +#define CONFIG_SYS_SPL_MALLOC_START 0x900000 +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x8000 #define CONFIG_SYS_TEXT_BASE 0x17800000 #endif #endif
but still getting spi_alloc_slave() to fail.
Regards,
Fabio Estevam

On Thursday, August 06, 2015 at 04:14:34 PM, Fabio Estevam wrote:
Hi Stefano and Marek,
Thanks for the suggestions.
On Thu, Aug 6, 2015 at 10:38 AM, Stefano Babic sbabic@denx.de wrote:
There is the possibility to set a malloc area inside SPL:
CONFIG_SYS_SPL_MALLOC_START CONFIG_SYS_SPL_MALLOC_SIZE you do not need a lot of space, and you can try to put it inside the IRAM.
This should guarantee that spi_alloc_slave() works.
So I tried moving them to the internal RAM:
--- a/include/configs/imx6_spl.h +++ b/include/configs/imx6_spl.h @@ -70,8 +70,8 @@ #else #define CONFIG_SPL_BSS_START_ADDR 0x18200000 #define CONFIG_SPL_BSS_MAX_SIZE 0x100000 /* 1 MB */ -#define CONFIG_SYS_SPL_MALLOC_START 0x18300000 -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x3200000 /* 50 MB */ +#define CONFIG_SYS_SPL_MALLOC_START 0x900000 +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x8000 #define CONFIG_SYS_TEXT_BASE 0x17800000 #endif #endif
but still getting spi_alloc_slave() to fail.
You want to avoid this "CONFIG_SYS_SPL_MALLOC_*" stuff, as it increases the SPL size by 3kiB compared to MALLOC_F . Also, MALLOC_F needs just the base address of the malloc area to work (see my email).
Do you know the return value ?
Best regards, Marek Vasut

Hi Marek,
On Thu, Aug 6, 2015 at 11:28 AM, Marek Vasut marex@denx.de wrote:
You want to avoid this "CONFIG_SYS_SPL_MALLOC_*" stuff, as it increases the SPL size by 3kiB compared to MALLOC_F . Also, MALLOC_F needs just the base address of the malloc area to work (see my email).
Thanks, I removed CONFIG_SYS_SPL_MALLOC_*.
Do you know the return value ?
The malloc() inside spi_do_alloc_slave() returns NULL in the SPL case:
void *spi_do_alloc_slave(int offset, int size, unsigned int bus, unsigned int cs) { struct spi_slave *slave; void *ptr;
ptr = malloc(size);
if (ptr) { memset(ptr, '\0', size); slave = (struct spi_slave *)(ptr + offset); slave->bus = bus; slave->cs = cs; slave->wordlen = SPI_DEFAULT_WORDLEN; }
return ptr; }
size is only 56.
Also tried to modify spi_do_alloc_slave() as per your earlier recommendation, but no success so far.
I was not able to find a way to make malloc() to be happy with SPL yet.
Thanks,
Fabio Estevam

Hi Marek,
On 06/08/2015 16:28, Marek Vasut wrote:
On Thursday, August 06, 2015 at 04:14:34 PM, Fabio Estevam wrote:
Hi Stefano and Marek,
Thanks for the suggestions.
On Thu, Aug 6, 2015 at 10:38 AM, Stefano Babic sbabic@denx.de wrote:
There is the possibility to set a malloc area inside SPL:
CONFIG_SYS_SPL_MALLOC_START CONFIG_SYS_SPL_MALLOC_SIZE you do not need a lot of space, and you can try to put it inside the IRAM.
This should guarantee that spi_alloc_slave() works.
So I tried moving them to the internal RAM:
--- a/include/configs/imx6_spl.h +++ b/include/configs/imx6_spl.h @@ -70,8 +70,8 @@ #else #define CONFIG_SPL_BSS_START_ADDR 0x18200000 #define CONFIG_SPL_BSS_MAX_SIZE 0x100000 /* 1 MB */ -#define CONFIG_SYS_SPL_MALLOC_START 0x18300000 -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x3200000 /* 50 MB */ +#define CONFIG_SYS_SPL_MALLOC_START 0x900000 +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x8000 #define CONFIG_SYS_TEXT_BASE 0x17800000 #endif #endif
but still getting spi_alloc_slave() to fail.
You want to avoid this "CONFIG_SYS_SPL_MALLOC_*" stuff, as it increases the SPL size by 3kiB compared to MALLOC_F . Also, MALLOC_F needs just the base address of the malloc area to work (see my email).
It does not matter at the moment, because Fabio's issue is not yet solved. But is it not CONFIG_SYS_SPL_MALLOC the preferred way for SPL ? Setting the array with MALLOC_F looks like a hack. And CONFIG_SYS_SPL_MALLOC is *already* set for i.MX6, lool at include/configs/imx6_spl.h:
#define CONFIG_SYS_SPL_MALLOC_START 0x18300000 #define CONFIG_SYS_SPL_MALLOC_SIZE 0x3200000 /* 50 MB */
This is in RAM, of course. Increasing the size by 3KiB is not IMHO for i.MX6 a problem, there is enough space in IRAM. But what is surprising is that Fabio gets a Null pointer by malloc().
Best regards, Stefano Babic

On Thu, Aug 6, 2015 at 2:03 PM, Stefano Babic sbabic@denx.de wrote:
This is in RAM, of course. Increasing the size by 3KiB is not IMHO for i.MX6 a problem, there is enough space in IRAM. But what is surprising is that Fabio gets a Null pointer by malloc().
Yes, so I did a simpler patch that shows the malloc() issue with SPL:
--- a/board/freescale/mx6sabresd/mx6sabresd.c +++ b/board/freescale/mx6sabresd/mx6sabresd.c @@ -692,6 +692,7 @@ int checkboard(void) #ifdef CONFIG_SPL_BUILD #include <spl.h> #include <libfdt.h> +#include <malloc.h>
const struct mx6dq_iomux_ddr_regs mx6_ddr_ioregs = { .dram_sdclk_0 = 0x00020030, @@ -822,6 +823,7 @@ static void spl_dram_init(void)
void board_init_f(ulong dummy) { + void __iomem *ptr; /* setup AIPS and disable watchdog */ arch_cpu_init();
@@ -837,6 +839,10 @@ void board_init_f(ulong dummy) /* UART clocks enabled and gd valid - init serial console */ preloader_console_init();
+ ptr = malloc(64); + if (!ptr) + puts("*** malloc returned NULL\n"); + /* DDR initialization */ spl_dram_init();
when I run it:
U-Boot SPL 2015.07-08201-gfb44bcd-dirty (Aug 06 2015 - 15:19:54) *** malloc returned NULL
Even if I put the malloc() after spl_dram_init() it still returns NULL.
Regards,
Fabio Estevam

On Thu, Aug 6, 2015 at 3:24 PM, Fabio Estevam festevam@gmail.com wrote:
On Thu, Aug 6, 2015 at 2:03 PM, Stefano Babic sbabic@denx.de wrote:
This is in RAM, of course. Increasing the size by 3KiB is not IMHO for i.MX6 a problem, there is enough space in IRAM. But what is surprising is that Fabio gets a Null pointer by malloc().
Yes, so I did a simpler patch that shows the malloc() issue with SPL:
Ok, with this change malloc() does not fail with SPL:
--- a/board/freescale/mx6sabresd/mx6sabresd.c +++ b/board/freescale/mx6sabresd/mx6sabresd.c @@ -692,6 +692,7 @@ int checkboard(void) #ifdef CONFIG_SPL_BUILD #include <spl.h> #include <libfdt.h> +#include <malloc.h>
const struct mx6dq_iomux_ddr_regs mx6_ddr_ioregs = { .dram_sdclk_0 = 0x00020030, @@ -822,6 +823,7 @@ static void spl_dram_init(void)
void board_init_f(ulong dummy) { + void __iomem *ptr; /* setup AIPS and disable watchdog */ arch_cpu_init();
@@ -837,6 +839,12 @@ void board_init_f(ulong dummy) /* UART clocks enabled and gd valid - init serial console */ preloader_console_init();
+ gd->malloc_limit = CONFIG_SYS_MALLOC_F_LEN; + gd->malloc_ptr = 0; + ptr = malloc(64); + if (!ptr) + puts("*** malloc returned NULL\n"); + /* DDR initialization */ spl_dram_init();
Regards,
Fabio Estevam

Hi,
On 6 August 2015 at 12:24, Fabio Estevam festevam@gmail.com wrote:
On Thu, Aug 6, 2015 at 2:03 PM, Stefano Babic sbabic@denx.de wrote:
This is in RAM, of course. Increasing the size by 3KiB is not IMHO for i.MX6 a problem, there is enough space in IRAM. But what is surprising is that Fabio gets a Null pointer by malloc().
Yes, so I did a simpler patch that shows the malloc() issue with SPL:
--- a/board/freescale/mx6sabresd/mx6sabresd.c +++ b/board/freescale/mx6sabresd/mx6sabresd.c @@ -692,6 +692,7 @@ int checkboard(void) #ifdef CONFIG_SPL_BUILD #include <spl.h> #include <libfdt.h> +#include <malloc.h>
const struct mx6dq_iomux_ddr_regs mx6_ddr_ioregs = { .dram_sdclk_0 = 0x00020030, @@ -822,6 +823,7 @@ static void spl_dram_init(void)
void board_init_f(ulong dummy) {
void __iomem *ptr; /* setup AIPS and disable watchdog */ arch_cpu_init();
@@ -837,6 +839,10 @@ void board_init_f(ulong dummy) /* UART clocks enabled and gd valid - init serial console */ preloader_console_init();
ptr = malloc(64);
if (!ptr)
puts("*** malloc returned NULL\n");
/* DDR initialization */ spl_dram_init();
when I run it:
U-Boot SPL 2015.07-08201-gfb44bcd-dirty (Aug 06 2015 - 15:19:54) *** malloc returned NULL
Even if I put the malloc() after spl_dram_init() it still returns NULL.
Please check the README about the SPL flow. From what I can see malloc() is not available before board_init_r() in SPL.
However, if you add a call to spl_init() from your board_init_f(), then early malloc would be available. Enable this with CONFIG_SYS_MALLOC_F_... as people explained.
It would be nice if someone could tidy this up so there is only one generic board_init_f(), and it calls board-specific code from there.
Regards, Simon

Hi Simon,
On Thu, Aug 6, 2015 at 4:31 PM, Simon Glass sjg@chromium.org wrote:
Please check the README about the SPL flow. From what I can see malloc() is not available before board_init_r() in SPL.
However, if you add a call to spl_init() from your board_init_f(), then early malloc would be available. Enable this with CONFIG_SYS_MALLOC_F_... as people explained.
Cool, calling spl_init() from board_init_f() allows malloc to work fine.
Thanks a lot!
Fabio Estevam

Hi Fabio, guys,
On 08/07/2015 01:13 AM, Fabio Estevam wrote:
Hi Simon,
On Thu, Aug 6, 2015 at 4:31 PM, Simon Glass sjg@chromium.org wrote:
Please check the README about the SPL flow. From what I can see malloc() is not available before board_init_r() in SPL.
However, if you add a call to spl_init() from your board_init_f(), then early malloc would be available. Enable this with CONFIG_SYS_MALLOC_F_... as people explained.
Cool, calling spl_init() from board_init_f() allows malloc to work fine.
Thanks a lot!
Fabio Estevam
Thanks for sharing this info, it seems it will solve a similar problem of mine on the latest U-Boot code (SPI slave malloc not working during SPL boot from SPI NOR).
Regards, Nikolay

Hi Nikolay,
On Thu, Aug 13, 2015 at 9:37 AM, Nikolay Dimitrov picmaster@mail.bg wrote:
Thanks for sharing this info, it seems it will solve a similar problem of mine on the latest U-Boot code (SPI slave malloc not working during SPL boot from SPI NOR).
Unfortunately this is not working in U-boot mainline now. I haven't had a chance to debug it, but if you are able to get spi slave malloc to work in SPL with mainline, please let me know.
Regards,
Fabio Estevam

On Mon, Nov 9, 2015 at 10:05 PM, Fabio Estevam festevam@gmail.com wrote:
Hi Nikolay,
On Thu, Aug 13, 2015 at 9:37 AM, Nikolay Dimitrov picmaster@mail.bg wrote:
Thanks for sharing this info, it seems it will solve a similar problem of mine on the latest U-Boot code (SPI slave malloc not working during SPL boot from SPI NOR).
Unfortunately this is not working in U-boot mainline now. I haven't had a chance to debug it, but if you are able to get spi slave malloc to work in SPL with mainline, please let me know.
Commit 5ba534d247d418 broke this.

On Thu, Aug 6, 2015 at 7:13 PM, Fabio Estevam festevam@gmail.com wrote:
Hi Simon,
On Thu, Aug 6, 2015 at 4:31 PM, Simon Glass sjg@chromium.org wrote:
Please check the README about the SPL flow. From what I can see malloc() is not available before board_init_r() in SPL.
However, if you add a call to spl_init() from your board_init_f(), then early malloc would be available. Enable this with CONFIG_SYS_MALLOC_F_... as people explained.
Cool, calling spl_init() from board_init_f() allows malloc to work fine.
Just noticed that this no longer works in mainline U-boot:
U-Boot SPL 2015.10-00527-g8800bee (Nov 09 2015 - 21:23:54) mxc_spi: SPI Slave not allocated !

On Thursday, August 06, 2015 at 07:03:24 PM, Stefano Babic wrote:
Hi Marek,
On 06/08/2015 16:28, Marek Vasut wrote:
On Thursday, August 06, 2015 at 04:14:34 PM, Fabio Estevam wrote:
Hi Stefano and Marek,
Thanks for the suggestions.
On Thu, Aug 6, 2015 at 10:38 AM, Stefano Babic sbabic@denx.de wrote:
There is the possibility to set a malloc area inside SPL:
CONFIG_SYS_SPL_MALLOC_START CONFIG_SYS_SPL_MALLOC_SIZE you do not need a lot of space, and you can try to put it inside the IRAM.
This should guarantee that spi_alloc_slave() works.
So I tried moving them to the internal RAM:
--- a/include/configs/imx6_spl.h +++ b/include/configs/imx6_spl.h @@ -70,8 +70,8 @@
#else #define CONFIG_SPL_BSS_START_ADDR 0x18200000 #define CONFIG_SPL_BSS_MAX_SIZE 0x100000 /* 1 MB */
-#define CONFIG_SYS_SPL_MALLOC_START 0x18300000 -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x3200000 /* 50 MB */ +#define CONFIG_SYS_SPL_MALLOC_START 0x900000 +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x8000
#define CONFIG_SYS_TEXT_BASE 0x17800000 #endif #endif
but still getting spi_alloc_slave() to fail.
You want to avoid this "CONFIG_SYS_SPL_MALLOC_*" stuff, as it increases the SPL size by 3kiB compared to MALLOC_F . Also, MALLOC_F needs just the base address of the malloc area to work (see my email).
It does not matter at the moment, because Fabio's issue is not yet solved. But is it not CONFIG_SYS_SPL_MALLOC the preferred way for SPL ? Setting the array with MALLOC_F looks like a hack. And CONFIG_SYS_SPL_MALLOC is *already* set for i.MX6, lool at include/configs/imx6_spl.h:
#define CONFIG_SYS_SPL_MALLOC_START 0x18300000 #define CONFIG_SYS_SPL_MALLOC_SIZE 0x3200000 /* 50 MB */
This is in RAM, of course. Increasing the size by 3KiB is not IMHO for i.MX6 a problem, there is enough space in IRAM. But what is surprising is that Fabio gets a Null pointer by malloc().
The malloc is set up only at the beginning of board_init_r(), but Fabio needs to use the SPI in board_init_f(), so of course he does not have the malloc operational at that point.
The MALLOC_F if intended to be used before RAM is operational (ie. before board_init_r() is invoked), so it looks like the correct tool to use here. Setting up a full malloc() in your small OCRAM is on the other hand a really bad idea.
Best regards, Marek Vasut
participants (5)
-
Fabio Estevam
-
Marek Vasut
-
Nikolay Dimitrov
-
Simon Glass
-
Stefano Babic