[Patch] Removing legacy method from arm csb337

[Sebastian Huber]

On 07/02/2013 01:26 PM, Vipul Nayyar wrote:
> Hello,
>
> Thanks for pointing out the mistakes in the previous patches. Updated the
> csb337 patch, and did away with the use of rtems_irq_connect_data structure in
> the file. Is this what you were suggesting ? Hoping I've followed the
> indentation rules.
>
> commit d24fbcb472b0164ed4e6b51233155df7f6dd57b4
> Author: Vipul Nayyar <nayyar_vipul at yahoo.com>
> Date:   Tue Jul 2 16:52:29 2013 +0530
>
>      Updated Legacy code in arm csb337
>
> diff --git a/c/src/lib/libbsp/arm/csb337/network/network.c
> b/c/src/lib/libbsp/arm/csb337/network/network.c
> index e31bcc0..1c1dd0e 100644
> --- a/c/src/lib/libbsp/arm/csb337/network/network.c
> +++ b/c/src/lib/libbsp/arm/csb337/network/network.c
> @@ -87,17 +87,6 @@ static void at91rm9200_emac_isr_on(const
> rtems_irq_connect_data *unused);
>   static void at91rm9200_emac_isr_off(const rtems_irq_connect_data *unused);
>   static int at91rm9200_emac_isr_is_on(const rtems_irq_connect_data *irq);
> -/* Replace the first value with the clock's interrupt name. */
> -rtems_irq_connect_data at91rm9200_emac_isr_data = {
> -    AT91RM9200_INT_EMAC,
> -    at91rm9200_emac_isr,
> -    NULL,
> -    at91rm9200_emac_isr_on,
> -    at91rm9200_emac_isr_off,
> -    at91rm9200_emac_isr_is_on

Where are the at91rm9200_emac_isr_on(), at91rm9200_emac_isr_off() and 
at91rm9200_emac_isr_is_on() called now?  Did you compile the code after the 
changes?  You should get warnings now about unused static functions.

Are they really all superfluous?

> -};
> -
> -
>   /* use the values defined in linkcmds for our use of SRAM */
>   extern void * at91rm9200_emac_rxbuf_hdrs;
>   extern void * at91rm9200_emac_txbuf;
> @@ -358,6 +347,7 @@ void at91rm9200_emac_init(void *arg)
>   {
>       at91rm9200_emac_softc_t     *sc = arg;
>       struct ifnet *ifp = &sc->arpcom.ac_if;
> +    rtems_status_code status = RTEMS_SUCCESSFUL;
>       /*
>        *This is for stuff that only gets done once (at91rm9200_emac_init()
> @@ -382,7 +372,19 @@ void at91rm9200_emac_init(void *arg)
>       AIC_SMR_REG(AIC_SMR_EMAC) = AIC_SMR_PRIOR(EMAC_INT_PRIORITY);
>       /* install the interrupt handler */
> -  BSP_install_rtems_irq_handler(&at91rm9200_emac_isr_data);
> +    status = rtems_interrupt_handler_install(
> +      AT91RM9200_INT_EMAC,
> +      "Network",
> +      RTEMS_INTERRUPT_UNIQUE,
> +      at91rm9200_emac_isr,
> +      NULL
> +    );
> +    if (status != RTEMS_SUCCESSFUL) {
> +        rtems_fatal(
> +          RTEMS_FATAL_SOURCE_BSP_SPECIFIC,
> +          BSP_GENERIC_FATAL_INTERRUPT_INITIALIZATION
> +        );
> +    }
[...]

As I wrote before, the indentation level is two characters.

The purpose of the fatal errors is

1. terminate the system, and
2. allow the exact identification of the error source in the code.

 From 2. follows that a (source, code) pair must be used a most once in the 
RTEMS source code.

In the documentation we have:

http://www.rtems.org/onlinedocs/doxygen/cpukit/html/group__ScoreIntErr.html#ga878b4de77df7d0b83d19609d4de42c26

[...]
RTEMS_FATAL_SOURCE_BSP_GENERIC 	

Fatal source for generic BSP errors.

The fatal codes are defined in <bsp/generic-fatal.h>. Examples are interrupt 
and exception initialization.

See also:
     bsp_generic_fatal_code and bsp_generic_fatal().

RTEMS_FATAL_SOURCE_BSP_SPECIFIC 	

Fatal source for BSP specific errors.

The fatal code is BSP specific.
[...]

The BSP_GENERIC_FATAL_INTERRUPT_INITIALIZATION is generic BSP error code, so 
don't use it with RTEMS_FATAL_SOURCE_BSP_SPECIFIC.

I would use simply assert().  If there is time, we can change all the assert() 
into BSP specific fatal errors later.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.