[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