[U-Boot] [PATCH 7/10] driver/mtd/spi:Read 8KB data chunk during u-boot load in SPL

[U-Boot] [PATCH 7/10] driver/mtd/spi:Read 8KB data chunk during u-boot load in SPL

[Prabhakar Kushwaha]

SPI driver perform its operation(read/write) on 64KB buffer chunk for data
greater than 64KB. This buffer chunk is allocated from system heap.

During SPL boot, 768KB of data is read from SPI flash.
Here, heap size may not be sufficient enough to full-fill 64KB buffer
requirement of SPI driver. So break down u-boot read operation at 8KB of chunk.

Also, fix a warning i.e. "unused variable buf" during CONFIG_FSL_CORENET

Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
---
 drivers/mtd/spi/fsl_espi_spl.c |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/spi/fsl_espi_spl.c b/drivers/mtd/spi/fsl_espi_spl.c
index e5ac79b..a55d741 100644
--- a/drivers/mtd/spi/fsl_espi_spl.c
+++ b/drivers/mtd/spi/fsl_espi_spl.c
@@ -20,8 +20,10 @@
 void spi_boot(void)
 {
 	void (*uboot)(void) __noreturn;
-	u32 offset, code_len;
+	u32 offset, code_len, copy_len = 0;
+#ifndef CONFIG_FSL_CORENET
 	unsigned char *buf = NULL;
+#endif
 	struct spi_flash *flash;
 
 	flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
@@ -56,8 +58,15 @@ void spi_boot(void)
 	code_len = code_len - CONFIG_SPL_MAX_SIZE;
 #endif
 	/* copy code to DDR */
-	spi_flash_read(flash, offset, code_len,
-		       (void *)CONFIG_SYS_SPI_FLASH_U_BOOT_DST);
+	printf("Loading second stage boot loader ");
+	while (copy_len <= code_len) {
+		spi_flash_read(flash, offset + copy_len, 0x2000,
+			       (void *)(CONFIG_SYS_SPI_FLASH_U_BOOT_DST
+			       + copy_len));
+		copy_len = copy_len + 0x2000;
+		putc('.');
+	}
+
 	/*
 	* Jump to U-Boot image
 	*/
-- 
1.7.9.5

[U-Boot] [PATCH 7/10] driver/mtd/spi:Read 8KB data chunk during u-boot load in SPL

[Scott Wood]

On Mon, 2014-03-31 at 15:34 +0530, Prabhakar Kushwaha wrote:
> SPI driver perform its operation(read/write) on 64KB buffer chunk for data
> greater than 64KB. This buffer chunk is allocated from system heap.
> 
> During SPL boot, 768KB of data is read from SPI flash.
> Here, heap size may not be sufficient enough to full-fill 64KB buffer
> requirement of SPI driver. So break down u-boot read operation at 8KB of chunk.
> 
> Also, fix a warning i.e. "unused variable buf" during CONFIG_FSL_CORENET
> 
> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
> ---
>  drivers/mtd/spi/fsl_espi_spl.c |   15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/spi/fsl_espi_spl.c b/drivers/mtd/spi/fsl_espi_spl.c
> index e5ac79b..a55d741 100644
> --- a/drivers/mtd/spi/fsl_espi_spl.c
> +++ b/drivers/mtd/spi/fsl_espi_spl.c
> @@ -20,8 +20,10 @@
>  void spi_boot(void)
>  {
>  	void (*uboot)(void) __noreturn;
> -	u32 offset, code_len;
> +	u32 offset, code_len, copy_len = 0;
> +#ifndef CONFIG_FSL_CORENET
>  	unsigned char *buf = NULL;
> +#endif
>  	struct spi_flash *flash;
>  
>  	flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
> @@ -56,8 +58,15 @@ void spi_boot(void)
>  	code_len = code_len - CONFIG_SPL_MAX_SIZE;
>  #endif
>  	/* copy code to DDR */
> -	spi_flash_read(flash, offset, code_len,
> -		       (void *)CONFIG_SYS_SPI_FLASH_U_BOOT_DST);
> +	printf("Loading second stage boot loader ");
> +	while (copy_len <= code_len) {
> +		spi_flash_read(flash, offset + copy_len, 0x2000,
> +			       (void *)(CONFIG_SYS_SPI_FLASH_U_BOOT_DST
> +			       + copy_len));
> +		copy_len = copy_len + 0x2000;
> +		putc('.');
> +	}

How long does this take?  If it's long enough to warrant a progress
meter, that ought to be fixed first.

-Scott

[U-Boot] [PATCH 7/10] driver/mtd/spi:Read 8KB data chunk during u-boot load in SPL

[Prabhakar Kushwaha]

On 4/2/2014 3:32 AM, Scott Wood wrote:
> On Mon, 2014-03-31 at 15:34 +0530, Prabhakar Kushwaha wrote:
>> SPI driver perform its operation(read/write) on 64KB buffer chunk for data
>> greater than 64KB. This buffer chunk is allocated from system heap.
>>
>> During SPL boot, 768KB of data is read from SPI flash.
>> Here, heap size may not be sufficient enough to full-fill 64KB buffer
>> requirement of SPI driver. So break down u-boot read operation at 8KB of chunk.
>>
>> Also, fix a warning i.e. "unused variable buf" during CONFIG_FSL_CORENET
>>
>> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
>> ---
>>   drivers/mtd/spi/fsl_espi_spl.c |   15 ++++++++++++---
>>   1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mtd/spi/fsl_espi_spl.c b/drivers/mtd/spi/fsl_espi_spl.c
>> index e5ac79b..a55d741 100644
>> --- a/drivers/mtd/spi/fsl_espi_spl.c
>> +++ b/drivers/mtd/spi/fsl_espi_spl.c
>> @@ -20,8 +20,10 @@
>>   void spi_boot(void)
>>   {
>>   	void (*uboot)(void) __noreturn;
>> -	u32 offset, code_len;
>> +	u32 offset, code_len, copy_len = 0;
>> +#ifndef CONFIG_FSL_CORENET
>>   	unsigned char *buf = NULL;
>> +#endif
>>   	struct spi_flash *flash;
>>   
>>   	flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
>> @@ -56,8 +58,15 @@ void spi_boot(void)
>>   	code_len = code_len - CONFIG_SPL_MAX_SIZE;
>>   #endif
>>   	/* copy code to DDR */
>> -	spi_flash_read(flash, offset, code_len,
>> -		       (void *)CONFIG_SYS_SPI_FLASH_U_BOOT_DST);
>> +	printf("Loading second stage boot loader ");
>> +	while (copy_len <= code_len) {
>> +		spi_flash_read(flash, offset + copy_len, 0x2000,
>> +			       (void *)(CONFIG_SYS_SPI_FLASH_U_BOOT_DST
>> +			       + copy_len));
>> +		copy_len = copy_len + 0x2000;
>> +		putc('.');
>> +	}
> How long does this take?  If it's long enough to warrant a progress
> meter, that ought to be fixed first.
>
>
It take 20 sec to boot.  it is long enough.  This issue has already been 
reported.

Problem is reading 1 MB from SPI flash.  It take almost 20s in on u-boot 
prompt from any boot.

I agree, we should first fix it.
But This patch should not be stopped to get apply because of issue 
somewhere else. That issue can be fixed separately in other patch.

Regards,
Prabhakar

[U-Boot] [PATCH 7/10] driver/mtd/spi:Read 8KB data chunk during u-boot load in SPL

[Scott Wood]

On Wed, 2014-04-02 at 08:34 +0530, Prabhakar Kushwaha wrote:
> On 4/2/2014 3:32 AM, Scott Wood wrote:
> > On Mon, 2014-03-31 at 15:34 +0530, Prabhakar Kushwaha wrote:
> >> SPI driver perform its operation(read/write) on 64KB buffer chunk for data
> >> greater than 64KB. This buffer chunk is allocated from system heap.
> >>
> >> During SPL boot, 768KB of data is read from SPI flash.
> >> Here, heap size may not be sufficient enough to full-fill 64KB buffer
> >> requirement of SPI driver. So break down u-boot read operation at 8KB of chunk.
> >>
> >> Also, fix a warning i.e. "unused variable buf" during CONFIG_FSL_CORENET
> >>
> >> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
> >> ---
> >>   drivers/mtd/spi/fsl_espi_spl.c |   15 ++++++++++++---
> >>   1 file changed, 12 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/mtd/spi/fsl_espi_spl.c b/drivers/mtd/spi/fsl_espi_spl.c
> >> index e5ac79b..a55d741 100644
> >> --- a/drivers/mtd/spi/fsl_espi_spl.c
> >> +++ b/drivers/mtd/spi/fsl_espi_spl.c
> >> @@ -20,8 +20,10 @@
> >>   void spi_boot(void)
> >>   {
> >>   	void (*uboot)(void) __noreturn;
> >> -	u32 offset, code_len;
> >> +	u32 offset, code_len, copy_len = 0;
> >> +#ifndef CONFIG_FSL_CORENET
> >>   	unsigned char *buf = NULL;
> >> +#endif
> >>   	struct spi_flash *flash;
> >>   
> >>   	flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
> >> @@ -56,8 +58,15 @@ void spi_boot(void)
> >>   	code_len = code_len - CONFIG_SPL_MAX_SIZE;
> >>   #endif
> >>   	/* copy code to DDR */
> >> -	spi_flash_read(flash, offset, code_len,
> >> -		       (void *)CONFIG_SYS_SPI_FLASH_U_BOOT_DST);
> >> +	printf("Loading second stage boot loader ");
> >> +	while (copy_len <= code_len) {
> >> +		spi_flash_read(flash, offset + copy_len, 0x2000,
> >> +			       (void *)(CONFIG_SYS_SPI_FLASH_U_BOOT_DST
> >> +			       + copy_len));
> >> +		copy_len = copy_len + 0x2000;
> >> +		putc('.');
> >> +	}
> > How long does this take?  If it's long enough to warrant a progress
> > meter, that ought to be fixed first.
> >
> >
> It take 20 sec to boot.  it is long enough.  This issue has already been 
> reported.

It wasn't mentioned in this patch submission...

> Problem is reading 1 MB from SPI flash.  It take almost 20s in on u-boot 
> prompt from any boot.
> 
> I agree, we should first fix it.
> But This patch should not be stopped to get apply because of issue 
> somewhere else. That issue can be fixed separately in other patch.

But this patch seems to assume the bug hasn't been fixed -- it still has
a progress meter, and it still breaks the SPI read into 8K chunks (not
sure if that part would change as part of the fix).

-Scott

[U-Boot] [PATCH 7/10] driver/mtd/spi:Read 8KB data chunk during u-boot load in SPL

[Prabhakar Kushwaha]

On 4/3/2014 4:40 AM, Scott Wood wrote:
> On Wed, 2014-04-02 at 08:34 +0530, Prabhakar Kushwaha wrote:
>> On 4/2/2014 3:32 AM, Scott Wood wrote:
>>> On Mon, 2014-03-31 at 15:34 +0530, Prabhakar Kushwaha wrote:
>>>> SPI driver perform its operation(read/write) on 64KB buffer chunk for data
>>>> greater than 64KB. This buffer chunk is allocated from system heap.
>>>>
>>>> During SPL boot, 768KB of data is read from SPI flash.
>>>> Here, heap size may not be sufficient enough to full-fill 64KB buffer
>>>> requirement of SPI driver. So break down u-boot read operation at 8KB of chunk.
>>>>
>>>> Also, fix a warning i.e. "unused variable buf" during CONFIG_FSL_CORENET
>>>>
>>>> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
>>>> ---
>>>>    drivers/mtd/spi/fsl_espi_spl.c |   15 ++++++++++++---
>>>>    1 file changed, 12 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/mtd/spi/fsl_espi_spl.c b/drivers/mtd/spi/fsl_espi_spl.c
>>>> index e5ac79b..a55d741 100644
>>>> --- a/drivers/mtd/spi/fsl_espi_spl.c
>>>> +++ b/drivers/mtd/spi/fsl_espi_spl.c
>>>> @@ -20,8 +20,10 @@
>>>>    void spi_boot(void)
>>>>    {
>>>>    	void (*uboot)(void) __noreturn;
>>>> -	u32 offset, code_len;
>>>> +	u32 offset, code_len, copy_len = 0;
>>>> +#ifndef CONFIG_FSL_CORENET
>>>>    	unsigned char *buf = NULL;
>>>> +#endif
>>>>    	struct spi_flash *flash;
>>>>    
>>>>    	flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
>>>> @@ -56,8 +58,15 @@ void spi_boot(void)
>>>>    	code_len = code_len - CONFIG_SPL_MAX_SIZE;
>>>>    #endif
>>>>    	/* copy code to DDR */
>>>> -	spi_flash_read(flash, offset, code_len,
>>>> -		       (void *)CONFIG_SYS_SPI_FLASH_U_BOOT_DST);
>>>> +	printf("Loading second stage boot loader ");
>>>> +	while (copy_len <= code_len) {
>>>> +		spi_flash_read(flash, offset + copy_len, 0x2000,
>>>> +			       (void *)(CONFIG_SYS_SPI_FLASH_U_BOOT_DST
>>>> +			       + copy_len));
>>>> +		copy_len = copy_len + 0x2000;
>>>> +		putc('.');
>>>> +	}
>>> How long does this take?  If it's long enough to warrant a progress
>>> meter, that ought to be fixed first.
>>>
>>>
>> It take 20 sec to boot.  it is long enough.  This issue has already been
>> reported.
> It wasn't mentioned in this patch submission...

I will add this  information..

>> Problem is reading 1 MB from SPI flash.  It take almost 20s in on u-boot
>> prompt from any boot.
>>
>> I agree, we should first fix it.
>> But This patch should not be stopped to get apply because of issue
>> somewhere else. That issue can be fixed separately in other patch.
> But this patch seems to assume the bug hasn't been fixed -- it still has
> a progress meter, and it still breaks the SPI read into 8K chunks (not
> sure if that part would change as part of the fix).
>

I will make sure of removing this progress meter once issue is fixed.

Regards,
Prabhakar