[En-Nut-Discussion] Cortex ix_xxx_interuptY code duplication

[Ole Reinhardt]

Hi Uwe,

> at present nearly each nut/arch/cm3/dev/xxx/ih_xxx_*.c file implements it's
> own xxxIrqCtl(int cmd, void *param) function.  The core of that function is
> always the same, apart from a different xxxIrqEntry and xxx_IRQn
> settings. Not a very maintainable situation.

This is a _very_ good point and I thought about this before.

For the LPC every IRQ handler uses exactly the same code, I think for
the STM it will be the same.

Even the AT91 architecture uses the same code in most cases.


> ...
> Shouldn't we extend above TwoWireIrqCtl() to a IrqCtl() function in
> e.g. arch/cm3/os, replacing all xxxIrqCtl() functtions.

Yes we could easily write a common IrqCtl function, but would have to
pass the signal as additional argument. This would need changes in the
API, but from the user land code this should not be called anyway. So an
API change here should not hurt, right?

> I think the additional redirection in the case of the common IrqCtl()
> doesn't matter. That way, much code duplication could be avoided. 

I think there wouldn't be any further redirection needed as long as we
pass the IRQ_HANDLER instead of a user argument, as the user argument is
stored in the IRQ_HANDLER struct, isn't it?

>Perhaps even one file for all IRQ_HANDLERs.

Yes, I fully agree. We could stay with one file for all interrupts. The
only thing shich would have to be defined is the IRQ_HANDLER struct
instance.

> NUT_IRQCTL_GETMODE/NUT_IRQCTL_SETMODE might need a special consideration, as
> Ulrich marked some interrupt handlers as NUT_IRQMODE_LEVEL versus the
> default NUT_IRQMODE_EDGE everywhere in nxp and mostly in stm32.

AFAIK the CM3 does not even support NUT_IRQMODE_LEVEL as the AT91 does.

On the AT91, the edge driven interrupts might get lost on some interrupt
sources (see the errata) so we will need to either define a default
value for each interrupt (could be placed in the IRQ_HANDLER struct as
well) or better add a IrqCtl(...,
NUT_IRQCTL_GETMODE/NUT_IRQCTL_SETMODE, ...) at each driver right after
IRQ registration where we would need to change the IRQ mode. In my eyes
this is more comprehensive anyway instead of defining different default
values for each driver.

> However I have not seen any consumer of the
> NUT_IRQCTL_GETMODE/NUT_IRQCTL_SETMODE function in our codebase yet. 

On AT91 is is used by several drivers or (if I'm wrong) there are at
least differen default values used. See my idea above.

> GPIO is a
> consumer for sure, but for  NXP Ole just implemented GPIO_SIGNAL redirection,
> handling that case and I will probably reuse for STM.

Yes, I would appreciate we could go the same way there too. From the
users point of view I implement the same API as it is used by AT91 as
well (beside of the fact that AT91 does only support an interrupt
trigger on both edges).

bye,

Ole

-- 

Thermotemp GmbH, Embedded-IT

Embedded Hard-/ Software and Open Source Development, 
Integration and Consulting

http://www.embedded-it.de

Geschäftsstelle Siegen - Steinstraße 67 - D-57072 Siegen - 
tel +49 (0)271 5513597, +49 (0)271-73681 - fax +49 (0)271 736 97

Hauptsitz - Hademarscher Weg 7 - 13503 Berlin
Tel +49 (0)30 4315205 - Fax +49 (0)30 43665002
Geschäftsführer: Jörg Friedrichs, Ole Reinhardt
Handelsregister Berlin Charlottenburg HRB 45978 UstID DE 156329280