This is the mail archive of the
ecos-bugs@sourceware.org
mailing list for the eCos project.
[Bug 1001275] Cortex-M (armV7) architecture endian instructions /Applied on lwIP
- From: bugzilla-daemon at bugs dot ecos dot sourceware dot org
- To: unassigned at bugs dot ecos dot sourceware dot org
- Date: Mon, 22 Aug 2011 19:26:22 +0100
- Subject: [Bug 1001275] Cortex-M (armV7) architecture endian instructions /Applied on lwIP
- Auto-submitted: auto-generated
- References: <bug-1001275-777@http.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
--- Comment #15 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2011-08-22 19:26:17 BST ---
(In reply to comment #14)
> Created an attachment (id=1345)
--> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1345)
> Cortex-M architecture endian.
Ilija, thank you. The patch looks good for me, minors:
hal_cortexm.cdl: a few trailing whitespaces.
cortexm_regs.h: s/CYGONCE_HALCORTEXM_REGS_H/CYGONCE_CORTEXM_REGS_H/
No need to repost the patch. I'll fix that on commit.
> (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.
As for me I have no objection to include CPSID/CPSIE in cortexm_regs.h,
more that you test the macros on real hardware what is much appreciated.
> > (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.
(snip snippets)
> > 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.
Yep.
> It makes sense, but IMHO that, when available, /architecture optimized/
> functions should be enabled by default.
Agreed.
> 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.
Agreed. IMHO, you offered clear names for us (who reads this thread),
however, I would stick on a set from PKG, or BLD either IMP.
[OFF-TOPIC]
My recent find. CYGOPT_* names use itself in eCos config files more than
100 times! It seems to mee I used such names too :-) and reading this
http://ecos.sourceware.org/docs-latest/cdl-guide/language.naming.html
chapter I believe in that what to use CYGOPT is some kind of tautology.
> 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
> //----------------------------------------------------------------
Ilija, it's even more proper check. Once again thank you for your tests.
If you plan to check proposed CYGxxx_LWIP_ENDIAN_BY_HAL in lwipopts.h
#ifdef CYGxxx_LWIP_ENDIAN_BY_HAL
# include <cyg/hal/hal_endian.h>
#endif
I will very appreciate your final patches for lwipopts.h, and config
files.
Thank you,
Sergei
--
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.