[U-Boot] [PATCH v2] serial: ns16550: Add RX interrupt buffer support

[Stefan Roese]

Hi Bin,

On 17.07.2017 11:26, Bin Meng wrote:

<snip>

>>>> +       }
>>>> +#endif
>>>> +
>>>>           return 0;
>>>>    }
>>>>
>>>> @@ -459,6 +559,15 @@ int ns16550_serial_ofdata_to_platdata(struct udevice
>>>> *dev)
>>>>           if (port_type == PORT_JZ4780)
>>>>                   plat->fcr |= UART_FCR_UME;
>>>>
>>>> +#if CONFIG_IS_ENABLED(SERIAL_IRQ_BUFFER)
>>>> +       plat->irq = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),
>>>> +                                  "interrupts", 0);
>>>
>>>
>>> Should we handle NS16550 on the PCI bus? If not, better put a comment
>>> here.
>>
>>
>> Actually I'm also using this RX IRQ buffer for the HS-UART on BayTrail
>> connected via PCI (pciuart0: uart at 1e,3). I've added the interrupt
>> property to the DT for this to work for now. I'll check, if I can read
>> the interrupt from the PCI config space instead.
>>
> 
> Yes, the interrupt line register is programmed with the value of its
> interrupt vector in irq_router_probe(). But you should wait for the
> IRQ device to be probed before NS16550.

I'm currently working on this and this dependency with the PCI interrupt
assignment is problematic. The serial driver is (intentionally) probed
earlier and the PCI interrupts are still unassigned at that stage.

I currently have no real good idea on how to solve this. We could
add a new "probe_late() / probe_irqs_enabled()" DM function for such
cases, but this seems overly complex and adds new bloat to the DM
infrastructure.

Do you have some other (simpler) ideas on how to solve this?

<snip>

>>> I've tested the patch on MinnowMax, and pasted a really long text.
>>> Below are 5 times results: (using SecureCRT as my terminal application
>>> on Windows)
>>>
>>> => Note: when interrupt transfer complets, the xHC generates an event
>>> TRB with TRB type 'Transfer Event', which is exactly the same as a
>>> control or bulk transfer. In our xHCI driver, xhci_wait_foer_event()
>>> checks the event TRB t ype and depending on the timing,g it may
>>> wrongly return an event TRB to the caller which originates from
>>> another USB device (different slot ID is checked by the driver and if
>>> different a "BUG" will be thrown).
>>> Unknown command 'Note:' - try 'help'
>>> => Note: when interrupt transfer complets, thee xHC generates an event
>>> TRB withn TRB type 'Transfer Event', which is exactly the same as a
>>> control or bulk transfer. In our xHCI driver, xhci_wait_for_event()
>>> checks the event TRB type and depending on the timing,g it may wrongly
>>> return an event rTRB to the caller which originates from another USB
>>> device (different slot ID is checked by the driver and if different a
>>> "BUG" will be thrown).
>>> Unknown command 'Note:' - try 'help'
>>> => Note: when interrupt transfer complets, thee xHC generates an event
>>> TRB withn TRB type 'Transfer Event', which is exactly the same as a
>>> control or bulk transfer. In our xHCI driver, xhci_wait_for_event()
>>> checks the event TRB type and depending on the timing, it may wrongly
>>> return an event rTRB to the caller which originates from another USB
>>> device (different slot ID is checked by the driver and if different a
>>> "BUG" will be thrown).
>>> Unknown command 'Note:' - try 'help'
>>> => Note: when interrupt transfer complets, the xHC generates an event
>>> TRB with TRB type 'Transfer Event', whicfh is exactly the same as a
>>> control or bulk transfer. In our xHCI driver, xhci_wait_for_event()
>>> checks the event TRB type and depending on the timing, it may wrongly
>>> return an event TRB to the caller which originatres from another USB
>>> device (different slot ID is checked by the driver and if different a
>>> "BUG" will be thrown).
>>> Unknown command 'Note:' - try 'help'
>>> => Note: when interrupt transfer complets, thee xHC generates an event
>>> TRB with TRB type 'Transfer Event', which is exactly the same as a
>>> control or bulk transfer. In our xHCI driver, xhci_wait_for_event()
>>> checks the event TRB type and depending on the timing, it may wrongly
>>> return an event rTRB to the caller which originatres from another USB
>>> device (different slot ID is checked by the driver and if different a
>>> "BUG" will be thrown).
>>> Unknown command 'Note:' - try 'help'
>>>
>>> As you see, none of 5 is completely correct. The 1st and 2nd have
>>> issues in "t ype" and "timing,g". The 2nd/4th/5th have issues in "thee
>>> xHC". The 2nd/3rd/5th have issues in "event rTRB". The 4th has
>>> "whicfh".
>>
>>
>> Interesting, I've not seen such issues. Probably since I only checked
>> the length of the received / echoed lines and roughly scanned the text
>> for some incorrectness. Is the length always correct in your tests?
>> Does increasing the buffer size help here?
>>
> 
> I increased the buffer size to 1024, but it did not help. I tested it
> with the MinnowMax internal UART (0x3f8). As you can see above, the
> length is not always equal due to these incorrectness.

Thanks for testing and confirming. Currently I have no idea, where
this problem might come from. And my time is limited right now,
as I'll be on vacation for 10 days starting on Wednesday. I still think
that this RX interrupt buffer support is a big improvement over the
"polling only" mode, even in this current version. I can re-visit this
driver in a few weeks to work on a fix for the issue that you have
found.

Thanks,
Stefan