Prev: [PATCH 04/31] lmb: Move lmb.c to mm/
Next: [PATCH 05/31] lmb: Seperate region array from lmb_region struct
From: Feng Tang on 28 Mar 2010 22:50
Hi,

I modified the code a little and run it on our HW platform, it really show
some sigh of life: it can boots to console (the print format is not so good),
I can input command and it execute correctly, but very slow, I type 3
characters and it takes about 2 seconds to echo back on screen and start the
execution, and after about 1 minute, the console hang there and input stopped
to work.

I also have some comments inline.

Thanks,
Feng


On Tue, 23 Mar 2010 18:29:30 +0800
Christian Pellegrin <chripell(a)fsfe.org> wrote:

> This patch adds console support for the MAX3100 UART
> (console=ttyMAX0,11500). The SPI subsystem and an

115200?

>
> +#ifdef CONFIG_SERIAL_MAX3100_CONSOLE
> +
> +static void max3100_console_work(struct work_struct *w)
> +{
> + struct max3100_port *s = container_of(w, struct max3100_port,
> + console_work);
> + unsigned long start;
> + u16 tx, rx;
> +
> + while (s->console_head != s->console_tail &&
> + (s->console_flags & MAX3100_SUSPENDING) == 0) {
> + start = jiffies;
> + do {
> + tx = MAX3100_RC;
> + max3100_sr(s, tx, &rx);
> + } while ((rx & MAX3100_T) == 0 &&
> + !time_after(jiffies, start +
> s->console_tout));
> + tx = s->console_buf[s->console_tail];
> + max3100_calc_parity(s, &tx);
> + tx |= MAX3100_WD | MAX3100_RTS;

Does this imply to have to work with HW flow control? on my platform
I have to remove the RTS bit to make it work.

> + max3100_sr(s, tx, &rx);

It doesn't handle received characters here? If the console is printing out
a bulk of message while user input some command, the command may be ignored.
Myself have met the same problem in our driver.


> + s->console_tail = (s->console_tail + 1) %
> CONSOLE_BUF_SIZE;
> + }
> +}
> +
> +static void max3100_console_putchar(struct uart_port *port, int ch)
> +{
> + struct max3100_port *s = to_max3100_port(port);
> + int next = (s->console_head + 1) % CONSOLE_BUF_SIZE;
> +
> + if (next != s->console_tail) {
> + s->console_buf[next] = ch;
> + s->console_head = next;
> + }

Also I saw max3100_sr() uses cpu_to_be16() and be16_to_cpu(), is it really
necessary, our platform is little-endian(x86), and I have to disable them
to make the code work. Is your test platform big-endian?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: christian pellegrin on 29 Mar 2010 02:20
On Mon, Mar 29, 2010 at 4:48 AM, Feng Tang <feng.tang(a)intel.com> wrote:
> Hi,

Hi,

>
> I modified the code a little and run it on our HW platform, it really show
> some sigh of life: it can boots to console (the print format is not so good),
> I can input command and it execute correctly, but very slow, I type 3
> characters and it takes about 2 seconds to echo back on screen and start the
> execution, and after about 1 minute, the console hang there and input stopped
> to work.

never seen such a behavior. Which platform are you using? Which SPI
driver? Do you have a low level printk (printascii) that puts output
somewhere else so I can send you a patch with some debugging output?
Can you log in some other way (like via network) and see if the CPU
load is at 100% for some reason?

>> This patch adds console support for the MAX3100 UART
>> (console=ttyMAX0,11500). The SPI subsystem and an
>
> 115200?
>

ack

> Does this imply to have to work with HW flow control? on my platform
> I have to remove the RTS bit to make it work.
>

no, I put RTS on because it looks like a good default. I can make it
configurable. I just noticed on the data sheet that RTS is actually
inverted so a more sensible default would be to put it off. For
testing you should have flow control set to none on the machine you
are using as a terminal emulator.

>> + � � � � � � max3100_sr(s, tx, &rx);
>
> It doesn't handle received characters here? If the console is printing out
> a bulk of message while user input some command, the command may be ignored.
> Myself have met the same problem in our driver.
>

yes but I think it's quite difficult to solve this problem in every
case. Console output is massively used only on boot when the user is
not supposed to type a lot.

>> + � � if (next != s->console_tail) {
>> + � � � � � � s->console_buf[next] = ch;
>> + � � � � � � s->console_head = next;
>> + � � }
>
> Also I saw max3100_sr() uses cpu_to_be16() and be16_to_cpu(), is it really
> necessary, our platform is little-endian(x86), and I have to disable them
> to make the code work. Is your test platform big-endian?
>

Have you configured your SPI controller as LSB first somehow, haven't
you? BTW my platform is a quite usual ARM9 S3C2440 which is little
endian.


--
Christian Pellegrin, see http://www.evolware.org/chri/
"Real Programmers don't play tennis, or any other sport which requires
you to change clothes. Mountain climbing is OK, and Real Programmers
wear their climbing boots to work in case a mountain should suddenly
spring up in the middle of the computer room."
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Feng Tang on 29 Mar 2010 03:10
On Mon, 29 Mar 2010 14:11:15 +0800
christian pellegrin <chripell(a)fsfe.org> wrote:

> > I modified the code a little and run it on our HW platform, it
> > really show some sigh of life: it can boots to console (the print
> > format is not so good), I can input command and it execute
> > correctly, but very slow, I type 3 characters and it takes about 2
> > seconds to echo back on screen and start the execution, and after
> > about 1 minute, the console hang there and input stopped to work.
>
> never seen such a behavior. Which platform are you using? Which SPI
> driver? Do you have a low level printk (printascii) that puts output
> somewhere else so I can send you a patch with some debugging output?
> Can you log in some other way (like via network) and see if the CPU
> load is at 100% for some reason?

Hi,

Our platform is Intel Moorestown platform, and use a spi controller core
from Designware (drivers/spi/dw_*.c). I know the problem may probably be
caused by my setting, but the dw_spi driver works fine with our own
3110 driver.

For debug method, sadly I don't get another output port yet, but if you
have some debug patch, that's great, it will help when I find another debug
output than max3110.

>
> >> +             max3100_sr(s, tx, &rx);
> >
> > It doesn't handle received characters here? If the console is
> > printing out a bulk of message while user input some command, the
> > command may be ignored. Myself have met the same problem in our
> > driver.
> >
>
> yes but I think it's quite difficult to solve this problem in every
> case. Console output is massively used only on boot when the user is
> not supposed to type a lot.

It's difficult but not impossible, actually our driver checks every word
read back and handle it if it contains a valid data

>
> >> +     if (next != s->console_tail) {
> >> +             s->console_buf[next] = ch;
> >> +             s->console_head = next;
> >> +     }
> >
> > Also I saw max3100_sr() uses cpu_to_be16() and be16_to_cpu(), is it
> > really necessary, our platform is little-endian(x86), and I have to
> > disable them to make the code work. Is your test platform
> > big-endian?
> >
>
> Have you configured your SPI controller as LSB first somehow, haven't
> you? BTW my platform is a quite usual ARM9 S3C2440 which is little
> endian.
>
yeah, you hit the point that our spi controller is LSB naturally (not
configured to), here may need a check for whether to do a swap

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: christian pellegrin on 29 Mar 2010 09:00
On Mon, Mar 29, 2010 at 9:06 AM, Feng Tang <feng.tang(a)intel.com> wrote:

> Our platform is Intel Moorestown platform, and use a spi controller core
> from Designware (drivers/spi/dw_*.c). I know the problem may probably be
> caused by my setting, but the dw_spi driver works fine with our own
> 3110 driver.

I had a look at the dw_spi driver. The spi_transfer path queues some
work to a worqueue that itself schedules a tasklet. I don't think this
is good for latency, I won't bet that such an architecture could
deliver good performance. Now I see why you needed to do only big fat
SPI transfers. Anyway this doesn't justify the 2 seconds delay between
chars coming in and going out through the max31x0 you are seeing. I
will try to analyze what's going on. BTW is only input slow or output
is sluggish too? The initial messages from the console are coming out
fast? If you disable the MAX3110 for console but you use just as a
normal terminal (set console=/dev/null in the kernel command line and
add getty 115200 /dev/ttyMAX0 in iniitab) is the interaction with the
system fine? Thanks for helping sorting out this.

>>
>> yes but I think it's quite difficult to solve this problem in every
>> case. Console output is massively used only on boot when the user is
>> not supposed to type a lot.
>
> It's difficult but not impossible, actually our driver checks every word
> read back and handle it if it contains a valid data
>

Of course it is possible, I just wanted to keep the max3100 a small
clean driver. Unfortunately console and serial drivers are two
different beings in the Linux kernel, but the max3100 implements the
tx-rx in one indivisible instruction (that is slow compared to
registers IO and has to be called in an preemptible contex for added
fun). To implement what you are saying we need:

1) the console code has to check if the serial port associated to the
same physical max3100 is up (console driver start their life much
before serial ones).

