From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Chou <thomas@wytron.com.tw>
Date: Thu, 5 Nov 2015 12:26:25 +0800
Subject: [U-Boot] [PATCH] mtd: add altera quadspi driver
In-Reply-To: <1446692747.2088.11.camel@clsee-VirtualBox>
References: <1446556932-14978-1-git-send-email-thomas@wytron.com.tw>
 <201511041718.35613.marex@denx.de> <1446691758.2088.5.camel@clsee-VirtualBox>
 <201511050353.09704.marex@denx.de>
 <1446692747.2088.11.camel@clsee-VirtualBox>
Message-ID: <563ADA71.9030007@wytron.com.tw>
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 Chin Liang,

On 2015?11?05? 11:05, Chin Liang See wrote:
>>>>> I notice you are writing in word style which might have concern in
>>>>> performance. As the burst count can go up to 64, we can write larger
>>>>> data through memcpy. This will avoid redundancy of data header (opcode
>>>>> + address + dummy).
>>>>
>>>> You cannot do that, memcpy works on memory while write*() operators work
>>>> on I/O. You should use readsl() and friends then.
>>>
>>> Actually I am thinking to take advantage the cache fill and dump. But
>>> after rethinking, this might limit some of the use case as we want the
>>> driver to support NIOS II without cache. With that, just ignore this
>>> comment for now.
>>
>> I'm not sure I want to ask for details here. I think we're reading data from
>> some sort of IO device, so we should just use readl() or readsl() to read
>> them out (and write*() for the other direction). I don't think cache operations
>> can be involved in any way. Correct me if I'm wrong please.
>>
>
> Sure, I can share more. Since the read can support up to burst of 64
> byte, we can use the a cache fill method which eventually trigger a read
> of 32 byte (which is size of a cache line) to the Quad SPI controller.
> To ensure we don't read from old data, we need to invalidate that cache
> line (through address). By doing this, we can gain better performance as
> we are reading 32 bytes of data instead 4 per transaction.
>
>>> But your comment lead to the fact that the read part is now using
>>> memcpy. Thomas needs to fix that to use the readl :)
>>
>> Uhm, I don't think I understand this remark, sorry. I never suggested to use
>> memcpy() in this entire thread, did I ?
>
>
> Nope, but this trigger me that we need to do the same for read. The
> memcpy might lead to the driver reading old data that stay on cache
> instead from controller. Another way to get rid of this is invalidate
> the cache.

Thank for the reminding about the read part. I should not use memcpy() 
indeed.

Maybe we could pull the memcpy_fromio() and memcpy_toio() or the 
asm-generic/io.h from Linux kernel?

For i/o access, we bypass the cache for u-boot nios2 with ioremap() or 
map_physaddr(uncached). So no worries or advantage about cache.

Best regards,
Thomas