On 02/18/2015 11:39 AM, George Spelvin wrote:
> tl;dr: I'm sorry to have needed remedial 16550 education.
> I was being thick, and you're absolutely right. Will fix.
No apology necessary.
[...]
>>>> Is UPF_CONS_FLOW part of the use case? If not, please drop.
>>>
>>> Um, what does UPF_CONS_FLOW have to do with this?
>>
>> UPF_CONS_FLOW usage is very limited, and I don't think rx is appropriate;
>> why not drop RTS? Not that I'm suggested this patch should do that, but
>> rather that this patch should focus on fixing only the observed problem.
>
> Oh! You meant "drop RTS". I thought you meant "drop this hunk".
I do mean "drop this hunk". The original patch context for my comment is
now lost in this email but the specific hunk I was referring to was this:
> @@ -1989,12 +2021,29 @@ static void wait_for_xmitr(struct uart_8250_port *up, int bits)
> up->msr_saved_flags |= msr & MSR_SAVE_FLAGS;
> if (msr & UART_MSR_CTS)
> break;
> +
> + if (rx && !emergency_situation(&up->port)) {
> + status = serial_in(up, UART_LSR);
> +
> + up->lsr_saved_flags |= status & LSR_SAVE_FLAGS;
> + if (status & (UART_LSR_DR | UART_LSR_BI))
> + serial8250_rx_chars(up, status, true);
> + }
> +
> udelay(1);
> touch_nmi_watchdog();
> }
> }
> }
This specific hunk is only evaluated if UPF_CONS_FLOW is set;
currently, there are only 3 in-tree users of UPF_CONS_FLOW and all of them
are samsung UARTs. Furthermore, UPF_CONS_FLOW cannot be set from userspace
with ioctl(TIOCSSERIAL).
IOW, the entire "if (up->port.flags & UPF_CONS_FLOW) { ... }" statement is
dead code, so let's not add to it ;)
[...]
>> The console output waits for the tx fifo to be _empty_ and then only writes
>> 1 char. It behaves this way because it has no idea how big the tx fifo is
>> and how much data might be in the tx fifo when console output started.
>
> I think the idea is more "I'm polling anyway, *and* I have to wait for the
> FIFO to drain, so I don't need the FIFO for interrupt mitigation".
>
> The emergency transmit could use the FIFO, but it wouldn't shrink the
> interrupts-disabled window unless we let it return with a non-empty FIFO, and
> that would require some interlocking with the regular serial8250_tx_chars path.
>
> I'm not I'd survive the torches & pitchforks if I seriously proposed that.
:)
>> If the actual shifter is sufficiently small (which I would think would be
>> the case on a low-cost design with a small fifo), then the rx fifo cannot
>> overflow in the time it takes for 1 char plus the remainder in the shifter
>> to be transmitted.
>
> If I understand what you mean by "the actual shifter", i.e. the difference
> between THRE and TEMT, then that's always 8 bits (+ start + stop).
I doubt that's true for every uart design supported by this driver. But
I do think the number is small, and I agree that it's almost certainly < 16 bits
for 16550 work-alikes.
Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
