Re: [PATCH 2/3] net: emaclite: fix xemaclite_alignedread/write functions

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