[Bug 1001456] HAL misses Interrupt Clear-Pending Registers handling: wasted processing power

[bugzilla-daemon@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=1001456

--- Comment #16 from Bernard Fouché <bernard.fouche@kuantic.com> 2012-02-16 16:45:35 GMT ---
(In reply to comment #15)
> (In reply to comment #14)
> > (In reply to comment #13)
> > > Changes to the Kernel should be done with caution. I think that this addition
> > > is not necessary. The desired functionality, for a given platform, can be
> > > introduced by #defining HAL_VAR_INTERRUPT_ACKNOWLEDGE (and/or its cousins).
> > 
> > There are two problems:
> > 
> > - clearing the pending interrupt bit is completely different from acknowledging
> > an interrupt. Acknowledgment is usually done at the end of DSR/ISR while
> > clearing the pending interrupt bit must be done *before* reading the peripheral
> > registers describing interrupt source(s): HAL_VAR_INTERRUPT_ACKNOWLEDGE is not
> > a solution.
> > 
> > - if HAL_VAR_INTERRUPT_CLEAR_PENDING is added only to Cortex-M targets, then
> > how can one share a driver between an arch not requiring clearing pending
> > interrupt and Cortex-M cores? For instance the generic serial 16x5x driver?
> > Today cyg_drv_interrupt_acknowledge() resolves to CYG_EMPTY_STATEMENT for
> > Cortex-M, IMHO we need cyg_drv_interrupt_clear_pending() to resolve to
> > CYG_EMPTY_STATEMENT for non-Cortex-M, hence one can write a driver using both
> > cyg_drv_interrupt_acknowledge() and cyg_drv_interrupt_clear_pending() at the
> > correct place so the driver can work in different arch.
> 
> Normally acknowledge should be enough. Regarding the IRQ re-triggering you have
> found out, maybe (some of) LPC17xx peripherals employ pulsed rather than level
> interrupts. Here is what's Cortex-M NVIC doc saying about it.
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0552a/Bhchgeei.html

I've read this, did you look at the section just afterwards ('NVIC usage hints
and tips') that states: "A interrupt can enter pending state even if it is
disabled. Disabling an interrupt only prevents the processor from taking that
interrupt.": there is no difference between pulse and level interrupts for
pending interrupt management.

Since eCos disable interrupts when using cyg_interrupt_mask() my guess is the
issue should show up on all Cortex-M cores having the ICPR0/1 registers (all?).

Most if not all drivers I've seen do:

1) read interrupt source(s)
2) work
3) acknowledge and unmask interrupt, exit

If there are multiple memory buffers (CAN), many interrupt conditions described
in a single register (ser16x5x,CAN,SPI), and/or a FIFO (ser16x5x,SPI) to
process then it's:

1) read interrupt source(s)
2) work
3) goto step 1 if there is still at least an interrupt condition
4) acknowledge and unmask interrupt, exit

The issue appears more clearly with this second type of drivers, they must be
changed to:

0) clear pending interrupts
1) read interrupt source(s)
2) work
3) goto step *0* if there is still at least an interrupt condition
4) acknowledge and unmask interrupt

You see that it's not possible to rewrite cyg_drv_interrupt_acknowledge() to
solve the problem: the need is different because it occurs at a different time.
Or you change the semantics of cyg_drv_interrupt_acknowledge() and then you
break driver re-use.

Doing:

1) read interrupt source(s)
2) work
3) goto step 1 if there is still at least an interrupt condition
4) clear pending interrupt, acknowledge and unmask interrupt

won't work: an interrupt could occur between steps 3 and 4 and you would lose
it, that's why clearing pending ints must be done at the beginning of the
processing.

> 
> Then, if we must implement HAL_VAR_INTERRUPT_CLEAR_PENDING I would suggest to
> do it on LPC17xx variant or, if the problem appears on other variants consider
> Cortex-M architecture level, and avoid patching kernel files. FYI as i
> mentioned earlier I'll do some tests on Kinetis during next week.

