Hi, I tested both versions to be sure, but the results are as can be expected:
1. both __raw_readl/__raw_writel and readl/writel functions work ok on riscv - only the original nonvolatile accesses were problematic 2. the additional barrier in readl/writel functions can introduce noticeable slowdown - e.g. with our setup, download speed over tftp for the same image decreased from 460 KiB/s to 118 KiB/s
Can we conclude to keep the raw versions in the patch?
Thanks, Samuel
On Tue, Sep 20, 2022 at 4:23 PM Michal Simek michal.simek@amd.com wrote:
On 9/19/22 19:03, Jan Remes wrote:
On Mon, Aug 8, 2022 at 10:05 AM Michal Simek michal.simek@amd.com
wrote:
On 8/6/22 19:33, Ramon Fried wrote:
On Wed, Jul 13, 2022 at 5:02 PM Samuel Obuch samuel.obuch@codasip.com
wrote:
Use __raw_read* and __raw_write* functions to ensure read/write is passed to the memory-mapped regions, as non-volatile accesses may get optimised out.
Signed-off-by: Samuel Obuch samuel.obuch@codasip.com
drivers/net/xilinx_emaclite.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/net/xilinx_emaclite.c
b/drivers/net/xilinx_emaclite.c
index 5cd88e04fe..de7a2dee0b 100644 --- a/drivers/net/xilinx_emaclite.c +++ b/drivers/net/xilinx_emaclite.c @@ -113,12 +113,12 @@ static void xemaclite_alignedread(u32 *srcptr,
void *destptr, u32 bytecount)
/* Word aligned buffer, no correction needed. */ to32ptr = (u32 *) destptr; while (bytecount > 3) {
*to32ptr++ = *from32ptr++;
*to32ptr++ = __raw_readl(from32ptr++); bytecount -= 4; } to8ptr = (u8 *) to32ptr;
alignbuffer = *from32ptr++;
alignbuffer = __raw_readl(from32ptr++); from8ptr = (u8 *) &alignbuffer; for (i = 0; i < bytecount; i++)@@ -136,8 +136,7 @@ static void xemaclite_alignedwrite(void *srcptr,
u32 *destptr, u32 bytecount)
from32ptr = (u32 *) srcptr; while (bytecount > 3) {
*to32ptr++ = *from32ptr++;
__raw_writel(*from32ptr++, to32ptr++); bytecount -= 4; }@@ -148,7 +147,7 @@ static void xemaclite_alignedwrite(void *srcptr,
u32 *destptr, u32 bytecount)
for (i = 0; i < bytecount; i++) *to8ptr++ = *from8ptr++;
*to32ptr++ = alignbuffer;
__raw_writel(alignbuffer, to32ptr++);}
static int wait_for_bit(const char *func, u32 *reg, const u32 mask,
-- 2.31.1
I think that using readl/writel is fine, no need for raw_...
For microblaze that should be fine but let me ask my team to rest it on
ARM.
I think that __raw version are safer because this IP can also run on
big endian
systems and I think that was the reason why readl/writel wasn't used in
past.
Thanks, Michal
Hi Michal,
Do you have any new information on this? For the v2, it would be good to have this resolved.
we are not testing emaclite on any ARM design. But in Linux you can find in this driver.
#ifdef __BIG_ENDIAN #define xemaclite_readl ioread32be #define xemaclite_writel iowrite32be #else #define xemaclite_readl ioread32 #define xemaclite_writel iowrite32 #endif
If you keep __raw variants it will ensure that native endian access is doing to be used. On ARM IIRC readl/writel also have barriers. Origin patch is also using raw variant that's why I expect it is working on your system.
Thanks, Michal