[PATCH] Updated Legacy code in arm csb336

[Gedare Bloom]

if (uart_name == UART_TX) return BSP_INT_UART2_TX;
+        else if (uart_name == UART_RX) return BSP_INT_UART2_RX;
+    }
UART_TX corresponds to a transmit interrupt. UART_RX to a receive. If
you use two functions, you will have something like
imx_uart_set_name_transmit(int minor) and
imx_uart_set_name_receive(int minor) and get rid of the uart_name
since you no longer need to have conditions. Instead, the calling code
will determine which function to call (transmit or receive version of
imx_uart_set_name).


On Mon, Jul 8, 2013 at 12:21 PM, Vipul Nayyar <nayyar_vipul at yahoo.com> wrote:
> 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
>
>
>
> _______________________________________________
> rtems-devel mailing list
> rtems-devel at rtems.org
> http://www.rtems.org/mailman/listinfo/rtems-devel
>