If there is a way to do that and keep driver code re-use between arch that's
would be fine!

When you perform your tests, please try clocking SLOWLY (no PLL for instance):
if the ISR/DSR sequence is faster than the time it takes to the MCU to generate
a new interrupt, then there won't be any pending interrupt and the dust will
stay hidden under the carpet and re-appear only when many different drivers
trigger more important amounts of interrupts. (my guess is this is what
happened to have this matter appearing only now, or if nobody counts the
numbers of ISR/DSR/DSR-that does-nothing/volume-of-data-handled-per-DSR-run
when testing drivers)

-- 
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.
>From ecos-bugs-return-8896-listarch-ecos-bugs=sources.redhat.com@sourceware.org Thu Feb 16 16:46:42 2012
Return-Path: <ecos-bugs-return-8896-listarch-ecos-bugs=sources.redhat.com@sourceware.org>
Delivered-To: listarch-ecos-bugs@sources.redhat.com
Received: (qmail 10122 invoked by alias); 16 Feb 2012 16:46:35 -0000
Received: (qmail 10100 invoked by uid 22791); 16 Feb 2012 16:46:32 -0000
X-SWARE-Spam-Status: No, hits=-1.9 required=5.0
	tests=AWL,BAYES_00,T_RP_MATCHES_RCVD
X-Spam-Check-By: sourceware.org
Received: from hagrid.ecoscentric.com (HELO mail.ecoscentric.com) (212.13.207.197)
    by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 16 Feb 2012 16:45:48 +0000
Received: by mail.ecoscentric.com (Postfix, from userid 48)
	id B92172F78009; Thu, 16 Feb 2012 16:45:46 +0000 (GMT)
X-Original-To: unassigned@bugs.ecos.sourceware.org
Delivered-To: unassigned@bugs.ecos.sourceware.org
From: bugzilla-daemon@bugs.ecos.sourceware.org
To: unassigned@bugs.ecos.sourceware.org
Subject: [Bug 1001456] HAL misses Interrupt Clear-Pending Registers handling:
 wasted processing power
X-Bugzilla-Reason: AssignedTo
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: eCos
X-Bugzilla-Component: HAL
X-Bugzilla-Keywords:
X-Bugzilla-Severity: major
X-Bugzilla-Who: bernard.fouche@kuantic.com
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Priority: low
X-Bugzilla-Assigned-To: unassigned@bugs.ecos.sourceware.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Changed-Fields:
In-Reply-To: <bug-1001456-777@http.bugs.ecos.sourceware.org/>
References: <bug-1001456-777@http.bugs.ecos.sourceware.org/>
X-Bugzilla-URL: http://bugs.ecos.sourceware.org/
Auto-Submitted: auto-generated
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Date: Thu, 16 Feb 2012 16:46:00 -0000
Message-Id: <20120216164539.0692C2F78005@mail.ecoscentric.com>
Mailing-List: contact ecos-bugs-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <ecos-bugs.sourceware.org>
List-Subscribe: <mailto:ecos-bugs-subscribe@sourceware.org>
List-Post: <mailto:ecos-bugs@sourceware.org>
List-Help: <mailto:ecos-bugs-help@sourceware.org>, <http://sourceware.org/lists.html#faqs>
Sender: ecos-bugs-owner@sourceware.org
Delivered-To: mailing list ecos-bugs@sourceware.org
X-SW-Source: 2012/txt/msg00325.txt.bz2
Content-length: 4850

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

