On 7/4/2012 8:39 AM, #! wrote: [...]>>>> bool write_to_fifo(int8 d) { >>>> >>>> if ((fifo->wr != fifo->rd) || fifo->ef == EMPTY) { >>>> fifo->bf[fifo->wr] = d; >>>> fifo->wr = MOD(fifo->wr+1, DIM(fifo->bf)); >>>> if (fifo->wr == fifo->rd) fifo->ef = FULL; >>>> return TRUE; >>>> } >>>> >>>> return FALSE; >>>> } >>>> >>> >>>> bool read_from_fifo(int8 *d) { >>>> >>>> if ((fifo->wr != fifo->rd) || fifo->ef == FULL) { >>>> *d = fifo->bf[fifo->rd]; >>>> fifo->rd = MOD(fifo->rd+1, DIM(fifo->bf)); >>>> if (fifo->rd == f->wr) fifo->ef = EMPTY; >>>> return TRUE; >>>> } >>>> >>>> return FALSE; >>>> } >>>> >>> >>> where fifo has the following structure: >>> >>>> typedef struct { >>>> int8 bf[FIFO_LENGTH]; >>>> int16 wr; // write pointer >>>> int16 rd; // read pointer >>>> bool ef; // fifo empty/full flag >>>> } _fifo;[...]> Just looked at your code > > An interrupt could change the value of rd while evaluating this line > if (fifo->wr == fifo->rd) fifo->ef = FULL; > > The value of wr may be partly incremented when an interrupt tries to > evaluate this line > if (fifo->rd == f->wr) fifo->ef = EMPTY;Yep, I also noticed that, even though I do not understand how this problem can produce the effect I see. If the pointers are moved while reading or writing the flag will be wrong. But in either case if the flag is wrong the fifo will be wrongly interpreted as full while is empty or viceversa, meaning data transmitted will be wrong... but they would still be transmitted. I will nevertheless disable the interrupts and reenable inside write_to_fifo() and read_from_fifo() and see what will happen.
[cross-post] nested interrupts
Started by ●July 1, 2012
Reply by ●July 4, 20122012-07-04
Reply by ●July 4, 20122012-07-04
On 2012-07-04, Meindert Sprang <ms@NOJUNKcustomORSPAMware.nl> wrote:> The code above is flawed. Your main should not write to the sport at all. If > no transmission is in progress, the TX interrupt of sport should be > disabled. If you write something to the fifo, you just enable the TX > interrupt, nothing more. At that time, the sport TX buffers are empty and an > interrupt will occur immediately, reading a byte from the fifo and writing > it to sport. When the next interrupt occurs, the next byte will be read from > the fifo and written to the sport. If the fifo is empty when an interrupt > occurs, the interrupt handler should disable the TX interrupt and exit.Exactly. And you do not need "fifo empty/full" flags. If write_pointer == read_pointer, fifo is empty. If write_pointer+1 (wrapping at end of buffer) == read_pointer, fifo is full. This way the main program only alters write_pointer (and buffer contents) and the irs code only alters read_pointer and there are no race conditions, ever. Simple. -jm
Reply by ●July 4, 20122012-07-04
On 7/4/2012 9:12 AM, Meindert Sprang wrote: [...]>> >>> int main() { >>> ... >>> while(1) { >>> write_to_fifo(data); >>> if (state != TRANSMITTING) { >>> if (read_from_fifo(&byte) == OK) { >>> write_to_sport(byte); >>> state = TRANSMITTING; >>> } >>> } >>> } >>> } >>[...]> The code above is flawed. Your main should not write to the sport at all. If > no transmission is in progress, the TX interrupt of sport should be > disabled. If you write something to the fifo, you just enable the TX > interrupt, nothing more. At that time, the sport TX buffers are empty and an > interrupt will occur immediately, reading a byte from the fifo and writing > it to sport.Uhm that is interesting, but since my interrupt is edge sensitive I do not believe that when I enable it the interrupt will be seen immediately. I may change the interrupt to level sensitive, but I'm not sure if it is beneficial. Nevertheless I don't see how this change will make me understand why the nested interrupts break my code. When the next interrupt occurs, the next byte will be read from> the fifo and written to the sport. If the fifo is empty when an interrupt > occurs, the interrupt handler should disable the TX interrupt and exit. >Your proposal is certainly valid, but it does not help me understanding what I'm doing wrong. Other people have correctly pointed out several flaws in my code but the logic of 'starting' the transmission in main() is not flawed 'per se', it is maybe wrongly implemented.
Reply by ●July 4, 20122012-07-04
On 7/4/2012 11:19 AM, Jukka Marin wrote:> On 2012-07-04, Meindert Sprang <ms@NOJUNKcustomORSPAMware.nl> wrote: >> The code above is flawed. Your main should not write to the sport at all. If >> no transmission is in progress, the TX interrupt of sport should be >> disabled. If you write something to the fifo, you just enable the TX >> interrupt, nothing more. At that time, the sport TX buffers are empty and an >> interrupt will occur immediately, reading a byte from the fifo and writing >> it to sport. When the next interrupt occurs, the next byte will be read from >> the fifo and written to the sport. If the fifo is empty when an interrupt >> occurs, the interrupt handler should disable the TX interrupt and exit. > > Exactly. And you do not need "fifo empty/full" flags. If write_pointer == > read_pointer, fifo is empty. If write_pointer+1 (wrapping at end of buffer) > == read_pointer, fifo is full. This way the main program only alters > write_pointer (and buffer contents) and the irs code only alters read_pointer > and there are no race conditions, ever.You are suggesting to remove the flag and keep one memory location of the fifo not used (if I understood correctly). This will prevent the race condition on the flag, it will still not prevent the race on 'state'. And I should remind you that my problem is why nested interrupts will break my code w.r.t. non nested interrupts. It is still true that I have to prevent fifo to be misused but this will affect only the data being wrong (fifo will be interpreted as full while is empty), not the logic on the transmission.
Reply by ●July 4, 20122012-07-04
On 2012-07-04, alb <alessandro.basili@cern.ch> wrote:>> Exactly. And you do not need "fifo empty/full" flags. If write_pointer == >> read_pointer, fifo is empty. If write_pointer+1 (wrapping at end of buffer) >> == read_pointer, fifo is full. This way the main program only alters >> write_pointer (and buffer contents) and the irs code only alters read_pointer >> and there are no race conditions, ever. > > You are suggesting to remove the flag and keep one memory location of > the fifo not used (if I understood correctly).Yes.> This will prevent the > race condition on the flag, it will still not prevent the race on 'state'.Yes, it does. You update the read and write pointers after you have read or written the buffer. No race.> And I should remind you that my problem is why nested interrupts will > break my code w.r.t. non nested interrupts. It is still true that I have > to prevent fifo to be misused but this will affect only the data being > wrong (fifo will be interpreted as full while is empty), not the logic > on the transmission.I'm not sure it's the nested interrupts that's the problem but the race problems in your fifo/isr code. (I'm also not sure if you need nested interrupts at all. ;-) -jm
Reply by ●July 4, 20122012-07-04
On 04/07/12 21:19, alb wrote:> On 7/4/2012 8:39 AM, #! wrote: > [...] >>>>> bool write_to_fifo(int8 d) { >>>>> >>>>> if ((fifo->wr != fifo->rd) || fifo->ef == EMPTY) { >>>>> fifo->bf[fifo->wr] = d; >>>>> fifo->wr = MOD(fifo->wr+1, DIM(fifo->bf)); >>>>> if (fifo->wr == fifo->rd) fifo->ef = FULL; >>>>> return TRUE; >>>>> } >>>>> >>>>> return FALSE; >>>>> } >>>>> >>>> >>>>> bool read_from_fifo(int8 *d) { >>>>> >>>>> if ((fifo->wr != fifo->rd) || fifo->ef == FULL) { >>>>> *d = fifo->bf[fifo->rd]; >>>>> fifo->rd = MOD(fifo->rd+1, DIM(fifo->bf)); >>>>> if (fifo->rd == f->wr) fifo->ef = EMPTY; >>>>> return TRUE; >>>>> } >>>>> >>>>> return FALSE; >>>>> } >>>>> >>>> >>>> where fifo has the following structure: >>>> >>>>> typedef struct { >>>>> int8 bf[FIFO_LENGTH]; >>>>> int16 wr; // write pointer >>>>> int16 rd; // read pointer >>>>> bool ef; // fifo empty/full flag >>>>> } _fifo; > [...] > >> Just looked at your code >> >> An interrupt could change the value of rd while evaluating this line >> if (fifo->wr == fifo->rd) fifo->ef = FULL; >> >> The value of wr may be partly incremented when an interrupt tries to >> evaluate this line >> if (fifo->rd == f->wr) fifo->ef = EMPTY; > > Yep, I also noticed that, even though I do not understand how this > problem can produce the effect I see. > If the pointers are moved while reading or writing the flag will be > wrong. But in either case if the flag is wrong the fifo will be wrongly > interpreted as full while is empty or viceversa, meaning data > transmitted will be wrong... but they would still be transmitted. > > I will nevertheless disable the interrupts and reenable inside > write_to_fifo() and read_from_fifo() and see what will happen. >To understand, you will need to look at the machine code and work out what happens when an interrupt occurs at every position in the critical section of code, with every possible combination of rd and wr As Niklas has suggested maybe the extra interrupts changed the timing, causing you luck to run out.
Reply by ●July 4, 20122012-07-04
>On 7/4/2012 9:12 AM, Meindert Sprang wrote: >[...] >>> >>>> int main() { >>>> ... >>>> while(1) { >>>> write_to_fifo(data); >>>> if (state != TRANSMITTING) { >>>> if (read_from_fifo(&byte) == OK) { >>>> write_to_sport(byte); >>>> state = TRANSMITTING; >>>> } >>>> } >>>> } >>>> } >>> >[...] > >> The code above is flawed. Your main should not write to the sport atall. If>> no transmission is in progress, the TX interrupt of sport should be >> disabled. If you write something to the fifo, you just enable the TX >> interrupt, nothing more. At that time, the sport TX buffers are emptyand an>> interrupt will occur immediately, reading a byte from the fifo andwriting>> it to sport. > >Uhm that is interesting, but since my interrupt is edge sensitive I do >not believe that when I enable it the interrupt will be seen immediately. > >I may change the interrupt to level sensitive, but I'm not sure if it is >beneficial. Nevertheless I don't see how this change will make me >understand why the nested interrupts break my code. > > When the next interrupt occurs, the next byte will be read from >> the fifo and written to the sport. If the fifo is empty when aninterrupt>> occurs, the interrupt handler should disable the TX interrupt and exit. >> > >Your proposal is certainly valid, but it does not help me understanding >what I'm doing wrong. Other people have correctly pointed out several >flaws in my code but the logic of 'starting' the transmission in main() >is not flawed 'per se', it is maybe wrongly implemented. >It is a while since I last looked at the topic, but ISTR that edge-triggered interrupts were to be considered an Abomination Unto Nuggan, at least if you have more than one interrput source. --------------------------------------- Posted through http://www.EmbeddedRelated.com
Reply by ●July 4, 20122012-07-04
"Rocky" <robertgush@gmail.com> wrote in message news:55d65d0a-6660-4cca-827e-4f23ac1cf93b@q29g2000vby.googlegroups.com... On Jul 4, 9:12 am, "Meindert Sprang" <m...@NOJUNKcustomORSPAMware.nl> wrote:> - Show quoted text - > This is not true for all processors. Some of them have edge triggered > interrupts the only interrupt when the transmitter finishes > transmission.What would be the advantage of having an edge triggered interrupt on a UART empty event? Do you have an example of this? Meindert
Reply by ●July 4, 20122012-07-04
"alb" <alessandro.basili@cern.ch> wrote in message news:a5igkjFb0oU1@mid.individual.net...> On 7/4/2012 9:12 AM, Meindert Sprang wrote: > [...] > >> > >>> int main() { > >>> ... > >>> while(1) { > >>> write_to_fifo(data); > >>> if (state != TRANSMITTING) { > >>> if (read_from_fifo(&byte) == OK) { > >>> write_to_sport(byte); > >>> state = TRANSMITTING; > >>> } > >>> } > >>> } > >>> } > >> > [...] > > > The code above is flawed. Your main should not write to the sport atall. If> > no transmission is in progress, the TX interrupt of sport should be > > disabled. If you write something to the fifo, you just enable the TX > > interrupt, nothing more. At that time, the sport TX buffers are emptyand an> > interrupt will occur immediately, reading a byte from the fifo andwriting> > it to sport. > > Uhm that is interesting, but since my interrupt is edge sensitive I do > not believe that when I enable it the interrupt will be seen immediately. > I may change the interrupt to level sensitive, but I'm not sure if it is > beneficial.Yes it is because then you can use the procedure I outlined. And this is not pure theoretical, I use this in a commercial product. And I have no race conditions and use nested interrupts.> Your proposal is certainly valid, but it does not help me understanding > what I'm doing wrong. Other people have correctly pointed out several > flaws in my code but the logic of 'starting' the transmission in main() > is not flawed 'per se', it is maybe wrongly implemented.It sure makes life a lot easier if you don't write to the same hardware in main code AND in an interrupt. Meindert
Reply by ●July 4, 20122012-07-04
On Jul 4, 1:14=A0pm, "Meindert Sprang" <m...@NOJUNKcustomORSPAMware.nl> wrote:> "Rocky" <robertg...@gmail.com> wrote in message > > news:55d65d0a-6660-4cca-827e-4f23ac1cf93b@q29g2000vby.googlegroups.com... > On Jul 4, 9:12 am, "Meindert Sprang" <m...@NOJUNKcustomORSPAMware.nl> > wrote: > > > - Show quoted text - > > This is not true for all processors. Some of them have edge triggered > > interrupts the only interrupt when the transmitter finishes > > transmission. > > What would be the advantage of having an edge triggered interrupt on a UA=RT> empty event? Do you have an example of this? > > Meindert8032. Bit of a pain, but once the code is in place it is OK. IIRC the Z80 SIO had a similar issue. The scheme in the PIC for example of disabling the specific interrupt is much easier to use because you just enable the interrupt after adding data to the circular buffer. It doesn't matter if was enabled or not when you re-enable. Worst case is a gratuitous interrupt.
