| Message ID | CAOMZO5AdFJUvtnhTwwdBbpxPET0uRRWiANHEwdBRC4TUGOqW6g@mail.gmail.com |
|---|---|
| State | Not Applicable |
| Delegated to: | Simon Glass |
| Headers | show |
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 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
--- 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