<html><body><div style="color:#000; background-color:#fff; font-family:arial, helvetica, sans-serif;font-size:10pt"><div style="font-family: arial, helvetica, sans-serif; font-size: 10pt;"><span>Hello,</span></div><div style="font-family: arial, helvetica, sans-serif; font-size: 13px; color: rgb(0, 0, 0); background-color: transparent; font-style: normal;"><span><br></span></div><div style="font-family: arial, helvetica, sans-serif; font-size: 13px; color: rgb(0, 0, 0); background-color: transparent; font-style: normal;"><span>Before submitting a patch again, I wanted to clarify some things which I understood & places where I need a little more guidance, so that I do things the right way. </span></div><div style="font-family: arial, helvetica, sans-serif; font-size: 13px; color: rgb(0, 0, 0); background-color: transparent; font-style: normal;"><span><br></span></div><div style="background-color: transparent;"><span><font size="2">1) Before
submitting any patch I'll definitely compile it first. </font></span><span style="font-size: 13px;">So do I need to bootstrap, run the configure </span><span style="font-size: 10pt;">using specific BSP name & target in cmd options </span><span style="background-color: transparent;"><font size="2">at top of RTEMS tree and do make every time, I change something ? Or is there a simpler method ?</font></span></div><div style="background-color: transparent; color: rgb(0, 0, 0); font-size: 13px; font-family: arial, helvetica, sans-serif; font-style: normal;"><span style="background-color: transparent;"><font size="2"><br></font></span></div><div style="background-color: transparent; color: rgb(0, 0, 0); font-size: 13px; font-family: arial, helvetica, sans-serif; font-style: normal;"><span style="background-color: transparent;"><font size="2">2) According to the legacy method BSP_install_rtems_irq_handler, it uses
the 'on' ( at91rm9200_emac_isr_on() in case of csb337) function only after calling rtems_interrupt_handler_install. And these functions require a pointer to a rtems_irq_connect_data type.So I should probably remove that parameter from the function definition right ?</font></span></div><div style="background-color: transparent; color: rgb(0, 0, 0); font-size: 13px; font-family: arial, helvetica, sans-serif; font-style: normal;"><span style="background-color: transparent;"><font size="2"><br></font></span></div><div style="background-color: transparent; color: rgb(0, 0, 0); font-size: 13px; font-family: arial, helvetica, sans-serif; font-style: normal;"><span style="background-color: transparent;"><font size="2">3) rtems_interrupt_handler_install() does not accept pointer to the </font></span><span style="font-size: small;"> </span><span style="font-size: 13px;">rtems_irq_connect_data type , so it definitely does not
use at91rm9200_emac_isr_off() & at91rm9200_emac_isr_is_on() functions. I think we should remove these 2 functions, because they're not being used in the any csb337 file.</span></div><div style="background-color: transparent; color: rgb(0, 0, 0); font-size: 13px; font-family: arial, helvetica, sans-serif; font-style: normal;"><span style="font-size: 13px;"><br></span></div><div style="background-color: transparent; color: rgb(0, 0, 0); font-size: 13px; font-family: arial, helvetica, sans-serif; font-style: normal;"><span style="font-size: 13px;">4) If you say so, I'll use assert() for time being. Plus, do I need to include <assert.h> ? If yes, then in each file where I used assert() or it can be included somewhere central ?</span></div><div style="background-color: transparent; color: rgb(0, 0, 0); font-size: 13px; font-family: arial, helvetica, sans-serif; font-style: normal;"><br></div><div style="background-color: transparent;
color: rgb(0, 0, 0); font-size: 13px; font-family: arial, helvetica, sans-serif; font-style: normal;">Hoping to hear from you</div><div style="font-family: arial, helvetica, clean, sans-serif; font-size: 13px; color: rgb(0, 0, 0); background-color: transparent; font-style: normal;"><br></div><div style="font-family: arial, helvetica, clean, sans-serif; font-size: 13px; color: rgb(0, 0, 0); background-color: transparent; font-style: normal;">Regards</div><div style="font-family: arial, helvetica, clean, sans-serif; font-size: 13px; color: rgb(0, 0, 0); background-color: transparent; font-style: normal;"><span style="background-color:transparent;line-height:1.22;">Vipul Nayyar </span></div><div style="font-family: arial, helvetica, clean, sans-serif; font-size: 12.727272033691406px; color: rgb(0, 0, 0); background-color: transparent; font-style: normal;"><br></div><div style="font-family: arial, helvetica, sans-serif; font-size: 10pt;"><br></div>
<div style="font-family: arial, helvetica, sans-serif; font-size: 10pt;"> <div style="font-family: 'times new roman', 'new york', times, serif; font-size: 12pt;"> <div dir="ltr"> <hr size="1"> <font size="2" face="Arial"> <b><span style="font-weight:bold;">From:</span></b> Sebastian Huber <sebastian.huber@embedded-brains.de><br> <b><span style="font-weight: bold;">To:</span></b> Vipul Nayyar <nayyar_vipul@yahoo.com> <br><b><span style="font-weight: bold;">Cc:</span></b> "rtems-devel@rtems.org" <rtems-devel@rtems.org>; Joel Sherrill <joel.sherrill@oarcorp.com> <br> <b><span style="font-weight: bold;">Sent:</span></b> Wednesday, 3 July 2013 12:37 PM<br> <b><span style="font-weight: bold;">Subject:</span></b> Re: [Patch] Removing legacy method from arm csb337<br> </font> </div> <div class="y_msg_container"><br>On 07/02/2013 01:26 PM, Vipul Nayyar wrote:<br>> Hello,<br>><br>> Thanks for pointing out the mistakes in the
previous patches. Updated the<br>> csb337 patch, and did away with the use of rtems_irq_connect_data structure in<br>> the file. Is this what you were suggesting ? Hoping I've followed the<br>> indentation rules.<br>><br>> commit d24fbcb472b0164ed4e6b51233155df7f6dd57b4<br>> Author: Vipul Nayyar <<a ymailto="mailto:nayyar_vipul@yahoo.com" href="mailto:nayyar_vipul@yahoo.com">nayyar_vipul@yahoo.com</a>><br>> Date: Tue Jul 2 16:52:29 2013 +0530<br>><br>> Updated Legacy code in arm csb337<br>><br>> diff --git a/c/src/lib/libbsp/arm/csb337/network/network.c<br>> b/c/src/lib/libbsp/arm/csb337/network/network.c<br>> index e31bcc0..1c1dd0e 100644<br>> --- a/c/src/lib/libbsp/arm/csb337/network/network.c<br>> +++ b/c/src/lib/libbsp/arm/csb337/network/network.c<br>> @@ -87,17 +87,6 @@ static void at91rm9200_emac_isr_on(const<br>> rtems_irq_connect_data *unused);<br>>
static void at91rm9200_emac_isr_off(const rtems_irq_connect_data *unused);<br>> static int at91rm9200_emac_isr_is_on(const rtems_irq_connect_data *irq);<br>> -/* Replace the first value with the clock's interrupt name. */<br>> -rtems_irq_connect_data at91rm9200_emac_isr_data = {<br>> - AT91RM9200_INT_EMAC,<br>> - at91rm9200_emac_isr,<br>> - NULL,<br>> - at91rm9200_emac_isr_on,<br>> - at91rm9200_emac_isr_off,<br>> - at91rm9200_emac_isr_is_on<br><br>Where are the at91rm9200_emac_isr_on(), at91rm9200_emac_isr_off() and <br>at91rm9200_emac_isr_is_on() called now? Did you compile the code after the <br>changes? You should get warnings now about unused static functions.<br><br>Are they really all superfluous?<br><br>> -};<br>> -<br>> -<br>> /* use the values defined in linkcmds for our use of SRAM */<br>>
extern void * at91rm9200_emac_rxbuf_hdrs;<br>> extern void * at91rm9200_emac_txbuf;<br>> @@ -358,6 +347,7 @@ void at91rm9200_emac_init(void *arg)<br>> {<br>> at91rm9200_emac_softc_t *sc = arg;<br>> struct ifnet *ifp = &sc->arpcom.ac_if;<br>> + rtems_status_code status = RTEMS_SUCCESSFUL;<br>> /*<br>> *This is for stuff that only gets done once (at91rm9200_emac_init()<br>> @@ -382,7 +372,19 @@ void at91rm9200_emac_init(void *arg)<br>> AIC_SMR_REG(AIC_SMR_EMAC) = AIC_SMR_PRIOR(EMAC_INT_PRIORITY);<br>> /* install the interrupt handler */<br>> - BSP_install_rtems_irq_handler(&at91rm9200_emac_isr_data);<br>> + status = rtems_interrupt_handler_install(<br>> + AT91RM9200_INT_EMAC,<br>> +
"Network",<br>> + RTEMS_INTERRUPT_UNIQUE,<br>> + at91rm9200_emac_isr,<br>> + NULL<br>> + );<br>> + if (status != RTEMS_SUCCESSFUL) {<br>> + rtems_fatal(<br>> + RTEMS_FATAL_SOURCE_BSP_SPECIFIC,<br>> + BSP_GENERIC_FATAL_INTERRUPT_INITIALIZATION<br>> + );<br>> + }<br>[...]<br><br>As I wrote before, the indentation level is two characters.<br><br>The purpose of the fatal errors is<br><br>1. terminate the system, and<br>2. allow the exact identification of the error source in the code.<br><br> From 2. follows that a (source, code) pair must be used a most once in the <br>RTEMS source code.<br><br>In the documentation we have:<br><br><a
href="http://www.rtems.org/onlinedocs/doxygen/cpukit/html/group__ScoreIntErr.html#ga878b4de77df7d0b83d19609d4de42c26" target="_blank">http://www.rtems.org/onlinedocs/doxygen/cpukit/html/group__ScoreIntErr.html#ga878b4de77df7d0b83d19609d4de42c26</a><br><br>[...]<br>RTEMS_FATAL_SOURCE_BSP_GENERIC <br><br>Fatal source for generic BSP errors.<br><br>The fatal codes are defined in <bsp/generic-fatal.h>. Examples are interrupt <br>and exception initialization.<br><br>See also:<br> bsp_generic_fatal_code and bsp_generic_fatal().<br><br>RTEMS_FATAL_SOURCE_BSP_SPECIFIC <br><br>Fatal source for BSP specific errors.<br><br>The fatal code is BSP specific.<br>[...]<br><br>The BSP_GENERIC_FATAL_INTERRUPT_INITIALIZATION is generic BSP error code, so <br>don't use it with RTEMS_FATAL_SOURCE_BSP_SPECIFIC.<br><br>I would use simply assert(). If there is time, we can change all the assert() <br>into BSP
specific fatal errors later.<br><br>-- <br>Sebastian Huber, embedded brains GmbH<br><br>Address : Dornierstr. 4, D-82178 Puchheim, Germany<br>Phone : +49 89 189 47 41-16<br>Fax : +49 89 189 47 41-09<br>E-Mail : <a ymailto="mailto:sebastian.huber@embedded-brains.de" href="mailto:sebastian.huber@embedded-brains.de">sebastian.huber@embedded-brains.de</a><br>PGP : Public key available on request.<br><br>Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.<br><br><br></div> </div> </div> </div></body></html>