On 08/03/2015 18:31, Peter Hurley wrote:
Au1x00/RT2800+ hardware has an alternate register layout which is remapped with lookup tables by the au_serial_in()/out() i/o accessors. However, the h/w does not support the complete 8250 register set, and accesses to unmapped registers cause out-of-bounds lookups. Further, because the lookup tables are defined by designated initializers, the tables may contain unmapped entries (although the current tables do not). Add the AU_MAP() macro to distinguish mapped and unmapped register lookups (unmapped register lookups (initialized to 0) result in negative register offsets). Validate the register index (ie., 'offset') is in the range [0, table size). Return fixed value for unmapped register reads and ignore unmapped register writes. Reported-by: Mason <slash.tmp@xxxxxxx> Signed-off-by: Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> --- drivers/tty/serial/8250/8250_core.c | 41 ++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index 5eb95fd..87df908 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -358,34 +358,43 @@ static void default_serial_dl_write(struct uart_8250_port *up, int value) #if defined(CONFIG_MIPS_ALCHEMY) || defined(CONFIG_SERIAL_8250_RT288X) /* Au1x00/RT288x UART hardware has a weird register layout */ +#define AU_MAP(x) (0x80 + x) static const u8 au_io_in_map[] = { - [UART_RX] = 0, - [UART_IER] = 2, - [UART_IIR] = 3, - [UART_LCR] = 5, - [UART_MCR] = 6, - [UART_LSR] = 7, - [UART_MSR] = 8, + [UART_RX] = AU_MAP(0), + [UART_IER] = AU_MAP(2), + [UART_IIR] = AU_MAP(3), + [UART_LCR] = AU_MAP(5), + [UART_MCR] = AU_MAP(6), + [UART_LSR] = AU_MAP(7), + [UART_MSR] = AU_MAP(8), }; static const u8 au_io_out_map[] = { - [UART_TX] = 1, - [UART_IER] = 2, - [UART_FCR] = 4, - [UART_LCR] = 5, - [UART_MCR] = 6, + [UART_TX] = AU_MAP(1), + [UART_IER] = AU_MAP(2), + [UART_FCR] = AU_MAP(4), + [UART_LCR] = AU_MAP(5), + [UART_MCR] = AU_MAP(6), }; static unsigned int au_serial_in(struct uart_port *p, int offset) { - offset = au_io_in_map[offset] << p->regshift; - return __raw_readl(p->membase + offset); + if (offset >= ARRAYSIZE(au_io_in_map)) + return UINT_MAX; + offset = au_io_in_map[offset] - AU_MAP(0); + if (offset < 0) + return UINT_MAX; + return __raw_readl(p->membase + (offset << p->regshift)); } static void au_serial_out(struct uart_port *p, int offset, int value) { - offset = au_io_out_map[offset] << p->regshift; - __raw_writel(value, p->membase + offset); + if (offset >= ARRAYSIZE(au_io_out_map)) + return; + offset = au_io_out_map[offset] - AU_MAP(0); + if (offset < 0) + return; + __raw_writel(value, p->membase + (offset << p->regshift)); }
Hello Peter, I understand your concern about the lookup tables being sparse, but once you've checked that they are not, why go to great length to detect the issue at run-time? Do you expect register offsets to change in the future? (They've been stable for a very long time, according to git-log) I would just go with "if (offset >= ARRAYSIZE(foo))" and not the AU_MAP gymnastics. On a tangential subject, as Mans stated, Au1x00 does not support the scratch register; but Ralink and Tango do. These three platforms share the weird register layout (requiring the LUTs), but they differ in scratch support. Would it make any sense to differentiate them, to support scratch where it is implemented? (Although adding offset 7 to au_io_out_map would then make that table sparse!) Regards. -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html