net: emaclite: Fixes, enable driver for other archs
These patches solve a few issues with the driver observed on a RISC-V platform.
Function ioremap_nocache seems to be defined only for mips and microblaze architectures. Therefore, the function call in the emaclite driver causes this driver to be unusable with other architectures, for example riscv.
Signed-off-by: Samuel Obuch samuel.obuch@codasip.com --- drivers/net/xilinx_emaclite.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/net/xilinx_emaclite.c b/drivers/net/xilinx_emaclite.c index 6c9f1f7c27..5cd88e04fe 100644 --- a/drivers/net/xilinx_emaclite.c +++ b/drivers/net/xilinx_emaclite.c @@ -615,8 +615,12 @@ static int emaclite_of_to_plat(struct udevice *dev) int offset = 0;
pdata->iobase = dev_read_addr(dev); +#if defined(CONFIG_MICROBLAZE) || defined(CONFIG_MIPS) emaclite->regs = (struct emaclite_regs *)ioremap_nocache(pdata->iobase, 0x10000); +#else + emaclite->regs = (struct emaclite_regs *)pdata->iobase; +#endif
emaclite->phyaddr = -1;
On Wed, Jul 13, 2022 at 5:02 PM Samuel Obuch samuel.obuch@codasip.com wrote:
Function ioremap_nocache seems to be defined only for mips and microblaze architectures. Therefore, the function call in the emaclite driver causes this driver to be unusable with other architectures, for example riscv.
Signed-off-by: Samuel Obuch samuel.obuch@codasip.com
drivers/net/xilinx_emaclite.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/net/xilinx_emaclite.c b/drivers/net/xilinx_emaclite.c index 6c9f1f7c27..5cd88e04fe 100644 --- a/drivers/net/xilinx_emaclite.c +++ b/drivers/net/xilinx_emaclite.c @@ -615,8 +615,12 @@ static int emaclite_of_to_plat(struct udevice *dev) int offset = 0;
pdata->iobase = dev_read_addr(dev);+#if defined(CONFIG_MICROBLAZE) || defined(CONFIG_MIPS) emaclite->regs = (struct emaclite_regs *)ioremap_nocache(pdata->iobase, 0x10000); +#else
emaclite->regs = (struct emaclite_regs *)pdata->iobase;+#endif
emaclite->phyaddr = -1;-- 2.31.1
Hm... Well, this isn't right,The right solution is to replace ioremap_nocache() with ioremap(). This way it will work both for MIPS and other architectures. I can do it myself, you can fix your patch. let me know. Thanks, Ramon.
On 8/6/22 19:31, Ramon Fried wrote:
On Wed, Jul 13, 2022 at 5:02 PM Samuel Obuch samuel.obuch@codasip.com wrote:
Function ioremap_nocache seems to be defined only for mips and microblaze architectures. Therefore, the function call in the emaclite driver causes this driver to be unusable with other architectures, for example riscv.
Signed-off-by: Samuel Obuch samuel.obuch@codasip.com
drivers/net/xilinx_emaclite.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/net/xilinx_emaclite.c b/drivers/net/xilinx_emaclite.c index 6c9f1f7c27..5cd88e04fe 100644 --- a/drivers/net/xilinx_emaclite.c +++ b/drivers/net/xilinx_emaclite.c @@ -615,8 +615,12 @@ static int emaclite_of_to_plat(struct udevice *dev) int offset = 0;
pdata->iobase = dev_read_addr(dev);+#if defined(CONFIG_MICROBLAZE) || defined(CONFIG_MIPS) emaclite->regs = (struct emaclite_regs *)ioremap_nocache(pdata->iobase, 0x10000); +#else
emaclite->regs = (struct emaclite_regs *)pdata->iobase;+#endif
emaclite->phyaddr = -1;-- 2.31.1
Hm... Well, this isn't right,The right solution is to replace ioremap_nocache() with ioremap(). This way it will work both for MIPS and other architectures. I can do it myself, you can fix your patch. let me know.
Microblaze doesn't define it now. But I agree that using ioremap which has implicit nocache is the right way to go. It means please create the first patch which creates ioremap for microblaze, Then second to replace ioremap_nocache() in emaclite driver to ioremap. And third to remove ioremap_nocache from microblaze io.h.
Thanks, Michal
Hi,
On 8/8/22 09:35, Michal Simek wrote:
On 8/6/22 19:31, Ramon Fried wrote:
On Wed, Jul 13, 2022 at 5:02 PM Samuel Obuch samuel.obuch@codasip.com wrote:
Function ioremap_nocache seems to be defined only for mips and microblaze architectures. Therefore, the function call in the emaclite driver causes this driver to be unusable with other architectures, for example riscv.
Signed-off-by: Samuel Obuch samuel.obuch@codasip.com
drivers/net/xilinx_emaclite.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/net/xilinx_emaclite.c b/drivers/net/xilinx_emaclite.c index 6c9f1f7c27..5cd88e04fe 100644 --- a/drivers/net/xilinx_emaclite.c +++ b/drivers/net/xilinx_emaclite.c @@ -615,8 +615,12 @@ static int emaclite_of_to_plat(struct udevice *dev) int offset = 0;
pdata->iobase = dev_read_addr(dev); +#if defined(CONFIG_MICROBLAZE) || defined(CONFIG_MIPS) emaclite->regs = (struct emaclite_regs *)ioremap_nocache(pdata->iobase, 0x10000); +#else + emaclite->regs = (struct emaclite_regs *)pdata->iobase; +#endif
emaclite->phyaddr = -1;
-- 2.31.1
Hm... Well, this isn't right,The right solution is to replace ioremap_nocache() with ioremap(). This way it will work both for MIPS and other architectures. I can do it myself, you can fix your patch. let me know.
Microblaze doesn't define it now. But I agree that using ioremap which has implicit nocache is the right way to go. It means please create the first patch which creates ioremap for microblaze, Then second to replace ioremap_nocache() in emaclite driver to ioremap. And third to remove ioremap_nocache from microblaze io.h.
I did closer look when I looked at other patches. You should switch to linux/io.h which automatically create ioremap if not defined by architecture.
And ioremap_nocache for microblaze can be removed later.
diff --git a/drivers/net/xilinx_emaclite.c b/drivers/net/xilinx_emaclite.c index 48aee77ab509..3299eefd999f 100644 --- a/drivers/net/xilinx_emaclite.c +++ b/drivers/net/xilinx_emaclite.c @@ -21,7 +21,7 @@ #include <linux/delay.h> #include <linux/errno.h> #include <linux/kernel.h> -#include <asm/io.h> +#include <linux/io.h>
M
Hi, using linux/io.h and ioremap works well for us, thanks. I will update the patch.
S
On Mon, Aug 8, 2022 at 9:44 AM Michal Simek michal.simek@amd.com wrote:
Hi,
On 8/8/22 09:35, Michal Simek wrote:
On 8/6/22 19:31, Ramon Fried wrote:
On Wed, Jul 13, 2022 at 5:02 PM Samuel Obuch samuel.obuch@codasip.com
wrote:
Function ioremap_nocache seems to be defined only for mips and
microblaze
architectures. Therefore, the function call in the emaclite driver
causes
this driver to be unusable with other architectures, for example riscv.
Signed-off-by: Samuel Obuch samuel.obuch@codasip.com
drivers/net/xilinx_emaclite.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/net/xilinx_emaclite.c
b/drivers/net/xilinx_emaclite.c
index 6c9f1f7c27..5cd88e04fe 100644 --- a/drivers/net/xilinx_emaclite.c +++ b/drivers/net/xilinx_emaclite.c @@ -615,8 +615,12 @@ static int emaclite_of_to_plat(struct udevice
*dev)
int offset = 0; pdata->iobase = dev_read_addr(dev);+#if defined(CONFIG_MICROBLAZE) || defined(CONFIG_MIPS) emaclite->regs = (struct emaclite_regs
*)ioremap_nocache(pdata->iobase,
0x10000);
+#else
emaclite->regs = (struct emaclite_regs *)pdata->iobase;+#endif
emaclite->phyaddr = -1;-- 2.31.1
Hm... Well, this isn't right,The right solution is to replace ioremap_nocache() with ioremap(). This way it will work both for MIPS and other architectures. I can do it myself, you can fix your patch. let me know.
Microblaze doesn't define it now. But I agree that using ioremap which
has
implicit nocache is the right way to go. It means please create the first patch which creates ioremap for
microblaze,
Then second to replace ioremap_nocache() in emaclite driver to ioremap.
And
third to remove ioremap_nocache from microblaze io.h.
I did closer look when I looked at other patches. You should switch to linux/io.h which automatically create ioremap if not defined by architecture.
And ioremap_nocache for microblaze can be removed later.
diff --git a/drivers/net/xilinx_emaclite.c b/drivers/net/xilinx_emaclite.c index 48aee77ab509..3299eefd999f 100644 --- a/drivers/net/xilinx_emaclite.c +++ b/drivers/net/xilinx_emaclite.c @@ -21,7 +21,7 @@ #include <linux/delay.h> #include <linux/errno.h> #include <linux/kernel.h> -#include <asm/io.h> +#include <linux/io.h>
M
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,
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_...
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
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.
Thanks, Jan
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
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
Hi,
On 9/23/22 11:17, Samuel Obuch wrote:
Hi, I tested both versions to be sure, but the results are as can be expected:
- 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?
I prefer raw versions too.
M
On Fri, Sep 23, 2022 at 12:36 PM Michal Simek michal.simek@amd.com wrote:
Hi,
On 9/23/22 11:17, Samuel Obuch wrote:
Hi, I tested both versions to be sure, but the results are as can be expected:
- 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?
I prefer raw versions too.
Let's keep it raw.
The maximum length is capped similarly to the emaclite_send function. Avoid integer underflow for values of ip->ip_len < 30, the minimum length of an IP packet is 21 bytes.
Signed-off-by: Samuel Obuch samuel.obuch@codasip.com --- drivers/net/xilinx_emaclite.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/xilinx_emaclite.c b/drivers/net/xilinx_emaclite.c index de7a2dee0b..21c450ec46 100644 --- a/drivers/net/xilinx_emaclite.c +++ b/drivers/net/xilinx_emaclite.c @@ -518,6 +518,8 @@ try_again: length = ntohs(ip->ip_len); length += ETHER_HDR_SIZE + ETH_FCS_LEN; debug("IP Packet %x\n", length); + if (length > PKTSIZE) + length = PKTSIZE; break; default: debug("Other Packet\n"); @@ -526,7 +528,7 @@ try_again: }
/* Read the rest of the packet which is longer then first read */ - if (length != first_read) + if (length > first_read) xemaclite_alignedread(addr + first_read, etherrxbuff + first_read, length - first_read);
On Wed, Jul 13, 2022 at 5:02 PM Samuel Obuch samuel.obuch@codasip.com wrote:
The maximum length is capped similarly to the emaclite_send function. Avoid integer underflow for values of ip->ip_len < 30, the minimum length of an IP packet is 21 bytes.
Signed-off-by: Samuel Obuch samuel.obuch@codasip.com
drivers/net/xilinx_emaclite.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/xilinx_emaclite.c b/drivers/net/xilinx_emaclite.c index de7a2dee0b..21c450ec46 100644 --- a/drivers/net/xilinx_emaclite.c +++ b/drivers/net/xilinx_emaclite.c @@ -518,6 +518,8 @@ try_again: length = ntohs(ip->ip_len); length += ETHER_HDR_SIZE + ETH_FCS_LEN; debug("IP Packet %x\n", length);
if (length > PKTSIZE)length = PKTSIZE; break; default: debug("Other Packet\n");@@ -526,7 +528,7 @@ try_again: }
/* Read the rest of the packet which is longer then first read */
if (length != first_read)
if (length > first_read) xemaclite_alignedread(addr + first_read, etherrxbuff + first_read, length - first_read);-- 2.31.1
+ Michal
On 8/6/22 19:35, Ramon Fried wrote:
On Wed, Jul 13, 2022 at 5:02 PM Samuel Obuch samuel.obuch@codasip.com wrote:
The maximum length is capped similarly to the emaclite_send function. Avoid integer underflow for values of ip->ip_len < 30, the minimum length of an IP packet is 21 bytes.
Signed-off-by: Samuel Obuch samuel.obuch@codasip.com
drivers/net/xilinx_emaclite.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/xilinx_emaclite.c b/drivers/net/xilinx_emaclite.c index de7a2dee0b..21c450ec46 100644 --- a/drivers/net/xilinx_emaclite.c +++ b/drivers/net/xilinx_emaclite.c @@ -518,6 +518,8 @@ try_again: length = ntohs(ip->ip_len); length += ETHER_HDR_SIZE + ETH_FCS_LEN; debug("IP Packet %x\n", length);
if (length > PKTSIZE)length = PKTSIZE; break; default: debug("Other Packet\n");@@ -526,7 +528,7 @@ try_again: }
/* Read the rest of the packet which is longer then first read */
if (length != first_read)
if (length > first_read) xemaclite_alignedread(addr + first_read, etherrxbuff + first_read, length - first_read);-- 2.31.1
- Michal
This looks good. Reviewed-by: Michal Simek michal.simek@amd.com
Thanks, Michal
Hi,
On 7/13/22 15:52, Samuel Obuch wrote:
These patches solve a few issues with the driver observed on a RISC-V platform.
How does that system look like that you are using standard Xilinx IPs? Do you use any other standard Xilinx IP like console?
Is it available somewhere that I can run it too?
Thanks, Michal
Hi,
its a soft-core in an FPGA. Other than the ethernet, I think we only use Xilinx bridges.
Unfortunately theres no publicly available version.
S.
On Mon, Aug 8, 2022 at 10:08 AM Michal Simek michal.simek@amd.com wrote:
Hi,
On 7/13/22 15:52, Samuel Obuch wrote:
These patches solve a few issues with the driver observed on a RISC-V
platform.
How does that system look like that you are using standard Xilinx IPs? Do you use any other standard Xilinx IP like console?
Is it available somewhere that I can run it too?
Thanks, Michal
Hi Samuel,
po 8. 8. 2022 v 10:34 odesílatel Samuel Obuch samuel.obuch@codasip.com napsal:
Hi,
its a soft-core in an FPGA. Other than the ethernet, I think we only use Xilinx bridges.
Unfortunately theres no publicly available version.
S.
On Mon, Aug 8, 2022 at 10:08 AM Michal Simek michal.simek@amd.com wrote:
Hi,
On 7/13/22 15:52, Samuel Obuch wrote:
These patches solve a few issues with the driver observed on a RISC-V
platform.
How does that system look like that you are using standard Xilinx IPs? Do you use any other standard Xilinx IP like console?
Is it available somewhere that I can run it too?
Are you going to send a new version of this patchset?
Thanks, Michal
participants (5)
-
Jan Remes -
Michal Simek -
Michal Simek -
Ramon Fried -
Samuel Obuch