Re: [PATCH 26/27 v8] xen/arm: vpl011: Correct the logic for asserting/de-asserting SBSA UART TX interrupt

[Julien Grall <julien.grall@arm.com>]

Hi,

On 07/09/17 15:37, Andre Przywara wrote:
>>   
>>       vpl011->uartfr |= BUSY;
>>   
>> @@ -368,7 +376,16 @@ static void vpl011_data_avail(struct domain *d)
>>       if ( out_ring_qsize != sizeof(intf->out) )
>>       {
>>           vpl011->uartfr &= ~TXFF;
>> -        vpl011->uartris |= TXI;
>> +
>> +        /*
>> +         * Ensure that there is space for atleast 16 bytes before asserting the
>> +         * TXI interrupt status bit because the SBSA UART driver may write upto
>> +         * 16 bytes (i.e. half the SBSA UART fifo size of 32) on getting
>> +         * a TX interrupt.
> 
> The comment sounds a bit like this is hack, where it actually is a
> totally legit spec requirement (the interrupt is asserted/deasserted
> around the *trigger level*, which is half way by default and always half
> for the SBSA).
> 
> Also I think the same logic/fix needs to be applied to the receiving side.
> 
> And while I see that Julien requested a follow-up patch, I believe this
> should eventually be squashed into 02/27, to not have wrong code in the
> repo. But can could be done at commit time, I guess.

There is nothing wrong to keep this patch separately. This can be 
considered as a bug fix and I like the idea of having it separately 
because it explain the rationale behind it. After all, there are nothing 
said on the SBSA so far and just an assumption on how the spec will be 
updated.

Cheers,

-- 
Julien Grall

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