--- Comment #16 from Bernard Fouché <bernard.fouche@kuantic.com> 2012-02-16 16:45:35 GMT ---
(In reply to comment #15)
> (In reply to comment #14)
> > (In reply to comment #13)
> > > Changes to the Kernel should be done with caution. I think that this addition
> > > is not necessary. The desired functionality, for a given platform, can be
> > > introduced by #defining HAL_VAR_INTERRUPT_ACKNOWLEDGE (and/or its cousins).
> > 
> > There are two problems:
> > 
> > - clearing the pending interrupt bit is completely different from acknowledging
> > an interrupt. Acknowledgment is usually done at the end of DSR/ISR while
> > clearing the pending interrupt bit must be done *before* reading the peripheral
> > registers describing interrupt source(s): HAL_VAR_INTERRUPT_ACKNOWLEDGE is not
> > a solution.
> > 
> > - if HAL_VAR_INTERRUPT_CLEAR_PENDING is added only to Cortex-M targets, then
> > how can one share a driver between an arch not requiring clearing pending
> > interrupt and Cortex-M cores? For instance the generic serial 16x5x driver?
> > Today cyg_drv_interrupt_acknowledge() resolves to CYG_EMPTY_STATEMENT for
> > Cortex-M, IMHO we need cyg_drv_interrupt_clear_pending() to resolve to
> > CYG_EMPTY_STATEMENT for non-Cortex-M, hence one can write a driver using both
> > cyg_drv_interrupt_acknowledge() and cyg_drv_interrupt_clear_pending() at the
> > correct place so the driver can work in different arch.
> 
> Normally acknowledge should be enough. Regarding the IRQ re-triggering you have
> found out, maybe (some of) LPC17xx peripherals employ pulsed rather than level
> interrupts. Here is what's Cortex-M NVIC doc saying about it.
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0552a/Bhchgeei.html

I've read this, did you look at the section just afterwards ('NVIC usage hints
and tips') that states: "A interrupt can enter pending state even if it is
disabled. Disabling an interrupt only prevents the processor from taking that
interrupt.": there is no difference between pulse and level interrupts for
pending interrupt management.

Since eCos disable interrupts when using cyg_interrupt_mask() my guess is the
issue should show up on all Cortex-M cores having the ICPR0/1 registers (all?).

Most if not all drivers I've seen do:

1) read interrupt source(s)
2) work
3) acknowledge and unmask interrupt, exit

If there are multiple memory buffers (CAN), many interrupt conditions described
in a single register (ser16x5x,CAN,SPI), and/or a FIFO (ser16x5x,SPI) to
process then it's:

1) read interrupt source(s)
2) work
3) goto step 1 if there is still at least an interrupt condition
4) acknowledge and unmask interrupt, exit

The issue appears more clearly with this second type of drivers, they must be
changed to:

0) clear pending interrupts
1) read interrupt source(s)
2) work
3) goto step *0* if there is still at least an interrupt condition
4) acknowledge and unmask interrupt

You see that it's not possible to rewrite cyg_drv_interrupt_acknowledge() to
solve the problem: the need is different because it occurs at a different time.
Or you change the semantics of cyg_drv_interrupt_acknowledge() and then you
break driver re-use.

Doing:

1) read interrupt source(s)
2) work
3) goto step 1 if there is still at least an interrupt condition
4) clear pending interrupt, acknowledge and unmask interrupt

won't work: an interrupt could occur between steps 3 and 4 and you would lose
it, that's why clearing pending ints must be done at the beginning of the
processing.

> 
> Then, if we must implement HAL_VAR_INTERRUPT_CLEAR_PENDING I would suggest to
> do it on LPC17xx variant or, if the problem appears on other variants consider
> Cortex-M architecture level, and avoid patching kernel files. FYI as i
> mentioned earlier I'll do some tests on Kinetis during next week.

If there is a way to do that and keep driver code re-use between arch that's
would be fine!

