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

[Michal Simek]

On 9/19/22 19:03, Jan Remes wrote:
> On Mon, Aug 8, 2022 at 10:05 AM Michal Simek <michal.simek at 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 at 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 at 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