All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] Using spi_alloc_slave() from SPL
@ 2015-08-06 13:25 Fabio Estevam
  2015-08-06 13:33 ` Marek Vasut
  2015-08-06 13:38 ` Stefano Babic
  0 siblings, 2 replies; 16+ messages in thread
From: Fabio Estevam @ 2015-08-06 13:25 UTC (permalink / raw)
  To: u-boot

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
-- 
1.9.1

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

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?

Thanks,

Fabio Estevam

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot] Using spi_alloc_slave() from SPL
  2015-08-06 13:25 [U-Boot] Using spi_alloc_slave() from SPL Fabio Estevam
@ 2015-08-06 13:33 ` Marek Vasut
  2015-08-06 13:38 ` Stefano Babic
  1 sibling, 0 replies; 16+ messages in thread
From: Marek Vasut @ 2015-08-06 13:33 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] Using spi_alloc_slave() from SPL
  2015-08-06 13:25 [U-Boot] Using spi_alloc_slave() from SPL Fabio Estevam
  2015-08-06 13:33 ` Marek Vasut
@ 2015-08-06 13:38 ` Stefano Babic
  2015-08-06 14:14   ` Fabio Estevam
  1 sibling, 1 reply; 16+ messages in thread
From: Stefano Babic @ 2015-08-06 13:38 UTC (permalink / raw)
  To: u-boot

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


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] Using spi_alloc_slave() from SPL
  2015-08-06 13:38 ` Stefano Babic
@ 2015-08-06 14:14   ` Fabio Estevam
  2015-08-06 14:28     ` Marek Vasut
  0 siblings, 1 reply; 16+ messages in thread
From: Fabio Estevam @ 2015-08-06 14:14 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] Using spi_alloc_slave() from SPL
  2015-08-06 14:14   ` Fabio Estevam
@ 2015-08-06 14:28     ` Marek Vasut
  2015-08-06 16:44       ` Fabio Estevam
  2015-08-06 17:03       ` Stefano Babic
  0 siblings, 2 replies; 16+ messages in thread
From: Marek Vasut @ 2015-08-06 14:28 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] Using spi_alloc_slave() from SPL
  2015-08-06 14:28     ` Marek Vasut
@ 2015-08-06 16:44       ` Fabio Estevam
  2015-08-06 17:03       ` Stefano Babic
  1 sibling, 0 replies; 16+ messages in thread
From: Fabio Estevam @ 2015-08-06 16:44 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] Using spi_alloc_slave() from SPL
  2015-08-06 14:28     ` Marek Vasut
  2015-08-06 16:44       ` Fabio Estevam
@ 2015-08-06 17:03       ` Stefano Babic
  2015-08-06 18:24         ` Fabio Estevam
  2015-08-06 22:22         ` Marek Vasut
  1 sibling, 2 replies; 16+ messages in thread
From: Stefano Babic @ 2015-08-06 17:03 UTC (permalink / raw)
  To: u-boot

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

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] Using spi_alloc_slave() from SPL
  2015-08-06 17:03       ` Stefano Babic
@ 2015-08-06 18:24         ` Fabio Estevam
  2015-08-06 19:29           ` Fabio Estevam
  2015-08-06 19:31           ` Simon Glass
  2015-08-06 22:22         ` Marek Vasut
  1 sibling, 2 replies; 16+ messages in thread
From: Fabio Estevam @ 2015-08-06 18:24 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] Using spi_alloc_slave() from SPL
  2015-08-06 18:24         ` Fabio Estevam
@ 2015-08-06 19:29           ` Fabio Estevam
  2015-08-06 19:31           ` Simon Glass
  1 sibling, 0 replies; 16+ messages in thread
From: Fabio Estevam @ 2015-08-06 19:29 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] Using spi_alloc_slave() from SPL
  2015-08-06 18:24         ` Fabio Estevam
  2015-08-06 19:29           ` Fabio Estevam
@ 2015-08-06 19:31           ` Simon Glass
  2015-08-06 22:13             ` Fabio Estevam
  1 sibling, 1 reply; 16+ messages in thread
From: Simon Glass @ 2015-08-06 19:31 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] Using spi_alloc_slave() from SPL
  2015-08-06 19:31           ` Simon Glass
@ 2015-08-06 22:13             ` Fabio Estevam
  2015-08-13 12:37               ` Nikolay Dimitrov
  2015-11-09 23:28               ` Fabio Estevam
  0 siblings, 2 replies; 16+ messages in thread
From: Fabio Estevam @ 2015-08-06 22:13 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] Using spi_alloc_slave() from SPL
  2015-08-06 17:03       ` Stefano Babic
  2015-08-06 18:24         ` Fabio Estevam
@ 2015-08-06 22:22         ` Marek Vasut
  1 sibling, 0 replies; 16+ messages in thread
From: Marek Vasut @ 2015-08-06 22:22 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] Using spi_alloc_slave() from SPL
  2015-08-06 22:13             ` Fabio Estevam
@ 2015-08-13 12:37               ` Nikolay Dimitrov
  2015-11-10  0:05                 ` Fabio Estevam
  2015-11-09 23:28               ` Fabio Estevam
  1 sibling, 1 reply; 16+ messages in thread
From: Nikolay Dimitrov @ 2015-08-13 12:37 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] Using spi_alloc_slave() from SPL
  2015-08-06 22:13             ` Fabio Estevam
  2015-08-13 12:37               ` Nikolay Dimitrov
@ 2015-11-09 23:28               ` Fabio Estevam
  1 sibling, 0 replies; 16+ messages in thread
From: Fabio Estevam @ 2015-11-09 23:28 UTC (permalink / raw)
  To: u-boot

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 !

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] Using spi_alloc_slave() from SPL
  2015-08-13 12:37               ` Nikolay Dimitrov
@ 2015-11-10  0:05                 ` Fabio Estevam
  2015-11-10 14:41                   ` Fabio Estevam
  0 siblings, 1 reply; 16+ messages in thread
From: Fabio Estevam @ 2015-11-10  0:05 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] Using spi_alloc_slave() from SPL
  2015-11-10  0:05                 ` Fabio Estevam
@ 2015-11-10 14:41                   ` Fabio Estevam
  0 siblings, 0 replies; 16+ messages in thread
From: Fabio Estevam @ 2015-11-10 14:41 UTC (permalink / raw)
  To: u-boot

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2015-11-10 14:41 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-06 13:25 [U-Boot] Using spi_alloc_slave() from SPL Fabio Estevam
2015-08-06 13:33 ` Marek Vasut
2015-08-06 13:38 ` Stefano Babic
2015-08-06 14:14   ` Fabio Estevam
2015-08-06 14:28     ` Marek Vasut
2015-08-06 16:44       ` Fabio Estevam
2015-08-06 17:03       ` Stefano Babic
2015-08-06 18:24         ` Fabio Estevam
2015-08-06 19:29           ` Fabio Estevam
2015-08-06 19:31           ` Simon Glass
2015-08-06 22:13             ` Fabio Estevam
2015-08-13 12:37               ` Nikolay Dimitrov
2015-11-10  0:05                 ` Fabio Estevam
2015-11-10 14:41                   ` Fabio Estevam
2015-11-09 23:28               ` Fabio Estevam
2015-08-06 22:22         ` Marek Vasut

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.