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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2013-08-27 at 11:58 +0100, Julien Grall wrote:
> 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.

The IPMI console can take input too though and is a pl011 from the main
processor's PoV

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

THanks!

> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel