[Bug 1001480] New: Generic 16x5x driver: inverted chronology inpc_serial_start_xmit(): superfluous ISR/DSR run

[bugzilla-daemon at bugs dot ecos dot sourceware dot org]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001480

           Summary: Generic 16x5x driver: inverted chronology in
                    pc_serial_start_xmit(): superfluous ISR/DSR run
           Product: eCos
           Version: CVS
          Platform: All
        OS/Version: Other
            Status: UNCONFIRMED
          Severity: normal
          Priority: low
         Component: Serial
        AssignedTo: unassigned@bugs.ecos.sourceware.org
        ReportedBy: bernard.fouche@kuantic.com
                CC: ecos-bugs@ecos.sourceware.org
             Class: Advice Request


Created an attachment (id=1569)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1569)
logic analyzer screenshot demonstrating the issue

pc_serial_start_xmit() first setups the TX interrupt and then calls the
xmt_char() callback that feeds the TX FIFO. This sequence makes the driver to
fire useless ISR/DSR sequences if the TX FIFO was empty when start_xmit() is
called.

Here is the function code:

// Enable the transmitter on the device
static void
pc_serial_start_xmit(serial_channel *chan)
{
    pc_serial_info *ser_chan = (pc_serial_info *)chan->dev_priv;
    cyg_addrword_t base = ser_chan->base;
    cyg_uint8 _ier;

    HAL_READ_UINT8(base+REG_ier, _ier);
    _ier |= IER_XMT;                    // Enable xmit interrupt
    HAL_WRITE_UINT8(base+REG_ier, _ier);
#ifdef CYGPKG_IO_SERIAL_GENERIC_16X5X_XMIT_REQUIRE_PRIME
    (chan->callbacks->xmt_char)(chan);
#endif
}

The problem is:

- as soon as the TX interrupt is activated, if the FIFO is empty at that time,
then an interrupt fires immediately. DSR is posted.

- in start_xmit(), following TX interrupt activation, xmt_char() is called, it
calls pc_serial_putc(), TX FIFO is fed, at least a byte starts to be clocked
out on the serial link.

- DSR runs, and read the register that tells it why an interrupt has been
fired. But at that time the TX FIFO isn't empty anymore: DSR does nothing.

This is demonstrated by the attached screen shot done with a logic analyzer:

- channel 0: ISR: a GPIO pin is set a low level when entering ISR, to a high
level when ISR exits.
- channel 1: a GPIO pin changes of state each time the DSR runs without doing
any processing.
- channel 2: DSR: a GPIO pin is set a low level when entering DSR, to a high
level when DSR exits.
- channel 3: start_xmit: a GPIO pin is set a low level when entering
start_xmit(), to a high level when start_xmit() exits.
- the other channels show serial link activity (RTS/TX/RX/CTS)
- green marker 1: set to start_xmit() beginning: nearly immediately, ISR is
fired (there is very small delay between start_xmit() and ISR entry).
- green marker 2: set to end of start_xmit(): a byte starts to be clocked out
on TXD1.

Channel 5 shows that DSR is called when the first TX byte is being output.
Channel 1 shows that DSR did nothing.

target configuration: MCU runs at 16MHz, serial is at 115200bps, characters are
output using repeated calls to fputc(), hence the delay between two TX bytes.

Solution:

inverse interrupt setup and call to xmt_char:

// Enable the transmitter on the device
static void
pc_serial_start_xmit(serial_channel *chan)
{
    pc_serial_info *ser_chan = (pc_serial_info *)chan->dev_priv;
    cyg_addrword_t base = ser_chan->base;
    cyg_uint8 _ier;

#ifdef CYGPKG_IO_SERIAL_GENERIC_16X5X_XMIT_REQUIRE_PRIME
    (chan->callbacks->xmt_char)(chan);
#endif
    HAL_READ_UINT8(base+REG_ier, _ier);
    _ier |= IER_XMT;                    // Enable xmit interrupt
    HAL_WRITE_UINT8(base+REG_ier, _ier);
}

In that case the byte(s) to send start to be clocked out, but the TX FIFO isn't
empty when the TX interrupt is setup: no interrupt will be fired until the FIFO
is empty.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.