[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

On 08/27/2013 11:45 AM, Ian Campbell wrote:
> On Tue, 2013-08-27 at 11:37 +0100, Ian Campbell wrote:
>> On Mon, 2013-08-26 at 16:55 +0100, Julien Grall wrote:
>>> 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...
>> Docs say:
>>         The receive timeout interrupt is asserted when the receive FIFO
>>         is not empty, and no more data is received during a 32-bit
>>         period.
> I see now that we do actually enable the FIFOs, so this functionality
> makes sense -- if your RX FIFO trigger level is e.g. 1/2 and the FIFO is
> 1/4 full when the sender stops then this interrupt provides a backstop
> such that you will receive those bytes without waiting for the sender to
> resume (which might be a long time).
> So I think enabling and handling these timeout interrupts makes sense. I
> cannot explain why it only affects vexpress though.

I only see the issue with the serial input. I guess this is needed on
midway but we don't see anything because the serial console doesn't
receive input.

I will send a correct patch + commit message this afternoon.

Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.