[Tinyos-devel] Bug in

[Jan Hauer]

Yes, it is a bug, but I think the race condition is slightly different
from what you describe (because removing the clrT/RxIntr() lines
doesn't solve the problem for me). The problem is that in the
continueOp() function (in file
tinyos-2.x/tos/chips/msp430/usart/Msp430SpiNoDmaP.nc) two bytes are
written to the U0TXBUF before one is read from the U0RXBUF. This works
at 500Khz, because the time it takes for the second byte to be output
is enough for the first byte to be read from U0RXBUF. At 2Mhz it is
not, i.e. the second byte is moved from the receive-shift register to
U0RXBUF *before* the first byte has been read. This is actually
signalled through the OE (overrun) flag in U0RCTL, but it is never
checked in the code. It can be fixed by first reading U0RXBUF before
sending the second byte (see code snippet below). Also, we might
consider increasing SPI_ATOMIC_SIZE to more than the default value of
2 otherwise the gain of a higher bus clock is (almost) neutralized by
the ISR overhead. Eric, can you please check whether the code below
works for you?

Jan

  void continueOp() {

    uint8_t end;
    uint8_t tmp;

    atomic {
      call Usart.tx( m_tx_buf ? m_tx_buf[ m_pos ] : 0 );

      end = m_pos + SPI_ATOMIC_SIZE;
      if ( end > m_len )
        end = m_len;

      while ( ++m_pos < end ) {
        while( !call Usart.isRxIntrPending() );
        tmp = call Usart.rx();
        if ( m_rx_buf )
          m_rx_buf[ m_pos - 1 ] = tmp;
        call Usart.tx( m_tx_buf ? m_tx_buf[ m_pos ] : 0 );
      }
    }

  }



On Thu, May 15, 2008 at 7:07 PM, Eric Decker <cire831 at gmail.com> wrote:
> in the file tinyos-2.x/tos/chips/msp430/usart/Msp430SpiNoDmaP.nc (current
> CVS repository)
> there are some bugs with how the Usart is being handled in Spi mode with no
> DMA.
>
> Background: The default setup in the tree is to run SMCLK at 1 MHz and the
> CPU clock
> at 4 MHz.  This means the SPI is being clocked at 1 MHz.  The MM3 platform
> (very
> similar to a telosb) has a SD (secure digital flash device) attached to the
> SPI that
> we want to run as fast as possible.  So we are running SMCLK at 4 MHz and
> doing
> a /2 for the SPI.  We adjusted the initilization for the underlying timing
> for the Timers
> so the main timers are still ticking correctly.  (/4).
>
> Now here is the problem.  In continueOp (part of doing SpiPacket.send
> without DMA)
> the following code is explicitly clearing both the tx and rx interrupt
> flags.   Here is what
> is happening:
>
> write first byte out
> wait for the byte to get copied into the tx output shift register (tx ifg is
> asserted)
> clear the tx interrupt flag
> write the next byte to go out.
> wait for rx interrupt flag
> clear rx interupt flag
> read rx byte
>
> If the SPI is fast enough there is a race condition.  The first tx causes
> an rx byte to come in.  If this happens quickly enough (and it does
> with the SPI being clocked at 2 MHz vs. 500KHz) this byte will show
> up prior to the "wait for rx interrupt flag".  Note that the next tx byte
> has already been sent so the next rx byte is also on its way.  If this
> byte arrives prior to the "clear rx interrupt" then the corresponding
> rx interrupt flag will be lost.  This is exactly the behaviour I am seeing.
>
> The reason it works with the SPI being clocked at 500 MHz is it is running
> 4 times slower.
>
> The msp430 code that interacts with rx and tx interrupts shouldn't
> explicitly
> clear the interrupt flags.  The msp430 user manual states that writing to
> TXBUF will automatically clear the TX interrupt flag.  Similarily reading
> RXBUF will clear the RX interrupt flag.  So in the continueOp code there
> is no need to explicitly clear the interrupt flags.
>
> eric
>
>
>   void continueOp() {
>
>     uint8_t end;
>     uint8_t tmp;
>
>     atomic {
>       call Usart.tx( m_tx_buf ? m_tx_buf[ m_pos ] : 0 );
>
>       end = m_pos + SPI_ATOMIC_SIZE;
>       if ( end > m_len )
>         end = m_len;
>
>       while ( ++m_pos < end ) {
>         while( !call Usart.isTxIntrPending() );
>         call Usart.clrTxIntr();                 // writing txbuf
> automatically clears the interrupt.
>         call Usart.tx( m_tx_buf ? m_tx_buf[ m_pos ] : 0 );
>         while( !call Usart.isRxIntrPending() );
>         call Usart.clrRxIntr();                 // reading rxbuf
> automatically clears so this is a problem
>         tmp = call Usart.rx();
>         if ( m_rx_buf )
>           m_rx_buf[ m_pos - 1 ] = tmp;
>       }
>     }
>
>   }
>
>
> _______________________________________________
> Tinyos-devel mailing list
> Tinyos-devel at millennium.berkeley.edu
> https://www.millennium.berkeley.edu/cgi-bin/mailman/listinfo/tinyos-devel
>
>