This is the mail archive of the
ecos-patches@sourceware.org
mailing list for the eCos project.
Re: Fixed some bugs in hal_io.h
- From: Jonathan Larmour <jifl at eCosCentric dot com>
- To: dfernandez at cct dot co dot uk
- Cc: ecos-patches at ecos dot sourceware dot org
- Date: Tue, 19 Dec 2006 02:55:55 +0000
- Subject: Re: Fixed some bugs in hal_io.h
- References: <1166434539.23934.3.camel@software.cct.co.uk>
David Fernandez wrote:
Hi there,
This fixes some bugs in hal_io.h regarding the STRING macros that were
lacking the "rep" prefix.
Those changes are fair enough, good catch.
It also adds memory string macros,
I'm a bit hesitant about these, as they aren't part of the HAL API, which
could be detrimental to code reuse. But I'll let them pass as they're
entirely novel and no-one can fail to notice if their HAL doesn't have them.
and does a
slight reformat to keep everything with 80 columns.
Ok.
But those aren't the only changes:
#define HAL_READ_UINT8( _register_, _value_ ) \
-CYG_MACRO_START \
-{ \
- asm volatile ( "xor %%eax,%%eax ;" \
- "inb %%dx, %%al" \
+({ \
+ asm volatile( \
+ " xor %%eax,%%eax \n" \
+ " inb %%dx, %%al \n" \
: "=a" (_value_) \
: "d"(_register_) \
); \
-} \
-CYG_MACRO_END
+ (_value_); \
+})
You make this an expression which can be an rvalue. This is outside the
HAL spec, and would mean that code that might initially seem usable by
non-i386 packages, would not be. It does not really add anything. I would
be against this change and the others like it.
#define HAL_READ_UINT8_STRING( _register_, _buf_, _count_) \
CYG_MACRO_START \
- asm volatile ( "insb" \
- : \
- : "c" (_count_), "d"(_register_), "D"(_buf_) \
+ typedef volatile struct { CYG_BYTE m[_count_]; } *mp; \
+ asm volatile( \
+ " rep insb \n" \
+ : "=m" (*(mp) (_buf_)) \
+ : "c" (_count_), \
+ "d" (_register_), \
+ "D" (_buf_) \
); \
CYG_MACRO_END
Here you have added an output constraint for the _buf_ array. Won't this
make GCC allocate an extra unused register? I'm not sure what it is
intended to achieve either. Similarly in other _STRING functions.
A ChangeLog entry would also be appreciated, thanks.
Jifl
--
eCosCentric http://www.eCosCentric.com/ The eCos and RedBoot experts
------["The best things in life aren't things."]------ Opinions==mine