lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 14 Feb 2017 20:39:40 -0600 From: Timur Tabi <timur@...eaurora.org> To: Christopher Covington <cov@...eaurora.org>, Jonathan Corbet <corbet@....net>, Marc Zyngier <marc.zyngier@....com>, Catalin Marinas <catalin.marinas@....com>, Will Deacon <will.deacon@....com>, linux-doc@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, Mark Rutland <mark.rutland@....com>, Peter Hurley <peter@...leysoftware.com>, Aleksey Makarov <aleksey.makarov@...aro.org>, Robin Murphy <robin.murphy@....com>, linux-kernel@...r.kernel.org, shankerd@...eaurora.org, "Rafael J. Wysocki" <rjw@...ysocki.net>, Len Brown <lenb@...nel.org>, Russell King <linux@...linux.org.uk>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Jiri Slaby <jslaby@...e.com>, linux-acpi@...r.kernel.org, linux-serial@...r.kernel.org Cc: Mark Langsdorf <mlangsdo@...hat.com>, Mark Salter <msalter@...hat.com>, Jon Masters <jcm@...hat.com>, Neil Leeder <nleeder@...eaurora.org> Subject: Re: [PATCH v2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit Christopher Covington wrote: > The Qualcomm Datacenter Technologies QDF2400 family of SoCs contains a > custom (non-PrimeCell) implementation of the SBSA UART. Occasionally the > BUSY bit in the Flag Register gets stuck as 1, erratum 44 for both 2432v1 > and 2400v1 SoCs. Checking that the Transmit FIFO Empty (TXFE) bit is 0, > instead of checking that the BUSY bit is 1, works around the issue. To > facilitate this substitution when UART AMBA Port (UAP) data is available, > introduce vendor-specific inversion of Feature Register bits. For the > earlycon case, implement alternative putc and early_write functions. > Similar to what how ARMv8 ACPI PCI quirks are detected during MCFG parsing, > check the OEM fields of the Serial Port Console Redirection (SPCR) ACPI > table to determine if the current platform is known to be affected by the > erratum. Please break this up into paragraphs. > + if (!memcmp(table->header.oem_id, "QCOM ", ACPI_OEM_ID_SIZE)) > + if (!memcmp(table->header.oem_table_id, "QDF2432 ", > + ACPI_OEM_TABLE_ID_SIZE) || > + (!memcmp(table->header.oem_table_id, > + "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) && > + table->header.oem_revision == 0)) > + uart = "qdf2400_e44"; > + Hey, you just wrote this And it looks crazy But here's my function So call it, maybe? static bool needs_qdf2400_e44(struct acpi_table_header *h) { const char *id; /* The erratum only applies to Qualcomm Technologies SOCs */ if (memcmp(h->oem_id, "QCOM ", ACPI_OEM_ID_SIZE)) return False; id = h->oem_table_id; if (!memcmp(id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE)) return True; if (!memcmp(id, "QDF2000 ", ACPI_OEM_TABLE_ID_SIZE) && h->oem_revision == 0) return True; return False; } ... if (needs_qdf2400_e44(&table->header)) uart = "qdf2400_e44"; > +/* > + * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as > + * occasionally getting stuck as 1. To avoid the potential for a hang, check > + * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART > + * implementations, so only do so if an affected platform is detected in > + * parse_spcr(). > + */ > +static bool qdf2400_e44 = false; Please rename this to needs_qdf2400_e44 > static u16 pl011_st_offsets[REG_ARRAY_SIZE] = { > [REG_DR] = UART01x_DR, > [REG_ST_DMAWM] = ST_UART011_DMAWM, > @@ -1518,7 +1543,8 @@ static unsigned int pl011_tx_empty(struct uart_port *port) > { > struct uart_amba_port *uap = > container_of(port, struct uart_amba_port, port); > - unsigned int status = pl011_read(uap, REG_FR); > + /* Allow feature register bits to be inverted to work around errata */ Blank line above the comment, please. > + unsigned int status = pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr; > return status & (uap->vendor->fr_busy | UART01x_FR_TXFF) ? > 0 : TIOCSER_TEMT; > } > @@ -2215,10 +2241,12 @@ pl011_console_write(struct console *co, const char *s, unsigned int count) > uart_console_write(&uap->port, s, count, pl011_console_putchar); > > /* > - * Finally, wait for transmitter to become empty > - * and restore the TCR > + * Finally, wait for transmitter to become empty and restore the > + * TCR. Allow feature register bits to be inverted to work around > + * errata. > */ > - while (pl011_read(uap, REG_FR) & uap->vendor->fr_busy) > + while ((pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr) > + & uap->vendor->fr_busy) > cpu_relax(); > if (!uap->vendor->always_enabled) > pl011_write(old_cr, uap, REG_CR); > @@ -2340,8 +2368,13 @@ static int __init pl011_console_match(struct console *co, char *name, int idx, > resource_size_t addr; > int i; > > - if (strcmp(name, "pl011") != 0) > + if (strcmp(name, "qdf2400_e44") == 0) { > + if (!qdf2400_e44) > + pr_info("UART: Working around QDF2400 SoC erratum 44"); Just use pr_once(), and you can skip the "if (!qdf2400_e44)". Although I don't understand why you need to do that. pl011_console_match() should only be called once anyway, right? > + qdf2400_e44 = true; > + } else if (strcmp(name, "pl011") != 0) { > return -ENODEV; > + } > > if (uart_parse_earlycon(options, &iotype, &addr, &options)) > return -ENODEV; > @@ -2383,6 +2416,25 @@ static struct console amba_console = { > > #define AMBA_CONSOLE (&amba_console) > > +static void qdf2400_e44_putc(struct uart_port *port, int c) > +{ > + while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF) > + cpu_relax(); > + if (port->iotype == UPIO_MEM32) > + writel(c, port->membase + UART01x_DR); > + else > + writeb(c, port->membase + UART01x_DR); I think you can ignore the UPIO_MEM32 test and always use writel(), even on the QDF2400. -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation.
Powered by blists - more mailing lists