When you perform your tests, please try clocking SLOWLY (no PLL for instance):
if the ISR/DSR sequence is faster than the time it takes to the MCU to generate
a new interrupt, then there won't be any pending interrupt and the dust will
stay hidden under the carpet and re-appear only when many different drivers
trigger more important amounts of interrupts. (my guess is this is what
happened to have this matter appearing only now, or if nobody counts the
numbers of ISR/DSR/DSR-that does-nothing/volume-of-data-handled-per-DSR-run
when testing drivers)

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
>From ecos-bugs-return-8898-listarch-ecos-bugs=sources.redhat.com@sourceware.org Thu Feb 16 16:49:39 2012
Return-Path: <ecos-bugs-return-8898-listarch-ecos-bugs=sources.redhat.com@sourceware.org>
Delivered-To: listarch-ecos-bugs@sources.redhat.com
Received: (qmail 12888 invoked by alias); 16 Feb 2012 16:49:37 -0000
Received: (qmail 12786 invoked by uid 22791); 16 Feb 2012 16:49:35 -0000
X-SWARE-Spam-Status: No, hits=-1.9 required=5.0
	tests=AWL,BAYES_00,T_RP_MATCHES_RCVD
X-Spam-Check-By: sourceware.org
Received: from hagrid.ecoscentric.com (HELO mail.ecoscentric.com) (212.13.207.197)
    by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 16 Feb 2012 16:48:55 +0000
Received: by mail.ecoscentric.com (Postfix, from userid 48)
	id F23BC2F78005; Thu, 16 Feb 2012 16:48:53 +0000 (GMT)
X-Original-To: unassigned@bugs.ecos.sourceware.org
Delivered-To: unassigned@bugs.ecos.sourceware.org
From: bugzilla-daemon@bugs.ecos.sourceware.org
To: unassigned@bugs.ecos.sourceware.org
Subject: [Bug 1001466] /dev/null serial driver
X-Bugzilla-Reason: AssignedTo
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: eCos
X-Bugzilla-Component: Patches and contributions
X-Bugzilla-Keywords:
X-Bugzilla-Severity: enhancement
X-Bugzilla-Who: bernard.fouche@kuantic.com
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Priority: low
X-Bugzilla-Assigned-To: unassigned@bugs.ecos.sourceware.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Changed-Fields:
In-Reply-To: <bug-1001466-777@http.bugs.ecos.sourceware.org/>
References: <bug-1001466-777@http.bugs.ecos.sourceware.org/>
X-Bugzilla-URL: http://bugs.ecos.sourceware.org/
Auto-Submitted: auto-generated
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Date: Thu, 16 Feb 2012 16:49:00 -0000
Message-Id: <20120216164842.E17952F78008@mail.ecoscentric.com>
Mailing-List: contact ecos-bugs-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <ecos-bugs.sourceware.org>
List-Subscribe: <mailto:ecos-bugs-subscribe@sourceware.org>
List-Post: <mailto:ecos-bugs@sourceware.org>
List-Help: <mailto:ecos-bugs-help@sourceware.org>, <http://sourceware.org/lists.html#faqs>
Sender: ecos-bugs-owner@sourceware.org
Delivered-To: mailing list ecos-bugs@sourceware.org
X-SW-Source: 2012/txt/msg00327.txt.bz2
Content-length: 901

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

--- Comment #12 from Bernard Fouché <bernard.fouche@kuantic.com> 2012-02-16 16:48:39 GMT ---
(In reply to comment #11)
> (In reply to comment #10)
> 
> [snip]
> 
> > > And what you get?
> > 
> > Excellent! Just need some documentation and then /dev/null can go to
> > itself ;-)
> 
> So, let's 
> 
>   Bug 1001466 > /dev/null
> 
> :-)

