2002-04-12 15:35:21

by Guennadi Liakhovetski

[permalink] [raw]
Subject: serial driver question

(asking on kernelnewbies didn't produceany results, so, I'm protected:-))

Hello all

The function
static int size_fifo(struct async_struct *info)
{
...
ends as follows:
serial_outp(info, UART_LCR, UART_LCR_DLAB);
serial_outp(info, UART_DLL, old_dll);
serial_outp(info, UART_DLM, old_dlm);

return count;
}

Which means, that DLAB is not re-set, and, in particular, all subsequent
read/write operations on offsets 0 and 1 will not affect the data and
interrupt enable registers, but the divisor latch register... Or is this
register somehow magically restored elsewhere or by the hardware (say, on
an interrupt)? This function seems to be only called for startech UARTs.

Thanks
Guennadi
---------------------------------
Guennadi Liakhovetski, Ph.D.
DSA Daten- und Systemtechnik GmbH
Pascalstr. 28
D-52076 Aachen
Germany



2002-04-12 15:54:16

by Ed Vance

[permalink] [raw]
Subject: RE: serial driver question

Guennadi Liakhovetski wrote:
> Hello all
>
> The function
> static int size_fifo(struct async_struct *info)
> {
> ...
> ends as follows:
> serial_outp(info, UART_LCR, UART_LCR_DLAB);
> serial_outp(info, UART_DLL, old_dll);
> serial_outp(info, UART_DLM, old_dlm);
>
> return count;
> }
>
> Which means, that DLAB is not re-set, and, in particular, all
> subsequent read/write operations on offsets 0 and 1 will not
> affect the data and interrupt enable registers, but the divisor
> latch register... Or is this register somehow magically restored
> elsewhere or by the hardware (say, on an interrupt)? This
> function seems to be only called for startech UARTs.

Hi Guennadi,

I'll look at it as soon as my system is up. It's morning boot-up time here.
Have a good evening.

Ed Vance

----------------------------------------------------------------
Ed Vance [email protected]
Macrolink, Inc. 1500 N. Kellogg Dr Anaheim, CA 92807
----------------------------------------------------------------

2002-04-12 21:54:39

by Ed Vance

[permalink] [raw]
Subject: RE: serial driver question

Guennadi Liakhovetski wrote:
> Hello all
>
> The function
> static int size_fifo(struct async_struct *info)
> {
> ...
> ends as follows:
> serial_outp(info, UART_LCR, UART_LCR_DLAB);
> serial_outp(info, UART_DLL, old_dll);
> serial_outp(info, UART_DLM, old_dlm);
>
> return count;
> }
>
> Which means, that DLAB is not re-set, and, in particular, all
> subsequent read/write operations on offsets 0 and 1 will not
> affect the data and interrupt enable registers, but the divisor
> latch register... Or is this register somehow magically restored
> elsewhere or by the hardware (say, on an interrupt)? This
> function seems to be only called for startech UARTs.

Hi Guennadi,

It is somehow magically restored elsewhere. The divisor latch bit (DLAB)
does remain set when size_fifo() returns, but the state does not persist.
Here is why the code works anyway, albeit with a trap or two for the
insufficiently paranoid ... and it can be explained without magic. :)

Notice that function size_fifo() does not save the LCR value before changing
it. Static function size_fifo() is called only from the very end of static
function autoconfig_startech_uarts(). Notice that function
autoconfig_startech_uarts() never returns with state->type set to
PORT_16550A. All code paths change state->type to something else. Static
function autoconfig_startech_uarts() is only called from the middle of
static function autoconfig(). Function autoconfig() saves the LCR value
before the testing of the alleged UART, which may include calling
autoconfig_startech_uarts(). When autoconfig_startech_uarts() returns to
autoconfig(), state->type cannot be PORT_16550A so the bodies of the next
two "if" statements that check state->type are skipped. The next line of
UART touching code that runs after size_fifo() and
autoconfig_startech_uarts() return to autoconfig(), is at about 50 lines
after the call to autoconfig_startech_uarts(), just before the check for
state->type == PORT_16450:

serial_outp(info, UART_LCR, save_lcr);

which restores autoconfig()'s saved LCR value, including the DLAB bit.

Yeah, I agree that size_fifo() should clean up after itself, but it doesn't
break anything as the code currently sits. Care to submit a one-line patch
for size_fifo() to clean up the UART state?

Best regards,
Ed Vance

----------------------------------------------------------------
Ed Vance [email protected]
Macrolink, Inc. 1500 N. Kellogg Dr Anaheim, CA 92807
----------------------------------------------------------------