[Tinyos-devel] Bug in

[Kevin Klues]

Eric, did you get a chance to test this and see if it fixes your
problem?  Vlado will comiit if you can verify this solves your
problems.

Kevin

On Wed, May 21, 2008 at 4:56 AM, Jan Hauer <jan.hauer at gmail.com> wrote:
> 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
>>
>>
> _______________________________________________
> Tinyos-devel mailing list
> Tinyos-devel at millennium.berkeley.edu
> https://www.millennium.berkeley.edu/cgi-bin/mailman/listinfo/tinyos-devel
>



-- 
~Kevin