Please change bug's state only when there is some doc about the trick you used,
and that doc uses the term '/dev/null': this will avoid losing time to some
other users if they are looking for a solution to the absence of /dev/null in
eCos.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
>From ecos-bugs-return-8899-listarch-ecos-bugs=sources.redhat.com@sourceware.org Thu Feb 16 17:22:36 2012
Return-Path: <ecos-bugs-return-8899-listarch-ecos-bugs=sources.redhat.com@sourceware.org>
Delivered-To: listarch-ecos-bugs@sources.redhat.com
Received: (qmail 27000 invoked by alias); 16 Feb 2012 17:22:33 -0000
Received: (qmail 26990 invoked by uid 22791); 16 Feb 2012 17:22:31 -0000
X-SWARE-Spam-Status: No, hits=-1.9 required=5.0
	tests=AWL,BAYES_00,T_RP_MATCHES_RCVD
X-Spam-Check-By: sourceware.org
Received: from hagrid.ecoscentric.com (HELO mail.ecoscentric.com) (212.13.207.197)
    by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 16 Feb 2012 17:21:15 +0000
Received: by mail.ecoscentric.com (Postfix, from userid 48)
	id C06E42F78008; Thu, 16 Feb 2012 17:21:14 +0000 (GMT)
X-Original-To: unassigned@bugs.ecos.sourceware.org
Delivered-To: unassigned@bugs.ecos.sourceware.org
From: bugzilla-daemon@bugs.ecos.sourceware.org
To: unassigned@bugs.ecos.sourceware.org
Subject: [Bug 1001466] /dev/null serial driver
X-Bugzilla-Reason: AssignedTo
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: eCos
X-Bugzilla-Component: Patches and contributions
X-Bugzilla-Keywords:
X-Bugzilla-Severity: enhancement
X-Bugzilla-Who: sergei.gavrikov@gmail.com
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Priority: low
X-Bugzilla-Assigned-To: unassigned@bugs.ecos.sourceware.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Changed-Fields:
In-Reply-To: <bug-1001466-777@http.bugs.ecos.sourceware.org/>
References: <bug-1001466-777@http.bugs.ecos.sourceware.org/>
X-Bugzilla-URL: http://bugs.ecos.sourceware.org/
Auto-Submitted: auto-generated
Content-Type: text/plain; charset="UTF-8"
MIME-Version: 1.0
Date: Thu, 16 Feb 2012 17:22:00 -0000
Message-Id: <20120216172103.CE00F2F78009@mail.ecoscentric.com>
Mailing-List: contact ecos-bugs-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <ecos-bugs.sourceware.org>
List-Subscribe: <mailto:ecos-bugs-subscribe@sourceware.org>
List-Post: <mailto:ecos-bugs@sourceware.org>
List-Help: <mailto:ecos-bugs-help@sourceware.org>, <http://sourceware.org/lists.html#faqs>
Sender: ecos-bugs-owner@sourceware.org
Delivered-To: mailing list ecos-bugs@sourceware.org
X-SW-Source: 2012/txt/msg00328.txt.bz2
Content-length: 1390

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

--- Comment #13 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2012-02-16 17:21:00 GMT ---
(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> >
> > [snip]
> >
> > > > And what you get?
> > >
> > > Excellent! Just need some documentation and then /dev/null can go
> > > to itself ;-)
> >
> > So, let's
> >
> >   Bug 1001466 > /dev/null
      ^^^^^^^^^^^^^^^^^^^^^^^

Excuse my '... > /dev/null' jargon:
http://en.wikipedia.org/wiki//dev/null

> Please change bug's state only when there is some doc about the trick
> you used, and that doc uses the term '/dev/null': this will avoid
> losing time to some other users if they are looking for a solution to
> the absence of /dev/null in eCos.

Good approach. I'll change the state to 'NEED INFO'.

As for my guess and test in GDB. I decided to check that example when I
saw in ecos.ecc:

  cdl_option CYGDAT_LIBC_STDIO_DEFAULT_CONSOLE {
      # Default value:  CYGDAT_IO_SERIAL_TTY_CONSOLE ?
CYGDAT_IO_SERIAL_TTY_CONSOLE : "\"/dev/null\""
  }

Hm, /dev/null? And I was looking for what will happen then.

--
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.