[Bug 1001184] Cortex-m arch: hal_intr.h includes var_intr.h too early.

[Bug 1001184] Cortex-m arch: hal_intr.h includes var_intr.h too early.

[bugzilla-daemon]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ecos-patches@ecos.sourcewar
                   |                            |e.org
          Component|HAL                         |Patches and contributions

--- Comment #1 from Ilija Kocho <ilijak@siva.com.mk> 2011-04-03 10:13:46 BST ---
In context of relation to Bug 1001187 Component changed to "Patches and
Contributions"

Ilija

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

[Bug 1001184] Cortex-m arch: hal_intr.h includes var_intr.h too early.

[bugzilla-daemon]

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

--- Comment #2 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2011-04-03 22:03:25 BST ---
(In reply on comment #0)
> Created an attachment (id=1195)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1195)
> Include var_intr.h after definition of hal_intr.h macros but allows
> overriding of CYGNUM_HAL_INTERRUPT_RTC.
> 
> In Cortex-M architecture, the header hal_intr.h includes var_intr.h
> twice.  Second one is of course with no effect, but the first one is
> too early.  Namely some macros that are defined in hal_intr.h, such as
> CYGNUM_HAL_INTERRUPT_EXTERNAL, are used in hal-intr.h. This doesn't
> seem to show ill effect so far since usage is by other macros.
> 
> Note: The early inclusion of var_intr.h has been introduced by Bug
> 1001090 Attachment 1092 in order to enable overriding of
> CYGNUM_HAL_INTERRUPT_RTC by variant/platform.
> 
> The proposed patch includes var_intr.h after definition of hal_intr.h
> macros but also allows overriding of CYGNUM_HAL_INTERRUPT_RTC.

Thanks for catch. On my look CYGNUM_HAL_INTERRUPT_RTC definition became
misplaced lines via that request (no header, in fact). So, I would shift
down the definition and point on this on CL entry. Certainly, the first
inclusion of the header can be removed safely then.

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

[Bug 1001184] Cortex-m arch: hal_intr.h includes var_intr.h too early.

[bugzilla-daemon]

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

