[Patch] Removing legacy method from arm csb336
Sebastian Huber
sebastian.huber at embedded-brains.de
Sun Jul 7 13:16:42 UTC 2013
Overall this patch looks good, but there are some minor issues.
On 07/07/13 13:33, Vipul Nayyar wrote:
> Hello,
>
> Worked again on csb336 patch. Following points I've kept in mind while doing this :
>
> 1) Update legacy install and remove methods to the new generic ones
> 2)Removed all uses of tems_irq_connect_data type
> 3) Inserted protoypes for functions which didn't had one
> 4) Made functions static that are only used in that file
>
> Hoping to get reviews, so that I can move on with other BSPs.
>
> commit e22d51a1f9d2efeba77e97ff0c81a9a5800af20c
> Author: Vipul Nayyar <nayyar_vipul at yahoo.com>
> Date: Sun Jul 7 16:51:07 2013 +0530
>
> Updated Legacy code in arm csb336
>
> diff --git a/c/src/lib/libbsp/arm/csb336/console/uart.c b/c/src/lib/libbsp/arm/csb336/console/uart.c
> index 4dc409b..96126fa 100644
> --- a/c/src/lib/libbsp/arm/csb336/console/uart.c
> +++ b/c/src/lib/libbsp/arm/csb336/console/uart.c
> @@ -13,6 +13,7 @@
> #include <libchip/sersupp.h>
> #include <rtems/error.h>
> #include <rtems/bspIo.h>
> +#include <assert.h>
> #include <termios.h>
> #include <rtems/irq.h>
> #include <bsp/irq.h>
> @@ -34,13 +35,17 @@ static int imx_uart_set_attrs(int, const struct termios *);
> static void imx_uart_init(int minor);
> static void imx_uart_set_baud(int, int);
> static ssize_t imx_uart_poll_write(int, const char *, size_t);
> +static int imx_uart_poll_read_char(int minor);
> +static void imx_uart_poll_write_char(int minor, char c);
> +static void _BSP_output_char(char c);
> +static int _BSP_poll_char(void);
> +
One empty line is enough.
>
> #if defined(USE_INTERRUPTS)
> static void imx_uart_tx_isr(rtems_irq_hdl_param);
> static void imx_uart_rx_isr(rtems_irq_hdl_param);
> -static void imx_uart_isr_on(const rtems_irq_connect_data *irq);
> -static void imx_uart_isr_off(const rtems_irq_connect_data *irq);
> -static int imx_uart_isr_is_on(const rtems_irq_connect_data *irq);
> +static void imx_uart_isr_on(const rtems_irq_number);
> +static void imx_uart_isr_off(const rtems_irq_number);
> static ssize_t imx_uart_intr_write(int, const char *, size_t);
> #endif
>
> @@ -71,11 +76,6 @@ rtems_termios_callbacks imx_uart_cbacks = {
> };
> #endif
>
> -#if defined(USE_INTERRUPTS)
> -static rtems_irq_connect_data imx_uart_tx_isr_data[NUM_DEVS];
> -static rtems_irq_connect_data imx_uart_rx_isr_data[NUM_DEVS];
> -#endif
> -
> typedef struct {
> int minor;
> mc9328mxl_uart_regs_t * regs;
> @@ -184,17 +184,9 @@ static void imx_uart_init(int minor)
> imx_uart_data[minor].idx = 0;
>
> if (minor == 0) {
> -#if defined(USE_INTERRUPTS)
> - imx_uart_tx_isr_data[minor].name = BSP_INT_UART1_TX;
> - imx_uart_rx_isr_data[minor].name = BSP_INT_UART1_RX;
> -#endif
> imx_uart_data[minor].regs =
> (mc9328mxl_uart_regs_t *) MC9328MXL_UART1_BASE;
> } else if (minor == 1) {
> -#if defined(USE_INTERRUPTS)
> - imx_uart_tx_isr_data[minor].name = BSP_INT_UART2_TX;
> - imx_uart_rx_isr_data[minor].name = BSP_INT_UART2_RX;
> -#endif
> imx_uart_data[minor].regs =
> (mc9328mxl_uart_regs_t *) MC9328MXL_UART2_BASE;
> } else {
> @@ -202,20 +194,6 @@ static void imx_uart_init(int minor)
> __FUNCTION__, __LINE__, minor);
> }
>
> -#if defined(USE_INTERRUPTS)
> - imx_uart_tx_isr_data[minor].hdl = imx_uart_tx_isr;
> - imx_uart_tx_isr_data[minor].handle = &imx_uart_data[minor];
> - imx_uart_tx_isr_data[minor].on = imx_uart_isr_on;
> - imx_uart_tx_isr_data[minor].off = imx_uart_isr_off;
> - imx_uart_tx_isr_data[minor].isOn = imx_uart_isr_is_on;
> -
> - imx_uart_rx_isr_data[minor].hdl = imx_uart_rx_isr;
> - imx_uart_rx_isr_data[minor].handle = &imx_uart_data[minor];
> - imx_uart_rx_isr_data[minor].on = imx_uart_isr_on;
> - imx_uart_rx_isr_data[minor].off = imx_uart_isr_off;
> - imx_uart_rx_isr_data[minor].isOn = imx_uart_isr_is_on;
> -#endif
> -
> imx_uart_data[minor].regs->cr1 = (
> MC9328MXL_UART_CR1_UARTCLKEN |
> MC9328MXL_UART_CR1_UARTEN);
> @@ -243,13 +221,43 @@ static void imx_uart_init(int minor)
> static int imx_uart_first_open(int major, int minor, void *arg)
> {
> rtems_libio_open_close_args_t *args = arg;
> + rtems_status_code status = RTEMS_SUCCESSFUL;
> + rtems_irq_number name_uart_tx, name_uart_rx;
These variables are unused if USE_INTERRUPTS is not defined. Please
compile this file with USE_INTERRUPTS defined and undefined and watch
for warnings.
>
> imx_uart_data[minor].tty = args->iop->data1;
>
> -#if defined(USE_INTERRUPTS)
> - BSP_install_rtems_irq_handler(&imx_uart_tx_isr_data[minor]);
> - BSP_install_rtems_irq_handler(&imx_uart_rx_isr_data[minor]);
> + if (minor == 0) {
> + name_uart_tx = BSP_INT_UART1_TX;
> + name_uart_rx = BSP_INT_UART1_RX;
> + } else if (minor == 1) {
> + name_uart_tx = BSP_INT_UART2_TX;
> + name_uart_rx = BSP_INT_UART2_RX;
> + } else {
> + rtems_panic("%s:%d Unknown UART minor number %d\n",
> + __FUNCTION__, __LINE__, minor);
> + }
>
> +#if defined(USE_INTERRUPTS)
> + status = rtems_interrupt_handler_install(
> + name_uart_tx,
> + "Network",
> + RTEMS_INTERRUPT_UNIQUE,
> + imx_uart_tx_isr,
> + &imx_uart_data[minor]
> + );
> + assert(status == RTEMS_SUCCESSFUL);
> + imx_uart_isr_on(name_uart_tx);
> +
> + status = rtems_interrupt_handler_install(
> + name_uart_rx,
> + "Network",
> + RTEMS_INTERRUPT_UNIQUE,
> + imx_uart_rx_isr,
> + &imx_uart_data[minor]
> + );
> + assert(status == RTEMS_SUCCESSFUL);
> + imx_uart_isr_on(name_uart_rx);
> +
> imx_uart_data[minor].regs->cr1 |= MC9328MXL_UART_CR1_RRDYEN;
> #endif
>
> @@ -258,9 +266,35 @@ static int imx_uart_first_open(int major, int minor, void *arg)
>
> static int imx_uart_last_close(int major, int minor, void *arg)
> {
> + rtems_status_code status = RTEMS_SUCCESSFUL;
> + rtems_irq_number name_uart_tx, name_uart_rx;
> +
> + if (minor == 0) {
> + name_uart_tx = BSP_INT_UART1_TX;
> + name_uart_rx = BSP_INT_UART1_RX;
> + } else if (minor == 1) {
> + name_uart_tx = BSP_INT_UART2_TX;
> + name_uart_rx = BSP_INT_UART2_RX;
> + } else {
> + rtems_panic("%s:%d Unknown UART minor number %d\n",
> + __FUNCTION__, __LINE__, minor);
> + }
The lines above are a copy and paste version of some code earlier. You
should avoid copy and past in general.
[...]
--
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.
More information about the devel
mailing list