2) if yes send data to the tty associated to the serial driver.
Locking is needed here.

I will implement this ASAP.

>> Have you configured your SPI controller as LSB first somehow, haven't
>> you? BTW my platform is a quite usual ARM9 S3C2440 which is little
>> endian.
>>
> yeah, you hit the point that our spi controller is LSB naturally (not
> configured to), here may need a check for whether to do a swap
>

ok, I think the dw_spi driver has to be fixed.


--
Christian Pellegrin, see http://www.evolware.org/chri/
"Real Programmers don't play tennis, or any other sport which requires
you to change clothes. Mountain climbing is OK, and Real Programmers
wear their climbing boots to work in case a mountain should suddenly
spring up in the middle of the computer room."
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: christian pellegrin on 8 Apr 2010 05:40
On Mon, Mar 29, 2010 at 4:48 AM, Feng Tang <feng.tang(a)intel.com> wrote:

>> + � � � � � � tx |= MAX3100_WD | MAX3100_RTS;
>
> Does this imply to have to work with HW flow control? on my platform
> I have to remove the RTS bit to make it work.
>

Finally I had time to check this. If you compare the 8250 (or similar)
data-sheet with the MAX31x0 one you see that the handling of the
RTX/CTS bits is exactly the same (8250 about RTS bit for example:
"When any of these bits are cleared, the associated output is forced
high." and MAX3110: "Request-to-Send Bit. Controls the state of the
RTS output. This bit is reset on power-up (RTS
bit = 0 sets the RTS pin = logic high)."). If you look at the 8250.c
driver (grep for UART_MCR_RTS) you notice that the bit is set on
device open (together with DTR of course). So I think the driver is
doing the right thing here.

--
Christian Pellegrin, see http://www.evolware.org/chri/
"Real Programmers don't play tennis, or any other sport which requires
you to change clothes. Mountain climbing is OK, and Real Programmers
wear their climbing boots to work in case a mountain should suddenly
spring up in the middle of the computer room."
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 |  Next  |  Last
Pages: 1 2
Prev: [PATCH 04/31] lmb: Move lmb.c to mm/
Next: [PATCH 05/31] lmb: Seperate region array from lmb_region struct