This is the mail archive of the ecos-patches@sourceware.org mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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

--- Comment #13 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2011-08-22 15:24:02 BST ---
(In reply to comment #11)
> (In reply to comment #9)

[snip]

> > If I understood you correctly you again returned back to CYGARC
> > prefix for macros. If it is not a typo then I would add below those
> > definitions (CYGARC_SWAPxx macros) in ``cortexm_endian.h'' the same
> > check and far definitions (overrides)
> 
> It is a my mistake:( sorry, CYG_SWAPxx() it should be.
> 
> [snip (as it is consequence to my error)]

I see, however your "mistake" would provide us two versions of
CYG_SWAPxx() macros (one from hal_endian.h and another from
cortex_endian.h if it's needed). Ok, let's forget it.

> Here is cortexm_endian.h snippet:
> 
> #include <cyg/hal/cortexm_regs.h>
> 
> static __inline__ cyg_uint32 cyg_hal_swap32(cyg_uint32 original)
> {
>     cyg_uint32 swapped;
>     CYGARC_REV(swapped, original);
>     return swapped;
> }
> 
> static __inline__ cyg_uint16 cyg_hal_swap16(cyg_uint16 original)
> {
>     cyg_uint16 swapped;
>     CYGARC_REV16(swapped, original);
>     return swapped;
> }
> 
> #define CYG_SWAP32(__val) cyg_hal_swap32(__val)
> #define CYG_SWAP16(__val) cyg_hal_swap16(__val)

Looks okay for me.

> > So, with your proposal we can leave hal_arch.h without any changes.
> > Did you mean it? I read so and I guess that we have to manage the
> > definition of LWIP_PLATFORM_BYTESWAP from configtool. In ideal world
> > this value must be defined from lwip config file, e.g.
> 
> I can see 2 options:
> 
> Option 1. We include cortexm_endian.h in hal_arch.h (that's what i
> have tested).  here are all changes to hal_arch.h:
> 
> diff -u -r1.3 hal_arch.h
> --- hal_arch.h	13 Feb 2009 17:04:18 -0000	1.3
> +++ hal_arch.h	22 Aug 2011 12:45:24 -0000
> @@ -55,6 +55,9 @@
>  
>  #include <cyg/hal/var_arch.h>
>  
> +#include <cyg/hal/cortexm_regs.h>
> +#include <cyg/hal/cortexm_endian.h>
> + 
>  
> Option 2. Probably we can include hal_endian.h in lwipots.h (not tested).
> 
> My vote is for Option 1. since it would propagate cortexm_endian.h
> everywhere (for better, or for worse) Cortex-M is used.

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.

> > 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?

> If you agree with adding cortexm_endian.h and #include it in
> hal_arch.h I could submit patches for review.
> 
> Ilija

It seems to me that you can send cortexm_{regs,endian.h} for now (arch's
side) and the lwip's side needs further discussion.

Sergei

-- 
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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]