[Patch] Removing legacy method from arm csb336

Joel Sherrill Joel.Sherrill at OARcorp.com
Sat Jun 29 20:34:17 UTC 2013


Please return the status code into a variable and test it separately in the assert.

I will defer to Sebastian on whether assert should be used. I tend to think it should be a fatal error call with an explicit test. Using assert increases the code size. Plus this first one gives you a pattern for the rest.

Vipul Nayyar <nayyar_vipul at yahoo.com> wrote:



Hello,

Thanks for guidance on the previous patch. I hope I've rectified them in the following patch. I was wondering that while checking the return value of the functions with assert() , I need to include <assert.h> . So, should I include it individually in both the files, or somewhere else ? Please do suggest any other changes that maybe required.


commit 70aee0d8827888b5c8230f45594e85fda5bd5a6d
Author: Vipul Nayyar <nayyar_vipul at yahoo.com>
Date:   Sun Jun 30 00:17:44 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..c0505bb 100644
--- a/c/src/lib/libbsp/arm/csb336/console/uart.c
+++ b/c/src/lib/libbsp/arm/csb336/console/uart.c
@@ -247,8 +247,28 @@ static int imx_uart_first_open(int major, int minor, void *arg)
     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]);
+    assert(rtems_interrupt_handler_install(
+      imx_uart_tx_isr_data[minor].name,
+      "Consle",
+      RTEMS_INTERRUPT_UNIQUE,
+      imx_uart_tx_isr_data[minor].hdl,
+      imx_uart_tx_isr_data[minor].handle
+     ) == RTEMS_SUCCESSFUL
+    );
+    if (imx_uart_tx_isr_data[minor].on != NULL) {
+    imx_uart_tx_isr_data[minor].on(&imx_uart_tx_isr_data[minor]);
+  }
+    assert(rtems_interrupt_handler_install(
+      imx_uart_rx_isr_data[minor].name,
+      "Console",
+      RTEMS_INTERRUPT_UNIQUE,
+      imx_uart_rx_isr_data[minor].hdl,
+      imx_uart_rx_isr_data[minor].handle
+     ) == RTEMS_SUCCESSFUL
+    );
+    if (imx_uart_rx_isr_data[minor].on != NULL) {
+    imx_uart_rx_isr_data[minor].on(&imx_uart_rx_isr_data[minor]);
+  }

     imx_uart_data[minor].regs->cr1 |= MC9328MXL_UART_CR1_RRDYEN;
 #endif
@@ -259,8 +279,24 @@ static int imx_uart_first_open(int major, int minor, void *arg)
 static int imx_uart_last_close(int major, int minor, void *arg)
 {
 #if defined(USE_INTERRUPTS)
-    BSP_remove_rtems_irq_handler(&imx_uart_tx_isr_data[minor]);
-    BSP_remove_rtems_irq_handler(&imx_uart_rx_isr_data[minor]);
+    if (imx_uart_tx_isr_data[minor].off != NULL) {
+    imx_uart_tx_isr_data[minor].off(&imx_uart_tx_isr_data[minor]);
+    }
+    assert( rtems_interrupt_handler_remove(
+        imx_uart_tx_isr_data[minor].name,
+        imx_uart_tx_isr_data[minor].hdl,
+        imx_uart_tx_isr_data[minor].handle
+        ) == RTEMS_SUCCESSFUL
+    );
+    if (imx_uart_rx_isr_data[minor].off != NULL) {
+    imx_uart_rx_isr_data[minor].off(&imx_uart_rx_isr_data[minor]);
+    }
+    assert( rtems_interrupt_handler_remove(
+        imx_uart_rx_isr_data[minor].name,
+        imx_uart_rx_isr_data[minor].hdl,
+        imx_uart_rx_isr_data[minor].handle
+        ) == RTEMS_SUCCESSFUL
+    );
 #endif

     return 0;
diff --git a/c/src/lib/libbsp/arm/csb336/network/network.c b/c/src/lib/libbsp/arm/csb336/network/network.c
index e513595..9ae970e 100644
--- a/c/src/lib/libbsp/arm/csb336/network/network.c
+++ b/c/src/lib/libbsp/arm/csb336/network/network.c
@@ -334,7 +334,17 @@ void  mc9328mxl_enet_init_hw(mc9328mxl_enet_softc_t *sc)
     MC9328MXL_GPIOA_IMR |= bit(3);

     /* Install the interrupt handler */
-    BSP_install_rtems_irq_handler(&mc9328mxl_enet_isr_data);
+    assert(rtems_interrupt_handler_install(
+      mc9328mxl_enet_isr_data.name,
+      "Network",
+      RTEMS_INTERRUPT_UNIQUE,
+      mc9328mxl_enet_isr_data.hdl,
+      mc9328mxl_enet_isr_data.handle
+     ) == RTEMS_SUCCESSFUL
+    );
+    if (mc9328mxl_enet_isr_data.on != NULL) {
+    mc9328mxl_enet_isr_data.on(&mc9328mxl_enet_isr_data);
+  }

 } /* mc9328mxl_enet_init_hw() */






