[ECOS] MPC555 serial receive drops bytes

[ECOS] MPC555 serial receive drops bytes

[Steven Clugston]

I've been developing an application which makes use of the mpc555 serial
driver (xxx555_serial_with_ints.c).

I was having problems with my app just hanging after receiving only a
few bytes on either of the two serial ports at 57600 baud.

After fixing this (I'll explain the cause below), I've then also had
problems with bytes going missing.

To test this I've used the serial_echo program and pasted individual
lines of text into hyperterminal/minicom and characters are consistently
lost ant any baud rate above 2400bps, getting worse as the baud rate is
increased.

The following code is the DSR which handles a received byte interrupt:


#define MPC555_SERIAL_SCxSR_ERRORS (MPC555_SERIAL_SCxSR_OR | \
                                    MPC555_SERIAL_SCxSR_NF | \
                                    MPC555_SERIAL_SCxSR_FE | \
                                    MPC555_SERIAL_SCxSR_PF)

static void mpc555_serial_rx_DSR(cyg_vector_t vector, cyg_ucount32
count, cyg_addrword_t data)
{
  serial_channel * chan = (serial_channel *)data;
  mpc555_serial_info * mpc555_chan = (mpc555_serial_info
*)chan->dev_priv;
  cyg_addrword_t port = mpc555_chan->base;
  cyg_uint16 scdr;
  cyg_uint16 scsr;

  // Allways read out the received character, in order to clear receiver
flags
  HAL_READ_UINT16(port + MPC555_SERIAL_SCxDR, scdr);

  HAL_READ_UINT16(port + MPC555_SERIAL_SCxSR, scsr);
  if(scsr & (cyg_uint16)MPC555_SERIAL_SCxSR_ERRORS)
  {
    scsr &= ~((cyg_uint16)MPC555_SERIAL_SCxSR_ERRORS);
    HAL_WRITE_UINT16(port + MPC555_SERIAL_SCxSR, scsr);
  }
  else
  {
    (chan->callbacks->rcv_char)(chan, (cyg_uint8)scdr);
  }

  cyg_drv_interrupt_unmask(mpc555_chan->rx_interrupt_num);
}

In this configuration the mpc555 has only a single byte receive buffer
for which an interrupt is generated each time it is filled. The SCI
status register SCSR has some bits which can flag status info and
errors, specifically Overrun error(NR), Noise error (NF), Framing error
(FE) and Parity error.

Using a hardware debugger I've found that the OR, overrun error bit was
being set.
The code above reads the data register SCDR buffer then reads the status
register, then if any error bits are set, attempts to clear them.

According to the mpc555 user manual, the act of reading the data
register clears or invalidates the status register, and at least some of
the bits are read-only. Attempting to clear these bits (specifically the
overrun bit) was causing a processor execption in my application.

Ok, it was an easy fix to read the data register after the status
register, but is there a way to report read errors back to the
application so that it doesn't fail silently? It might be useful for
example to return the line noise error as an error code unique to that
driver or the serial IO layer, whereas maybe an overrun should be an
assert as this shouldn't really happen.

With the exception fixed, I was still getting overruns so I moved the
code out of the DSR code into the ISR to improve latency. Since its only
moving a byte out of a register, I thought it doesn't really need a DSR.
This improved matters, but the overrun was still occuring.

In the status register there is also an IDLE, or idle line detected bit
which is flagged when there is a gap of one frame in the data stream. It
seems that a first level fix would be to stay in a hard loop in the DSR
until the IDLE bit is set. This seems to be the only way to ensure that
all incomming bytes are read in time. There is still a risk that the DSR
would not be serviced in time to get the first byte out of the buffer
though. The knock-on effect of this is that application code would be
blocked for as long as a continuous stream of serial bytes are being
sent.

The next level fix would require a partial re-implementation of the
driver. One of the two serial channels supports the QSCI, or queued
receive buffer which uses a 16-byte buffer and half-water and full-water
mark interrupts to allow it to be used as two 8-byte buffers. You read
one half whilst the other is being filled until you get the idle line
bit then you read whatever is left over. This would allow normal async
interrupt operation on one of the serial ports. I think this should be
implemented as a cdl option for the serial port that supports it, but
would require differentiation of which port is being accessed at runtime
in the code.

Steven.

--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] MPC555 serial receive drops bytes

[Andrew Lunn]

> With the exception fixed, I was still getting overruns so I moved the
> code out of the DSR code into the ISR to improve latency. Since its only
> moving a byte out of a register, I thought it doesn't really need a DSR.

You have to be very careful here. You cannot call the 

