Re: [PATCH] PL011: fix reverse logic for interrupt mask register

[Julien Grall <julien.grall@linaro.org>]

On 08/22/2013 08:23 AM, Ian Campbell wrote:
> On Wed, 2013-08-21 at 17:11 +0100, Julien Grall wrote:
>> On 08/13/2013 04:12 PM, Andre Przywara wrote:
>>> The PL011 IMSC register description is somehow fuzzy in the
>>> documentation; by comparing it with the Linux implementation one can
>>> see that the logic is actually reversed to Xen's implementation:
>>> A "0" in field means interrupt disabled, a "1" enables it.
>>> Therefore we enabled all interrupts instead of disabling them in the
>>> beginning and later on masked the wrong interrupts.
>>> Unclear how this worked on the Versatile Express, but this fix is
>>> needed to get Calxeda Midway running (and works on VExpress, too).
>>
>> On my Versatile Express, the keyboard is unusable with this patch.
>> Xen receives random keys and sometimes nothing is printed on the serial
>> port.
>>
>> By reverting this patch on my tree, I'm able to use correctly the serial
>> port.
> 
> :-/ Andre did say this patch worked on vexpress for him.
> 
> I'm pretty certain Andre's patch is correct in its own right. But the
> fact that it worked before does seem to imply that there are other
> issues with the pl011 driver, it's likely that this change has just
> exposed a latent one.
> 
> Could this be related somehow to the baud rate setting change? I
> wouldn't have expected so but "random keys" and nothingness could be a
> symptom of incorrect baud too.
> 
> Does anyone have time to look into this?
> 

If RTI interrupt are enabled (see small patch below), the UART works perfectly
on the versatile express.

The PL011 documentation says the bit is used to mask/unmask receive interrupt
timeout. I don't understand why this interrupt is useful and the documentation
doesn't help me...

====================================================================================
diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 0e1eb64..e4bd702 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -140,7 +140,7 @@ static void __init pl011_init_postirq(struct serial_port *port)
     pl011_write(uart, ICR, OEI|BEI|PEI|FEI);
 
     /* Unmask interrupts */
-    pl011_write(uart, IMSC, OEI|BEI|PEI|FEI|TXI|RXI);
+    pl011_write(uart, IMSC, RTI|OEI|BEI|PEI|FEI|TXI|RXI);
 }
 
 static void pl011_suspend(struct serial_port *port)
======================================================================================

-- 
Julien Grall