public inbox for ecos-bugs@sourceware.org help / color / mirror / Atom feed
From: bugzilla-daemon@bugs.ecos.sourceware.org To: unassigned@bugs.ecos.sourceware.org Subject: [Bug 1001275] Cortex-M (armV7) architecture endian instructions / Applied on lwIP Date: Fri, 19 Aug 2011 08:41:00 -0000 [thread overview] Message-ID: <20110819084047.B0C3C2F78011@mail.ecoscentric.com> (raw) In-Reply-To: <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 #5 from Ilija Kocho <ilijak@siva.com.mk> 2011-08-19 09:40:37 BST --- (In reply to comment #4) > (In reply to comment #3 from IK) > > (In reply to comment #2 from SG) > > > Instead hal_arch_instr.h I would create <arch>_regs.h header file to > > > define a few instructions in assembler. In your case it is > > > cortexm_regs.h. > > > > > > NOTE: There are the precendents of such definitions in other > > > architecture trees. > > > > > > > Right! Actually I was looking for such a header but obviously not hard > > enough. I would have been happier if file name was hal_regs.h, but > > let's not break the habit, and use cortexm_regs.h > > > > > I mean (snippets not tested!) > > > > > > #ifndef CYGONCE_HAL_CORTEXM_REGS_H > > > #define CYGONCE_HAL_CORTEXM_REGS_H > > > //==================================================== ... > > > // > > > // cortexm_regs.h > > > // > > > // Cortex-M CPU definitions > > > // > > > // ... > > > > > > #include <pkgconf/hal.h> > > > > > > #include <cyg/hal/var_regs.h> > > > > > > // Macro to embed rev instructions in C code > > > #define CYGARC_REV(_origin_, _swapped_) \ > > > asm volatile( "rev %0, %1\n" \ > > > : "=r" (_swapped_) \ > > > : "r" (_origin_) \ > > > ); > > > > > > #define CYGARC_REV16(_origin_, _swapped_) \ > > > asm volatile( "rev16 %0, %1\n" \ > > > : "=r" (_swapped_) \ > > > : "r" (_origin_) \ > > > ); > > > > > > // We could also define the functions, now using macros. > > Then my question, Where did you plan to define and implement them? The idea was to have CMSIS like functions http://www.arm.com/products/processors/cortex-m/cortex-microcontroller-software-interface-standard.php in cortexm_regs.h, but I changed my mind so pse forget. > > > // In context with this please look at discussion > > // regarding lwIP below. > > // Regarding functions I have one dillema: > > // Could we rename cygarc_arm_rev*() functions into cygarc_byteswap*()? > > IMHO, as hal_endian.h contains > > #ifndef CYG_SWAP16 > # define CYG_SWAP16(_x_) > ... > > #ifndef CYG_SWAP32 > # define CYG_SWAP32(_x_) > ... > > And you would just define own versions (see below). > Agree. [snip] > > Ilija, Do you really need in C90 inline attribute? Do you really need > extern inlines. AFAIK, extern + inline is almost macro and what is about If I reconsider, not really. These functions are single instruction and compiler should be crazy not to inline. [snip] > > > // Platform supports endian operations. > > #define CYGARC_BYTESWAP 1 > > OK, or may be CYGARC_SWAPXX. > > > #define CYGARC_BYTESWAP32(__val) cygarc_arm_rev(__val) > > #define CYGARC_BYTESWAP16(__val) ((cyg_uint16)cygarc_arm_rev16(__val)) > > I would prefer those short names or may be even CYG_SWAP16, CYG_SWAP32; Personally I would like CYGARC_SWAPxx(), but since CYG_SWAPxx() are already present in hal_endian.h let's use them. It will propagate REV instructions in other parts of code that may benefit (i guess it is safe). > also I would not put the above definitions in cortexm_regs.h. In my > opinion CYGARC_SWAP16, CYGARC_SWAP32 can be placed in hal_arch.h. I am thinking this way too, only I didn't want to do lot's of changes in hal_arch.h. Now, especially if we use CYG_SWAPxx() it shouldn't be a problem. [snip] > Agreed. But, I told about some invented `hal_net.h' thought. Above and > below was for such a header. See below. IMHO this would need further consideration since as it is supposed to propagate through other architectures too. Likewise, I would be happy to have hal_devs.h, (actually var_devs.h) for HAL defines borrowed to devices (pin configurations, clock sources, power management etc). [snip] > > Ilija, IMHO > > 1) <hal>_regs.h is for registers, opcodes, mnemonics, and asm inlines > definitions. So, let's put a few CYGARC_REV* defines there for now. > Agree. > 2) We can define CYGARC_SWAPxx or CYG_SWAPxx macros and put there static > inlines for them in hal_arch.h. Agree. > > I would avoid C90 inline attributes (always_inline) in hal_arch.h as > it is GCC4-ish and I would quote here "one never knows what the next > annoying compiler will demand" :-) As mentioned before it should be safe to omit them, I'll re-confirm during my tests. > [snip] > > 3) If we will define SWAP16/32 in hal_arch.h then lwiopts.h can get > almost yours (just an example) > > #ifdef CYGARC_SWAPXX > # define LWIP_PLATFORM_BYTESWAP 1 > # define LWIP_PLATFORM_HTONS(__val) CYGARC_SWAP16(__val) > # define LWIP_PLATFORM_HTONL(__val) CYGARC_SWAP32(__val) > #endif Clear. > > Well, it is my view only. I hope that I did not miss something. Thanks again for considerations. At present I am away, but today I return home and I am going to test during weekend. FAOD, unless you object I intend to use CYG_SWAPxx() naming. Ilija -- Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the assignee for the bug.
next prev parent reply other threads:[~2011-08-19 8:41 UTC|newest] Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top 2011-07-03 17:46 [Bug 1001275] New: Cortex-M (armV7) architecture endian instructions / lwIP bugzilla-daemon 2011-07-03 17:52 ` [Bug 1001275] " bugzilla-daemon 2011-07-03 17:56 ` [Bug 1001275] Cortex-M (armV7) architecture endian instructions / Applied on lwIP bugzilla-daemon 2011-08-15 20:18 ` bugzilla-daemon 2011-08-17 20:26 ` bugzilla-daemon 2011-08-18 21:14 ` bugzilla-daemon 2011-08-19 8:41 ` bugzilla-daemon [this message] 2011-08-19 15:10 ` bugzilla-daemon 2011-08-19 15:18 ` bugzilla-daemon 2011-08-21 13:46 ` bugzilla-daemon 2011-08-22 12:10 ` bugzilla-daemon 2011-08-22 13:01 ` bugzilla-daemon 2011-08-22 13:23 ` bugzilla-daemon 2011-08-22 13:36 ` bugzilla-daemon 2011-08-22 14:24 ` bugzilla-daemon 2011-08-22 18:26 ` bugzilla-daemon 2011-08-23 13:32 ` bugzilla-daemon 2011-08-23 14:18 ` bugzilla-daemon 2011-08-23 16:14 ` bugzilla-daemon 2011-08-23 16:17 ` bugzilla-daemon 2011-08-23 16:19 ` bugzilla-daemon 2011-08-23 16:21 ` bugzilla-daemon 2011-08-23 19:28 ` bugzilla-daemon 2011-08-23 20:15 ` bugzilla-daemon 2011-08-23 20:47 ` bugzilla-daemon 2011-08-24 6:51 ` bugzilla-daemon 2011-08-24 7:08 ` bugzilla-daemon 2011-08-24 13:34 ` bugzilla-daemon 2011-08-24 13:47 ` bugzilla-daemon 2011-08-24 16:43 ` bugzilla-daemon 2011-08-24 17:01 ` bugzilla-daemon 2011-08-24 17:56 ` bugzilla-daemon 2011-08-25 8:02 ` bugzilla-daemon 2011-08-25 8:20 ` bugzilla-daemon 2011-08-25 14:03 ` bugzilla-daemon
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20110819084047.B0C3C2F78011@mail.ecoscentric.com \ --to=bugzilla-daemon@bugs.ecos.sourceware.org \ --cc=unassigned@bugs.ecos.sourceware.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).