* net: emaclite: Fixes, enable driver for other archs
@ 2022-07-13 13:52 Samuel Obuch
2022-07-13 13:52 ` [PATCH 1/3] net: emaclite: fix broken build Samuel Obuch
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: Samuel Obuch @ 2022-07-13 13:52 UTC (permalink / raw)
To: u-boot
These patches solve a few issues with the driver observed on a RISC-V platform.
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH 1/3] net: emaclite: fix broken build 2022-07-13 13:52 net: emaclite: Fixes, enable driver for other archs Samuel Obuch @ 2022-07-13 13:52 ` Samuel Obuch 2022-08-06 17:31 ` Ramon Fried 2022-07-13 13:52 ` [PATCH 2/3] net: emaclite: fix xemaclite_alignedread/write functions Samuel Obuch ` (2 subsequent siblings) 3 siblings, 1 reply; 20+ messages in thread From: Samuel Obuch @ 2022-07-13 13:52 UTC (permalink / raw) To: u-boot; +Cc: Samuel Obuch 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] net: emaclite: fix broken build 2022-07-13 13:52 ` [PATCH 1/3] net: emaclite: fix broken build Samuel Obuch @ 2022-08-06 17:31 ` Ramon Fried 2022-08-08 7:35 ` Michal Simek 0 siblings, 1 reply; 20+ messages in thread From: Ramon Fried @ 2022-08-06 17:31 UTC (permalink / raw) To: Samuel Obuch; +Cc: U-Boot Mailing List 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] net: emaclite: fix broken build 2022-08-06 17:31 ` Ramon Fried @ 2022-08-08 7:35 ` Michal Simek 2022-08-08 7:44 ` Michal Simek 0 siblings, 1 reply; 20+ messages in thread From: Michal Simek @ 2022-08-08 7:35 UTC (permalink / raw) To: Ramon Fried, Samuel Obuch; +Cc: U-Boot Mailing List 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] net: emaclite: fix broken build 2022-08-08 7:35 ` Michal Simek @ 2022-08-08 7:44 ` Michal Simek 2022-09-23 9:29 ` Samuel Obuch 0 siblings, 1 reply; 20+ messages in thread From: Michal Simek @ 2022-08-08 7:44 UTC (permalink / raw) To: Ramon Fried, Samuel Obuch; +Cc: U-Boot Mailing List 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] net: emaclite: fix broken build 2022-08-08 7:44 ` Michal Simek @ 2022-09-23 9:29 ` Samuel Obuch 0 siblings, 0 replies; 20+ messages in thread From: Samuel Obuch @ 2022-09-23 9:29 UTC (permalink / raw) To: Michal Simek; +Cc: Ramon Fried, U-Boot Mailing List 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 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/3] net: emaclite: fix xemaclite_alignedread/write functions 2022-07-13 13:52 net: emaclite: Fixes, enable driver for other archs Samuel Obuch 2022-07-13 13:52 ` [PATCH 1/3] net: emaclite: fix broken build Samuel Obuch @ 2022-07-13 13:52 ` Samuel Obuch 2022-08-06 17:33 ` Ramon Fried 2022-07-13 13:52 ` [PATCH 3/3] net: emaclite: fix handling for IP packets with specific lengths Samuel Obuch 2022-08-08 8:08 ` net: emaclite: Fixes, enable driver for other archs Michal Simek 3 siblings, 1 reply; 20+ messages in thread From: Samuel Obuch @ 2022-07-13 13:52 UTC (permalink / raw) To: u-boot; +Cc: Samuel Obuch 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] net: emaclite: fix xemaclite_alignedread/write functions 2022-07-13 13:52 ` [PATCH 2/3] net: emaclite: fix xemaclite_alignedread/write functions Samuel Obuch @ 2022-08-06 17:33 ` Ramon Fried 2022-08-08 8:04 ` Michal Simek 0 siblings, 1 reply; 20+ messages in thread From: Ramon Fried @ 2022-08-06 17:33 UTC (permalink / raw) To: Samuel Obuch; +Cc: U-Boot Mailing List 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_... ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] net: emaclite: fix xemaclite_alignedread/write functions 2022-08-06 17:33 ` Ramon Fried @ 2022-08-08 8:04 ` Michal Simek 2022-09-19 17:03 ` Jan Remes 0 siblings, 1 reply; 20+ messages in thread From: Michal Simek @ 2022-08-08 8:04 UTC (permalink / raw) To: Ramon Fried, Samuel Obuch; +Cc: U-Boot Mailing List 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] net: emaclite: fix xemaclite_alignedread/write functions 2022-08-08 8:04 ` Michal Simek @ 2022-09-19 17:03 ` Jan Remes 2022-09-20 14:23 ` Michal Simek 0 siblings, 1 reply; 20+ messages in thread From: Jan Remes @ 2022-09-19 17:03 UTC (permalink / raw) To: Michal Simek; +Cc: Ramon Fried, Samuel Obuch, U-Boot Mailing List 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] net: emaclite: fix xemaclite_alignedread/write functions 2022-09-19 17:03 ` Jan Remes @ 2022-09-20 14:23 ` Michal Simek 2022-09-23 9:17 ` Samuel Obuch 0 siblings, 1 reply; 20+ messages in thread From: Michal Simek @ 2022-09-20 14:23 UTC (permalink / raw) To: Jan Remes; +Cc: Ramon Fried, Samuel Obuch, U-Boot Mailing List 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] net: emaclite: fix xemaclite_alignedread/write functions 2022-09-20 14:23 ` Michal Simek @ 2022-09-23 9:17 ` Samuel Obuch 2022-09-23 9:36 ` Michal Simek 0 siblings, 1 reply; 20+ messages in thread From: Samuel Obuch @ 2022-09-23 9:17 UTC (permalink / raw) To: Michal Simek; +Cc: Jan Remes, Ramon Fried, U-Boot Mailing List 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 > > > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] net: emaclite: fix xemaclite_alignedread/write functions 2022-09-23 9:17 ` Samuel Obuch @ 2022-09-23 9:36 ` Michal Simek 2022-10-05 2:09 ` Ramon Fried 0 siblings, 1 reply; 20+ messages in thread From: Michal Simek @ 2022-09-23 9:36 UTC (permalink / raw) To: Samuel Obuch; +Cc: Jan Remes, Ramon Fried, U-Boot Mailing List 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: > > 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? I prefer raw versions too. M ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] net: emaclite: fix xemaclite_alignedread/write functions 2022-09-23 9:36 ` Michal Simek @ 2022-10-05 2:09 ` Ramon Fried 0 siblings, 0 replies; 20+ messages in thread From: Ramon Fried @ 2022-10-05 2:09 UTC (permalink / raw) To: Michal Simek; +Cc: Samuel Obuch, Jan Remes, U-Boot Mailing List 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: > > > > 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? > > I prefer raw versions too. > > Let's keep it raw. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/3] net: emaclite: fix handling for IP packets with specific lengths 2022-07-13 13:52 net: emaclite: Fixes, enable driver for other archs Samuel Obuch 2022-07-13 13:52 ` [PATCH 1/3] net: emaclite: fix broken build Samuel Obuch 2022-07-13 13:52 ` [PATCH 2/3] net: emaclite: fix xemaclite_alignedread/write functions Samuel Obuch @ 2022-07-13 13:52 ` Samuel Obuch 2022-08-06 17:35 ` Ramon Fried 2022-08-08 8:08 ` net: emaclite: Fixes, enable driver for other archs Michal Simek 3 siblings, 1 reply; 20+ messages in thread From: Samuel Obuch @ 2022-07-13 13:52 UTC (permalink / raw) To: u-boot; +Cc: Samuel Obuch 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] net: emaclite: fix handling for IP packets with specific lengths 2022-07-13 13:52 ` [PATCH 3/3] net: emaclite: fix handling for IP packets with specific lengths Samuel Obuch @ 2022-08-06 17:35 ` Ramon Fried 2022-08-08 7:54 ` Michal Simek 0 siblings, 1 reply; 20+ messages in thread From: Ramon Fried @ 2022-08-06 17:35 UTC (permalink / raw) To: Samuel Obuch, michal.simek; +Cc: U-Boot Mailing List 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] net: emaclite: fix handling for IP packets with specific lengths 2022-08-06 17:35 ` Ramon Fried @ 2022-08-08 7:54 ` Michal Simek 0 siblings, 0 replies; 20+ messages in thread From: Michal Simek @ 2022-08-08 7:54 UTC (permalink / raw) To: Ramon Fried, Samuel Obuch; +Cc: U-Boot Mailing List 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: net: emaclite: Fixes, enable driver for other archs 2022-07-13 13:52 net: emaclite: Fixes, enable driver for other archs Samuel Obuch ` (2 preceding siblings ...) 2022-07-13 13:52 ` [PATCH 3/3] net: emaclite: fix handling for IP packets with specific lengths Samuel Obuch @ 2022-08-08 8:08 ` Michal Simek 2022-08-08 8:33 ` Samuel Obuch 3 siblings, 1 reply; 20+ messages in thread From: Michal Simek @ 2022-08-08 8:08 UTC (permalink / raw) To: Samuel Obuch, u-boot 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: net: emaclite: Fixes, enable driver for other archs 2022-08-08 8:08 ` net: emaclite: Fixes, enable driver for other archs Michal Simek @ 2022-08-08 8:33 ` Samuel Obuch 2022-09-19 7:31 ` Michal Simek 0 siblings, 1 reply; 20+ messages in thread From: Samuel Obuch @ 2022-08-08 8:33 UTC (permalink / raw) To: Michal Simek; +Cc: u-boot 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 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: net: emaclite: Fixes, enable driver for other archs 2022-08-08 8:33 ` Samuel Obuch @ 2022-09-19 7:31 ` Michal Simek 0 siblings, 0 replies; 20+ messages in thread From: Michal Simek @ 2022-09-19 7:31 UTC (permalink / raw) To: Samuel Obuch; +Cc: Michal Simek, u-boot 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 -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Xilinx Microblaze Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2022-10-05 2:09 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-07-13 13:52 net: emaclite: Fixes, enable driver for other archs Samuel Obuch 2022-07-13 13:52 ` [PATCH 1/3] net: emaclite: fix broken build Samuel Obuch 2022-08-06 17:31 ` Ramon Fried 2022-08-08 7:35 ` Michal Simek 2022-08-08 7:44 ` Michal Simek 2022-09-23 9:29 ` Samuel Obuch 2022-07-13 13:52 ` [PATCH 2/3] net: emaclite: fix xemaclite_alignedread/write functions Samuel Obuch 2022-08-06 17:33 ` Ramon Fried 2022-08-08 8:04 ` Michal Simek 2022-09-19 17:03 ` Jan Remes 2022-09-20 14:23 ` Michal Simek 2022-09-23 9:17 ` Samuel Obuch 2022-09-23 9:36 ` Michal Simek 2022-10-05 2:09 ` Ramon Fried 2022-07-13 13:52 ` [PATCH 3/3] net: emaclite: fix handling for IP packets with specific lengths Samuel Obuch 2022-08-06 17:35 ` Ramon Fried 2022-08-08 7:54 ` Michal Simek 2022-08-08 8:08 ` net: emaclite: Fixes, enable driver for other archs Michal Simek 2022-08-08 8:33 ` Samuel Obuch 2022-09-19 7:31 ` Michal Simek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).