--- Comment #3 from Ilija Kocho <ilijak@siva.com.mk> 2011-04-03 23:37:49 BST ---
(In reply to comment #2)
> (In reply on comment #0)
> > Created an attachment (id=1195)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1195) [details]
> > Include var_intr.h after definition of hal_intr.h macros but allows
> > overriding of CYGNUM_HAL_INTERRUPT_RTC.
> > 
> > In Cortex-M architecture, the header hal_intr.h includes var_intr.h
> > twice.  Second one is of course with no effect, but the first one is
> > too early.  Namely some macros that are defined in hal_intr.h, such as
> > CYGNUM_HAL_INTERRUPT_EXTERNAL, are used in hal-intr.h. This doesn't
> > seem to show ill effect so far since usage is by other macros.
> > 
> > Note: The early inclusion of var_intr.h has been introduced by Bug
> > 1001090 Attachment 1092 [details] in order to enable overriding of
> > CYGNUM_HAL_INTERRUPT_RTC by variant/platform.
> > 
> > The proposed patch includes var_intr.h after definition of hal_intr.h
> > macros but also allows overriding of CYGNUM_HAL_INTERRUPT_RTC.
> 
> Thanks for catch. On my look CYGNUM_HAL_INTERRUPT_RTC definition became
> misplaced lines via that request (no header, in fact). So, I would shift
> down the definition and point on this on CL entry. Certainly, the first
> inclusion of the header can be removed safely then.

Thanks for the reply Sergei. As far as I understand you are going to revert
hal_intr.h to state before Attachment 1092 and in addition shift
CYGNUM_HAL_INTERRUPT_RTC definition after inclusion of var_intr.h. Am I right?
It would fit, since var_intr.h shall be included after definition of Cortex-M
system vectors and before CYGNUM_HAL_INTERRUPT_RTC.

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

[Bug 1001184] Cortex-m arch: hal_intr.h includes var_intr.h too early.

[bugzilla-daemon]

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

--- Comment #4 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2011-04-04 06:53:45 BST ---
(In reply to comment #3)
> Thanks for the reply Sergei. As far as I understand you are going to
> revert hal_intr.h to state before Attachment 1092 and in addition
> shift CYGNUM_HAL_INTERRUPT_RTC definition after inclusion of
> var_intr.h. Am I right?  It would fit, since var_intr.h shall be
> included after definition of Cortex-M system vectors and before
> CYGNUM_HAL_INTERRUPT_RTC.

Right. Also, please, fix CL entry, as there is unsubjected mention of
CYGNUM_HAL_INTERRUPT_EXTERNAL.

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

[Bug 1001184] Cortex-m arch: hal_intr.h includes var_intr.h too early.

[bugzilla-daemon]

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

--- Comment #5 from Ilija Kocho <ilijak@siva.com.mk> 2011-04-04 11:09:58 BST ---
(In reply to comment #4)
> (In reply to comment #3)
> > Thanks for the reply Sergei. As far as I understand you are going to
> > revert hal_intr.h to state before Attachment 1092 [details] and in addition
> > shift CYGNUM_HAL_INTERRUPT_RTC definition after inclusion of
> > var_intr.h. Am I right?  It would fit, since var_intr.h shall be
> > included after definition of Cortex-M system vectors and before
> > CYGNUM_HAL_INTERRUPT_RTC.
> 
> Right. Also, please, fix CL entry, as there is unsubjected mention of
> CYGNUM_HAL_INTERRUPT_EXTERNAL.
> 

Sergei, Sorry but I do not quite understand what to do? What is the acronym CL?

Ilija

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

[Bug 1001184] Cortex-m arch: hal_intr.h includes var_intr.h too early.

[bugzilla-daemon]

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

--- Comment #6 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2011-04-04 11:23:52 BST ---
(In reply to comment #5)
> Sergei, Sorry but I do not quite understand what to do? What is the acronym
> CL?

Ah, Ilija, I'm sorry CL = ChangeLog. I meaned and to fix your ChangeLog's
record.

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.

[Bug 1001184] Cortex-m arch: hal_intr.h includes var_intr.h too early.

[bugzilla-daemon]

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

--- Comment #7 from Ilija Kocho <ilijak@siva.com.mk> 2011-04-04 13:24:08 BST ---
(In reply to comment #6)
> (In reply to comment #5)
> > Sergei, Sorry but I do not quite understand what to do? What is the acronym
> > CL?
> 
> Ah, Ilija, I'm sorry CL = ChangeLog. I meaned and to fix your ChangeLog's
> record.
> 
> Sergei

OK - Could it be:
 ... after definition of
- CYGNUM_HAL_INTERRUPT_EXTERNAL
+ Cortex-M architecture common exception vectors
 ...

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

[Bug 1001184] Cortex-m arch: hal_intr.h includes var_intr.h too early.

[bugzilla-daemon]

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

--- Comment #8 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2011-04-04 14:29:12 BST ---
(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > Sergei, Sorry but I do not quite understand what to do? What is the
> > > acronym CL?
> > 
> > Ah, Ilija, I'm sorry CL = ChangeLog. I meaned and to fix your ChangeLog's
> > record.
> > 
> > Sergei
> 
> OK - Could it be:
>  ... after definition of
> - CYGNUM_HAL_INTERRUPT_EXTERNAL
> + Cortex-M architecture common exception vectors

Ilija, I kept in mind something likes the below

* include/hal_intr.h: Place CYGNUM_HAL_INTERRUPT_RTC
cond. definition properly. Remove duplicated inclusion of
var_intr.h header.

Why do we need to talk about the exception vectors? As far as I could
understand CYGNUM_HAL_INTERRUPT_RTC definition is misplaced and there
is dup. header inclusion in hal_intr.h. So, you fix it and should add
record *about* it. Did I miss something?

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

[Bug 1001184] Cortex-m arch: hal_intr.h includes var_intr.h too early.

[bugzilla-daemon]

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

--- Comment #9 from Ilija Kocho <ilijak@siva.com.mk> 2011-04-04 21:50:03 BST ---
(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > (In reply to comment #5)
> > > > Sergei, Sorry but I do not quite understand what to do? What is the
> > > > acronym CL?
> > > 
> > > Ah, Ilija, I'm sorry CL = ChangeLog. I meaned and to fix your ChangeLog's
> > > record.
> > > 
> > > Sergei
> > 
> > OK - Could it be:
> >  ... after definition of
> > - CYGNUM_HAL_INTERRUPT_EXTERNAL
> > + Cortex-M architecture common exception vectors
> 
> Ilija, I kept in mind something likes the below
> 
> * include/hal_intr.h: Place CYGNUM_HAL_INTERRUPT_RTC
> cond. definition properly. Remove duplicated inclusion of
> var_intr.h header.
> 
> Why do we need to talk about the exception vectors? As far as I could
> understand CYGNUM_HAL_INTERRUPT_RTC definition is misplaced and there
> is dup. header inclusion in hal_intr.h. So, you fix it and should add
> record *about* it. Did I miss something?

Maybe I tried to be too descriptive. If we opt for short record I think the
main point is placing #include var_intr.h at right place so I would restate:

 * include/hal_intr.h: Remove double inclusion of var_intr.h header and include
   it at proper place.

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

[Bug 1001184] Cortex-m arch: hal_intr.h includes var_intr.h too early.

[bugzilla-daemon]

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

--- Comment #10 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2011-04-04 21:56:22 BST ---
(In reply to comment #9)

[snip]

> Maybe I tried to be too descriptive. If we opt for short record I think the
> main point is placing #include var_intr.h at right place so I would restate:
> 
>  * include/hal_intr.h: Remove double inclusion of var_intr.h header and
>  include it at proper place.

This CL entry looks great for me. Can you re-post the patch, please?

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

[Bug 1001184] Cortex-m arch: hal_intr.h includes var_intr.h too early.

[bugzilla-daemon]

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

--- Comment #11 from Ilija Kocho <ilijak@siva.com.mk> 2011-04-04 22:27:43 BST ---
Created an attachment (id=1202)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1202)
Remove double inclusion of var_intr.h - include at proper place.

(In reply to comment #10)
> (In reply to comment #9)
> 
> [snip]
> 
> > Maybe I tried to be too descriptive. If we opt for short record I think the
> > main point is placing #include var_intr.h at right place so I would restate:
> > 
> >  * include/hal_intr.h: Remove double inclusion of var_intr.h header and
> >  include it at proper place.
> 
> This CL entry looks great for me. Can you re-post the patch, please?

Here it is.

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

[Bug 1001184] Cortex-m arch: hal_intr.h includes var_intr.h too early.

[bugzilla-daemon]

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

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

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

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

[Bug 1001184] Cortex-m arch: hal_intr.h includes var_intr.h too early.

[bugzilla-daemon]

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

--- Comment #12 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2011-04-05 17:18:04 BST ---
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > 
> > [snip]
> > 
> > > Maybe I tried to be too descriptive. If we opt for short record I think
> > > the main point is placing #include var_intr.h at right place so I would
> > > restate:
> > > 
> > >  * include/hal_intr.h: Remove double inclusion of var_intr.h header and
> > >  include it at proper place.
> > 
> > This CL entry looks great for me. Can you re-post the patch, please?
> 
> Here it is.

Now checked-in. Thank you for contribution.

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.

[Bug 1001184] Cortex-m arch: hal_intr.h includes var_intr.h too early.

[bugzilla-daemon]

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

Sergei Gavrikov <sergei.gavrikov@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
            Version|3.0                         |CVS
         Resolution|                            |CURRENTRELEASE

--- Comment #13 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2011-04-05 17:48:28 BST ---
Set status to RESOLVED

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

[Bug 1001184] Cortex-m arch: hal_intr.h includes var_intr.h too early.

[bugzilla-daemon]

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

--- Comment #14 from Ilija Kocho <ilijak@siva.com.mk> 2011-04-05 18:02:29 BST ---
(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > (In reply to comment #9)
> > > 
> > > [snip]
> > > 
> > > > Maybe I tried to be too descriptive. If we opt for short record I think
> > > > the main point is placing #include var_intr.h at right place so I would
> > > > restate:
> > > > 
> > > >  * include/hal_intr.h: Remove double inclusion of var_intr.h header and
> > > >  include it at proper place.
> > > 
> > > This CL entry looks great for me. Can you re-post the patch, please?
> > 
> > Here it is.
> 
> Now checked-in. Thank you for contribution.

Many thanks Sergei.
Ilija

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