* [PATCH] ns16550: Add support for UART present in Broadcom TruManage capable NetXtreme chips
@ 2013-11-14 16:29 Aravind Gopalakrishnan
2013-11-14 16:42 ` Andrew Cooper
0 siblings, 1 reply; 3+ messages in thread
From: Aravind Gopalakrishnan @ 2013-11-14 16:29 UTC (permalink / raw)
To: keir, xen-devel
Cc: Thomas Lendacky, jbeulich, Aravind Gopalakrishnan,
Suravee Suthikulpanit, sherry.hurwitz, shurd
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ns16550: Add support for UART present in Broadcom TruManage capable NetXtreme chips
2013-11-14 16:29 [PATCH] ns16550: Add support for UART present in Broadcom TruManage capable NetXtreme chips Aravind Gopalakrishnan
@ 2013-11-14 16:42 ` Andrew Cooper
2013-11-14 17:16 ` Aravind Gopalakrioshnan
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Cooper @ 2013-11-14 16:42 UTC (permalink / raw)
To: Aravind Gopalakrishnan
Cc: Thomas Lendacky, keir, jbeulich, xen-devel, Suravee Suthikulpanit,
sherry.hurwitz, shurd
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);
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ns16550: Add support for UART present in Broadcom TruManage capable NetXtreme chips
2013-11-14 16:42 ` Andrew Cooper
@ 2013-11-14 17:16 ` Aravind Gopalakrioshnan
0 siblings, 0 replies; 3+ messages in thread
From: Aravind Gopalakrioshnan @ 2013-11-14 17:16 UTC (permalink / raw)
To: Andrew Cooper
Cc: Thomas Lendacky, keir, jbeulich, xen-devel, Suravee Suthikulpanit,
sherry.hurwitz, shurd
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.
>
> ~Andrew
Noted.
Will send out changes in a V2.
Thanks,
-Aravind.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-11-14 17:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-14 16:29 [PATCH] ns16550: Add support for UART present in Broadcom TruManage capable NetXtreme chips Aravind Gopalakrishnan
2013-11-14 16:42 ` Andrew Cooper
2013-11-14 17:16 ` Aravind Gopalakrioshnan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).