This is the mail archive of the ecos-discuss@sources.redhat.com 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]

Re: bug in mips arch code ????


"HG" <henri@broadbandnetdevices.com> writes:

> Hi All
> 
> 1-    interrupt acknowledge
> 
> the macro HAL_INTERRUPT_ACKNOWLEDGE found in /mips/arch/hal_intr.h tries to
> write in a region of the cause register ($13) that is read only ( the ip
> bits 10 to 15) . As per the Sweetman book and the idt mips ref manual ,
> these bits show the interrupts that want to happen. To acknowledge those
> hardware interrupt(s)  , the  condition is cleared by correcting the
> condition causing the interrupt pin to be asserted.
> example : the vector 1 causes an attempted  write to bit 11 of the cause cp0
> register


It is possible that this was how interrupts were acknowledged on some
earlier MIPS devices. I have a vague memory that this was needed on
the TX39. Since this was our first MIPS target, the write to the
cause register probably made its way into the generic code in the
mistaken impression that this was needed for all MIPS CPUs.

> 
> while this does not break anything , what is broken is that the routines
> think they are acknowledging the interrupt but they are not.
> in the case of the timer interrupt , the isr in clock.cxx calls the
> HAL_CLOCK_RESET macro that does a write to the compare register ($11) . As
> per the mips documentation this is what is required to acknowledge the
> interrupt and deassert the interrupt request.  All is well.
>

HAL_INTERRUPT_ACKNOWLEDGE() is only ever intended to cancel the
interrupt in the interrupt controller, if needed. There is always the
need to deal with the interrupting device separately.

> 
> a clean way to fix this is to define the macros found in the comments
> section of the 82559 driver :
> //               Platform code can also define these three:
> //               CYGPRI_DEVS_ETH_INTEL_I82559_MASK_INTERRUPTS(p_i82559,old)
> //
> CYGPRI_DEVS_ETH_INTEL_I82559_UNMASK_INTERRUPTS(p_i82559,old)
> //               CYGPRI_DEVS_ETH_INTEL_I82559_ACK_INTERRUPTS(p_i82559)
> 
> some of the platforms appear to define this,  none of the mips boards do
> however.

It's possible that these macros were added after the MIPS drivers were
written, and those drivers were not updated, since they appear to work
correctly without. Or maybe the authors of the drivers were unaware of
the existence of these macros, or their importance.

> The platforms that do dont appear to follow the datasheets
> recommendation???? .

The i8255x devices don't always obey the datasheets themselves. The
i8255x driver has been worked on a lot to get it to work reliably.

> I will write and test those macros for my platform and
> contribute them to the patch list in the form of source code that the
> maintainers of the specific boards can use to generate patches for the
> specifics of their board.

Please do.

> 
> 
> 2-    Interrupt state control
> 
> the documentation found in the reference manual appears to contradict some
> of the macros in mips/arch/include/hal_intr.h :
> 
> from the documentation
> HAL_DISABLE_INTERRUPTS(old) disables the delivery of interrupts and stores
> the original state of the interrupt mask
> in the variable passed in the old argument.
> 
> from the code :
> the macro saves only the ie bit (global interrupt enable)  but does not save
> the im field (interrupt mask)

>From the point of view of that macro, only the IE bit is of
interest. The IM bits are viewed as part of the interrupt controller,
and should be manipulated by HAL_INTERRUPT_MASK() and
HAL_INTERRUPT_UNMASK() if necessary. For many platforms, it is often
sufficient to just set the IM bits at startup and do all masking via
the external interrupt controller.

Possibly the use of the term "interrupt mask" for the CPU interrupt
enable is confusing. But this will always be the case where different
CPU manufacturers use similar terms for different things.

-- 
Nick Garnett                    eCos Kernel Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts


-- 
Before posting, please read the FAQ: http://sources.redhat.com/fom/ecos
and search the list archive: http://sources.redhat.com/ml/ecos-discuss


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]