Atmega128RFA1 locks up if packet is received when turning radio off

[GoogleCodeExporter]

Madis Uusjärv found following issue:

When turning radio off the accidential incoming message causes radio IRQ that 
will be handled with Tasklet.schedule() instead of command CMD_TURNOFF. But 
RadioState.turnOff() returns SUCCESS anyway which means that the upper layer 
will wait for RadioState.done() event that will never occur causing radio 
driver to hang. 

The solution is to clear a pending radio interrupt...

 tasklet_async command error_t RadioState.turnOff()
 {
   if( cmd != CMD_NONE )
     return EBUSY;
   else if( state == STATE_SLEEP )
     return EALREADY;

   atomic
   {
       cmd = CMD_TURNOFF;
       radioIrq =  IRQ_NONE;    // clear radio IRQ, we want to stop radio, do not handle RX interrupt!
   }
   call Tasklet.schedule();
   return SUCCESS;
 }

... and do not set interrupt flag if CMD_TURNOFF is pending.

 /**
  * Indicates the completion of a frame reception
  */
 AVR_NONATOMIC_HANDLER(TRX24_RX_END_vect){
   RADIO_ASSERT( ! radioIrq );
   if (cmd != CMD_TURNOFF)
   {
       atomic radioIrq |= IRQ_RX_END;
       call Tasklet.schedule();
   }
 }

It is fixed in attached file. Unfortunately it is not patch file but I guess 
Miklos is able to merge it with the trunk.

Original issue reported on code.google.com by andres.v...@smartdustsolutions.com on 28 Nov 2011 at 9:16

Attachments:

• RFA1DriverLayerP.nc

[GoogleCodeExporter]

Original comment by philip.l...@gmail.com on 29 Nov 2011 at 6:12

[GoogleCodeExporter]

RF230 driver is affected with the same problem.
For example there are cases when changing channel fails if at the same time 
some packet is receive.

Original comment by andres.v...@smartdustsolutions.com on 2 Dec 2011 at 1:24

[GoogleCodeExporter]

Thanks for the detailed error report!

Yes, the Atmega128RFA1 radio driver is affected. However, the proposed solution 
is not correct. We take extra care not to read or modify the command and state 
and other variables in interrupt context. So you CANNOT check do the check in 
your TRX24_RX_END_vect interrupt vector (unless we protect all cmd updates with 
atomic, but we do not want to, since tasklets will do that for us). 

The real problem is in the handling of the interrupt code, in particular in 
line 607: 

    if( irq & IRQ_RX_END )
    {
      ...
      cmd = CMD_DOWNLOAD;
    }

It should not do that, it should check if CMD is NONE, or some other things and 
only then should it proceed to download stuff. Note, that the RF230 drivcer is 
NOT affected with this behavior, there we take pain to check the CMD. 

Can you reproduce the error on the RF230 (or RF212)? 

My proposed change is to do this:

1) if( cmd == NONE && (irq & IRQ_RX_END) != 0 )
2) do some safety checking for if( irq & IRQ_TX_END ) case as well
3) reorganize the three ifs, so that we check first for TX_END, then for 
RX_START and then for RX_END. I did similar thing for RF230, so you could send 
a packet and receive another one handle the interrupt after all these three 
events have occurred.

Here is a link to my proposed changes. Can someone check it?

Miklos

Original comment by mmar...@gmail.com on 13 Mar 2012 at 9:47

[GoogleCodeExporter]

The link is here:

https://github.com/mmaroti/tinyos/commit/05ea35ff59e8dfaff738218042551f75e5bef31
3

Original comment by mmar...@gmail.com on 13 Mar 2012 at 9:48

[GoogleCodeExporter]

> Can you reproduce the error on the RF230 (or RF212)? 

I have not noticed such a behavior with RF230.

Original comment by andres.v...@smartdustsolutions.com on 14 Mar 2012 at 6:01

[GoogleCodeExporter]

Fixed.

Original comment by mmar...@gmail.com on 29 Mar 2012 at 2:38

• Changed state: Fixed

[GoogleCodeExporter]

Can any one provide solution for RF230 in file RF230DriverLayerP.nc

Original comment by rdprav...@iith.ac.in on 20 Apr 2012 at 1:55

[GoogleCodeExporter]

This issue did not affect the RF230 driver.

Original comment by mmar...@gmail.com on 20 Apr 2012 at 3:33