This is the mail archive of the ecos-bugs@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 1001456] HAL misses Interrupt Clear-Pending Registers handling:wasted processing power


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

--- Comment #19 from Bernard Fouchà <bernard.fouche@kuantic.com> 2012-02-23 10:29:10 GMT ---
(In reply to comment #18)
> (In reply to comment #17)
> > If you have at hand "The definitive guide to the ARM Cortex-M3" by Joseph Yiu:
> > see chapter 7, "Interrupt Inputs and Pending Behavior" section,  especially
> > figure 7.13, this is exactly what I observed.
> 
> I guess that in context of this bug you probably mean fig 7.14. (i guess the
> number at the bottom of fig.).

Well on my edition there isn't any figure 7.14 ;-) (I think I have the first
edition)

> 
> For solution I would return to cyg_drv_interrupt_acknowledge() since that's
> what it's supposed to do: "Perform any processing required at the interrupt
> controller and in the CPU to cancel the current interrupt request on the
> vector. An ISR may also need to program the hardware of the device to prevent
> an immediate re-triggering of the interrupt."

Our problem isn't usually in the ISR (unless it does all interrupt handling)
and the feature concerned is different: we don't need to cancel "the current
interrupt request" but to manage possible pending requests: we have to manage
interrupt requests THAT ARE NOT THE CURRENT INTERRUPT REQUEST, exactly what
cyg_drv_interrupt_acknowledge() does not!

> Only, it should be called later than it is called now. I acknowledge your
> concern about generic serial driver but it has some conditional code already so
> adding little-bit more wouldn't harm (or IMO the impact will be lower than
> adding functions on Kernel level).

If we take the serial driver as an example, cyg_drv_interrupt_acknowledge() is
called at the end of the ISR while interrupt pending conditions should be
processed at the beginning or in the middle of the DSR (a strange place to
acknowledge an interrupt , if we consider the driver code being read by someone
unaware of the Cortex-M NVIC).

You'll end up with multiple calls to cyg_drv_interrupt_acknowledge() in the
code, bordered by conditional compilation specific to Cortex-M cores.

And then you have all other 'Prime-cell' related drivers, shared between arch
(CAN, ADC, SPI for the driver I ported/checked) so the mysterious calls to
cyg_drv_interrupt_acknowledge() in the middle or at the beginning of DSR will
start to sprout in different places and if one looks at the original definition
of cyg_drv_interrupt_acknowledge() you shown then it's really impossible to
grab the reason why such calls have to be inserted here and there.

Or you change the definition of cyg_drv_interrupt_acknowledge(), describing it
as something that has a different meaning according to the type of arch
involved while at the same time all cyg_drv_*() functions are arch independent.

I understand that changing a decade old API isn't a good news but I'm not sure
that making the API difficult to understand because of a change of meaning of a
well known function is a better solution. Cortex-M brings a new interrupt
handling model, maybe it's time to update the API spec also, instead of keeping
it at any cost.

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