- Add synchronization on input and output buffers - Respect timeouts on IRP_MJ_READ - Get right buffer in read/write routines Modified: trunk/reactos/drivers/dd/serial/devctrl.c Modified: trunk/reactos/drivers/dd/serial/misc.c Modified: trunk/reactos/drivers/dd/serial/pnp.c Modified: trunk/reactos/drivers/dd/serial/rw.c Modified: trunk/reactos/drivers/dd/serial/serial.h _____
Modified: trunk/reactos/drivers/dd/serial/devctrl.c --- trunk/reactos/drivers/dd/serial/devctrl.c 2005-03-23 22:11:20 UTC (rev 14296) +++ trunk/reactos/drivers/dd/serial/devctrl.c 2005-03-24 07:50:41 UTC (rev 14297) @@ -215,14 +215,23 @@
OUT PSERIAL_STATUS pSerialStatus, IN PSERIAL_DEVICE_EXTENSION DeviceExtension) { + KIRQL Irql; + RtlZeroMemory(pSerialStatus, sizeof(SERIAL_STATUS)); pSerialStatus->Errors = 0; /* FIXME */ pSerialStatus->HoldReasons = 0; /* FIXME */ + + KeAcquireSpinLock(&DeviceExtension->InputBufferLock, &Irql); pSerialStatus->AmountInInQueue = (DeviceExtension->InputBuffer.WritePosition + DeviceExtension->InputBuffer.Length - DeviceExtension->InputBuffer.ReadPosition) % DeviceExtension->InputBuffer.Length; + KeReleaseSpinLock(&DeviceExtension->InputBufferLock, Irql); + + KeAcquireSpinLock(&DeviceExtension->OutputBufferLock, &Irql); pSerialStatus->AmountInOutQueue = (DeviceExtension->OutputBuffer.WritePosition + DeviceExtension->OutputBuffer.Length - DeviceExtension->OutputBuffer.ReadPosition) % DeviceExtension->OutputBuffer.Length; + KeReleaseSpinLock(&DeviceExtension->OutputBufferLock, Irql); + pSerialStatus->EofReceived = FALSE; /* FIXME */ pSerialStatus->WaitForImmediate = FALSE; /* FIXME */ @@ -497,8 +506,10 @@ } case IOCTL_SERIAL_PURGE: { + KIRQL Irql1, Irql2; DPRINT("Serial: IOCTL_SERIAL_PURGE\n"); - /* FIXME: lock input and output queues */ + KeAcquireSpinLock(&DeviceExtension->InputBufferLock, &Irql1); + KeAcquireSpinLock(&DeviceExtension->OutputBufferLock, &Irql2); DeviceExtension->InputBuffer.ReadPosition = DeviceExtension->InputBuffer.WritePosition = 0; DeviceExtension->OutputBuffer.ReadPosition = DeviceExtension->OutputBuffer.WritePosition = 0; /* Clear receive/transmit buffers */ @@ -507,7 +518,8 @@ WRITE_PORT_UCHAR(SER_FCR(ComPortBase), SR_FCR_CLEAR_RCVR | SR_FCR_CLEAR_XMIT); } - /* FIXME: unlock input and output queues */ + KeReleaseSpinLock(&DeviceExtension->OutputBufferLock, Irql2); + KeReleaseSpinLock(&DeviceExtension->InputBufferLock, Irql1); Status = STATUS_SUCCESS; break; } @@ -618,18 +630,19 @@ return STATUS_INVALID_PARAMETER; else { + KIRQL Irql; Status = STATUS_SUCCESS; if (((PSERIAL_QUEUE_SIZE)Buffer)->InSize
DeviceExtension->InputBuffer.Length)
{ - /* FIXME: lock input queue */ + KeAcquireSpinLock(&DeviceExtension->InputBufferLock, &Irql); Status = IncreaseCircularBufferSize(&DeviceExtension->InputBuffer, ((PSERIAL_QUEUE_SIZE)Buffer)->InSize); - /* FIXME: unlock input queue */ + KeReleaseSpinLock(&DeviceExtension->InputBufferLock, Irql); } if (NT_SUCCESS(Status) && ((PSERIAL_QUEUE_SIZE)Buffer)->OutSize > DeviceExtension->OutputBuffer.Length) { - /* FIXME: lock output queue */ + KeAcquireSpinLock(&DeviceExtension->OutputBufferLock, &Irql); Status = IncreaseCircularBufferSize(&DeviceExtension->OutputBuffer, ((PSERIAL_QUEUE_SIZE)Buffer)->OutSize); - /* FIXME: unlock output queue */ + KeReleaseSpinLock(&DeviceExtension->OutputBufferLock, Irql); } } break; _____
Modified: trunk/reactos/drivers/dd/serial/misc.c --- trunk/reactos/drivers/dd/serial/misc.c 2005-03-23 22:11:20 UTC (rev 14296) +++ trunk/reactos/drivers/dd/serial/misc.c 2005-03-24 07:50:41 UTC (rev 14297) @@ -60,6 +60,67 @@
return IoCallDriver(LowerDevice, Irp); }
+VOID STDCALL +SerialReceiveByte( + IN PKDPC Dpc, + IN PVOID pDeviceExtension, // real type PSERIAL_DEVICE_EXTENSION + IN PVOID pByte, // real type UCHAR + IN PVOID Unused) +{ + PSERIAL_DEVICE_EXTENSION DeviceExtension; + UCHAR Byte; + KIRQL Irql; + NTSTATUS Status; + + DeviceExtension = (PSERIAL_DEVICE_EXTENSION)pDeviceExtension; + Byte = (UCHAR)(ULONG_PTR)pByte; + DPRINT1("Serial: received byte on COM%lu: 0x%02x (%c)\n", + DeviceExtension->ComPort, Byte, Byte); + + KeAcquireSpinLock(&DeviceExtension->InputBufferLock, &Irql); + Status = PushCircularBufferEntry(&DeviceExtension->InputBuffer, Byte); + if (!NT_SUCCESS(Status)) + { + /* FIXME: count buffer overflow */ + return; + } + DPRINT1("Serial: push to buffer done\n"); + KeReleaseSpinLock(&DeviceExtension->InputBufferLock, Irql); + InterlockedIncrement(&DeviceExtension->SerialPerfStats.ReceivedCount); +} + +VOID STDCALL +SerialSendByte( + IN PKDPC Dpc, + IN PVOID pDeviceExtension, // real type PSERIAL_DEVICE_EXTENSION + IN PVOID Unused1, + IN PVOID Unused2) +{ + PSERIAL_DEVICE_EXTENSION DeviceExtension; + PUCHAR ComPortBase; + UCHAR Byte; + KIRQL Irql; + NTSTATUS Status; + + DeviceExtension = (PSERIAL_DEVICE_EXTENSION)pDeviceExtension; + ComPortBase = (PUCHAR)DeviceExtension->BaseAddress; + + DPRINT1("Serial: sending bytes (if any) on COM%lu\n", + DeviceExtension->ComPort); + + KeAcquireSpinLock(&DeviceExtension->OutputBufferLock, &Irql); + while (!IsCircularBufferEmpty(&DeviceExtension->OutputBuffer) + && READ_PORT_UCHAR(SER_LSR(ComPortBase)) & SR_LSR_TBE) + { + Status = PopCircularBufferEntry(&DeviceExtension->OutputBuffer, &Byte); + if (!NT_SUCCESS(Status)) + break; + WRITE_PORT_UCHAR(SER_THR(ComPortBase), Byte); + DeviceExtension->SerialPerfStats.TransmittedCount++; + } + KeReleaseSpinLock(&DeviceExtension->OutputBufferLock, Irql); +} + BOOLEAN STDCALL SerialInterruptService( IN PKINTERRUPT Interrupt, @@ -70,7 +131,6 @@ UCHAR Byte; PUCHAR ComPortBase; UCHAR Iir; - NTSTATUS Status; DeviceObject = (PDEVICE_OBJECT)ServiceContext; DeviceExtension = (PSERIAL_DEVICE_EXTENSION)DeviceObject->DeviceExtension; @@ -96,18 +156,7 @@ case SR_IIR_THR_EMPTY: { DPRINT("Serial: SR_IIR_THR_EMPTY\n"); - /* FIXME: lock OutputBuffer */ - while (!IsCircularBufferEmpty(&DeviceExtension->OutputBuffer) - && READ_PORT_UCHAR(SER_LSR(ComPortBase)) & SR_LSR_TBE) - { - Status = PopCircularBufferEntry(&DeviceExtension->OutputBuffer, &Byte); - if (!NT_SUCCESS(Status)) - break; - WRITE_PORT_UCHAR(SER_THR(ComPortBase), Byte); - DeviceExtension->SerialPerfStats.TransmittedCount++; - } - /* FIXME: unlock OutputBuffer */ - return TRUE; + return KeInsertQueueDpc(&DeviceExtension->SendByteDpc, NULL, NULL); } case SR_IIR_DATA_RECEIVED: { @@ -116,19 +165,10 @@ { Byte = READ_PORT_UCHAR(SER_RBR(ComPortBase)); DPRINT1("Serial: Byte received: 0x%02x (%c)\n", Byte, Byte); - /* FIXME: lock InputBuffer */ - Status = PushCircularBufferEntry(&DeviceExtension->InputBuffer, Byte); - if (!NT_SUCCESS(Status)) - { - /* FIXME: count buffer overflow */ + if (!KeInsertQueueDpc(&DeviceExtension->ReceivedByteDpc, (PVOID)(ULONG_PTR)Byte, NULL)) break; - } - DPRINT1("Serial: push to buffer done\n"); - /* FIXME: unlock InputBuffer */ - DeviceExtension->SerialPerfStats.ReceivedCount++; } return TRUE; - break; } case SR_IIR_ERROR: { _____
Modified: trunk/reactos/drivers/dd/serial/pnp.c --- trunk/reactos/drivers/dd/serial/pnp.c 2005-03-23 22:11:20 UTC (rev 14296) +++ trunk/reactos/drivers/dd/serial/pnp.c 2005-03-24 07:50:41 UTC (rev 14297) @@ -76,6 +76,10 @@
Status = InitializeCircularBuffer(&DeviceExtension->OutputBuffer, 16); if (!NT_SUCCESS(Status)) goto ByeBye; IoInitializeRemoveLock(&DeviceExtension->RemoveLock, SERIAL_TAG, 0, 0); + KeInitializeSpinLock(&DeviceExtension->InputBufferLock); + KeInitializeSpinLock(&DeviceExtension->OutputBufferLock); + KeInitializeDpc(&DeviceExtension->ReceivedByteDpc, SerialReceiveByte, DeviceExtension); + KeInitializeDpc(&DeviceExtension->SendByteDpc, SerialSendByte, DeviceExtension); //Fdo->Flags |= DO_POWER_PAGEABLE (or DO_POWER_INRUSH?) Status = IoAttachDeviceToDeviceStackSafe(Fdo, Pdo, &DeviceExtension->LowerDevice); if (!NT_SUCCESS(Status)) @@ -152,13 +156,7 @@ DeviceExtension = (PSERIAL_DEVICE_EXTENSION)DeviceObject->DeviceExtension; - /* FIXME: actually, IRP_MN_START_DEVICE is sent twice to each serial device: - * - one when loading serial.sys - * - one when loading attached upper filter serenum.sys - * This behaviour MUST NOT exist. - * As PnP handling isn't right anyway, I didn't search how to correct this. - */ - if (DeviceExtension->PnpState == dsStarted) return STATUS_SUCCESS; + ASSERT(DeviceExtension->PnpState == dsStopped); DeviceExtension->ComPort = DeviceExtension->SerialPortNumber + 1; DeviceExtension->BaudRate = 19200 | SERIAL_BAUD_USER; _____
Modified: trunk/reactos/drivers/dd/serial/rw.c --- trunk/reactos/drivers/dd/serial/rw.c 2005-03-23 22:11:20 UTC (rev 14296) +++ trunk/reactos/drivers/dd/serial/rw.c 2005-03-24 07:50:41 UTC (rev 14297) @@ -16,12 +16,8 @@
SerialGetUserBuffer(IN PIRP Irp) { ASSERT(Irp); - - if (Irp->MdlAddress) - return MmGetSystemAddressForMdlSafe(Irp->MdlAddress, NormalPagePriority); - else - /* FIXME: try buffer */ - return Irp->UserBuffer; + + return Irp->AssociatedIrp.SystemBuffer; }
NTSTATUS STDCALL @@ -36,6 +32,7 @@ PUCHAR Buffer; PUCHAR ComPortBase; UCHAR ReceivedByte; + KIRQL Irql; NTSTATUS Status = STATUS_SUCCESS; DPRINT("Serial: IRP_MJ_READ\n"); @@ -64,8 +61,8 @@ if (!NT_SUCCESS(Status)) goto ByeBye; - /* FIXME: lock InputBuffer */ - while (Length-- > 0 && !IsCircularBufferEmpty(&DeviceExtension->InputBuffer)) + KeAcquireSpinLock(&DeviceExtension->InputBufferLock, &Irql); + while (Length-- > 0) { Status = PopCircularBufferEntry(&DeviceExtension->InputBuffer, &ReceivedByte); if (!NT_SUCCESS(Status)) @@ -73,54 +70,117 @@ DPRINT("Serial: read from buffer 0x%x (%c)\n", ReceivedByte, ReceivedByte); Buffer[Information++] = ReceivedByte; } + KeReleaseSpinLock(&DeviceExtension->InputBufferLock, Irql); if (Length > 0 && !(DeviceExtension->SerialTimeOuts.ReadIntervalTimeout == INFINITE &&
DeviceExtension->SerialTimeOuts.ReadTotalTimeoutConstant == 0 &&
DeviceExtension->SerialTimeOuts.ReadTotalTimeoutMultiplier == 0)) { - if (DeviceExtension->SerialTimeOuts.ReadIntervalTimeout == 0 - || DeviceExtension->SerialTimeOuts.ReadTotalTimeoutMultiplier == 0) + ULONG IntervalTimeout; + ULONG TotalTimeout; + BOOLEAN UseIntervalTimeout = FALSE; + BOOLEAN UseTotalTimeout = FALSE; + ULONG ThisByteTimeout; + BOOLEAN IsByteReceived; + ULONG i; + /* Extract timeouts informations */ + if (DeviceExtension->SerialTimeOuts.ReadIntervalTimeout == INFINITE && + DeviceExtension->SerialTimeOuts.ReadTotalTimeoutMultiplier == INFINITE && + DeviceExtension->SerialTimeOuts.ReadTotalTimeoutConstant > 0 && + DeviceExtension->SerialTimeOuts.ReadTotalTimeoutConstant < INFINITE) { - DPRINT("Serial: we must wait for %lu characters!\n", Length); -#if 1 - /* Disable interrupts */ - WRITE_PORT_UCHAR(SER_IER((PUCHAR)DeviceExtension->BaseAddress), DeviceExtension->IER & ~1); - - /* Polling code */ - while (Length > 0) + if (Information > 0) { - while ((READ_PORT_UCHAR(SER_LSR(ComPortBase)) & SR_LSR_DR) == 0) - ; - ReceivedByte = READ_PORT_UCHAR(SER_RBR(ComPortBase)); - Buffer[Information++] = ReceivedByte; - Length--; + /* don't read mode bytes */ + Length = 0; } - /* Enable interrupts */ - WRITE_PORT_UCHAR(SER_IER((PUCHAR)DeviceExtension->BaseAddress), DeviceExtension->IER); -#else - while (Length > 0) + else { - if (!IsCircularBufferEmpty(&DeviceExtension->InputBuffer)) + /* read only one byte */ + UseTotalTimeout = TRUE; + TotalTimeout = DeviceExtension->SerialTimeOuts.ReadTotalTimeoutConstant; + Length = 1; + } + } + else + { + if (DeviceExtension->SerialTimeOuts.ReadIntervalTimeout != 0) + { + UseIntervalTimeout = TRUE; + IntervalTimeout = DeviceExtension->SerialTimeOuts.ReadIntervalTimeout; + } + if (DeviceExtension->SerialTimeOuts.ReadTotalTimeoutConstant != 0 || + DeviceExtension->SerialTimeOuts.ReadTotalTimeoutMultiplier != 0) + { + UseTotalTimeout = TRUE; + TotalTimeout = DeviceExtension->SerialTimeOuts.ReadTotalTimeoutConstant + + DeviceExtension->SerialTimeOuts.ReadTotalTimeoutMultiplier * Length; + } + } + DPRINT("Serial: UseIntervalTimeout = %ws, IntervalTimeout = %lu\n", + UseIntervalTimeout ? L"YES" : L"NO", + UseIntervalTimeout ? IntervalTimeout : 0); + DPRINT("Serial: UseTotalTimeout = %ws, TotalTimeout = %lu\n", + UseTotalTimeout ? L"YES" : L"NO", + UseTotalTimeout ? TotalTimeout : 0); + + /* FIXME: it should be better to use input buffer instead of + * disabling interrupts, and try to directly read for port! */ + + /* FIXME: NtQueryPerformanceCounter gives a more accurate + * timer, but it is not available on all computers. First try + * NtQueryPerformanceCounter, and current method if it is not + * implemented. */ + + /* FIXME: remove disabling interrupts */ + WRITE_PORT_UCHAR(SER_IER(ComPortBase), DeviceExtension->IER & ~1); + while (Length > 0) + { + ThisByteTimeout = IntervalTimeout; + IsByteReceived = FALSE; + while (TRUE) + { + for (i = 0; i < 1000; i++) { - Status = PopCircularBufferEntry(&DeviceExtension->InputBuffer, &ReceivedByte); - if (!NT_SUCCESS(Status)) +#if 1 + if ((READ_PORT_UCHAR(SER_LSR(ComPortBase)) & SR_LSR_DR) != 0) + { + ReceivedByte = READ_PORT_UCHAR(ComPortBase); + DPRINT("Serial: received byte 0x%02x (%c)\n", ReceivedByte, ReceivedByte); + IsByteReceived = TRUE; break; - DPRINT1("Serial: read from buffer 0x%x (%c)\n", ReceivedByte, ReceivedByte); - Buffer[Information++] = ReceivedByte; - Length--; + } +#else + KeAcquireSpinLock(&DeviceExtension->InputBufferLock, &Irql); + if (!IsCircularBufferEmpty(&DeviceExtension->InputBuffer)) + { + PopCircularBufferEntry(&DeviceExtension->InputBuffer, &ReceivedByte); + KeReleaseSpinLock(&DeviceExtension->InputBufferLock, Irql); + DPRINT("Serial: reading byte from buffer 0x%02x (%c)\n", ReceivedByte, ReceivedByte); + IsByteReceived = TRUE; + break; + } + KeReleaseSpinLock(&DeviceExtension->InputBufferLock, Irql); +#endif + KeStallExecutionProcessor(1); } + if (IsByteReceived) break; + if (UseIntervalTimeout) + { + if (ThisByteTimeout == 0) break; else ThisByteTimeout--; + } + if (UseTotalTimeout) + { + if (TotalTimeout == 0) break; else TotalTimeout--; + } } -#endif + if (!IsByteReceived) break; + Buffer[Information++] = ReceivedByte; + Length--; } - else - { - /* FIXME: use ReadTotalTimeoutMultiplier and ReadTotalTimeoutConstant */ - DPRINT1("Serial: we must wait for %lu characters at maximum within %lu milliseconds! UNIMPLEMENTED\n", - Length, - Stack->Parameters.Read.Length * DeviceExtension->SerialTimeOuts.ReadTotalTimeoutMultiplier + DeviceExtension->SerialTimeOuts.ReadTotalTimeoutConstant); - } + /* FIXME: remove enabling interrupts */ + WRITE_PORT_UCHAR(SER_IER(ComPortBase), DeviceExtension->IER); } - /* FIXME: unlock InputBuffer */ Status = STATUS_SUCCESS; IoReleaseRemoveLock(&DeviceExtension->RemoveLock, (PVOID)DeviceExtension->ComPort); @@ -143,6 +203,7 @@ ULONG Information = 0; PUCHAR Buffer; PUCHAR ComPortBase; + KIRQL Irql; NTSTATUS Status = STATUS_SUCCESS; DPRINT("Serial: IRP_MJ_WRITE\n"); @@ -166,7 +227,7 @@ if (!NT_SUCCESS(Status)) goto ByeBye; - /* FIXME: lock OutputBuffer */ + KeAcquireSpinLock(&DeviceExtension->OutputBufferLock, &Irql); if (IsCircularBufferEmpty(&DeviceExtension->OutputBuffer)) { /* Put the maximum amount of data in UART output buffer */ @@ -197,7 +258,7 @@ DPRINT1("Serial: write to buffer 0x%02x\n", Buffer[Information]); Information++; } - /* FIXME: unlock OutputBuffer */ + KeReleaseSpinLock(&DeviceExtension->OutputBufferLock, Irql); IoReleaseRemoveLock(&DeviceExtension->RemoveLock, (PVOID)DeviceExtension->ComPort);
ByeBye: _____
Modified: trunk/reactos/drivers/dd/serial/serial.h --- trunk/reactos/drivers/dd/serial/serial.h 2005-03-23 22:11:20 UTC (rev 14296) +++ trunk/reactos/drivers/dd/serial/serial.h 2005-03-24 07:50:41 UTC (rev 14297) @@ -81,6 +81,8 @@
ULONG BaudRate; ULONG BaseAddress; PKINTERRUPT Interrupt; + KDPC ReceivedByteDpc; + KDPC SendByteDpc; SERIAL_LINE_CONTROL SerialLineControl; UART_TYPE UartType; @@ -90,7 +92,9 @@ SERIAL_TIMEOUTS SerialTimeOuts; BOOLEAN IsOpened; CIRCULAR_BUFFER InputBuffer; + KSPIN_LOCK InputBufferLock; CIRCULAR_BUFFER OutputBuffer; + KSPIN_LOCK OutputBufferLock; /* Current values */ UCHAR IER; /* Base+1, Interrupt Enable Register */ @@ -250,6 +254,20 @@ IN PDEVICE_OBJECT DeviceObject, IN PIRP Irp);
+VOID STDCALL +SerialReceiveByte( + IN PKDPC Dpc, + IN PVOID pDeviceExtension, // real type PSERIAL_DEVICE_EXTENSION + IN PVOID pByte, // real type UCHAR + IN PVOID Unused); + +VOID STDCALL +SerialSendByte( + IN PKDPC Dpc, + IN PVOID pDeviceExtension, // real type PSERIAL_DEVICE_EXTENSION + IN PVOID Unused1, + IN PVOID Unused2); + BOOLEAN STDCALL SerialInterruptService( IN PKINTERRUPT Interrupt,