From mboxrd@z Thu Jan  1 00:00:00 1970
From: vikasm <vikas.manocha@st.com>
Date: Thu, 13 Aug 2015 09:30:37 -0700
Subject: [U-Boot] [v2 3/6] spi: cadence_qspi: remove sram polling from
 flash write
In-Reply-To: <201508130411.53564.marex@denx.de>
References: <1437013654-29387-1-git-send-email-vikas.manocha@st.com>
 <1437013654-29387-4-git-send-email-vikas.manocha@st.com>
 <201508130411.53564.marex@denx.de>
Message-ID: <55CCC62D.5020701@st.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Marek,

On 08/12/2015 07:11 PM, Marek Vasut wrote:
> On Thursday, July 16, 2015 at 04:27:31 AM, Vikas Manocha wrote:
>> There is no need to poll sram level before writing to flash, data going to
>> SRAM till sram is full, after that backpressure will take over.
> 
> Please see the question I posed in 2/6 v2 .

replied in 2/6 v2.

> 
>> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
>> ---
>>
>> Changes in v2: Rebased to master
>>
>>  drivers/spi/cadence_qspi_apb.c |   61
>> ++++++++++------------------------------ 1 file changed, 15 insertions(+),
>> 46 deletions(-)
>>
>> diff --git a/drivers/spi/cadence_qspi_apb.c
>> b/drivers/spi/cadence_qspi_apb.c index 487bbef..ad8b79a 100644
>> --- a/drivers/spi/cadence_qspi_apb.c
>> +++ b/drivers/spi/cadence_qspi_apb.c
>> @@ -214,67 +214,36 @@ static int cadence_qspi_apb_read_fifo_data(void
>> *dest, return 0;
>>  }
>>
>> -static void cadence_qspi_apb_write_fifo_data(const void *dest_ahb_addr,
>> -	const void *src, unsigned int bytes)
>> -{
>> -	unsigned int temp = 0;
>> -	int i;
>> -	int remaining = bytes;
>> -	unsigned int *dest_ptr = (unsigned int *)dest_ahb_addr;
>> -	unsigned int *src_ptr = (unsigned int *)src;
>> -
>> -	while (remaining >= CQSPI_FIFO_WIDTH) {
>> -		for (i = CQSPI_FIFO_WIDTH/sizeof(src_ptr) - 1; i >= 0; i--)
>> -			writel(*(src_ptr+i), dest_ptr+i);
>> -		src_ptr += CQSPI_FIFO_WIDTH/sizeof(src_ptr);
>> -		remaining -= CQSPI_FIFO_WIDTH;
>> -	}
>> -	if (remaining) {
>> -		/* dangling bytes */
>> -		i = remaining/sizeof(dest_ptr);
>> -		memcpy(&temp, src_ptr+i, remaining % sizeof(dest_ptr));
>> -		writel(temp, dest_ptr+i);
>> -		for (--i; i >= 0; i--)
>> -			writel(*(src_ptr+i), dest_ptr+i);
>> -	}
>> -	return;
>> -}
>> -
>>  /* Write to SRAM FIFO with polling SRAM fill level. */
>>  static int qpsi_write_sram_fifo_push(struct cadence_spi_platdata *plat,
>>  				const void *src_addr, unsigned int num_bytes)
>>  {
>> -	const void *reg_base = plat->regbase;
>> -	void *dest_addr = plat->ahbbase;
>> -	unsigned int retry = CQSPI_REG_RETRY;
>> -	unsigned int sram_level;
>> +	int i = 0;
>> +	unsigned int *dest_addr = plat->ahbbase;
>>  	unsigned int wr_bytes;
>> -	unsigned char *src = (unsigned char *)src_addr;
>> +	unsigned int *src_ptr = (unsigned int *)src_addr;
>>  	int remaining = num_bytes;
>>  	unsigned int page_size = plat->page_size;
>> -	unsigned int sram_threshold_words = CQSPI_REG_SRAM_THRESHOLD_WORDS;
>>
>>  	while (remaining > 0) {
>> -		retry = CQSPI_REG_RETRY;
>> -		while (retry--) {
>> -			sram_level = CQSPI_GET_WR_SRAM_LEVEL(reg_base);
>> -			if (sram_level <= sram_threshold_words)
>> -				break;
>> -		}
>> -		if (!retry) {
>> -			printf("QSPI: SRAM fill level (0x%08x) not hit lower 
> expected level
>> (0x%08x)", -			       sram_level, sram_threshold_words);
>> -			return -1;
>> -		}
>>  		/* Write a page or remaining bytes. */
>>  		wr_bytes = (remaining > page_size) ?
>>  					page_size : remaining;
>>
>> -		cadence_qspi_apb_write_fifo_data(dest_addr, src, wr_bytes);
>> -		src += wr_bytes;
>>  		remaining -= wr_bytes;
>> +		while (wr_bytes >= CQSPI_FIFO_WIDTH) {
>> +			for (i = 0; i < CQSPI_FIFO_WIDTH/sizeof(dest_addr); i++)
>> +				writel(*(src_ptr+i), dest_addr+i);
> 
> Why don't you use memcpy instead , didn't you say you're copying data to/from 
> SRAM? Is src_ptr value always aligned ?

i think you are right, i will check it & fix in the next version.

Rgds,
Vikas

> 
>> +			src_ptr += CQSPI_FIFO_WIDTH/sizeof(dest_addr);
>> +			wr_bytes -= CQSPI_FIFO_WIDTH;
>> +		}
>> +		if (wr_bytes) {
>> +			/* dangling bytes */
>> +			i = wr_bytes/sizeof(dest_addr);
>> +			for (i = wr_bytes/sizeof(dest_addr); i >= 0; i--)
>> +				writel(*(src_ptr+i), dest_addr+i);
>> +		}
>>  	}
>> -
>>  	return 0;
>>  }
> .
>