[PATCH v5 2/2] serial: mxc: have putc use the TXFIFO

[SCHNEIDER Johannes]

Hoi,

>> the thing with only checking !TXEMPTY is that it limits the fifo to one byte only; and as far as i understood, drivermodel >code is supposed to return immediately and t
>
>Why does it limit the FIFO to one byte only? The FIFO has 32 bytes.
>

we're in putc, so dealing with one byte/character at a time.
with the original !TXEMPTY the function would immediately return with an egain as soon as the first spot in the fifo was occupied by the previous putc, never filling the fifo up any further through the write-register call that follows the if block in mxc_serial_putc
so yes, the fifo in the imx is 32bytes long - but the code did not make any use of it

>I would like to understand what problem you were trying to solve when
>you did the change:
>
>-       if (!(readl(&uart->ts) & UTS_TXEMPTY))
>+       if (readl(&uart->ts) & UTS_TXFULL)
>
>Can you share a reproducer?
>
>We need to figure this out soon, as this is causing a regression.

sadly the previously mentioned printf with a long string (longer than 32bytes) does not trigger the issue on the debug-serial...
(running on an imx8mm -> uart3 out of 4, and with uboot/v2202.07)

but what puzzles me most is that with my current setup (different tool: old analog oscilloscope replaced by an usb logic-analyzer, and a more recent software versions also on the receiving end where the problem cropped up originally) i'm currently unable to reproduce the original issue i saw - and described in the commit message :-S
back then the situation was as follows: with the analog scope on uart4 visually comparing waveforms clearly showed different message lengths when comparing u-boot generated traffic (without the patch = short, with the patch = "correct" length) with a reference/working signal generated from within a running linux.
the receiving end did not respond to the messages from within uboot prior to the patch; but did so with the TXFULL applied and also always when communication was initiated from linux

i still would argue to keep the change, since both the linux kernel and $other bootloaders make use of the TXFULL flag

kind regard

________________________________________
From: Fabio Estevam <festevam at gmail.com>
Sent: Tuesday, November 1, 2022 21:39
To: SCHNEIDER Johannes
Cc: Pali Rohár; Fabio Estevam; Tim Harvey; u-boot at lists.denx.de; peng.fan at oss.nxp.com; sbabic at denx.de; trini; GEO-CHHER-bsp-development; Peng Fan
Subject: Re: [PATCH v5 2/2] serial: mxc: have putc use the TXFIFO

[Some people who received this message don't often get email from festevam at gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.


Johannes,

On Wed, Oct 26, 2022 at 7:23 PM SCHNEIDER Johannes
<johannes.schneider at leica-geosystems.com> wrote:
>
> Hi
>
> the thing with only checking !TXEMPTY is that it limits the fifo to one byte only; and as far as i understood, drivermodel code is supposed to return immediately and t

Why does it limit the FIFO to one byte only? The FIFO has 32 bytes.

I would like to understand what problem you were trying to solve when
you did the change:

-       if (!(readl(&uart->ts) & UTS_TXEMPTY))
+       if (readl(&uart->ts) & UTS_TXFULL)

Can you share a reproducer?

We need to figure this out soon, as this is causing a regression.