Regards
Vipul Nayyar



________________________________
From: Joel Sherrill <Joel.Sherrill at OARcorp.com>
To: Gedare Bloom <gedare at rtems.org>
Cc: Vipul Nayyar <nayyar_vipul at yahoo.com>; "rtems-devel at rtems.org" <rtems-devel at rtems.org>
Sent: Friday, 28 June 2013 2:18 AM
Subject: Re: [Patch] Removing legacy method from arm csb336

I agree with you Gedare. The legacy api support remains for a while. There is external code which could use it and we always hold legacy for at least one release branch.

I just want the code in the tree to use the new way consistently. And update check_submission to flag the old way

Gedare Bloom <gedare at rtems.org<mailto:gedare at rtems.org>> wrote:


On Thu, Jun 27, 2013 at 4:46 AM, Joel Sherrill
<Joel.Sherrill at oarcorp.com<mailto:Joel.Sherrill at oarcorp.com>> wrote:
> The goal of this is to kill bad examples. It is legacy and we don't want
> people to use it anymore. These should be mechanical changes. Vipul should
> be able to provide instructions for users with external drivers to make the
> changes.
>
> Fixing these could break here or there but if we don't remove uses of
> deprecated methods then users will use them in new BSPs and that is
> unacceptable.
>
I'd prefer to see 4.11 released before we start killing off legacy support.

> Vipul Nayyar <nayyar_vipul at yahoo.com<mailto:nayyar_vipul at yahoo.com>> wrote:
>
> Hello,
>
> According to the discussions with my mentor Joel Sherill, I was suggested to
> kill all legacy/deprecated API uses in the tree which are defined in
> irq-legacy.c as part of my project. I guess Dr Joel can guide us better
> whether all of the deprecated methods need to be removed or only some of
> them. cc'ing Dr Joel for guidance on this.
>
> Regards
> Vipul Nayyar
>
>
> ________________________________
> From: Sebastian Huber <sebastian.huber at embedded-brains.de<mailto:sebastian.huber at embedded-brains.de>>
> To: rtems-devel at rtems.org<mailto:rtems-devel at rtems.org>
> Sent: Thursday, 27 June 2013 1:48 PM
> Subject: Re: [Patch] Removing legacy method from arm csb336
>
> On 06/26/2013 05:28 PM, Vipul Nayyar wrote:
>>
>> For my GSOC Project Unified APIs,  a public google doc has been setup (
>>
>> https://docs.google.com/document/d/1W9DMnpocXUKXiQMxHbbp0aG-ecuh94P-3N7hxEJmcgs/edit?usp=sharing)
>> for community viewing. In order to establish this unified pattern across
>> all
>> BSPs, I felt that deprecated methods stored in irq-legacy.c should be
>> dealt
>> with first. So, I'm attaching a very basic patch which removes the
>> deprecated
>> method BSP_install_rtems_irq_handler from arm csb336 and instead makes use
>> of rtems_interrupt_handler_install, as suggested in irq-legacy.c. Please
>> do
>> give suggestions if I'm heading the right way, and any modifications that
>> this
>> patch may require.
>
> I would not touch the existing code that uses these legacy functions.  The
> irq-legacy.c is provided to keep existing code as is.
>
> -- 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<mailto: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<mailto:rtems-devel at rtems.org>
> http://www.rtems.org/mailman/listinfo/rtems-devel
>
>
>
> _______________________________________________
> rtems-devel mailing list
> rtems-devel at rtems.org<mailto: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/20130629/60ae6cd1/attachment-0001.html>


More information about the devel mailing list