Aravind Gopalakrishnan
2013-Nov-14 16:29 UTC
[PATCH] ns16550: Add support for UART present in Broadcom TruManage capable NetXtreme chips
There are few quirks regarding the chip:
Firstly, it is an MMIO device. Therefore, the code has been modified to
accept MMIO based devices as well. Settings particular to such devices are
populated in the table ''uart_config''. Currently, we only
support BCM5725
TruManage chip.
Some more quirks are - the need to shift the register offset by a specific
value and we also need to verify (UART_LSR_THRE && UART_LSR_TEMT) bits
before
transmitting data.
While testing, we ned to include com1=115200,8n1,pci,0 on the xen cmdline to
be able to observe output using SOL.
Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Signed-off-by: Thomas Lendacky <Thomas.Lendacky@amd.com>
---
xen/drivers/char/ns16550.c | 98 ++++++++++++++++++++++++++++++++++++++++------
1 file changed, 86 insertions(+), 12 deletions(-)
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 9c2cded..05d77ba 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -53,6 +53,7 @@ static struct ns16550 {
char __iomem *remapped_io_base; /* Remapped virtual address of MMIO. */
/* UART with IRQ line: interrupt-driven I/O. */
struct irqaction irqaction;
+ u32 lsr_mask;
#ifdef CONFIG_ARM
struct vuart_info vuart;
#endif
@@ -77,6 +78,37 @@ static struct ns16550 {
#endif
} ns16550_com[2] = { { 0 } };
+/* Defining uart config options for MMIO devices */
+struct ns16550_config_mmio {
+ u16 vendor_id;
+ u16 dev_id;
+ int reg_shift;
+ int reg_width;
+ int fifo_size;
+ u32 lsr_mask;
+ int max_bars;
+};
+
+/*
+ * Create lookup tables for specific MMIO devices..
+ * It is assumed that if the device found is MMIO,
+ * then you have indexed it here. Else, the driver
+ * does nothing.
+ */
+static struct ns16550_config_mmio uart_config[] +{
+ /* Broadcom TruManage device */
+ {
+ .vendor_id = 0x14e4,
+ .dev_id = 0x160a,
+ .reg_shift = 2,
+ .reg_width = 1,
+ .fifo_size = 64,
+ .lsr_mask = (UART_LSR_THRE | UART_LSR_TEMT),
+ .max_bars = 1,
+ },
+};
+
static void ns16550_delayed_resume(void *data);
static char ns_read_reg(struct ns16550 *uart, int reg)
@@ -134,7 +166,7 @@ static void ns16550_interrupt(
while ( !(ns_read_reg(uart, UART_IIR) & UART_IIR_NOINT) )
{
char lsr = ns_read_reg(uart, UART_LSR);
- if ( lsr & UART_LSR_THRE )
+ if ( (lsr & uart->lsr_mask) == uart->lsr_mask )
serial_tx_interrupt(port, regs);
if ( lsr & UART_LSR_DR )
serial_rx_interrupt(port, regs);
@@ -160,7 +192,7 @@ static void __ns16550_poll(struct cpu_user_regs *regs)
serial_rx_interrupt(port, regs);
}
- if ( ns_read_reg(uart, UART_LSR) & UART_LSR_THRE )
+ if ( ( ns_read_reg(uart, UART_LSR) & uart->lsr_mask ) ==
uart->lsr_mask )
serial_tx_interrupt(port, regs);
out:
@@ -183,7 +215,9 @@ static int ns16550_tx_ready(struct serial_port *port)
if ( ns16550_ioport_invalid(uart) )
return -EIO;
- return ns_read_reg(uart, UART_LSR) & UART_LSR_THRE ? uart->fifo_size
: 0;
+
+ return ( (ns_read_reg(uart, UART_LSR) &
+ uart->lsr_mask ) == uart->lsr_mask ) ? uart->fifo_size :
0;
}
static void ns16550_putc(struct serial_port *port, char c)
@@ -550,7 +584,8 @@ static int
pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
{
uint32_t bar, len;
- int b, d, f, nextf;
+ int b, d, f, nextf, i;
+ u16 vendor, device;
/* NB. Start at bus 1 to avoid AMT: a plug-in card cannot be on bus 0. */
for ( b = skip_amt ? 1 : 0; b < 0x100; b++ )
@@ -579,27 +614,63 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int
bar_idx)
bar = pci_conf_read32(0, b, d, f,
PCI_BASE_ADDRESS_0 + bar_idx*4);
+ if ( !bar )
+ continue;
+
/* Not IO */
if ( !(bar & PCI_BASE_ADDRESS_SPACE_IO) )
- continue;
+ {
+ vendor = pci_conf_read16(0, b, d, f, PCI_VENDOR_ID);
+ device = pci_conf_read16(0, b, d, f, PCI_DEVICE_ID);
+
+ /* Check for quirks in uart_config lookup table */
+ for ( i = 0; i < ARRAY_SIZE(uart_config); i++)
+ {
+ if ( uart_config[i].vendor_id != vendor )
+ continue;
+
+ if ( uart_config[i].dev_id != device )
+ continue;
+
+ if ( bar_idx >= uart_config[i].max_bars )
+ break;
+
+ uart->reg_shift = uart_config[i].reg_shift;
+ uart->reg_width = uart_config[i].reg_width;
+ uart->fifo_size = uart_config[i].fifo_size;
+ uart->lsr_mask = uart_config[i].lsr_mask;
+ uart->io_base = bar & PCI_BASE_ADDRESS_MEM_MASK;
+ break;
+ }
+
+ /* If we have an io_base, then we succeeded in the lookup
*/
+ if ( !uart->io_base )
+ continue;
+ }
+ /* IO based */
+ else
+ {
+ pci_conf_write32(0, b, d, f, PCI_BASE_ADDRESS_0, ~0u);
+ len = pci_conf_read32(0, b, d, f, PCI_BASE_ADDRESS_0);
+ pci_conf_write32(0, b, d, f,
+ PCI_BASE_ADDRESS_0 + bar_idx*4, bar);
- pci_conf_write32(0, b, d, f, PCI_BASE_ADDRESS_0, ~0u);
- len = pci_conf_read32(0, b, d, f, PCI_BASE_ADDRESS_0);
- pci_conf_write32(0, b, d, f, PCI_BASE_ADDRESS_0 + bar_idx*4,
bar);
+ /* Not 8 bytes */
+ if ( (len & 0xffff) != 0xfff9 )
+ continue;
- /* Not 8 bytes */
- if ( (len & 0xffff) != 0xfff9 )
- continue;
+ uart->io_base = bar & ~PCI_BASE_ADDRESS_SPACE_IO;
+ }
uart->ps_bdf[0] = b;
uart->ps_bdf[1] = d;
uart->ps_bdf[2] = f;
uart->bar = bar;
uart->bar_idx = bar_idx;
- uart->io_base = bar & ~PCI_BASE_ADDRESS_SPACE_IO;
uart->irq = pci_conf_read8(0, b, d, f, PCI_INTERRUPT_PIN) ?
pci_conf_read8(0, b, d, f, PCI_INTERRUPT_LINE) : 0;
+
return 0;
}
}
@@ -746,6 +817,9 @@ void __init ns16550_init(int index, struct ns16550_defaults
*defaults)
/* Default is no transmit FIFO. */
uart->fifo_size = 1;
+ /* Default lsr_mask = UART_LSR_THRE */
+ uart->lsr_mask = UART_LSR_THRE;
+
ns16550_parse_port_config(uart, (index == 0) ? opt_com1 : opt_com2);
}
--
1.8.1.2
Andrew Cooper
2013-Nov-14 16:42 UTC
Re: [PATCH] ns16550: Add support for UART present in Broadcom TruManage capable NetXtreme chips
On 14/11/13 16:29, Aravind Gopalakrishnan wrote:> There are few quirks regarding the chip: > Firstly, it is an MMIO device. Therefore, the code has been modified to > accept MMIO based devices as well. Settings particular to such devices are > populated in the table ''uart_config''. Currently, we only support BCM5725 > TruManage chip. > > Some more quirks are - the need to shift the register offset by a specific > value and we also need to verify (UART_LSR_THRE && UART_LSR_TEMT) bits before > transmitting data. > > While testing, we ned to include com1=115200,8n1,pci,0 on the xen cmdline to > be able to observe output using SOL. > > Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com> > Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> > Signed-off-by: Thomas Lendacky <Thomas.Lendacky@amd.com> > --- > xen/drivers/char/ns16550.c | 98 ++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 86 insertions(+), 12 deletions(-) > > diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c > index 9c2cded..05d77ba 100644 > --- a/xen/drivers/char/ns16550.c > +++ b/xen/drivers/char/ns16550.c > @@ -53,6 +53,7 @@ static struct ns16550 { > char __iomem *remapped_io_base; /* Remapped virtual address of MMIO. */ > /* UART with IRQ line: interrupt-driven I/O. */ > struct irqaction irqaction; > + u32 lsr_mask; > #ifdef CONFIG_ARM > struct vuart_info vuart; > #endif > @@ -77,6 +78,37 @@ static struct ns16550 { > #endif > } ns16550_com[2] = { { 0 } }; > > +/* Defining uart config options for MMIO devices */ > +struct ns16550_config_mmio { > + u16 vendor_id; > + u16 dev_id; > + int reg_shift;Shifts of signed quantities are a recipe for undefined behaviour. I dont see any reason for any of these values to be signed quantities.> + int reg_width; > + int fifo_size; > + u32 lsr_mask; > + int max_bars; > +}; > + > +/* > + * Create lookup tables for specific MMIO devices.. > + * It is assumed that if the device found is MMIO, > + * then you have indexed it here. Else, the driver > + * does nothing. > + */ > +static struct ns16550_config_mmio uart_config[] > +{ > + /* Broadcom TruManage device */ > + { > + .vendor_id = 0x14e4, > + .dev_id = 0x160a, > + .reg_shift = 2, > + .reg_width = 1, > + .fifo_size = 64, > + .lsr_mask = (UART_LSR_THRE | UART_LSR_TEMT), > + .max_bars = 1, > + }, > +}; > + > static void ns16550_delayed_resume(void *data); > > static char ns_read_reg(struct ns16550 *uart, int reg) > @@ -134,7 +166,7 @@ static void ns16550_interrupt( > while ( !(ns_read_reg(uart, UART_IIR) & UART_IIR_NOINT) ) > { > char lsr = ns_read_reg(uart, UART_LSR); > - if ( lsr & UART_LSR_THRE ) > + if ( (lsr & uart->lsr_mask) == uart->lsr_mask ) > serial_tx_interrupt(port, regs); > if ( lsr & UART_LSR_DR ) > serial_rx_interrupt(port, regs); > @@ -160,7 +192,7 @@ static void __ns16550_poll(struct cpu_user_regs *regs) > serial_rx_interrupt(port, regs); > } > > - if ( ns_read_reg(uart, UART_LSR) & UART_LSR_THRE ) > + if ( ( ns_read_reg(uart, UART_LSR) & uart->lsr_mask ) == uart->lsr_mask ) > serial_tx_interrupt(port, regs); > > out: > @@ -183,7 +215,9 @@ static int ns16550_tx_ready(struct serial_port *port) > > if ( ns16550_ioport_invalid(uart) ) > return -EIO; > - return ns_read_reg(uart, UART_LSR) & UART_LSR_THRE ? uart->fifo_size : 0; > + > + return ( (ns_read_reg(uart, UART_LSR) & > + uart->lsr_mask ) == uart->lsr_mask ) ? uart->fifo_size : 0; > } > > static void ns16550_putc(struct serial_port *port, char c) > @@ -550,7 +584,8 @@ static int > pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx) > { > uint32_t bar, len; > - int b, d, f, nextf; > + int b, d, f, nextf, i; > + u16 vendor, device; > > /* NB. Start at bus 1 to avoid AMT: a plug-in card cannot be on bus 0. */ > for ( b = skip_amt ? 1 : 0; b < 0x100; b++ ) > @@ -579,27 +614,63 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx) > bar = pci_conf_read32(0, b, d, f, > PCI_BASE_ADDRESS_0 + bar_idx*4); > > + if ( !bar ) > + continue; > +What is this check supposed to be doing? This could be a valid non-prefechable 32bit memory bar.> /* Not IO */ > if ( !(bar & PCI_BASE_ADDRESS_SPACE_IO) ) > - continue; > + { > + vendor = pci_conf_read16(0, b, d, f, PCI_VENDOR_ID); > + device = pci_conf_read16(0, b, d, f, PCI_DEVICE_ID); > + > + /* Check for quirks in uart_config lookup table */ > + for ( i = 0; i < ARRAY_SIZE(uart_config); i++) > + { > + if ( uart_config[i].vendor_id != vendor ) > + continue; > + > + if ( uart_config[i].dev_id != device ) > + continue; > + > + if ( bar_idx >= uart_config[i].max_bars ) > + break; > + > + uart->reg_shift = uart_config[i].reg_shift; > + uart->reg_width = uart_config[i].reg_width; > + uart->fifo_size = uart_config[i].fifo_size; > + uart->lsr_mask = uart_config[i].lsr_mask; > + uart->io_base = bar & PCI_BASE_ADDRESS_MEM_MASK; > + break; > + } > + > + /* If we have an io_base, then we succeeded in the lookup */ > + if ( !uart->io_base ) > + continue; > + } > + /* IO based */ > + else > + { > + pci_conf_write32(0, b, d, f, PCI_BASE_ADDRESS_0, ~0u); > + len = pci_conf_read32(0, b, d, f, PCI_BASE_ADDRESS_0); > + pci_conf_write32(0, b, d, f, > + PCI_BASE_ADDRESS_0 + bar_idx*4, bar); > > - pci_conf_write32(0, b, d, f, PCI_BASE_ADDRESS_0, ~0u); > - len = pci_conf_read32(0, b, d, f, PCI_BASE_ADDRESS_0); > - pci_conf_write32(0, b, d, f, PCI_BASE_ADDRESS_0 + bar_idx*4, bar); > + /* Not 8 bytes */ > + if ( (len & 0xffff) != 0xfff9 ) > + continue; > > - /* Not 8 bytes */ > - if ( (len & 0xffff) != 0xfff9 ) > - continue; > + uart->io_base = bar & ~PCI_BASE_ADDRESS_SPACE_IO; > + } > > uart->ps_bdf[0] = b; > uart->ps_bdf[1] = d; > uart->ps_bdf[2] = f; > uart->bar = bar; > uart->bar_idx = bar_idx; > - uart->io_base = bar & ~PCI_BASE_ADDRESS_SPACE_IO; > uart->irq = pci_conf_read8(0, b, d, f, PCI_INTERRUPT_PIN) ? > pci_conf_read8(0, b, d, f, PCI_INTERRUPT_LINE) : 0; > > +Spurious whitespace change. Please remove. ~Andrew> return 0; > } > } > @@ -746,6 +817,9 @@ void __init ns16550_init(int index, struct ns16550_defaults *defaults) > /* Default is no transmit FIFO. */ > uart->fifo_size = 1; > > + /* Default lsr_mask = UART_LSR_THRE */ > + uart->lsr_mask = UART_LSR_THRE; > + > ns16550_parse_port_config(uart, (index == 0) ? opt_com1 : opt_com2); > } >
Aravind Gopalakrioshnan
2013-Nov-14 17:16 UTC
Re: [PATCH] ns16550: Add support for UART present in Broadcom TruManage capable NetXtreme chips
On 11/14/2013 10:42 AM, Andrew Cooper wrote:> On 14/11/13 16:29, Aravind Gopalakrishnan wrote: >> >> +/* Defining uart config options for MMIO devices */ >> +struct ns16550_config_mmio { >> + u16 vendor_id; >> + u16 dev_id; >> + int reg_shift; > Shifts of signed quantities are a recipe for undefined behaviour. I > dont see any reason for any of these values to be signed quantities.You are right.. I will change this..>> + int reg_width; >> + int fifo_size; >> + u32 lsr_mask; >> + int max_bars; >> +}; >> + >> +/* >> + * Create lookup tables for specific MMIO devices.. >> + * It is assumed that if the device found is MMIO, >> + * then you have indexed it here. Else, the driver >> + * does nothing. >> + */ >> +static struct ns16550_config_mmio uart_config[] >> +{ >> + /* Broadcom TruManage device */ >> + { >> + .vendor_id = 0x14e4, >> + .dev_id = 0x160a, >> + .reg_shift = 2, >> + .reg_width = 1, >> + .fifo_size = 64, >> + .lsr_mask = (UART_LSR_THRE | UART_LSR_TEMT), >> + .max_bars = 1, >> + }, >> +}; >> + >> static void ns16550_delayed_resume(void *data); >> >> static char ns_read_reg(struct ns16550 *uart, int reg) >> @@ -134,7 +166,7 @@ static void ns16550_interrupt( >> while ( !(ns_read_reg(uart, UART_IIR) & UART_IIR_NOINT) ) >> { >> char lsr = ns_read_reg(uart, UART_LSR); >> - if ( lsr & UART_LSR_THRE ) >> + if ( (lsr & uart->lsr_mask) == uart->lsr_mask ) >> serial_tx_interrupt(port, regs); >> if ( lsr & UART_LSR_DR ) >> serial_rx_interrupt(port, regs); >> @@ -160,7 +192,7 @@ static void __ns16550_poll(struct cpu_user_regs *regs) >> serial_rx_interrupt(port, regs); >> } >> >> - if ( ns_read_reg(uart, UART_LSR) & UART_LSR_THRE ) >> + if ( ( ns_read_reg(uart, UART_LSR) & uart->lsr_mask ) == uart->lsr_mask ) >> serial_tx_interrupt(port, regs); >> >> out: >> @@ -183,7 +215,9 @@ static int ns16550_tx_ready(struct serial_port *port) >> >> if ( ns16550_ioport_invalid(uart) ) >> return -EIO; >> - return ns_read_reg(uart, UART_LSR) & UART_LSR_THRE ? uart->fifo_size : 0; >> + >> + return ( (ns_read_reg(uart, UART_LSR) & >> + uart->lsr_mask ) == uart->lsr_mask ) ? uart->fifo_size : 0; >> } >> >> static void ns16550_putc(struct serial_port *port, char c) >> @@ -550,7 +584,8 @@ static int >> pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx) >> { >> uint32_t bar, len; >> - int b, d, f, nextf; >> + int b, d, f, nextf, i; >> + u16 vendor, device; >> >> /* NB. Start at bus 1 to avoid AMT: a plug-in card cannot be on bus 0. */ >> for ( b = skip_amt ? 1 : 0; b < 0x100; b++ ) >> @@ -579,27 +614,63 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx) >> bar = pci_conf_read32(0, b, d, f, >> PCI_BASE_ADDRESS_0 + bar_idx*4); >> >> + if ( !bar ) >> + continue; >> + > What is this check supposed to be doing? This could be a valid > non-prefechable 32bit memory bar.Okay, will correct this.>> >> >> uart->ps_bdf[0] = b; >> uart->ps_bdf[1] = d; >> uart->ps_bdf[2] = f; >> uart->bar = bar; >> uart->bar_idx = bar_idx; >> - uart->io_base = bar & ~PCI_BASE_ADDRESS_SPACE_IO; >> uart->irq = pci_conf_read8(0, b, d, f, PCI_INTERRUPT_PIN) ? >> pci_conf_read8(0, b, d, f, PCI_INTERRUPT_LINE) : 0; >> >> + > Spurious whitespace change. Please remove. > > ~AndrewNoted. Will send out changes in a V2. Thanks, -Aravind.