(chan->callbacks->rcv_char)(chan, (cyg_uint8)scdr);

from ISR context. It has to be in DSR context. It will play around
with threads, unblocking readers etc, which can only be done safely in
DSR context.

    Andrew


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

RE: [ECOS] MPC555 serial receive drops bytes

[Steven Clugston]

>> With the exception fixed, I was still getting overruns so I 
>moved the 
>> code out of the DSR code into the ISR to improve latency. Since its 
>> only moving a byte out of a register, I thought it doesn't 
>really need 
>> a DSR.

>
>You have to be very careful here. You cannot call the 
>
>(chan->callbacks->rcv_char)(chan, (cyg_uint8)scdr);
>
>from ISR context. It has to be in DSR context. It will play 
>around with threads, unblocking readers etc, which can only be 
>done safely in DSR context.
>
>    Andrew

This was just something that I naïvely tried just to test if getting the latency down would be enough to fix the problem.
Thanks for pointing out that its not the correct thing to do from that context.

I had in mind that if it fixed the problem, then the ISR could be made to store the received byte somewhere until the DSR was called to process it, rather than waiting for the DSR to lift it out of the data register. As it happens the latency is still too much so this isn't the answer.

I neglected to mention that in Redboot the serial code works much better as you can upload using ymodem etc. 
I've not looked at the Redboot code, but I assume it stays in a very tight polling loop which is why it only occasionally drops bytes.

It seems that the processor is just not fast enough to process serial data in the way that is currently implemented, it needs more or less procedural code to do this without making use of idle line detection.

Just to re-iterate a previous question, is there any mechanism for reporting errors from a DSR? And if not should there be? Also is it considered to be a good or bad practise to have asserts in a DSR to catch unreported errors?

Steven

--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] MPC555 serial receive drops bytes

[Andrew Lunn]

> I had in mind that if it fixed the problem, then the ISR could be
> made to store the received byte somewhere until the DSR was called
> to process it, rather than waiting for the DSR to lift it out of the
> data register. As it happens the latency is still too much so this
> isn't the answer.

Yep, you could add our own buffering in the ISR and then call the DSR
periodically. However, it makes much more sense to use the hardware
receive buffer. It will considerably lower your CPU usage.
 
> Just to re-iterate a previous question, is there any mechanism for
> reporting errors from a DSR? 

Not errors. You can indicate flow control conditions with the callback
serial_indicate_status(). I've never used it, don't know how it works,
so i would suggest seeing if there is a driver which makes use of
this.

> Also is it considered to be a good or bad practise to have asserts
> in a DSR to catch unreported errors?

Asserts in general in DSR are O.K. Asserts are always fatal, so it
does not matter if calling an assert destroys the stack, corrupts
threads etc. You are dead, so shooting yourself in the foot as you die
makes little difference.

However, is an assert the correct thing to do? Is the error fatal?
Dropping a few characters is not fatal. It happens. The protocol
layers running above have to expect this and deal with it. So i would
not assert on an overrun error, a parity error, etc.

    Andrew

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

[ECOS] Re: MPC555 serial receive drops bytes

[Alex Upchurch]

Steven Clugston wrote:
> I've been developing an application which makes use of the mpc555 serial
> driver (xxx555_serial_with_ints.c).
> 
> ....
> 
> With the exception fixed, I was still getting overruns so I moved the
> code out of the DSR code into the ISR to improve latency. Since its only
> moving a byte out of a register, I thought it doesn't really need a DSR.
> This improved matters, but the overrun was still occuring.
> 
This sounds like your ISR code can take more than one character time to 
execute. Assuming 8-N-1 data format, a 57600 baudrate works out to 173.6 
microseconds per character. You don't say what speed your CPU is 
running, but I suspect 173.6 uS works out to a *lot* of CPU 
instructions. You should not need to sit in the ISR waiting for the line 
to go idle.

I had the same problem with the Coldfire serial driver. The original 
driver simply masked out the interrupt and called the DSR. That worked 
great until you got some other DSR taking more than one character time 
to execute. The serial DSRs got posted, but they didn't run in time. The 
solution for my driver was to move the data in/out of the UART to a 
circular buffer in the ISR.

I suspect this issue may occur on other platforms as well.

Alex.

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

[ECOS] Re: MPC555 serial receive drops bytes

[Steven Clugston]

>This sounds like your ISR code can take more than one 
>character time to 
>execute. Assuming 8-N-1 data format, a 57600 baudrate works 
>out to 173.6 
>microseconds per character. You don't say what speed your CPU is 
>running, but I suspect 173.6 uS works out to a *lot* of CPU 
>instructions. 

