On 14 Aug 2014, stefan@agner.ch wrote:
This adds initial support for Freescale NFC (NAND Flash Controller) found in ARM Vybrid SoC's, Power Architecture MPC5125 and others. However, this driver is only tested on Vybrid.
On Wed, 2014-08-13 at 22:32, Scott Wood wrote:
raw_writel() is itself something that should only be used for hand-optimized sections. For non-performance-critical code you should use normal writel() so that you don't need to worry about manually adding I/O barriers.
Am 2014-08-14 23:12, schrieb Bill Pringlemeir:
[regarding memcpy() in the driver]
Maybe a comment is fine? It seems the Vybrid is safe for different access sizes, but it is possible that some other CPU might not be able to access this memory via 32/16/8 bit accesses and 'memcpy()' may not be appropriate. It seems that 'natural' size of the NFC controller itself is 32bits and the CPU interface does lane masking. Ie, boot mode documentation talks about remapping 'sram_physical_addr[13:3] = {cpu_addr[11:3],cpu_addr[13:12]}' saying that bits 2,1 are not used (hopefully one based numbers). This is just my guess...
On 18 Aug 2014, stefan@agner.ch wrote:
What assumptions do you make how memcpy accesses memory? This latest patch now uses the optimized versions from the kernel... Maybe they even try to access 64-bit width (the NIC interconnect supports 64-bit access)
[snip]
Am 2014-08-18 18:38, schrieb Bill Pringlemeir:
My only point is that the SRAM buffers use the same interface as the main Nand controller register banks. So using 'readl/writel' for the register, but not the SRAM buffers seems inconsistent.
So to address this inconsistency, I was thinking that we should at least have a comment?
On 19 Aug 2014, stefan@agner.ch wrote:
IMHO, we just treat this as if its memory and I guess this is fine for a buffer. memcpy knows how to copy data, and takes care if the architecture needs aligned access when reading 32-bit width, or similar requirements. We do not know whether memcpy really uses 32-bit accesses, hence this comment might even be wrong. In a short test, I could also access the buffer in byte/word length (tested using md.b/md.w).
Also, I assume this just works for a different architecture too. If not, the one using this driver the first time on a different architecture would see this pretty quickly I guess :-)
[snip]
In our case, a barrier just after the memcpy would be sufficient.
I would suggest you make a 'vf610_nfc_memcpy()' [or even from/to variants if you are pendantic] which can be a wrapper function of just 'memcpy'. Just the like the readl/writel wrappers this will collect the BUS accesses into one place. So they are documented for people porting the code. Trying to accommodate some future insane hardware hookup seems futile beyond this?
Otherwise, I will add an 'Ack' or 'Reviewed-By' from me if you like. I am sorry, I don't know what if anything is appropriate.
Thanks, Bill Pringlemeir.