[Tinyos-devel] Bug in

[Jan Hauer]

now fixed in CVS.
Jan

On Sat, Jun 14, 2008 at 8:40 AM, Eric Decker <cire831 at gmail.com> wrote:
> Code works.
>
> The issue with keeping the uart busy using the cpu stuffing chars is
> difficult.  I'm not sure if it is a problem worth solving.  Using the
> DMA engine works much better.
>
> Anyway the code works as provided by Jan.
>
> thanks Jan.
>
> 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
>> >
>> >
>
>
>
> --
> Eric B. Decker
> Senior (over 50 :-) Researcher
> Autonomous Systems Lab
> Jack Baskin School of Engineering
> UCSC
>