The board is 40MHz which for 173.6 uS corresponds to 6944 instructions
assuming one instruction per clock cycle.
The board is really quite modest when running just the serial_echo test,
the only thing (that I can think of) that's running periodically in at
the ISR/DSR level is the wallclock and serial Rx and Tx ISRs and DSRs.

Thinking about this though, the Tx DSR does sit and try to clear the
send buffer into the hardware register which would make it take longer
than the Rx DSR. It usually only manages to transfer one or two bytes at
a time before giving up and waiting for the next ready to send
interrupt.
If the Tx DSR was blocking the Rx DSR from running in time then I guess
this would show up worst in an serial echo situation which is what I'm
seeing.
I think this warrants further investigation.

>You should not need to sit in the ISR waiting 
>for the line 
>to go idle.

I was thinking to do this in the DSR and not the ISR, but I think the
ISR circular buffer is better.

>I had the same problem with the Coldfire serial driver. The original 
>driver simply masked out the interrupt and called the DSR. That worked 
>great until you got some other DSR taking more than one character time 
>to execute. The serial DSRs got posted, but they didn't run in 
>time. The 
>solution for my driver was to move the data in/out of the UART to a 
>circular buffer in the ISR.
>
>I suspect this issue may occur on other platforms as well.
>
>Alex.

I've had a quick look at coldfire mcf5272_serial.c in cvs which still
does as you say the original driver does.
Does this mean then that this is a potential problem will all of the
serial drivers, particularly the ones which have only a single character
buffer?

I notice also that in the same file there is:

#ifdef CYGOPT_IO_SERIAL_SUPPORT_LINE_STATUS
 // Overrun error
 if (usr_ucsr & MCF5272_UART_USR_OE)
 {
   stat.which = CYGNUM_SERIAL_STATUS_OVERRUNERR;
   (chan->callbacks->indicate_status)(chan, &stat);
 }
#endif

And in packages/io/serial/include/serialio.h
#ifdef CYGOPT_IO_SERIAL_SUPPORT_LINE_STATUS

# define CYGNUM_SERIAL_STATUS_FLOW          0
# define CYGNUM_SERIAL_STATUS_BREAK         1
# define CYGNUM_SERIAL_STATUS_FRAMEERR      2
# define CYGNUM_SERIAL_STATUS_PARITYERR     3
# define CYGNUM_SERIAL_STATUS_OVERRUNERR    4
# define CYGNUM_SERIAL_STATUS_CARRIERDETECT 5
# define CYGNUM_SERIAL_STATUS_RINGINDICATOR 6


This is the error reporting mechanism I was after.

In io_serial.cdl there is the cdl option
CYGOPT_IO_SERIAL_SUPPORT_LINE_STATUS :

requires      { CYGINT_IO_SERIAL_LINE_STATUS_HW > 0 }
"This option indicates that if the serial driver supports it,
 serial line status and modem status information should be
 propagated to higher layers via callbacks."

And in mcf5272_serial.cdl:

implements    CYGINT_IO_SERIAL_LINE_STATUS_HW

So I know how to do this from the driver side, but does anybody know how
this gets reported to "higher layers" and if it currently works?
I can't find any API documentation for this. Ideally I would like the
cyg_io_read() to return with a spcific error code or at least something
that tells me I need to do a cyg_io_get_config() to find out what
happened at the app level.

Steven



--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

[ECOS] Re: Re: MPC555 serial receive drops bytes

[Alex Upchurch]

Steven Clugston wrote:
> I've had a quick look at coldfire mcf5272_serial.c in cvs which still
> does as you say the original driver does.
> Does this mean then that this is a potential problem will all of the
> serial drivers, particularly the ones which have only a single character
> buffer?
Any driver ISR that defers servicing the hardware to the DSR level is 
implicitly assuming that the DSR will run before another external event 
makes the current data invalid. Even FIFO equipped hardware will get 
bitten if the DSR doesn't run in time.

> 
> And in mcf5272_serial.cdl:
> 
> implements    CYGINT_IO_SERIAL_LINE_STATUS_HW
> 
> So I know how to do this from the driver side, but does anybody know how
> this gets reported to "higher layers" and if it currently works?
> I can't find any API documentation for this. Ideally I would like the
> cyg_io_read() to return with a spcific error code or at least something
> that tells me I need to do a cyg_io_get_config() to find out what
> happened at the app level.
As I recall, when I was investigating my problem I just modified the 
driver to make the driver variables visible at the application level. In 
the end I removed the eCos serial drivers and wrote my own application 
level drivers.

Alex.

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss