Hi Bin,
On 16.08.2017 11:45, Bin Meng wrote:
On Tue, Aug 15, 2017 at 5:33 PM, Stefan Roese <s...@denx.de> wrote:
This patch changes the RX interrupt buffer support in these ways, mostly
suggested by Bin Meng a few weeks ago:
- The RX interrupt buffers size is now configurable via Kconfig
(default still at 256 bytes)
- For NS16550 devices on the PCI bus, the interrupt number will be
read from the PCI config space. Please note that this might not
result in the correct non-zero interrupt number for this PCI
device, as the UART init code is called very early, currently on
x86 before the PCI config registers are initialized via U-Boot
- If the interrupt number is not provided, the code falls back to
the normal polling mode
- The RX interrupt generation is disabled in the UART in the remove
function
While reworking this RX interrupt buffer support, the "default n" is
also removed from Kconfig as its not needed as pointed out by Bin Meng.
Also, a missing comment for the 'irq' variable is added to the
header.
Signed-off-by: Stefan Roese <s...@denx.de>
Cc: Simon Glass <s...@chromium.org>
Cc: Bin Meng <bmeng...@gmail.com>
Cc: Tom Rini <tr...@konsulko.com>
---
drivers/serial/Kconfig | 8 +++++++-
drivers/serial/ns16550.c | 41 +++++++++++++++++++++++++++++++----------
include/ns16550.h | 2 +-
3 files changed, 39 insertions(+), 12 deletions(-)
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index a8e997834a..1b19b24f10 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -67,13 +67,19 @@ config DM_SERIAL
config SERIAL_IRQ_BUFFER
bool "Enable RX interrupt buffer for serial input"
depends on DM_SERIAL
- default n
help
Enable RX interrupt buffer support for the serial driver.
This enables pasting longer strings, even when the RX FIFO
of the UART is not big enough (e.g. 16 bytes on the normal
NS16550).
+config SERIAL_IRQ_BUFFER_SIZE
I don't see this CONFIG_SERIAL_IRQ_BUFFER_SIZE is referenced anywhere
in the ns16550 codes?
Hugh? I'll take a look later today.
+ int "RX interrupt buffer size"
+ depends on SERIAL_IRQ_BUFFER
+ default 256
+ help
+ The size of the RX interrupt buffer
+
config SPL_DM_SERIAL
bool "Enable Driver Model for serial drivers in SPL"
depends on DM_SERIAL
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index 607a1b8c1d..a24ba75031 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -453,11 +453,13 @@ int ns16550_serial_probe(struct udevice *dev)
/* Allocate the RX buffer */
plat->buf = malloc(BUF_COUNT);
- /* Install the interrupt handler */
- irq_install_handler(plat->irq, ns16550_handle_irq, dev);
+ if (plat->irq) {
+ /* Install the interrupt handler */
+ irq_install_handler(plat->irq, ns16550_handle_irq, dev);
- /* Enable RX interrupts */
- serial_out(UART_IER_RDI, &com_port->ier);
+ /* Enable RX interrupts */
+ serial_out(UART_IER_RDI, &com_port->ier);
+ }
}
#endif
@@ -469,9 +471,13 @@ int ns16550_serial_probe(struct udevice *dev)
static int ns16550_serial_remove(struct udevice *dev)
{
#if CONFIG_IS_ENABLED(SERIAL_IRQ_BUFFER)
- if (gd->flags & GD_FLG_RELOC) {
- struct ns16550_platdata *plat = dev->platdata;
+ struct ns16550_platdata *plat = dev->platdata;
+
+ if ((gd->flags & GD_FLG_RELOC) && (plat->irq)) {
+ struct NS16550 *const com_port = dev_get_priv(dev);
+ /* Disable RX interrupts */
+ serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier);
irq_free_handler(plat->irq);
}
#endif
@@ -504,6 +510,7 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
struct fdt_pci_addr pci_addr;
u32 bar;
int ret;
+ u8 irq;
/* we prefer to use a memory-mapped register */
ret = fdtdec_get_pci_addr(gd->fdt_blob, dev_of_offset(dev),
@@ -524,6 +531,10 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
return ret;
addr = bar;
+
+ /* Try to get the PCI interrupt number */
+ dm_pci_read_config8(dev, PCI_INTERRUPT_LINE, &irq);
+ plat->irq = irq;
}
#endif
@@ -562,12 +573,22 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
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);
+ /*
+ * If the interrupt is not provided via PCI, read the number from
+ * the DT instead
+ */
if (!plat->irq) {
- debug("ns16550 interrupt not provided\n");
- return -EINVAL;
+ plat->irq = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),
+ "interrupts", 0);
}
+
+ /*
+ * If no interrupt is provided the RX interrupt buffer code is
+ * still used, but the interrupt is not enabled and registered.
+ * The RX buffer code will be usedin polling mode in this case.
+ */
+ if (!plat->irq)
+ debug("ns16550 interrupt not provided, using polling\n");
But rx_pending() and rx_get() is still the IRQ buffer version, not the
polling version.
Yes, but still they should work just fine (or even better).
Having said that, I've tested this patch, without setting "interrupts"
in the dts file, and testing on MinnowMax showing that previous out of
order character issue is disappeared with such configuration! Here is
the test result:
=> This patch changes the RX interrupt buffer support in these ways,
mostly suggested by Bin Meng a few weeks ago: - The RX interrupt
buffers size is now configurable via Kconfig (default still at 256
bytes) - For NS16550 devices on the PCI bus, the interrupt number will
be read from the PCI config space. Please note that this might not
result in the correct non-zero interrupt number for this PCI device,
as the UART init code is called very early, currently on x86 before
the PCI config registers are initialize
Unknown command 'This' - try 'help'
The following is the test result with "interrupts" set toi 4 in the
dts file for MinnowMax:
=> This patch changes the RX interrupt buffer nsupport in these ways,
mostly suggested by Bin Meng a few weeks ago: - The RX interrupt
buffers size is now configurable via Kconfig (default still a(t 256
bytes) - For NS16550 devicoes on the PCI bus, the interrupts number
will be read from the PCI config space. Please note that this might
not result in the correct non-zero interrupt number for this PCI
device, as the UART init code is called very early, currently on x86
before the PCI config registers are initi
Unknown command 'This' - try 'help'
=> This patch changes the RX interrupt buffer support in these ways,
mostly su ggested by Bin Meng a few weeks ago: - The RX interrupt
buffers size is now configurable via Kconfig (default still at 256
bytes) - For NS16550 devices on the PCI bus, the interrupt number will
be read from the PCrI config space. Please note that this might not
result in the correct non-zero interrupt number for this PCI device,
as the UART init code is called very early, currently on x86 befeore
the PCI config registers are initial
So maybe there is no need to add IRQ support at all ... Only adding a
buffer will resolve the original issue you wanted to fix ..
Interesting idea - thanks for all the tests. The RX buffer has evolved
quite a bit, so your idea might very well be the much easier solution
for this problem. I need to test this on the DFI board though as well,
to see if this solves the char dropping here as well.
Thanks,
Stefan
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
