Re: intc - rework core code

[Magnus D. <mag...@gm...>]

Hi Markus!

On 8/14/07, EXTERNAL Brunner Markus (Praktikant; ST-FIR/Eng)
<ext...@de...> wrote:
> Hi Magnus,
>
> thanks for the new priority feature.

Hehe, you noticed. =)

> There are 3 things I noticed.
> First:
> The declaration of the new function should probably go to hw_irq.h.

I wanted to keep it semi-secret waiting to tie it in with some generic
code, but I think you are right that a prototype should be put in
hw_irq.h.

> Second:
> Is it forbidden to set the highest possible priority? If not there is a
> bug in set_priority.
>
> if (prio >= ((1 << _INTC_WIDTH(ihp->handle)) - 1))

You are right, we should have a ">" instead of ">=". Thanks.

> Third:
> The code doesn't work for processors without mask registers.

Is that so? I've tried the code on sh7750 among other cpus without any
problems. But I may of course have missed something...

Just to clarify (you probably know this already, but anyway):

The intc code requires at least one way to mask an interrupt. So intc
can for instance not be used in the cpu specific code for certain IRL
vectors on sh processors that don't provide any mask registers. For
sh7705 that means that the cpu specific code can use intc in IRQ mode
but not for IRL mode. So with IRL mode you need board specific
interrupt code that masks interrupts in some external register (FPGA
maybe) that handles the IRL pins. And that board specific code can of
course use intc... A good example where intc can be used in IRL mode
in the cpu specific code is sh7785 which provides bitmap support for
both IRQ mode and IRL.

> If the processor only has priority registers the "handle" will not be
> set in intc_register_irq (because of "if(!primary)")
> This leads to wrong results from _INTC_WIDTH, which causes
> intc_set_priority fail with EINVAL

You cannot set priorities for interrupt source with priority as
primary masking method, right? It sure looks like a bug. Thank you for
spending time on tracking this down.

> When I comment out the "if(!primary)" condition all priorities will be
> set immediatelly, which also enables them.
> When I hardcode the _INTC_WIDTH to the correct size in set_priority, the
> priority will be set as soon as the irq is requested.

Right. The correct fix is probably to setup the handle properly but
use a _INTC_FN(h) set to REG_FN_ERR to indicate that there is no point
in writing the priority value to hardware.

I'll post fixes in a little while. Thank you!

/ magnus