[PATCH rtems] arm/xilinx: Fix zynq-uart interrupt receive

[Chris Johns]

On 19/8/21 5:49 am, Kinsey Moore wrote:
> On 8/18/2021 13:20, Chris Johns wrote:
>> On 19/8/21 3:41 am, Kinsey Moore wrote:
>>> This is functional on the ZynqMP board I currently have setup for testing and on
>>> ZynqMP QEMU except for the data corruption/loss caused by the removal of the
>>> post-baud-set null write.
>> Thanks for the testing.
>>
>> I am not sure if you are saying both the ZyncMP hardware and qemu need the write
>> or just qemu. The write may work but it does not make sense because at some
>> point the software will print a character and achive the same thing?
> QEMU works fine either way. The ZynqMP hardware is what has issues without the
> null write.

OK

> The problem is that it's picky about which characters will actually fix the data
> loss/corruption.
> I've seen that both space and null will do it while letters and numbers won't.
> Null was chosen specifically because it's not printable. 

It shows up on Zynq consoles I have running a a junk character.

> Without this null print, I see garbage
> output and it eventually starts printing text correctly.

How many characters? It smells like a clock is hunting. I suggest you ask Joel
to raise this with Xilinx to see if there is any known errata.

Another suggestion is downloading the Xilinx SDK and looking over the bare metal
console support to see what they do there.

The Versal VCK190 eval board has a ZynqMP in the corner and it runs a standard
FSBL etc and I do not see any dirty characters on its UART console when it
reboots. We need to be sure this is not specific to the board you are using.

So I am sorry, I am not yet convinced we the issue is in the ZynqMP.

>>> One nit below.
>>>>> +    while (ctx->tx_queued < 32 && len > 0) {
>>> Is 32 the size of the TX FIFO? It would be nice to see that as a sizeof() or a
>>> #define for better context.
>> The Versal is 32 bytes and the Zynq is 64 bytes. The Versal is based on the Arm
>> IP r1p5-00rel1 and the Zynq and ZynqMP is based on Cadence IP. We either accept
>> the lesser size on the Zynq and ZyncMP or the zynq_uart struct will need
>> specialised info from each variant of device sharing this code.
> I'm fine using 32 bytes across the board(s).
>> It looks like the UARTs are not fully compatible. The Versal's FIFO trigger
>> level reg is:
>>
>> https://www.xilinx.com/html_docs/registers/am012/am012-versal-register-reference.html#uart___fifo_level.html
>>
>>
>> so it needs the timeout. I will add back the timeout support and see what
>> happens on the Zynq vs the Versal.
> 
> Sounds good.

I was mistaken, the Versal does not share the same driver so the change is
needed. Gedare has added a new serial driver for the Versal.

Chris