[Bug 1001275] Cortex-M (armV7) architecture endian instructions / Applied on lwIP

[bugzilla-daemon@bugs.ecos.sourceware.org]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001275

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1299|0                           |1
        is obsolete|                            |

--- Comment #14 from Ilija Kocho <ilijak@siva.com.mk> 2011-08-22 17:44:42 BST ---
Created an attachment (id=1345)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1345)
Cortex-M architecture endian.

(In reply to comment #13)

Here I submit Cortex-M architectural patches. I took the liberty to put/leave
some additional instructions  in cortexm_regs.h (that I am using for Kinetis
port). They are CPSID and CPSIE. I hope they will be accepted but if reviewers
find that they are too off-topic they may be removed.

> (In reply to comment #11)
> In opposite I would avoid from "Option 1". Usually <hal>_regs.h, and
> hal_endian.h include inself from hal_io.h, and hal_misc.c. I liked you
> find, because I thought, Great! We do not need hack hal_arch.h :-) I
> would vote for "Option 2" and that is what I did mean in my comment 9.

Now I agree. Patch follows the Option 2.

> 
> > > to add into lwip_net.cdl:
> > > 
> > > cdl_option CYGBLD_LWIP_HTONS_HTONL_INLINED {
> > >   default_value 0
> > > }
> > > 
> > 
> > In either case we don't need CDL. Following is what I have tested (a
> > patch to your input) :
> > 
> > > to add into lwipopts.h:
> > > 
> > > #if CYGPKG_LWIP_HTONS_HTONL_INLINED
> > -#if CYGPKG_LWIP_HTONS_HTONL_INLINED
> > 
> > > # include <cyg/hal/hal_endian.h>
> > +#ifdef CYG_SWAP32
> > > # define LWIP_PLATFORM_BYTESWAP 1
> > > # define LWIP_PLATFORM_HTONS(__val) CYG_SWAP16(__val)
> > > # define LWIP_PLATFORM_HTONL(__val) CYG_SWAP32(__val)
> > > #endif
> > > 
> > > What do you think?
> 
> I see. However, in such case you make lwip stack use only the inlined
> versions for htonX(s). I have a doubt about such a forcing. What is a
> bad with CDL option?

Sergei, now I see, you want to provide the user with choice between standard
and /architecture optimized/ endian functions.

It makes sense, but IMHO that, when available, /architecture optimized/
functions should be enabled by default.

It is likely that we shall need to add CDL options in both lwIP (bool) and in
architecture's CDL (provider of default value for lwIP).
For arch name I would propose something like CYGxxx_HAL_ARCH_ENDIAN where xxx
could be {PKG, OPT, BLD, INT}, and for lwIP - CYGxxx_LWIP_ENDIAN_BY_HAL.

At the end, for reference, here are lwip modifications (as are now) with which
I have tested. Please note that now I am using CYG_CPU_TO_BExx macros, also
defined in hal_endian.h and they consider existence of big endians.

diff -u -r1.8 lwipopts.h
--- current/include/lwipopts.h    31 Jan 2011 21:52:32 -0000    1.8
+++ current/include/lwipopts.h    22 Aug 2011 16:23:38 -0000
@@ -35,6 +35,7 @@
 #include <pkgconf/net_lwip.h>

 #include <cyg/hal/hal_arch.h>
+#include <cyg/hal/hal_endian.h>

 //-------------------------------------------------------------------
 // Platform specific locking
@@ -44,6 +45,16 @@
 #define NO_SYS                      defined(CYGFUN_LWIP_MODE_SIMPLE)

 //-------------------------------------------------------------------
+// Architecture specific options
+//--------------------------------------------------------------------
+
+#if defined(CYG_CPU_TO_BE16) && defined(CYG_CPU_TO_BE32)
+# define LWIP_PLATFORM_BYTESWAP 1
+# define LWIP_PLATFORM_HTONS(__val) CYG_CPU_TO_BE16(__val)
+# define LWIP_PLATFORM_HTONL(__val) CYG_CPU_TO_BE32(__val)
+#endif // defined(CYG_CPU_TO_BE16(__val)) && defined(CYG_CPU_TO_BE32(__val))
+
+//-----------------------------------------------------------------
 // Memory options
 //----------------------------------------------------------------

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.