[PATCH] Updated Legacy code in arm csb336

[Vipul Nayyar]

Hello,

Will use rtems_vector_number,an enum for uart_name and assert(0), or assert(!"Message describing error due to invalid value of minor or uart_name"). How do I use two functions ? I don't understand which part of code do you refer by receive and transmit interrupt. 

Regards
Vipul Nayyar 



________________________________
 From: Sebastian Huber <sebastian.huber at embedded-brains.de>
To: rtems-devel at rtems.org 
Sent: Monday, 8 July 2013 8:39 PM
Subject: Re: [PATCH] Updated Legacy code in arm csb336
 

On 07/08/2013 03:43 PM, Vipul Nayyar wrote:
> ---
>   c/src/lib/libbsp/arm/csb336/console/uart.c    | 128 +++++++++++++++-----------
>   c/src/lib/libbsp/arm/csb336/network/network.c |  46 +++------
>   2 files changed, 86 insertions(+), 88 deletions(-)
> 
> diff --git a/c/src/lib/libbsp/arm/csb336/console/uart.c b/c/src/lib/libbsp/arm/csb336/console/uart.c
> index 4dc409b..8156921 100644
> --- a/c/src/lib/libbsp/arm/csb336/console/uart.c
> +++ b/c/src/lib/libbsp/arm/csb336/console/uart.c
[...]
>   /* Define this to use interrupt driver UART driver */
>   #define USE_INTERRUPTS 1
> +#define UART_TX 1
> +#define UART_RX 2
[...]
>   static ssize_t imx_uart_intr_write(int, const char *, size_t);
> +static rtems_irq_number imx_uart_set_name(int,int);

This rtems_irq_number is defined in <rtems/irq.h>.  This it belongs to the legacy API we want to get rid of.  Use rtems_vector_number instead.

[...]
> +static rtems_irq_number imx_uart_set_name(int minor,int uart_name)
> +{
> +    if (minor == 0) {
> +        if (uart_name == UART_TX) return BSP_INT_UART1_TX;
> +        else if (uart_name == UART_RX)return BSP_INT_UART1_RX;
> +
> +    } else if (minor == 1) {
> +        if (uart_name == UART_TX) return BSP_INT_UART2_TX;
> +        else if (uart_name == UART_RX) return BSP_INT_UART2_RX;
> +    }
> +
> +    rtems_panic("%s:%d Unknown UART minor number %d\n",
> +                    __FUNCTION__, __LINE__, minor);
> +}
[...]

This function is better than the previous copy and paste approach.

If I naively read imx_uart_set_name() I think this function sets the name of the UART.  Instead this function returns an interrupt vector depending on a magic number uart_name.

I would use two functions for this, one for the receive and one for the transmit interrupt or use at least an enum instead of the int uart_name.  Since we already use assert(), you can replace the rtems_panic() (which is a very heavy weight function) with an assert(0).

-- 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.
_______________________________________________
rtems-devel mailing list
rtems-devel at rtems.org
http://www.rtems.org/mailman/listinfo/rtems-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20130709/c855adfd/attachment-0001.html>