Hi Russel,
I am working with Taku,
> On Thu, Sep 08, 2005 at 04:47:32PM +0900, Taku Izumi wrote:
>> >I don't think we want this. With early serial console, tx_loadsz is
>> >not guaranteed to be initialised, and may in fact be zero.
>>
>> >Plus there's no guarantee that the FIFOs will actually be enabled, so
>> >I think it's better that this patch doesn't go to mainline.
>>
>> Our server has a virtual serial port, but its performance seems to be poor.
>> It takes 10 seconds to output 4000 characters (from kernel) to serial
>> console. By applying my patch, its peformance could be improved. ( 0.4
>> seconds / 4000 characters output), so I think it is useful to use FIFO at
>> serial8250_console_write function like transmit_chars function. Where
>> should I correct in order to use FIFO?
>
> The problem is that we don't know:
>
> * if there is a FIFO
> * what size the FIFO is
I understand tx_loadsz is practical TX FIFO size.
If there is no FIFO, tx_loadsz becomes 1.
Is it wrong?
- tx_loadsz is properly initilized in autoconfig().
- FIFO is enabled in serial8250_clear_fifo() called from autoconfig(),
if FIFO exist.
- autoconfig() is called from serial8250_isa_init_ports().
- serial8250_isa_init_ports() is called from serial8250_console_init() etc.
I can't find the problem you are pointing out.
> * if it has been initialised
> * how much data is already contained in the FIFO
Right, we can't know how many byte exist in the FIFO.
So this patch is waiting the FIFO becomes empty at first
by calling "wait_for_xmitr(up)".
(This is the same logic with original.)
After TX FIFO become empty, we can decide the available
TX FIFO depth by up->tx_loadsize.
> for (i = 0; i < count; ) {
> int fifo;
>
> wait_for_xmitr(up);
> fifo = up->tx_loadsz;
> /*
> * Send the character out using FIFO.
> * If a LF, also do CR...
> */
> do {
> serial_out(up, UART_TX, *s);
> fifo--;
> if (*s == 10) {
> if (fifo > 0) {
> serial_out(up, UART_TX, 13);
> fifo--;
> } else {
> /* No room to add CR */
> wait_for_xmitr(up);
> fifo = up->tx_loadsz;
> serial_out(up, UART_TX, 13);
> fifo--;
> }
> }
> i++;
> s++;
> } while (fifo > 0 && i < count );
> }
>
> So we can't really blindly initialise the FIFO in the console write
> method. Neither can we initialise it in the console setup. If we
> could initialise it, we can't blindly load 16 bytes into the FIFO
> at a time.
>
> I don't think it's technically practical to use the FIFO for the
> console and still have a reliable serial port.
>
> --
> Russell King
> Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
> maintainer of: 2.6 Serial core
> -
Best regards,
Hironobu Ishii
On Tue, Sep 13, 2005 at 08:44:37PM +0900, Hironobu Ishii wrote:
> Hi Russel,
>
> I am working with Taku,
>
> > On Thu, Sep 08, 2005 at 04:47:32PM +0900, Taku Izumi wrote:
> >> >I don't think we want this. With early serial console, tx_loadsz is
> >> >not guaranteed to be initialised, and may in fact be zero.
> >>
> >> >Plus there's no guarantee that the FIFOs will actually be enabled, so
> >> >I think it's better that this patch doesn't go to mainline.
> >>
> >> Our server has a virtual serial port, but its performance seems to be poor.
> >> It takes 10 seconds to output 4000 characters (from kernel) to serial
> >> console. By applying my patch, its peformance could be improved. ( 0.4
> >> seconds / 4000 characters output), so I think it is useful to use FIFO at
> >> serial8250_console_write function like transmit_chars function. Where
> >> should I correct in order to use FIFO?
> >
> > The problem is that we don't know:
> >
> > * if there is a FIFO
> > * what size the FIFO is
>
> I understand tx_loadsz is practical TX FIFO size.
> If there is no FIFO, tx_loadsz becomes 1.
> Is it wrong?
>
> - tx_loadsz is properly initilized in autoconfig().
> - FIFO is enabled in serial8250_clear_fifo() called from autoconfig(),
> if FIFO exist.
> - autoconfig() is called from serial8250_isa_init_ports().
> - serial8250_isa_init_ports() is called from serial8250_console_init() etc.
>
> I can't find the problem you are pointing out.
autoconfig() is _not_ called from serial8250_isa_init_ports().
The problem is that the console write method may be called prior to
autoconfig() being run for the port in question, so tx_loadsz may be
uninitialised.
> > * if it has been initialised
> > * how much data is already contained in the FIFO
>
> Right, we can't know how many byte exist in the FIFO.
> So this patch is waiting the FIFO becomes empty at first
> by calling "wait_for_xmitr(up)".
> (This is the same logic with original.)
>
> After TX FIFO become empty, we can decide the available
> TX FIFO depth by up->tx_loadsize.
Only if you ignore the fact that tx_loadsz may not be initialised.
The only things which the console code can rely on being initialised
is the port address description (iobase / membase / iotype / regshift),
the flow control (UPF_CONS_FLOW) in flags, and in the case of Xscale
systems, the capabilities. Everything else will be in an indeterminent
state as far as the serial console code is concerned, and therefore can
not be relied upon.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
On Tue, Sep 13, 2005 at 12:53:26PM +0100, Russell King wrote:
> The only things which the console code can rely on being initialised
> is the port address description (iobase / membase / iotype / regshift),
> the flow control (UPF_CONS_FLOW) in flags, and in the case of Xscale
> systems, the capabilities. Everything else will be in an indeterminent
> state as far as the serial console code is concerned, and therefore can
> not be relied upon.
Additionally, once all architectures convert to initialising their
serial ports via platform devices (which means include/asm-*/serial.h
becomes essentially empty) and we eliminate serial8250_console_init(),
the 8250 console code can start assuming that more of the uart_port
structure will be initialised.
At that point, we can start to think about using FIFOs for the
console.
However, this is an exercise for architecture maintainers to do
who in theory know the requirements for their platforms. So far
I've seen little progress on this though.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core