[PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Christopher Covington]

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. To keep the
change small, this patch only works around the full console case, where UAP
data is available, and does not handle the erratum for earlycon, as the UAP
data is not available then.

Signed-off-by: Christopher Covington <cov@codeaurora.org>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
---
Changes between the previous RFC [1] and this PATCH:
* don't use arch/arm64/kernel/cpu_errata.c at Will's request
* separate out earlycon case to separate patch
* rework earlycon case to not depend on UAP as suggested by Timur

Because they need a newly-defined MIDR values, the patches are currently
based on:
https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/log/?h=for-next/core

I'm not confident that I know the best route for these two patches. Should
I request Catalin and Will to take them via arm64 as the essential MIDR
additions are their purview?

Thanks Russell for the ack. Compared to the RFC, I've made minor changes to
what is now patch 1/2 and am making an educated guess that the ack sticks
(but if not please let me know). Patch 2/2 is significantly revised from
the RFC so I've not included the ack on it.

1. https://patchwork.codeaurora.org/patch/163281/
---
 Documentation/arm64/silicon-errata.txt |  2 ++
 arch/arm64/include/asm/cputype.h       |  2 ++
 drivers/tty/serial/Kconfig             | 12 ++++++++++
 drivers/tty/serial/amba-pl011.c        | 40 +++++++++++++++++++++++++++++++---
 4 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
index 50da8391e9dd..0993ebb3e86b 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -65,3 +65,5 @@ stable kernels.
 | Freescale/NXP  | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585     |
 | Qualcomm Tech. | Falkor v1       | E1003           | QCOM_FALKOR_ERRATUM_1003|
 | Qualcomm Tech. | Falkor v1       | E1009           | QCOM_FALKOR_ERRATUM_1009|
+| Qualcomm Tech. | QDF2432v1 UART  | SoC E44         | QCOM_QDF2400_ERRATUM_44 |
+| Qualcomm Tech. | QDF2400v1 UART  | SoC E44         | QCOM_QDF2400_ERRATUM_44 |
diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index fc502713ab37..cb399c7fe6ec 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -88,12 +88,14 @@
 
 #define BRCM_CPU_PART_VULCAN		0x516
 
+#define QCOM_CPU_PART_KRYO_V1		0x281
 #define QCOM_CPU_PART_FALKOR_V1		0x800
 
 #define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
 #define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
 #define MIDR_THUNDERX	MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
 #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
+#define MIDR_QCOM_KRYO_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_KRYO_V1)
 #define MIDR_QCOM_FALKOR_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_FALKOR_V1)
 
 #ifndef __ASSEMBLY__
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index e9cf5b67f1b7..4cde8f48a540 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -73,6 +73,18 @@ config SERIAL_AMBA_PL011_CONSOLE
 	  your boot loader (lilo or loadlin) about how to pass options to the
 	  kernel at boot time.)
 
+config QCOM_QDF2400_ERRATUM_44
+	bool "Work around QDF2400 SoC E44 stuck BUSY bit"
+	depends on SERIAL_AMBA_PL011_CONSOLE=y
+	default y
+	help
+	  The BUSY bit in the Flag Register of the UART on the QDF2432v1 and
+	  QDF2400v1 SoCs may get stuck as 1, resulting in a hung serial console.
+	  Say Y here to work around the issue by checking TXFE == 0 instead of
+	  BUSY == 1 on affected systems.
+
+	  If unsure, say Y.
+
 config SERIAL_EARLYCON_ARM_SEMIHOST
 	bool "Early console using ARM semihosting"
 	depends on ARM64 || ARM
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index d4171d71a258..41e51901d6ef 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -97,6 +97,7 @@ struct vendor_data {
 	unsigned int		fr_dsr;
 	unsigned int		fr_cts;
 	unsigned int		fr_ri;
+	unsigned int		inv_fr;
 	bool			access_32b;
 	bool			oversampling;
 	bool			dma_threshold;
@@ -141,6 +142,25 @@ static struct vendor_data vendor_sbsa = {
 	.fixed_options		= true,
 };
 
+#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
+static struct vendor_data vendor_qdt_qdf2400_e44 = {
+	.reg_offset		= pl011_std_offsets,
+	.fr_busy		= UART011_FR_TXFE,
+	.fr_dsr			= UART01x_FR_DSR,
+	.fr_cts			= UART01x_FR_CTS,
+	.fr_ri			= UART011_FR_RI,
+	.inv_fr			= UART011_FR_TXFE,
+	.access_32b		= true,
+	.oversampling		= false,
+	.dma_threshold		= false,
+	.cts_event_workaround	= false,
+	.always_enabled		= true,
+	.fixed_options		= true,
+};
+#else
+#define vendor_qdt_qdf2400_e44 vendor_sbsa
+#endif
+
 static u16 pl011_st_offsets[REG_ARRAY_SIZE] = {
 	[REG_DR] = UART01x_DR,
 	[REG_ST_DMAWM] = ST_UART011_DMAWM,
@@ -1518,7 +1538,7 @@ 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);
+	unsigned int status = pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr;
 	return status & (uap->vendor->fr_busy | UART01x_FR_TXFF) ?
 							0 : TIOCSER_TEMT;
 }
@@ -2218,7 +2238,8 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
 	 *	Finally, wait for transmitter to become empty
 	 *	and restore the TCR
 	 */
-	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);
@@ -2383,6 +2404,13 @@ static struct console amba_console = {
 
 #define AMBA_CONSOLE	(&amba_console)
 
+static bool qdf2400_e44(void) {
+	u32 cpu_var_model = read_cpuid_id() & ~MIDR_REVISION_MASK;
+
+	return (cpu_var_model == MIDR_QCOM_KRYO_V1 ||
+	    cpu_var_model == MIDR_QCOM_FALKOR_V1);
+}
+
 static void pl011_putc(struct uart_port *port, int c)
 {
 	while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
@@ -2645,12 +2673,18 @@ static int sbsa_uart_probe(struct platform_device *pdev)
 	uap->port.irq	= ret;
 
 	uap->reg_offset	= vendor_sbsa.reg_offset;
-	uap->vendor	= &vendor_sbsa;
 	uap->fifosize	= 32;
 	uap->port.iotype = vendor_sbsa.access_32b ? UPIO_MEM32 : UPIO_MEM;
 	uap->port.ops	= &sbsa_uart_pops;
 	uap->fixed_baud = baudrate;
 
+	if (qdf2400_e44()) {
+		uap->vendor = &vendor_qdt_qdf2400_e44;
+		dev_info(&pdev->dev, "working around QDF2400 SoC erratum 44\n");
+	} else {
+		uap->vendor = &vendor_sbsa;
+	}
+
 	snprintf(uap->type, sizeof(uap->type), "SBSA");
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora
Forum, a Linux Foundation Collaborative Project.

[PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Christopher Covington]

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. To keep the
change small, this patch only works around the full console case, where UAP
data is available, and does not handle the erratum for earlycon, as the UAP
data is not available then.

Signed-off-by: Christopher Covington <cov@codeaurora.org>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
---
Changes between the previous RFC [1] and this PATCH:
* don't use arch/arm64/kernel/cpu_errata.c at Will's request
* separate out earlycon case to separate patch
* rework earlycon case to not depend on UAP as suggested by Timur

Because they need a newly-defined MIDR values, the patches are currently
based on:
https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/log/?h=for-next/core

I'm not confident that I know the best route for these two patches. Should
I request Catalin and Will to take them via arm64 as the essential MIDR
additions are their purview?

Thanks Russell for the ack. Compared to the RFC, I've made minor changes to
what is now patch 1/2 and am making an educated guess that the ack sticks
(but if not please let me know). Patch 2/2 is significantly revised from
the RFC so I've not included the ack on it.

1. https://patchwork.codeaurora.org/patch/163281/
---
 Documentation/arm64/silicon-errata.txt |  2 ++
 arch/arm64/include/asm/cputype.h       |  2 ++
 drivers/tty/serial/Kconfig             | 12 ++++++++++
 drivers/tty/serial/amba-pl011.c        | 40 +++++++++++++++++++++++++++++++---
 4 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
index 50da8391e9dd..0993ebb3e86b 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -65,3 +65,5 @@ stable kernels.
 | Freescale/NXP  | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585     |
 | Qualcomm Tech. | Falkor v1       | E1003           | QCOM_FALKOR_ERRATUM_1003|
 | Qualcomm Tech. | Falkor v1       | E1009           | QCOM_FALKOR_ERRATUM_1009|
+| Qualcomm Tech. | QDF2432v1 UART  | SoC E44         | QCOM_QDF2400_ERRATUM_44 |
+| Qualcomm Tech. | QDF2400v1 UART  | SoC E44         | QCOM_QDF2400_ERRATUM_44 |
diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index fc502713ab37..cb399c7fe6ec 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -88,12 +88,14 @@
 
 #define BRCM_CPU_PART_VULCAN		0x516
 
+#define QCOM_CPU_PART_KRYO_V1		0x281
 #define QCOM_CPU_PART_FALKOR_V1		0x800
 
 #define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
 #define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
 #define MIDR_THUNDERX	MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
 #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
+#define MIDR_QCOM_KRYO_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_KRYO_V1)
 #define MIDR_QCOM_FALKOR_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_FALKOR_V1)
 
 #ifndef __ASSEMBLY__
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index e9cf5b67f1b7..4cde8f48a540 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -73,6 +73,18 @@ config SERIAL_AMBA_PL011_CONSOLE
 	  your boot loader (lilo or loadlin) about how to pass options to the
 	  kernel at boot time.)
 
+config QCOM_QDF2400_ERRATUM_44
+	bool "Work around QDF2400 SoC E44 stuck BUSY bit"
+	depends on SERIAL_AMBA_PL011_CONSOLE=y
+	default y
+	help
+	  The BUSY bit in the Flag Register of the UART on the QDF2432v1 and
+	  QDF2400v1 SoCs may get stuck as 1, resulting in a hung serial console.
+	  Say Y here to work around the issue by checking TXFE == 0 instead of
+	  BUSY == 1 on affected systems.
+
+	  If unsure, say Y.
+
 config SERIAL_EARLYCON_ARM_SEMIHOST
 	bool "Early console using ARM semihosting"
 	depends on ARM64 || ARM
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index d4171d71a258..41e51901d6ef 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -97,6 +97,7 @@ struct vendor_data {
 	unsigned int		fr_dsr;
 	unsigned int		fr_cts;
 	unsigned int		fr_ri;
+	unsigned int		inv_fr;
 	bool			access_32b;
 	bool			oversampling;
 	bool			dma_threshold;
@@ -141,6 +142,25 @@ static struct vendor_data vendor_sbsa = {
 	.fixed_options		= true,
 };
 
+#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
+static struct vendor_data vendor_qdt_qdf2400_e44 = {
+	.reg_offset		= pl011_std_offsets,
+	.fr_busy		= UART011_FR_TXFE,
+	.fr_dsr			= UART01x_FR_DSR,
+	.fr_cts			= UART01x_FR_CTS,
+	.fr_ri			= UART011_FR_RI,
+	.inv_fr			= UART011_FR_TXFE,
+	.access_32b		= true,
+	.oversampling		= false,
+	.dma_threshold		= false,
+	.cts_event_workaround	= false,
+	.always_enabled		= true,
+	.fixed_options		= true,
+};
+#else
+#define vendor_qdt_qdf2400_e44 vendor_sbsa
+#endif
+
 static u16 pl011_st_offsets[REG_ARRAY_SIZE] = {
 	[REG_DR] = UART01x_DR,
 	[REG_ST_DMAWM] = ST_UART011_DMAWM,
@@ -1518,7 +1538,7 @@ 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);
+	unsigned int status = pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr;
 	return status & (uap->vendor->fr_busy | UART01x_FR_TXFF) ?
 							0 : TIOCSER_TEMT;
 }
@@ -2218,7 +2238,8 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
 	 *	Finally, wait for transmitter to become empty
 	 *	and restore the TCR
 	 */
-	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);
@@ -2383,6 +2404,13 @@ static struct console amba_console = {
 
 #define AMBA_CONSOLE	(&amba_console)
 
+static bool qdf2400_e44(void) {
+	u32 cpu_var_model = read_cpuid_id() & ~MIDR_REVISION_MASK;
+
+	return (cpu_var_model == MIDR_QCOM_KRYO_V1 ||
+	    cpu_var_model == MIDR_QCOM_FALKOR_V1);
+}
+
 static void pl011_putc(struct uart_port *port, int c)
 {
 	while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
@@ -2645,12 +2673,18 @@ static int sbsa_uart_probe(struct platform_device *pdev)
 	uap->port.irq	= ret;
 
 	uap->reg_offset	= vendor_sbsa.reg_offset;
-	uap->vendor	= &vendor_sbsa;
 	uap->fifosize	= 32;
 	uap->port.iotype = vendor_sbsa.access_32b ? UPIO_MEM32 : UPIO_MEM;
 	uap->port.ops	= &sbsa_uart_pops;
 	uap->fixed_baud = baudrate;
 
+	if (qdf2400_e44()) {
+		uap->vendor = &vendor_qdt_qdf2400_e44;
+		dev_info(&pdev->dev, "working around QDF2400 SoC erratum 44\n");
+	} else {
+		uap->vendor = &vendor_sbsa;
+	}
+
 	snprintf(uap->type, sizeof(uap->type), "SBSA");
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora
Forum, a Linux Foundation Collaborative Project.

[PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Christopher Covington]

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. To keep the
change small, this patch only works around the full console case, where UAP
data is available, and does not handle the erratum for earlycon, as the UAP
data is not available then.

Signed-off-by: Christopher Covington <cov@codeaurora.org>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
---
Changes between the previous RFC [1] and this PATCH:
* don't use arch/arm64/kernel/cpu_errata.c at Will's request
* separate out earlycon case to separate patch
* rework earlycon case to not depend on UAP as suggested by Timur

Because they need a newly-defined MIDR values, the patches are currently
based on:
https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/log/?h=for-next/core

I'm not confident that I know the best route for these two patches. Should
I request Catalin and Will to take them via arm64 as the essential MIDR
additions are their purview?

Thanks Russell for the ack. Compared to the RFC, I've made minor changes to
what is now patch 1/2 and am making an educated guess that the ack sticks
(but if not please let me know). Patch 2/2 is significantly revised from
the RFC so I've not included the ack on it.

1. https://patchwork.codeaurora.org/patch/163281/
---
 Documentation/arm64/silicon-errata.txt |  2 ++
 arch/arm64/include/asm/cputype.h       |  2 ++
 drivers/tty/serial/Kconfig             | 12 ++++++++++
 drivers/tty/serial/amba-pl011.c        | 40 +++++++++++++++++++++++++++++++---
 4 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
index 50da8391e9dd..0993ebb3e86b 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -65,3 +65,5 @@ stable kernels.
 | Freescale/NXP  | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585     |
 | Qualcomm Tech. | Falkor v1       | E1003           | QCOM_FALKOR_ERRATUM_1003|
 | Qualcomm Tech. | Falkor v1       | E1009           | QCOM_FALKOR_ERRATUM_1009|
+| Qualcomm Tech. | QDF2432v1 UART  | SoC E44         | QCOM_QDF2400_ERRATUM_44 |
+| Qualcomm Tech. | QDF2400v1 UART  | SoC E44         | QCOM_QDF2400_ERRATUM_44 |
diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index fc502713ab37..cb399c7fe6ec 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -88,12 +88,14 @@
 
 #define BRCM_CPU_PART_VULCAN		0x516
 
+#define QCOM_CPU_PART_KRYO_V1		0x281
 #define QCOM_CPU_PART_FALKOR_V1		0x800
 
 #define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
 #define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
 #define MIDR_THUNDERX	MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
 #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
+#define MIDR_QCOM_KRYO_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_KRYO_V1)
 #define MIDR_QCOM_FALKOR_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_FALKOR_V1)
 
 #ifndef __ASSEMBLY__
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index e9cf5b67f1b7..4cde8f48a540 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -73,6 +73,18 @@ config SERIAL_AMBA_PL011_CONSOLE
 	  your boot loader (lilo or loadlin) about how to pass options to the
 	  kernel at boot time.)
 
+config QCOM_QDF2400_ERRATUM_44
+	bool "Work around QDF2400 SoC E44 stuck BUSY bit"
+	depends on SERIAL_AMBA_PL011_CONSOLE=y
+	default y
+	help
+	  The BUSY bit in the Flag Register of the UART on the QDF2432v1 and
+	  QDF2400v1 SoCs may get stuck as 1, resulting in a hung serial console.
+	  Say Y here to work around the issue by checking TXFE == 0 instead of
+	  BUSY == 1 on affected systems.
+
+	  If unsure, say Y.
+
 config SERIAL_EARLYCON_ARM_SEMIHOST
 	bool "Early console using ARM semihosting"
 	depends on ARM64 || ARM
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index d4171d71a258..41e51901d6ef 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -97,6 +97,7 @@ struct vendor_data {
 	unsigned int		fr_dsr;
 	unsigned int		fr_cts;
 	unsigned int		fr_ri;
+	unsigned int		inv_fr;
 	bool			access_32b;
 	bool			oversampling;
 	bool			dma_threshold;
@@ -141,6 +142,25 @@ static struct vendor_data vendor_sbsa = {
 	.fixed_options		= true,
 };
 
+#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
+static struct vendor_data vendor_qdt_qdf2400_e44 = {
+	.reg_offset		= pl011_std_offsets,
+	.fr_busy		= UART011_FR_TXFE,
+	.fr_dsr			= UART01x_FR_DSR,
+	.fr_cts			= UART01x_FR_CTS,
+	.fr_ri			= UART011_FR_RI,
+	.inv_fr			= UART011_FR_TXFE,
+	.access_32b		= true,
+	.oversampling		= false,
+	.dma_threshold		= false,
+	.cts_event_workaround	= false,
+	.always_enabled		= true,
+	.fixed_options		= true,
+};
+#else
+#define vendor_qdt_qdf2400_e44 vendor_sbsa
+#endif
+
 static u16 pl011_st_offsets[REG_ARRAY_SIZE] = {
 	[REG_DR] = UART01x_DR,
 	[REG_ST_DMAWM] = ST_UART011_DMAWM,
@@ -1518,7 +1538,7 @@ 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);
+	unsigned int status = pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr;
 	return status & (uap->vendor->fr_busy | UART01x_FR_TXFF) ?
 							0 : TIOCSER_TEMT;
 }
@@ -2218,7 +2238,8 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
 	 *	Finally, wait for transmitter to become empty
 	 *	and restore the TCR
 	 */
-	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);
@@ -2383,6 +2404,13 @@ static struct console amba_console = {
 
 #define AMBA_CONSOLE	(&amba_console)
 
+static bool qdf2400_e44(void) {
+	u32 cpu_var_model = read_cpuid_id() & ~MIDR_REVISION_MASK;
+
+	return (cpu_var_model == MIDR_QCOM_KRYO_V1 ||
+	    cpu_var_model == MIDR_QCOM_FALKOR_V1);
+}
+
 static void pl011_putc(struct uart_port *port, int c)
 {
 	while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
@@ -2645,12 +2673,18 @@ static int sbsa_uart_probe(struct platform_device *pdev)
 	uap->port.irq	= ret;
 
 	uap->reg_offset	= vendor_sbsa.reg_offset;
-	uap->vendor	= &vendor_sbsa;
 	uap->fifosize	= 32;
 	uap->port.iotype = vendor_sbsa.access_32b ? UPIO_MEM32 : UPIO_MEM;
 	uap->port.ops	= &sbsa_uart_pops;
 	uap->fixed_baud = baudrate;
 
+	if (qdf2400_e44()) {
+		uap->vendor = &vendor_qdt_qdf2400_e44;
+		dev_info(&pdev->dev, "working around QDF2400 SoC erratum 44\n");
+	} else {
+		uap->vendor = &vendor_sbsa;
+	}
+
 	snprintf(uap->type, sizeof(uap->type), "SBSA");
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora
Forum, a Linux Foundation Collaborative Project.

[PATCH 2/2] tty: pl011: Work around QDF2400 E44 for earlycon

[Christopher Covington]

The previous change worked around QDF2432v1 and QDF2400v1 SoC erratum 44
for the full-fledged console, when UART AMBA Port (UAP) data is available.
Additionally provide a workaround the earlycon case, again checking TXFE ==
0 instead of BUSY == 1. As earlycon is operating before UAP data is
available, the implementation is different than in the preceding patch.

Signed-off-by: Christopher Covington <cov@codeaurora.org>
---
 drivers/tty/serial/amba-pl011.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 41e51901d6ef..f25e7c994f8e 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2411,6 +2411,29 @@ static bool qdf2400_e44(void) {
 	    cpu_var_model == MIDR_QCOM_FALKOR_V1);
 }
 
+#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
+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);
+	while (!(readl(port->membase + UART01x_FR) & UART011_FR_TXFE))
+		cpu_relax();
+}
+
+static void qdf2400_e44_early_write(struct console *con, const char *s, unsigned n)
+{
+	struct earlycon_device *dev = con->data;
+
+	uart_console_write(&dev->port, s, n, qdf2400_e44_putc);
+}
+#else
+#define qdf2400_e44_early_write pl011_early_write
+#endif
+
 static void pl011_putc(struct uart_port *port, int c)
 {
 	while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
@@ -2436,7 +2459,10 @@ static int __init pl011_early_console_setup(struct earlycon_device *device,
 	if (!device->port.membase)
 		return -ENODEV;
 
-	device->con->write = pl011_early_write;
+	if (qdf2400_e44())
+		device->con->write = qdf2400_e44_early_write;
+	else
+		device->con->write = pl011_early_write;
 	return 0;
 }
 OF_EARLYCON_DECLARE(pl011, "arm,pl011", pl011_early_console_setup);
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora
Forum, a Linux Foundation Collaborative Project.

[PATCH 2/2] tty: pl011: Work around QDF2400 E44 for earlycon

[Christopher Covington]

The previous change worked around QDF2432v1 and QDF2400v1 SoC erratum 44
for the full-fledged console, when UART AMBA Port (UAP) data is available.
Additionally provide a workaround the earlycon case, again checking TXFE ==
0 instead of BUSY == 1. As earlycon is operating before UAP data is
available, the implementation is different than in the preceding patch.

Signed-off-by: Christopher Covington <cov@codeaurora.org>
---
 drivers/tty/serial/amba-pl011.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 41e51901d6ef..f25e7c994f8e 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2411,6 +2411,29 @@ static bool qdf2400_e44(void) {
 	    cpu_var_model == MIDR_QCOM_FALKOR_V1);
 }
 
+#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
+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);
+	while (!(readl(port->membase + UART01x_FR) & UART011_FR_TXFE))
+		cpu_relax();
+}
+
+static void qdf2400_e44_early_write(struct console *con, const char *s, unsigned n)
+{
+	struct earlycon_device *dev = con->data;
+
+	uart_console_write(&dev->port, s, n, qdf2400_e44_putc);
+}
+#else
+#define qdf2400_e44_early_write pl011_early_write
+#endif
+
 static void pl011_putc(struct uart_port *port, int c)
 {
 	while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
@@ -2436,7 +2459,10 @@ static int __init pl011_early_console_setup(struct earlycon_device *device,
 	if (!device->port.membase)
 		return -ENODEV;
 
-	device->con->write = pl011_early_write;
+	if (qdf2400_e44())
+		device->con->write = qdf2400_e44_early_write;
+	else
+		device->con->write = pl011_early_write;
 	return 0;
 }
 OF_EARLYCON_DECLARE(pl011, "arm,pl011", pl011_early_console_setup);
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora
Forum, a Linux Foundation Collaborative Project.

[PATCH 2/2] tty: pl011: Work around QDF2400 E44 for earlycon

[Christopher Covington]

The previous change worked around QDF2432v1 and QDF2400v1 SoC erratum 44
for the full-fledged console, when UART AMBA Port (UAP) data is available.
Additionally provide a workaround the earlycon case, again checking TXFE ==
0 instead of BUSY == 1. As earlycon is operating before UAP data is
available, the implementation is different than in the preceding patch.

Signed-off-by: Christopher Covington <cov@codeaurora.org>
---
 drivers/tty/serial/amba-pl011.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 41e51901d6ef..f25e7c994f8e 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2411,6 +2411,29 @@ static bool qdf2400_e44(void) {
 	    cpu_var_model == MIDR_QCOM_FALKOR_V1);
 }
 
+#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
+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);
+	while (!(readl(port->membase + UART01x_FR) & UART011_FR_TXFE))
+		cpu_relax();
+}
+
+static void qdf2400_e44_early_write(struct console *con, const char *s, unsigned n)
+{
+	struct earlycon_device *dev = con->data;
+
+	uart_console_write(&dev->port, s, n, qdf2400_e44_putc);
+}
+#else
+#define qdf2400_e44_early_write pl011_early_write
+#endif
+
 static void pl011_putc(struct uart_port *port, int c)
 {
 	while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
@@ -2436,7 +2459,10 @@ static int __init pl011_early_console_setup(struct earlycon_device *device,
 	if (!device->port.membase)
 		return -ENODEV;
 
-	device->con->write = pl011_early_write;
+	if (qdf2400_e44())
+		device->con->write = qdf2400_e44_early_write;
+	else
+		device->con->write = pl011_early_write;
 	return 0;
 }
 OF_EARLYCON_DECLARE(pl011, "arm,pl011", pl011_early_console_setup);
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora
Forum, a Linux Foundation Collaborative Project.

Re: [PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Timur Tabi]

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. To keep the
> change small, this patch only works around the full console case, where UAP
> data is available, and does not handle the erratum for earlycon, as the UAP
> data is not available then.
>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> Changes between the previous RFC [1] and this PATCH:
> * don't use arch/arm64/kernel/cpu_errata.c at Will's request
> * separate out earlycon case to separate patch
> * rework earlycon case to not depend on UAP as suggested by Timur
>
> Because they need a newly-defined MIDR values, the patches are currently
> based on:
> https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/log/?h=for-next/core
>
> I'm not confident that I know the best route for these two patches. Should
> I request Catalin and Will to take them via arm64 as the essential MIDR
> additions are their purview?
>
> Thanks Russell for the ack. Compared to the RFC, I've made minor changes to
> what is now patch 1/2 and am making an educated guess that the ack sticks
> (but if not please let me know). Patch 2/2 is significantly revised from
> the RFC so I've not included the ack on it.
>
> 1. https://patchwork.codeaurora.org/patch/163281/
> ---
>  Documentation/arm64/silicon-errata.txt |  2 ++
>  arch/arm64/include/asm/cputype.h       |  2 ++
>  drivers/tty/serial/Kconfig             | 12 ++++++++++
>  drivers/tty/serial/amba-pl011.c        | 40 +++++++++++++++++++++++++++++++---
>  4 files changed, 53 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
> index 50da8391e9dd..0993ebb3e86b 100644
> --- a/Documentation/arm64/silicon-errata.txt
> +++ b/Documentation/arm64/silicon-errata.txt
> @@ -65,3 +65,5 @@ stable kernels.
>  | Freescale/NXP  | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585     |
>  | Qualcomm Tech. | Falkor v1       | E1003           | QCOM_FALKOR_ERRATUM_1003|
>  | Qualcomm Tech. | Falkor v1       | E1009           | QCOM_FALKOR_ERRATUM_1009|
> +| Qualcomm Tech. | QDF2432v1 UART  | SoC E44         | QCOM_QDF2400_ERRATUM_44 |
> +| Qualcomm Tech. | QDF2400v1 UART  | SoC E44         | QCOM_QDF2400_ERRATUM_44 |
> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> index fc502713ab37..cb399c7fe6ec 100644
> --- a/arch/arm64/include/asm/cputype.h
> +++ b/arch/arm64/include/asm/cputype.h
> @@ -88,12 +88,14 @@
>
>  #define BRCM_CPU_PART_VULCAN		0x516
>
> +#define QCOM_CPU_PART_KRYO_V1		0x281
>  #define QCOM_CPU_PART_FALKOR_V1		0x800
>
>  #define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
>  #define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
>  #define MIDR_THUNDERX	MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
>  #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
> +#define MIDR_QCOM_KRYO_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_KRYO_V1)
>  #define MIDR_QCOM_FALKOR_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_FALKOR_V1)
>
>  #ifndef __ASSEMBLY__
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index e9cf5b67f1b7..4cde8f48a540 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -73,6 +73,18 @@ config SERIAL_AMBA_PL011_CONSOLE
>  	  your boot loader (lilo or loadlin) about how to pass options to the
>  	  kernel at boot time.)
>
> +config QCOM_QDF2400_ERRATUM_44
> +	bool "Work around QDF2400 SoC E44 stuck BUSY bit"
> +	depends on SERIAL_AMBA_PL011_CONSOLE=y
> +	default y
> +	help
> +	  The BUSY bit in the Flag Register of the UART on the QDF2432v1 and
> +	  QDF2400v1 SoCs may get stuck as 1, resulting in a hung serial console.
> +	  Say Y here to work around the issue by checking TXFE == 0 instead of
> +	  BUSY == 1 on affected systems.
> +
> +	  If unsure, say Y.
> +
>  config SERIAL_EARLYCON_ARM_SEMIHOST
>  	bool "Early console using ARM semihosting"
>  	depends on ARM64 || ARM
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index d4171d71a258..41e51901d6ef 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -97,6 +97,7 @@ struct vendor_data {
>  	unsigned int		fr_dsr;
>  	unsigned int		fr_cts;
>  	unsigned int		fr_ri;
> +	unsigned int		inv_fr;
>  	bool			access_32b;
>  	bool			oversampling;
>  	bool			dma_threshold;
> @@ -141,6 +142,25 @@ static struct vendor_data vendor_sbsa = {
>  	.fixed_options		= true,
>  };
>
> +#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
> +static struct vendor_data vendor_qdt_qdf2400_e44 = {
> +	.reg_offset		= pl011_std_offsets,
> +	.fr_busy		= UART011_FR_TXFE,
> +	.fr_dsr			= UART01x_FR_DSR,
> +	.fr_cts			= UART01x_FR_CTS,
> +	.fr_ri			= UART011_FR_RI,
> +	.inv_fr			= UART011_FR_TXFE,
> +	.access_32b		= true,
> +	.oversampling		= false,
> +	.dma_threshold		= false,
> +	.cts_event_workaround	= false,
> +	.always_enabled		= true,
> +	.fixed_options		= true,
> +};
> +#else
> +#define vendor_qdt_qdf2400_e44 vendor_sbsa
> +#endif

Instead of the #else, just put the #ifdef inside qdf2400_e44().  That way, the 
function always returns False if CONFIG_QCOM_QDF2400_ERRATUM_44 is not defined.

Also, don't you need to add a definition of .inv_fr in vendor_sbsa and the other 
vendor_xxx structures?

> +
>  static u16 pl011_st_offsets[REG_ARRAY_SIZE] = {
>  	[REG_DR] = UART01x_DR,
>  	[REG_ST_DMAWM] = ST_UART011_DMAWM,
> @@ -1518,7 +1538,7 @@ 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);
> +	unsigned int status = pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr;
>  	return status & (uap->vendor->fr_busy | UART01x_FR_TXFF) ?
>  							0 : TIOCSER_TEMT;
>  }
> @@ -2218,7 +2238,8 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
>  	 *	Finally, wait for transmitter to become empty
>  	 *	and restore the TCR
>  	 */
> -	while (pl011_read(uap, REG_FR) & uap->vendor->fr_busy)
> +	while ((pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr)
> +						& uap->vendor->fr_busy)

I really think the XOR logic needs to be documented wherever it's used.  It's 
just too confusing.

>  		cpu_relax();
>  	if (!uap->vendor->always_enabled)
>  		pl011_write(old_cr, uap, REG_CR);
> @@ -2383,6 +2404,13 @@ static struct console amba_console = {
>
>  #define AMBA_CONSOLE	(&amba_console)
>
> +static bool qdf2400_e44(void) {

Call it needs_qdf2400_e44(void) ?

> +	u32 cpu_var_model = read_cpuid_id() & ~MIDR_REVISION_MASK;
> +
> +	return (cpu_var_model == MIDR_QCOM_KRYO_V1 ||
> +	    cpu_var_model == MIDR_QCOM_FALKOR_V1);
> +}
> +
>  static void pl011_putc(struct uart_port *port, int c)
>  {
>  	while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
> @@ -2645,12 +2673,18 @@ static int sbsa_uart_probe(struct platform_device *pdev)
>  	uap->port.irq	= ret;
>
>  	uap->reg_offset	= vendor_sbsa.reg_offset;
> -	uap->vendor	= &vendor_sbsa;
>  	uap->fifosize	= 32;
>  	uap->port.iotype = vendor_sbsa.access_32b ? UPIO_MEM32 : UPIO_MEM;
>  	uap->port.ops	= &sbsa_uart_pops;
>  	uap->fixed_baud = baudrate;
>
> +	if (qdf2400_e44()) {
> +		uap->vendor = &vendor_qdt_qdf2400_e44;
> +		dev_info(&pdev->dev, "working around QDF2400 SoC erratum 44\n");
> +	} else {
> +		uap->vendor = &vendor_sbsa;
> +	}
> +
>  	snprintf(uap->type, sizeof(uap->type), "SBSA");
>
>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>


-- 
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.

Re: [PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Timur Tabi]

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. To keep the
> change small, this patch only works around the full console case, where UAP
> data is available, and does not handle the erratum for earlycon, as the UAP
> data is not available then.
>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> Changes between the previous RFC [1] and this PATCH:
> * don't use arch/arm64/kernel/cpu_errata.c at Will's request
> * separate out earlycon case to separate patch
> * rework earlycon case to not depend on UAP as suggested by Timur
>
> Because they need a newly-defined MIDR values, the patches are currently
> based on:
> https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/log/?h=for-next/core
>
> I'm not confident that I know the best route for these two patches. Should
> I request Catalin and Will to take them via arm64 as the essential MIDR
> additions are their purview?
>
> Thanks Russell for the ack. Compared to the RFC, I've made minor changes to
> what is now patch 1/2 and am making an educated guess that the ack sticks
> (but if not please let me know). Patch 2/2 is significantly revised from
> the RFC so I've not included the ack on it.
>
> 1. https://patchwork.codeaurora.org/patch/163281/
> ---
>  Documentation/arm64/silicon-errata.txt |  2 ++
>  arch/arm64/include/asm/cputype.h       |  2 ++
>  drivers/tty/serial/Kconfig             | 12 ++++++++++
>  drivers/tty/serial/amba-pl011.c        | 40 +++++++++++++++++++++++++++++++---
>  4 files changed, 53 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
> index 50da8391e9dd..0993ebb3e86b 100644
> --- a/Documentation/arm64/silicon-errata.txt
> +++ b/Documentation/arm64/silicon-errata.txt
> @@ -65,3 +65,5 @@ stable kernels.
>  | Freescale/NXP  | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585     |
>  | Qualcomm Tech. | Falkor v1       | E1003           | QCOM_FALKOR_ERRATUM_1003|
>  | Qualcomm Tech. | Falkor v1       | E1009           | QCOM_FALKOR_ERRATUM_1009|
> +| Qualcomm Tech. | QDF2432v1 UART  | SoC E44         | QCOM_QDF2400_ERRATUM_44 |
> +| Qualcomm Tech. | QDF2400v1 UART  | SoC E44         | QCOM_QDF2400_ERRATUM_44 |
> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> index fc502713ab37..cb399c7fe6ec 100644
> --- a/arch/arm64/include/asm/cputype.h
> +++ b/arch/arm64/include/asm/cputype.h
> @@ -88,12 +88,14 @@
>
>  #define BRCM_CPU_PART_VULCAN		0x516
>
> +#define QCOM_CPU_PART_KRYO_V1		0x281
>  #define QCOM_CPU_PART_FALKOR_V1		0x800
>
>  #define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
>  #define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
>  #define MIDR_THUNDERX	MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
>  #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
> +#define MIDR_QCOM_KRYO_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_KRYO_V1)
>  #define MIDR_QCOM_FALKOR_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_FALKOR_V1)
>
>  #ifndef __ASSEMBLY__
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index e9cf5b67f1b7..4cde8f48a540 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -73,6 +73,18 @@ config SERIAL_AMBA_PL011_CONSOLE
>  	  your boot loader (lilo or loadlin) about how to pass options to the
>  	  kernel at boot time.)
>
> +config QCOM_QDF2400_ERRATUM_44
> +	bool "Work around QDF2400 SoC E44 stuck BUSY bit"
> +	depends on SERIAL_AMBA_PL011_CONSOLE=y
> +	default y
> +	help
> +	  The BUSY bit in the Flag Register of the UART on the QDF2432v1 and
> +	  QDF2400v1 SoCs may get stuck as 1, resulting in a hung serial console.
> +	  Say Y here to work around the issue by checking TXFE == 0 instead of
> +	  BUSY == 1 on affected systems.
> +
> +	  If unsure, say Y.
> +
>  config SERIAL_EARLYCON_ARM_SEMIHOST
>  	bool "Early console using ARM semihosting"
>  	depends on ARM64 || ARM
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index d4171d71a258..41e51901d6ef 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -97,6 +97,7 @@ struct vendor_data {
>  	unsigned int		fr_dsr;
>  	unsigned int		fr_cts;
>  	unsigned int		fr_ri;
> +	unsigned int		inv_fr;
>  	bool			access_32b;
>  	bool			oversampling;
>  	bool			dma_threshold;
> @@ -141,6 +142,25 @@ static struct vendor_data vendor_sbsa = {
>  	.fixed_options		= true,
>  };
>
> +#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
> +static struct vendor_data vendor_qdt_qdf2400_e44 = {
> +	.reg_offset		= pl011_std_offsets,
> +	.fr_busy		= UART011_FR_TXFE,
> +	.fr_dsr			= UART01x_FR_DSR,
> +	.fr_cts			= UART01x_FR_CTS,
> +	.fr_ri			= UART011_FR_RI,
> +	.inv_fr			= UART011_FR_TXFE,
> +	.access_32b		= true,
> +	.oversampling		= false,
> +	.dma_threshold		= false,
> +	.cts_event_workaround	= false,
> +	.always_enabled		= true,
> +	.fixed_options		= true,
> +};
> +#else
> +#define vendor_qdt_qdf2400_e44 vendor_sbsa
> +#endif

Instead of the #else, just put the #ifdef inside qdf2400_e44().  That way, the 
function always returns False if CONFIG_QCOM_QDF2400_ERRATUM_44 is not defined.

Also, don't you need to add a definition of .inv_fr in vendor_sbsa and the other 
vendor_xxx structures?

> +
>  static u16 pl011_st_offsets[REG_ARRAY_SIZE] = {
>  	[REG_DR] = UART01x_DR,
>  	[REG_ST_DMAWM] = ST_UART011_DMAWM,
> @@ -1518,7 +1538,7 @@ 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);
> +	unsigned int status = pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr;
>  	return status & (uap->vendor->fr_busy | UART01x_FR_TXFF) ?
>  							0 : TIOCSER_TEMT;
>  }
> @@ -2218,7 +2238,8 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
>  	 *	Finally, wait for transmitter to become empty
>  	 *	and restore the TCR
>  	 */
> -	while (pl011_read(uap, REG_FR) & uap->vendor->fr_busy)
> +	while ((pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr)
> +						& uap->vendor->fr_busy)

I really think the XOR logic needs to be documented wherever it's used.  It's 
just too confusing.

>  		cpu_relax();
>  	if (!uap->vendor->always_enabled)
>  		pl011_write(old_cr, uap, REG_CR);
> @@ -2383,6 +2404,13 @@ static struct console amba_console = {
>
>  #define AMBA_CONSOLE	(&amba_console)
>
> +static bool qdf2400_e44(void) {

Call it needs_qdf2400_e44(void) ?

> +	u32 cpu_var_model = read_cpuid_id() & ~MIDR_REVISION_MASK;
> +
> +	return (cpu_var_model == MIDR_QCOM_KRYO_V1 ||
> +	    cpu_var_model == MIDR_QCOM_FALKOR_V1);
> +}
> +
>  static void pl011_putc(struct uart_port *port, int c)
>  {
>  	while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
> @@ -2645,12 +2673,18 @@ static int sbsa_uart_probe(struct platform_device *pdev)
>  	uap->port.irq	= ret;
>
>  	uap->reg_offset	= vendor_sbsa.reg_offset;
> -	uap->vendor	= &vendor_sbsa;
>  	uap->fifosize	= 32;
>  	uap->port.iotype = vendor_sbsa.access_32b ? UPIO_MEM32 : UPIO_MEM;
>  	uap->port.ops	= &sbsa_uart_pops;
>  	uap->fixed_baud = baudrate;
>
> +	if (qdf2400_e44()) {
> +		uap->vendor = &vendor_qdt_qdf2400_e44;
> +		dev_info(&pdev->dev, "working around QDF2400 SoC erratum 44\n");
> +	} else {
> +		uap->vendor = &vendor_sbsa;
> +	}
> +
>  	snprintf(uap->type, sizeof(uap->type), "SBSA");
>
>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>


-- 
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.

[PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Timur Tabi]

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. To keep the
> change small, this patch only works around the full console case, where UAP
> data is available, and does not handle the erratum for earlycon, as the UAP
> data is not available then.
>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> Changes between the previous RFC [1] and this PATCH:
> * don't use arch/arm64/kernel/cpu_errata.c at Will's request
> * separate out earlycon case to separate patch
> * rework earlycon case to not depend on UAP as suggested by Timur
>
> Because they need a newly-defined MIDR values, the patches are currently
> based on:
> https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/log/?h=for-next/core
>
> I'm not confident that I know the best route for these two patches. Should
> I request Catalin and Will to take them via arm64 as the essential MIDR
> additions are their purview?
>
> Thanks Russell for the ack. Compared to the RFC, I've made minor changes to
> what is now patch 1/2 and am making an educated guess that the ack sticks
> (but if not please let me know). Patch 2/2 is significantly revised from
> the RFC so I've not included the ack on it.
>
> 1. https://patchwork.codeaurora.org/patch/163281/
> ---
>  Documentation/arm64/silicon-errata.txt |  2 ++
>  arch/arm64/include/asm/cputype.h       |  2 ++
>  drivers/tty/serial/Kconfig             | 12 ++++++++++
>  drivers/tty/serial/amba-pl011.c        | 40 +++++++++++++++++++++++++++++++---
>  4 files changed, 53 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
> index 50da8391e9dd..0993ebb3e86b 100644
> --- a/Documentation/arm64/silicon-errata.txt
> +++ b/Documentation/arm64/silicon-errata.txt
> @@ -65,3 +65,5 @@ stable kernels.
>  | Freescale/NXP  | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585     |
>  | Qualcomm Tech. | Falkor v1       | E1003           | QCOM_FALKOR_ERRATUM_1003|
>  | Qualcomm Tech. | Falkor v1       | E1009           | QCOM_FALKOR_ERRATUM_1009|
> +| Qualcomm Tech. | QDF2432v1 UART  | SoC E44         | QCOM_QDF2400_ERRATUM_44 |
> +| Qualcomm Tech. | QDF2400v1 UART  | SoC E44         | QCOM_QDF2400_ERRATUM_44 |
> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> index fc502713ab37..cb399c7fe6ec 100644
> --- a/arch/arm64/include/asm/cputype.h
> +++ b/arch/arm64/include/asm/cputype.h
> @@ -88,12 +88,14 @@
>
>  #define BRCM_CPU_PART_VULCAN		0x516
>
> +#define QCOM_CPU_PART_KRYO_V1		0x281
>  #define QCOM_CPU_PART_FALKOR_V1		0x800
>
>  #define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
>  #define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
>  #define MIDR_THUNDERX	MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
>  #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
> +#define MIDR_QCOM_KRYO_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_KRYO_V1)
>  #define MIDR_QCOM_FALKOR_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_FALKOR_V1)
>
>  #ifndef __ASSEMBLY__
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index e9cf5b67f1b7..4cde8f48a540 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -73,6 +73,18 @@ config SERIAL_AMBA_PL011_CONSOLE
>  	  your boot loader (lilo or loadlin) about how to pass options to the
>  	  kernel at boot time.)
>
> +config QCOM_QDF2400_ERRATUM_44
> +	bool "Work around QDF2400 SoC E44 stuck BUSY bit"
> +	depends on SERIAL_AMBA_PL011_CONSOLE=y
> +	default y
> +	help
> +	  The BUSY bit in the Flag Register of the UART on the QDF2432v1 and
> +	  QDF2400v1 SoCs may get stuck as 1, resulting in a hung serial console.
> +	  Say Y here to work around the issue by checking TXFE == 0 instead of
> +	  BUSY == 1 on affected systems.
> +
> +	  If unsure, say Y.
> +
>  config SERIAL_EARLYCON_ARM_SEMIHOST
>  	bool "Early console using ARM semihosting"
>  	depends on ARM64 || ARM
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index d4171d71a258..41e51901d6ef 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -97,6 +97,7 @@ struct vendor_data {
>  	unsigned int		fr_dsr;
>  	unsigned int		fr_cts;
>  	unsigned int		fr_ri;
> +	unsigned int		inv_fr;
>  	bool			access_32b;
>  	bool			oversampling;
>  	bool			dma_threshold;
> @@ -141,6 +142,25 @@ static struct vendor_data vendor_sbsa = {
>  	.fixed_options		= true,
>  };
>
> +#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
> +static struct vendor_data vendor_qdt_qdf2400_e44 = {
> +	.reg_offset		= pl011_std_offsets,
> +	.fr_busy		= UART011_FR_TXFE,
> +	.fr_dsr			= UART01x_FR_DSR,
> +	.fr_cts			= UART01x_FR_CTS,
> +	.fr_ri			= UART011_FR_RI,
> +	.inv_fr			= UART011_FR_TXFE,
> +	.access_32b		= true,
> +	.oversampling		= false,
> +	.dma_threshold		= false,
> +	.cts_event_workaround	= false,
> +	.always_enabled		= true,
> +	.fixed_options		= true,
> +};
> +#else
> +#define vendor_qdt_qdf2400_e44 vendor_sbsa
> +#endif

Instead of the #else, just put the #ifdef inside qdf2400_e44().  That way, the 
function always returns False if CONFIG_QCOM_QDF2400_ERRATUM_44 is not defined.

Also, don't you need to add a definition of .inv_fr in vendor_sbsa and the other 
vendor_xxx structures?

> +
>  static u16 pl011_st_offsets[REG_ARRAY_SIZE] = {
>  	[REG_DR] = UART01x_DR,
>  	[REG_ST_DMAWM] = ST_UART011_DMAWM,
> @@ -1518,7 +1538,7 @@ 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);
> +	unsigned int status = pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr;
>  	return status & (uap->vendor->fr_busy | UART01x_FR_TXFF) ?
>  							0 : TIOCSER_TEMT;
>  }
> @@ -2218,7 +2238,8 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
>  	 *	Finally, wait for transmitter to become empty
>  	 *	and restore the TCR
>  	 */
> -	while (pl011_read(uap, REG_FR) & uap->vendor->fr_busy)
> +	while ((pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr)
> +						& uap->vendor->fr_busy)

I really think the XOR logic needs to be documented wherever it's used.  It's 
just too confusing.

>  		cpu_relax();
>  	if (!uap->vendor->always_enabled)
>  		pl011_write(old_cr, uap, REG_CR);
> @@ -2383,6 +2404,13 @@ static struct console amba_console = {
>
>  #define AMBA_CONSOLE	(&amba_console)
>
> +static bool qdf2400_e44(void) {

Call it needs_qdf2400_e44(void) ?

> +	u32 cpu_var_model = read_cpuid_id() & ~MIDR_REVISION_MASK;
> +
> +	return (cpu_var_model == MIDR_QCOM_KRYO_V1 ||
> +	    cpu_var_model == MIDR_QCOM_FALKOR_V1);
> +}
> +
>  static void pl011_putc(struct uart_port *port, int c)
>  {
>  	while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
> @@ -2645,12 +2673,18 @@ static int sbsa_uart_probe(struct platform_device *pdev)
>  	uap->port.irq	= ret;
>
>  	uap->reg_offset	= vendor_sbsa.reg_offset;
> -	uap->vendor	= &vendor_sbsa;
>  	uap->fifosize	= 32;
>  	uap->port.iotype = vendor_sbsa.access_32b ? UPIO_MEM32 : UPIO_MEM;
>  	uap->port.ops	= &sbsa_uart_pops;
>  	uap->fixed_baud = baudrate;
>
> +	if (qdf2400_e44()) {
> +		uap->vendor = &vendor_qdt_qdf2400_e44;
> +		dev_info(&pdev->dev, "working around QDF2400 SoC erratum 44\n");
> +	} else {
> +		uap->vendor = &vendor_sbsa;
> +	}
> +
>  	snprintf(uap->type, sizeof(uap->type), "SBSA");
>
>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>


-- 
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.

Re: [PATCH 2/2] tty: pl011: Work around QDF2400 E44 for earlycon

[Timur Tabi]

Christopher Covington wrote:
> The previous change worked around QDF2432v1 and QDF2400v1 SoC erratum 44
> for the full-fledged console, when UART AMBA Port (UAP) data is available.
> Additionally provide a workaround the earlycon case, again checking TXFE ==
> 0 instead of BUSY == 1. As earlycon is operating before UAP data is
> available, the implementation is different than in the preceding patch.
>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> ---
>   drivers/tty/serial/amba-pl011.c | 28 +++++++++++++++++++++++++++-
>   1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 41e51901d6ef..f25e7c994f8e 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -2411,6 +2411,29 @@ static bool qdf2400_e44(void) {
>   	    cpu_var_model == MIDR_QCOM_FALKOR_V1);
>   }
>   
> +#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
> +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 believe 32-bit writes are safe on QDF2400v1, so I think you technically don't 
need the UPIO_MEM32 check.  Just always call writel.
> +	while (!(readl(port->membase + UART01x_FR) & UART011_FR_TXFE))
> +		cpu_relax();
> +}
> +
> +static void qdf2400_e44_early_write(struct console *con, const char *s, unsigned n)
> +{
> +	struct earlycon_device *dev = con->data;
> +
> +	uart_console_write(&dev->port, s, n, qdf2400_e44_putc);
> +}
> +#else
> +#define qdf2400_e44_early_write pl011_early_write
> +#endif
Same with patch 1/2.  If you change qdf2400_e44(), you don't need the #else block.
> +
>   static void pl011_putc(struct uart_port *port, int c)
>   {
>   	while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
> @@ -2436,7 +2459,10 @@ static int __init pl011_early_console_setup(struct earlycon_device *device,
>   	if (!device->port.membase)
>   		return -ENODEV;
>   
> -	device->con->write = pl011_early_write;
> +	if (qdf2400_e44())
> +		device->con->write = qdf2400_e44_early_write;
> +	else
> +		device->con->write = pl011_early_write;
>   	return 0;
>   }
>   OF_EARLYCON_DECLARE(pl011, "arm,pl011", pl011_early_console_setup);


-- 
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.

[PATCH 2/2] tty: pl011: Work around QDF2400 E44 for earlycon

[Timur Tabi]

Christopher Covington wrote:
> The previous change worked around QDF2432v1 and QDF2400v1 SoC erratum 44
> for the full-fledged console, when UART AMBA Port (UAP) data is available.
> Additionally provide a workaround the earlycon case, again checking TXFE ==
> 0 instead of BUSY == 1. As earlycon is operating before UAP data is
> available, the implementation is different than in the preceding patch.
>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> ---
>   drivers/tty/serial/amba-pl011.c | 28 +++++++++++++++++++++++++++-
>   1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 41e51901d6ef..f25e7c994f8e 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -2411,6 +2411,29 @@ static bool qdf2400_e44(void) {
>   	    cpu_var_model == MIDR_QCOM_FALKOR_V1);
>   }
>   
> +#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
> +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 believe 32-bit writes are safe on QDF2400v1, so I think you technically don't 
need the UPIO_MEM32 check.  Just always call writel.
> +	while (!(readl(port->membase + UART01x_FR) & UART011_FR_TXFE))
> +		cpu_relax();
> +}
> +
> +static void qdf2400_e44_early_write(struct console *con, const char *s, unsigned n)
> +{
> +	struct earlycon_device *dev = con->data;
> +
> +	uart_console_write(&dev->port, s, n, qdf2400_e44_putc);
> +}
> +#else
> +#define qdf2400_e44_early_write pl011_early_write
> +#endif
Same with patch 1/2.  If you change qdf2400_e44(), you don't need the #else block.
> +
>   static void pl011_putc(struct uart_port *port, int c)
>   {
>   	while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
> @@ -2436,7 +2459,10 @@ static int __init pl011_early_console_setup(struct earlycon_device *device,
>   	if (!device->port.membase)
>   		return -ENODEV;
>   
> -	device->con->write = pl011_early_write;
> +	if (qdf2400_e44())
> +		device->con->write = qdf2400_e44_early_write;
> +	else
> +		device->con->write = pl011_early_write;
>   	return 0;
>   }
>   OF_EARLYCON_DECLARE(pl011, "arm,pl011", pl011_early_console_setup);


-- 
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.

Re: [PATCH 2/2] tty: pl011: Work around QDF2400 E44 for earlycon

[Shanker Donthineni]

Hi Cov,

The same PL011 driver will be used in virtutal machine, make sure your 
changes have no side effects in VM.



On 02/07/2017 10:07 PM, Timur Tabi wrote:
> Christopher Covington wrote:
>> The previous change worked around QDF2432v1 and QDF2400v1 SoC erratum 44
>> for the full-fledged console, when UART AMBA Port (UAP) data is 
>> available.
>> Additionally provide a workaround the earlycon case, again checking 
>> TXFE ==
>> 0 instead of BUSY == 1. As earlycon is operating before UAP data is
>> available, the implementation is different than in the preceding patch.
>>
>> Signed-off-by: Christopher Covington <cov@codeaurora.org>
>> ---
>>   drivers/tty/serial/amba-pl011.c | 28 +++++++++++++++++++++++++++-
>>   1 file changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/amba-pl011.c 
>> b/drivers/tty/serial/amba-pl011.c
>> index 41e51901d6ef..f25e7c994f8e 100644
>> --- a/drivers/tty/serial/amba-pl011.c
>> +++ b/drivers/tty/serial/amba-pl011.c
>> @@ -2411,6 +2411,29 @@ static bool qdf2400_e44(void) {
>>           cpu_var_model == MIDR_QCOM_FALKOR_V1);
>>   }
>>   +#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
>> +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 believe 32-bit writes are safe on QDF2400v1, so I think you 
> technically don't need the UPIO_MEM32 check.  Just always call writel.
>> +    while (!(readl(port->membase + UART01x_FR) & UART011_FR_TXFE))
>> +        cpu_relax();
>> +}
>> +
>> +static void qdf2400_e44_early_write(struct console *con, const char 
>> *s, unsigned n)
>> +{
>> +    struct earlycon_device *dev = con->data;
>> +
>> +    uart_console_write(&dev->port, s, n, qdf2400_e44_putc);
>> +}
>> +#else
>> +#define qdf2400_e44_early_write pl011_early_write
>> +#endif
> Same with patch 1/2.  If you change qdf2400_e44(), you don't need the 
> #else block.
>> +
>>   static void pl011_putc(struct uart_port *port, int c)
>>   {
>>       while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
>> @@ -2436,7 +2459,10 @@ static int __init 
>> pl011_early_console_setup(struct earlycon_device *device,
>>       if (!device->port.membase)
>>           return -ENODEV;
>>   -    device->con->write = pl011_early_write;
>> +    if (qdf2400_e44())
>> +        device->con->write = qdf2400_e44_early_write;
>> +    else
>> +        device->con->write = pl011_early_write;
>>       return 0;
>>   }
>>   OF_EARLYCON_DECLARE(pl011, "arm,pl011", pl011_early_console_setup);
>
>

-- 
Shanker Donthineni
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH 2/2] tty: pl011: Work around QDF2400 E44 for earlycon

[Shanker Donthineni]

Hi Cov,

The same PL011 driver will be used in virtutal machine, make sure your 
changes have no side effects in VM.



On 02/07/2017 10:07 PM, Timur Tabi wrote:
> Christopher Covington wrote:
>> The previous change worked around QDF2432v1 and QDF2400v1 SoC erratum 44
>> for the full-fledged console, when UART AMBA Port (UAP) data is 
>> available.
>> Additionally provide a workaround the earlycon case, again checking 
>> TXFE ==
>> 0 instead of BUSY == 1. As earlycon is operating before UAP data is
>> available, the implementation is different than in the preceding patch.
>>
>> Signed-off-by: Christopher Covington <cov@codeaurora.org>
>> ---
>>   drivers/tty/serial/amba-pl011.c | 28 +++++++++++++++++++++++++++-
>>   1 file changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/amba-pl011.c 
>> b/drivers/tty/serial/amba-pl011.c
>> index 41e51901d6ef..f25e7c994f8e 100644
>> --- a/drivers/tty/serial/amba-pl011.c
>> +++ b/drivers/tty/serial/amba-pl011.c
>> @@ -2411,6 +2411,29 @@ static bool qdf2400_e44(void) {
>>           cpu_var_model == MIDR_QCOM_FALKOR_V1);
>>   }
>>   +#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
>> +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 believe 32-bit writes are safe on QDF2400v1, so I think you 
> technically don't need the UPIO_MEM32 check.  Just always call writel.
>> +    while (!(readl(port->membase + UART01x_FR) & UART011_FR_TXFE))
>> +        cpu_relax();
>> +}
>> +
>> +static void qdf2400_e44_early_write(struct console *con, const char 
>> *s, unsigned n)
>> +{
>> +    struct earlycon_device *dev = con->data;
>> +
>> +    uart_console_write(&dev->port, s, n, qdf2400_e44_putc);
>> +}
>> +#else
>> +#define qdf2400_e44_early_write pl011_early_write
>> +#endif
> Same with patch 1/2.  If you change qdf2400_e44(), you don't need the 
> #else block.
>> +
>>   static void pl011_putc(struct uart_port *port, int c)
>>   {
>>       while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
>> @@ -2436,7 +2459,10 @@ static int __init 
>> pl011_early_console_setup(struct earlycon_device *device,
>>       if (!device->port.membase)
>>           return -ENODEV;
>>   -    device->con->write = pl011_early_write;
>> +    if (qdf2400_e44())
>> +        device->con->write = qdf2400_e44_early_write;
>> +    else
>> +        device->con->write = pl011_early_write;
>>       return 0;
>>   }
>>   OF_EARLYCON_DECLARE(pl011, "arm,pl011", pl011_early_console_setup);
>
>

-- 
Shanker Donthineni
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 2/2] tty: pl011: Work around QDF2400 E44 for earlycon

[Shanker Donthineni]

Hi Cov,

The same PL011 driver will be used in virtutal machine, make sure your 
changes have no side effects in VM.



On 02/07/2017 10:07 PM, Timur Tabi wrote:
> Christopher Covington wrote:
>> The previous change worked around QDF2432v1 and QDF2400v1 SoC erratum 44
>> for the full-fledged console, when UART AMBA Port (UAP) data is 
>> available.
>> Additionally provide a workaround the earlycon case, again checking 
>> TXFE ==
>> 0 instead of BUSY == 1. As earlycon is operating before UAP data is
>> available, the implementation is different than in the preceding patch.
>>
>> Signed-off-by: Christopher Covington <cov@codeaurora.org>
>> ---
>>   drivers/tty/serial/amba-pl011.c | 28 +++++++++++++++++++++++++++-
>>   1 file changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/amba-pl011.c 
>> b/drivers/tty/serial/amba-pl011.c
>> index 41e51901d6ef..f25e7c994f8e 100644
>> --- a/drivers/tty/serial/amba-pl011.c
>> +++ b/drivers/tty/serial/amba-pl011.c
>> @@ -2411,6 +2411,29 @@ static bool qdf2400_e44(void) {
>>           cpu_var_model == MIDR_QCOM_FALKOR_V1);
>>   }
>>   +#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
>> +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 believe 32-bit writes are safe on QDF2400v1, so I think you 
> technically don't need the UPIO_MEM32 check.  Just always call writel.
>> +    while (!(readl(port->membase + UART01x_FR) & UART011_FR_TXFE))
>> +        cpu_relax();
>> +}
>> +
>> +static void qdf2400_e44_early_write(struct console *con, const char 
>> *s, unsigned n)
>> +{
>> +    struct earlycon_device *dev = con->data;
>> +
>> +    uart_console_write(&dev->port, s, n, qdf2400_e44_putc);
>> +}
>> +#else
>> +#define qdf2400_e44_early_write pl011_early_write
>> +#endif
> Same with patch 1/2.  If you change qdf2400_e44(), you don't need the 
> #else block.
>> +
>>   static void pl011_putc(struct uart_port *port, int c)
>>   {
>>       while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
>> @@ -2436,7 +2459,10 @@ static int __init 
>> pl011_early_console_setup(struct earlycon_device *device,
>>       if (!device->port.membase)
>>           return -ENODEV;
>>   -    device->con->write = pl011_early_write;
>> +    if (qdf2400_e44())
>> +        device->con->write = qdf2400_e44_early_write;
>> +    else
>> +        device->con->write = pl011_early_write;
>>       return 0;
>>   }
>>   OF_EARLYCON_DECLARE(pl011, "arm,pl011", pl011_early_console_setup);
>
>

-- 
Shanker Donthineni
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Robin Murphy]

On 08/02/17 00:57, 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. To keep the
> change small, this patch only works around the full console case, where UAP
> data is available, and does not handle the erratum for earlycon, as the UAP
> data is not available then.
> 
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> Changes between the previous RFC [1] and this PATCH:
> * don't use arch/arm64/kernel/cpu_errata.c at Will's request
> * separate out earlycon case to separate patch
> * rework earlycon case to not depend on UAP as suggested by Timur
> 
> Because they need a newly-defined MIDR values, the patches are currently
> based on:
> https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/log/?h=for-next/core
> 
> I'm not confident that I know the best route for these two patches. Should
> I request Catalin and Will to take them via arm64 as the essential MIDR
> additions are their purview?
> 
> Thanks Russell for the ack. Compared to the RFC, I've made minor changes to
> what is now patch 1/2 and am making an educated guess that the ack sticks
> (but if not please let me know). Patch 2/2 is significantly revised from
> the RFC so I've not included the ack on it.
> 
> 1. https://patchwork.codeaurora.org/patch/163281/
> ---
>  Documentation/arm64/silicon-errata.txt |  2 ++
>  arch/arm64/include/asm/cputype.h       |  2 ++
>  drivers/tty/serial/Kconfig             | 12 ++++++++++
>  drivers/tty/serial/amba-pl011.c        | 40 +++++++++++++++++++++++++++++++---
>  4 files changed, 53 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
> index 50da8391e9dd..0993ebb3e86b 100644
> --- a/Documentation/arm64/silicon-errata.txt
> +++ b/Documentation/arm64/silicon-errata.txt
> @@ -65,3 +65,5 @@ stable kernels.
>  | Freescale/NXP  | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585     |
>  | Qualcomm Tech. | Falkor v1       | E1003           | QCOM_FALKOR_ERRATUM_1003|
>  | Qualcomm Tech. | Falkor v1       | E1009           | QCOM_FALKOR_ERRATUM_1009|
> +| Qualcomm Tech. | QDF2432v1 UART  | SoC E44         | QCOM_QDF2400_ERRATUM_44 |
> +| Qualcomm Tech. | QDF2400v1 UART  | SoC E44         | QCOM_QDF2400_ERRATUM_44 |
> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> index fc502713ab37..cb399c7fe6ec 100644
> --- a/arch/arm64/include/asm/cputype.h
> +++ b/arch/arm64/include/asm/cputype.h
> @@ -88,12 +88,14 @@
>  
>  #define BRCM_CPU_PART_VULCAN		0x516
>  
> +#define QCOM_CPU_PART_KRYO_V1		0x281
>  #define QCOM_CPU_PART_FALKOR_V1		0x800
>  
>  #define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
>  #define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
>  #define MIDR_THUNDERX	MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
>  #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
> +#define MIDR_QCOM_KRYO_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_KRYO_V1)
>  #define MIDR_QCOM_FALKOR_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_FALKOR_V1)
>  
>  #ifndef __ASSEMBLY__
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index e9cf5b67f1b7..4cde8f48a540 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -73,6 +73,18 @@ config SERIAL_AMBA_PL011_CONSOLE
>  	  your boot loader (lilo or loadlin) about how to pass options to the
>  	  kernel at boot time.)
>  
> +config QCOM_QDF2400_ERRATUM_44
> +	bool "Work around QDF2400 SoC E44 stuck BUSY bit"
> +	depends on SERIAL_AMBA_PL011_CONSOLE=y
> +	default y
> +	help
> +	  The BUSY bit in the Flag Register of the UART on the QDF2432v1 and
> +	  QDF2400v1 SoCs may get stuck as 1, resulting in a hung serial console.
> +	  Say Y here to work around the issue by checking TXFE == 0 instead of
> +	  BUSY == 1 on affected systems.
> +
> +	  If unsure, say Y.

Is there a reason anyone would ever want to turn this off? AFAICS you
save a few dozen bytes in return for a kernel image which you know won't
work properly on some hardware. That doesn't seem particularly
worthwhile, and it's not like the PL011 driver isn't already ripe with
unconditional vendor-specific data.

> +
>  config SERIAL_EARLYCON_ARM_SEMIHOST
>  	bool "Early console using ARM semihosting"
>  	depends on ARM64 || ARM
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index d4171d71a258..41e51901d6ef 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -97,6 +97,7 @@ struct vendor_data {
>  	unsigned int		fr_dsr;
>  	unsigned int		fr_cts;
>  	unsigned int		fr_ri;
> +	unsigned int		inv_fr;
>  	bool			access_32b;
>  	bool			oversampling;
>  	bool			dma_threshold;
> @@ -141,6 +142,25 @@ static struct vendor_data vendor_sbsa = {
>  	.fixed_options		= true,
>  };
>  
> +#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
> +static struct vendor_data vendor_qdt_qdf2400_e44 = {
> +	.reg_offset		= pl011_std_offsets,
> +	.fr_busy		= UART011_FR_TXFE,
> +	.fr_dsr			= UART01x_FR_DSR,
> +	.fr_cts			= UART01x_FR_CTS,
> +	.fr_ri			= UART011_FR_RI,
> +	.inv_fr			= UART011_FR_TXFE,
> +	.access_32b		= true,
> +	.oversampling		= false,
> +	.dma_threshold		= false,
> +	.cts_event_workaround	= false,
> +	.always_enabled		= true,
> +	.fixed_options		= true,
> +};
> +#else
> +#define vendor_qdt_qdf2400_e44 vendor_sbsa
> +#endif
> +
>  static u16 pl011_st_offsets[REG_ARRAY_SIZE] = {
>  	[REG_DR] = UART01x_DR,
>  	[REG_ST_DMAWM] = ST_UART011_DMAWM,
> @@ -1518,7 +1538,7 @@ 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);
> +	unsigned int status = pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr;
>  	return status & (uap->vendor->fr_busy | UART01x_FR_TXFF) ?
>  							0 : TIOCSER_TEMT;
>  }
> @@ -2218,7 +2238,8 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
>  	 *	Finally, wait for transmitter to become empty
>  	 *	and restore the TCR
>  	 */
> -	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);
> @@ -2383,6 +2404,13 @@ static struct console amba_console = {
>  
>  #define AMBA_CONSOLE	(&amba_console)
>  
> +static bool qdf2400_e44(void) {
> +	u32 cpu_var_model = read_cpuid_id() & ~MIDR_REVISION_MASK;
> +
> +	return (cpu_var_model == MIDR_QCOM_KRYO_V1 ||
> +	    cpu_var_model == MIDR_QCOM_FALKOR_V1);

Are we to take it that every SoC now and always with any Kryo or Falkor
core which also has an SBSA UART will require this workaround? There's a
guarantee that the UART itself will definitely never be fixed in some
future revision? That it'll never be integrated into, say, some
Kryo/Cortex-Axx big.LITTLE system where you do still need the
workaround, but might be making this check on the wrong core?

If there's really no suitable ID register to identify this particular
UART implementation, it probably needs to be described appropriately by
the firmware - I can't claim knowledge of how that works under ACPI, but
I do note that the only device ID currently being matched is "ARMH0011",
which seems to me to be inappropriate to describe something which is not
an ARM PL011, bugs or no bugs.

Robin.

> +}
> +
>  static void pl011_putc(struct uart_port *port, int c)
>  {
>  	while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
> @@ -2645,12 +2673,18 @@ static int sbsa_uart_probe(struct platform_device *pdev)
>  	uap->port.irq	= ret;
>  
>  	uap->reg_offset	= vendor_sbsa.reg_offset;
> -	uap->vendor	= &vendor_sbsa;
>  	uap->fifosize	= 32;
>  	uap->port.iotype = vendor_sbsa.access_32b ? UPIO_MEM32 : UPIO_MEM;
>  	uap->port.ops	= &sbsa_uart_pops;
>  	uap->fixed_baud = baudrate;
>  
> +	if (qdf2400_e44()) {
> +		uap->vendor = &vendor_qdt_qdf2400_e44;
> +		dev_info(&pdev->dev, "working around QDF2400 SoC erratum 44\n");
> +	} else {
> +		uap->vendor = &vendor_sbsa;
> +	}
> +
>  	snprintf(uap->type, sizeof(uap->type), "SBSA");
>  
>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> 

[PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Robin Murphy]

On 08/02/17 00:57, 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. To keep the
> change small, this patch only works around the full console case, where UAP
> data is available, and does not handle the erratum for earlycon, as the UAP
> data is not available then.
> 
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> Changes between the previous RFC [1] and this PATCH:
> * don't use arch/arm64/kernel/cpu_errata.c at Will's request
> * separate out earlycon case to separate patch
> * rework earlycon case to not depend on UAP as suggested by Timur
> 
> Because they need a newly-defined MIDR values, the patches are currently
> based on:
> https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/log/?h=for-next/core
> 
> I'm not confident that I know the best route for these two patches. Should
> I request Catalin and Will to take them via arm64 as the essential MIDR
> additions are their purview?
> 
> Thanks Russell for the ack. Compared to the RFC, I've made minor changes to
> what is now patch 1/2 and am making an educated guess that the ack sticks
> (but if not please let me know). Patch 2/2 is significantly revised from
> the RFC so I've not included the ack on it.
> 
> 1. https://patchwork.codeaurora.org/patch/163281/
> ---
>  Documentation/arm64/silicon-errata.txt |  2 ++
>  arch/arm64/include/asm/cputype.h       |  2 ++
>  drivers/tty/serial/Kconfig             | 12 ++++++++++
>  drivers/tty/serial/amba-pl011.c        | 40 +++++++++++++++++++++++++++++++---
>  4 files changed, 53 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
> index 50da8391e9dd..0993ebb3e86b 100644
> --- a/Documentation/arm64/silicon-errata.txt
> +++ b/Documentation/arm64/silicon-errata.txt
> @@ -65,3 +65,5 @@ stable kernels.
>  | Freescale/NXP  | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585     |
>  | Qualcomm Tech. | Falkor v1       | E1003           | QCOM_FALKOR_ERRATUM_1003|
>  | Qualcomm Tech. | Falkor v1       | E1009           | QCOM_FALKOR_ERRATUM_1009|
> +| Qualcomm Tech. | QDF2432v1 UART  | SoC E44         | QCOM_QDF2400_ERRATUM_44 |
> +| Qualcomm Tech. | QDF2400v1 UART  | SoC E44         | QCOM_QDF2400_ERRATUM_44 |
> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> index fc502713ab37..cb399c7fe6ec 100644
> --- a/arch/arm64/include/asm/cputype.h
> +++ b/arch/arm64/include/asm/cputype.h
> @@ -88,12 +88,14 @@
>  
>  #define BRCM_CPU_PART_VULCAN		0x516
>  
> +#define QCOM_CPU_PART_KRYO_V1		0x281
>  #define QCOM_CPU_PART_FALKOR_V1		0x800
>  
>  #define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
>  #define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
>  #define MIDR_THUNDERX	MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
>  #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
> +#define MIDR_QCOM_KRYO_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_KRYO_V1)
>  #define MIDR_QCOM_FALKOR_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_FALKOR_V1)
>  
>  #ifndef __ASSEMBLY__
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index e9cf5b67f1b7..4cde8f48a540 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -73,6 +73,18 @@ config SERIAL_AMBA_PL011_CONSOLE
>  	  your boot loader (lilo or loadlin) about how to pass options to the
>  	  kernel at boot time.)
>  
> +config QCOM_QDF2400_ERRATUM_44
> +	bool "Work around QDF2400 SoC E44 stuck BUSY bit"
> +	depends on SERIAL_AMBA_PL011_CONSOLE=y
> +	default y
> +	help
> +	  The BUSY bit in the Flag Register of the UART on the QDF2432v1 and
> +	  QDF2400v1 SoCs may get stuck as 1, resulting in a hung serial console.
> +	  Say Y here to work around the issue by checking TXFE == 0 instead of
> +	  BUSY == 1 on affected systems.
> +
> +	  If unsure, say Y.

Is there a reason anyone would ever want to turn this off? AFAICS you
save a few dozen bytes in return for a kernel image which you know won't
work properly on some hardware. That doesn't seem particularly
worthwhile, and it's not like the PL011 driver isn't already ripe with
unconditional vendor-specific data.

> +
>  config SERIAL_EARLYCON_ARM_SEMIHOST
>  	bool "Early console using ARM semihosting"
>  	depends on ARM64 || ARM
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index d4171d71a258..41e51901d6ef 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -97,6 +97,7 @@ struct vendor_data {
>  	unsigned int		fr_dsr;
>  	unsigned int		fr_cts;
>  	unsigned int		fr_ri;
> +	unsigned int		inv_fr;
>  	bool			access_32b;
>  	bool			oversampling;
>  	bool			dma_threshold;
> @@ -141,6 +142,25 @@ static struct vendor_data vendor_sbsa = {
>  	.fixed_options		= true,
>  };
>  
> +#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
> +static struct vendor_data vendor_qdt_qdf2400_e44 = {
> +	.reg_offset		= pl011_std_offsets,
> +	.fr_busy		= UART011_FR_TXFE,
> +	.fr_dsr			= UART01x_FR_DSR,
> +	.fr_cts			= UART01x_FR_CTS,
> +	.fr_ri			= UART011_FR_RI,
> +	.inv_fr			= UART011_FR_TXFE,
> +	.access_32b		= true,
> +	.oversampling		= false,
> +	.dma_threshold		= false,
> +	.cts_event_workaround	= false,
> +	.always_enabled		= true,
> +	.fixed_options		= true,
> +};
> +#else
> +#define vendor_qdt_qdf2400_e44 vendor_sbsa
> +#endif
> +
>  static u16 pl011_st_offsets[REG_ARRAY_SIZE] = {
>  	[REG_DR] = UART01x_DR,
>  	[REG_ST_DMAWM] = ST_UART011_DMAWM,
> @@ -1518,7 +1538,7 @@ 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);
> +	unsigned int status = pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr;
>  	return status & (uap->vendor->fr_busy | UART01x_FR_TXFF) ?
>  							0 : TIOCSER_TEMT;
>  }
> @@ -2218,7 +2238,8 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
>  	 *	Finally, wait for transmitter to become empty
>  	 *	and restore the TCR
>  	 */
> -	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);
> @@ -2383,6 +2404,13 @@ static struct console amba_console = {
>  
>  #define AMBA_CONSOLE	(&amba_console)
>  
> +static bool qdf2400_e44(void) {
> +	u32 cpu_var_model = read_cpuid_id() & ~MIDR_REVISION_MASK;
> +
> +	return (cpu_var_model == MIDR_QCOM_KRYO_V1 ||
> +	    cpu_var_model == MIDR_QCOM_FALKOR_V1);

Are we to take it that every SoC now and always with any Kryo or Falkor
core which also has an SBSA UART will require this workaround? There's a
guarantee that the UART itself will definitely never be fixed in some
future revision? That it'll never be integrated into, say, some
Kryo/Cortex-Axx big.LITTLE system where you do still need the
workaround, but might be making this check on the wrong core?

If there's really no suitable ID register to identify this particular
UART implementation, it probably needs to be described appropriately by
the firmware - I can't claim knowledge of how that works under ACPI, but
I do note that the only device ID currently being matched is "ARMH0011",
which seems to me to be inappropriate to describe something which is not
an ARM PL011, bugs or no bugs.

Robin.

> +}
> +
>  static void pl011_putc(struct uart_port *port, int c)
>  {
>  	while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
> @@ -2645,12 +2673,18 @@ static int sbsa_uart_probe(struct platform_device *pdev)
>  	uap->port.irq	= ret;
>  
>  	uap->reg_offset	= vendor_sbsa.reg_offset;
> -	uap->vendor	= &vendor_sbsa;
>  	uap->fifosize	= 32;
>  	uap->port.iotype = vendor_sbsa.access_32b ? UPIO_MEM32 : UPIO_MEM;
>  	uap->port.ops	= &sbsa_uart_pops;
>  	uap->fixed_baud = baudrate;
>  
> +	if (qdf2400_e44()) {
> +		uap->vendor = &vendor_qdt_qdf2400_e44;
> +		dev_info(&pdev->dev, "working around QDF2400 SoC erratum 44\n");
> +	} else {
> +		uap->vendor = &vendor_sbsa;
> +	}
> +
>  	snprintf(uap->type, sizeof(uap->type), "SBSA");
>  
>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> 

Re: [PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Timur Tabi]

Robin Murphy wrote:
> Is there a reason anyone would ever want to turn this off? AFAICS you
> save a few dozen bytes in return for a kernel image which you know won't
> work properly on some hardware. That doesn't seem particularly
> worthwhile, and it's not like the PL011 driver isn't already ripe with
> unconditional vendor-specific data.
>
>> > +
>> >  config SERIAL_EARLYCON_ARM_SEMIHOST
>> >  	bool "Early console using ARM semihosting"
>> >  	depends on ARM64 || ARM
>> > diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
>> > index d4171d71a258..41e51901d6ef 100644
>> > --- a/drivers/tty/serial/amba-pl011.c
>> > +++ b/drivers/tty/serial/amba-pl011.c
>> > @@ -97,6 +97,7 @@ struct vendor_data {
>> >  	unsigned int		fr_dsr;
>> >  	unsigned int		fr_cts;
>> >  	unsigned int		fr_ri;
>> > +	unsigned int		inv_fr;
>> >  	bool			access_32b;
>> >  	bool			oversampling;
>> >  	bool			dma_threshold;
>> > @@ -141,6 +142,25 @@ static struct vendor_data vendor_sbsa = {
>> >  	.fixed_options		= true,
>> >  };
>> >
>> > +#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
>> > +static struct vendor_data vendor_qdt_qdf2400_e44 = {
>> > +	.reg_offset		= pl011_std_offsets,
>> > +	.fr_busy		= UART011_FR_TXFE,
>> > +	.fr_dsr			= UART01x_FR_DSR,
>> > +	.fr_cts			= UART01x_FR_CTS,
>> > +	.fr_ri			= UART011_FR_RI,
>> > +	.inv_fr			= UART011_FR_TXFE,
>> > +	.access_32b		= true,
>> > +	.oversampling		= false,
>> > +	.dma_threshold		= false,
>> > +	.cts_event_workaround	= false,
>> > +	.always_enabled		= true,
>> > +	.fixed_options		= true,
>> > +};
>> > +#else
>> > +#define vendor_qdt_qdf2400_e44 vendor_sbsa
>> > +#endif
>> > +
>> >  static u16 pl011_st_offsets[REG_ARRAY_SIZE] = {
>> >  	[REG_DR] = UART01x_DR,
>> >  	[REG_ST_DMAWM] = ST_UART011_DMAWM,
>> > @@ -1518,7 +1538,7 @@ 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);
>> > +	unsigned int status = pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr;
>> >  	return status & (uap->vendor->fr_busy | UART01x_FR_TXFF) ?
>> >  							0 : TIOCSER_TEMT;
>> >  }
>> > @@ -2218,7 +2238,8 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
>> >  	 *	Finally, wait for transmitter to become empty
>> >  	 *	and restore the TCR
>> >  	 */
>> > -	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);
>> > @@ -2383,6 +2404,13 @@ static struct console amba_console = {
>> >
>> >  #define AMBA_CONSOLE	(&amba_console)
>> >
>> > +static bool qdf2400_e44(void) {
>> > +	u32 cpu_var_model = read_cpuid_id() & ~MIDR_REVISION_MASK;
>> > +
>> > +	return (cpu_var_model == MIDR_QCOM_KRYO_V1 ||
>> > +	    cpu_var_model == MIDR_QCOM_FALKOR_V1);

> Are we to take it that every SoC now and always with any Kryo or Falkor
> core which also has an SBSA UART will require this workaround?

No, only Kryo and Falkor V1 based SOCs have this problem.  Falkor V2 will have 
this fixed.  We intend to revert these fixes after Falkor V1 SOCs are no longer 
supported.

 > There's a
> guarantee that the UART itself will definitely never be fixed in some
> future revision? That it'll never be integrated into, say, some
> Kryo/Cortex-Axx big.LITTLE system where you do still need the
> workaround, but might be making this check on the wrong core?

We are sure that this erratum is restricted to those specific SOCs.

>
> If there's really no suitable ID register to identify this particular
> UART implementation, it probably needs to be described appropriately by
> the firmware - I can't claim knowledge of how that works under ACPI, but
> I do note that the only device ID currently being matched is "ARMH0011",
> which seems to me to be inappropriate to describe something which is not
> an ARM PL011, bugs or no bugs.

ACPI is not like device tree.  You can't just define a "qcom,needs-busy-bit-fix" 
property and call it a day.

If you're saying that we should create a new ACPI HID, like QCOM0011, that 
probably would have worked as well, except hindsight is 20/20, and we already 
have firmware in the field.

-- 
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.

[PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Timur Tabi]

Robin Murphy wrote:
> Is there a reason anyone would ever want to turn this off? AFAICS you
> save a few dozen bytes in return for a kernel image which you know won't
> work properly on some hardware. That doesn't seem particularly
> worthwhile, and it's not like the PL011 driver isn't already ripe with
> unconditional vendor-specific data.
>
>> > +
>> >  config SERIAL_EARLYCON_ARM_SEMIHOST
>> >  	bool "Early console using ARM semihosting"
>> >  	depends on ARM64 || ARM
>> > diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
>> > index d4171d71a258..41e51901d6ef 100644
>> > --- a/drivers/tty/serial/amba-pl011.c
>> > +++ b/drivers/tty/serial/amba-pl011.c
>> > @@ -97,6 +97,7 @@ struct vendor_data {
>> >  	unsigned int		fr_dsr;
>> >  	unsigned int		fr_cts;
>> >  	unsigned int		fr_ri;
>> > +	unsigned int		inv_fr;
>> >  	bool			access_32b;
>> >  	bool			oversampling;
>> >  	bool			dma_threshold;
>> > @@ -141,6 +142,25 @@ static struct vendor_data vendor_sbsa = {
>> >  	.fixed_options		= true,
>> >  };
>> >
>> > +#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
>> > +static struct vendor_data vendor_qdt_qdf2400_e44 = {
>> > +	.reg_offset		= pl011_std_offsets,
>> > +	.fr_busy		= UART011_FR_TXFE,
>> > +	.fr_dsr			= UART01x_FR_DSR,
>> > +	.fr_cts			= UART01x_FR_CTS,
>> > +	.fr_ri			= UART011_FR_RI,
>> > +	.inv_fr			= UART011_FR_TXFE,
>> > +	.access_32b		= true,
>> > +	.oversampling		= false,
>> > +	.dma_threshold		= false,
>> > +	.cts_event_workaround	= false,
>> > +	.always_enabled		= true,
>> > +	.fixed_options		= true,
>> > +};
>> > +#else
>> > +#define vendor_qdt_qdf2400_e44 vendor_sbsa
>> > +#endif
>> > +
>> >  static u16 pl011_st_offsets[REG_ARRAY_SIZE] = {
>> >  	[REG_DR] = UART01x_DR,
>> >  	[REG_ST_DMAWM] = ST_UART011_DMAWM,
>> > @@ -1518,7 +1538,7 @@ 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);
>> > +	unsigned int status = pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr;
>> >  	return status & (uap->vendor->fr_busy | UART01x_FR_TXFF) ?
>> >  							0 : TIOCSER_TEMT;
>> >  }
>> > @@ -2218,7 +2238,8 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
>> >  	 *	Finally, wait for transmitter to become empty
>> >  	 *	and restore the TCR
>> >  	 */
>> > -	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);
>> > @@ -2383,6 +2404,13 @@ static struct console amba_console = {
>> >
>> >  #define AMBA_CONSOLE	(&amba_console)
>> >
>> > +static bool qdf2400_e44(void) {
>> > +	u32 cpu_var_model = read_cpuid_id() & ~MIDR_REVISION_MASK;
>> > +
>> > +	return (cpu_var_model == MIDR_QCOM_KRYO_V1 ||
>> > +	    cpu_var_model == MIDR_QCOM_FALKOR_V1);

> Are we to take it that every SoC now and always with any Kryo or Falkor
> core which also has an SBSA UART will require this workaround?

No, only Kryo and Falkor V1 based SOCs have this problem.  Falkor V2 will have 
this fixed.  We intend to revert these fixes after Falkor V1 SOCs are no longer 
supported.

 > There's a
> guarantee that the UART itself will definitely never be fixed in some
> future revision? That it'll never be integrated into, say, some
> Kryo/Cortex-Axx big.LITTLE system where you do still need the
> workaround, but might be making this check on the wrong core?

We are sure that this erratum is restricted to those specific SOCs.

>
> If there's really no suitable ID register to identify this particular
> UART implementation, it probably needs to be described appropriately by
> the firmware - I can't claim knowledge of how that works under ACPI, but
> I do note that the only device ID currently being matched is "ARMH0011",
> which seems to me to be inappropriate to describe something which is not
> an ARM PL011, bugs or no bugs.

ACPI is not like device tree.  You can't just define a "qcom,needs-busy-bit-fix" 
property and call it a day.

If you're saying that we should create a new ACPI HID, like QCOM0011, that 
probably would have worked as well, except hindsight is 20/20, and we already 
have firmware in the field.

-- 
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.

Re: [PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Mark Rutland]

On Wed, Feb 08, 2017 at 07:27:29AM -0600, Timur Tabi wrote:
> Robin Murphy wrote:

> >Are we to take it that every SoC now and always with any Kryo or Falkor
> >core which also has an SBSA UART will require this workaround?
> 
> No, only Kryo and Falkor V1 based SOCs have this problem.  Falkor V2
> will have this fixed.  We intend to revert these fixes after Falkor
> V1 SOCs are no longer supported.

Supported by whom?

Generally, once something's upstreamed we expect it to remain supported.

> > There's a
> >guarantee that the UART itself will definitely never be fixed in some
> >future revision? That it'll never be integrated into, say, some
> >Kryo/Cortex-Axx big.LITTLE system where you do still need the
> >workaround, but might be making this check on the wrong core?
> 
> We are sure that this erratum is restricted to those specific SOCs.

Ok. Then please identify the system as a whole (i.e. the SoC), and not
the CPU.

> >If there's really no suitable ID register to identify this particular
> >UART implementation, it probably needs to be described appropriately by
> >the firmware - I can't claim knowledge of how that works under ACPI, but
> >I do note that the only device ID currently being matched is "ARMH0011",
> >which seems to me to be inappropriate to describe something which is not
> >an ARM PL011, bugs or no bugs.
> 
> ACPI is not like device tree.  You can't just define a
> "qcom,needs-busy-bit-fix" property and call it a day.

ACPI and DT have pretty much the exact same set of issues w.r.t.
long term maintenance and support.

The important step is raising issues early, rather than shipping FW with
values that are not known to work (or known to not work)...

> If you're saying that we should create a new ACPI HID, like
> QCOM0011, that probably would have worked as well, except hindsight
> is 20/20, and we already have firmware in the field.

There are other way of identifying the system, if you look further than
the device.

Thanks,
Thanks.

Re: [PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Mark Rutland]

On Wed, Feb 08, 2017 at 07:27:29AM -0600, Timur Tabi wrote:
> Robin Murphy wrote:

> >Are we to take it that every SoC now and always with any Kryo or Falkor
> >core which also has an SBSA UART will require this workaround?
> 
> No, only Kryo and Falkor V1 based SOCs have this problem.  Falkor V2
> will have this fixed.  We intend to revert these fixes after Falkor
> V1 SOCs are no longer supported.

Supported by whom?

Generally, once something's upstreamed we expect it to remain supported.

> > There's a
> >guarantee that the UART itself will definitely never be fixed in some
> >future revision? That it'll never be integrated into, say, some
> >Kryo/Cortex-Axx big.LITTLE system where you do still need the
> >workaround, but might be making this check on the wrong core?
> 
> We are sure that this erratum is restricted to those specific SOCs.

Ok. Then please identify the system as a whole (i.e. the SoC), and not
the CPU.

> >If there's really no suitable ID register to identify this particular
> >UART implementation, it probably needs to be described appropriately by
> >the firmware - I can't claim knowledge of how that works under ACPI, but
> >I do note that the only device ID currently being matched is "ARMH0011",
> >which seems to me to be inappropriate to describe something which is not
> >an ARM PL011, bugs or no bugs.
> 
> ACPI is not like device tree.  You can't just define a
> "qcom,needs-busy-bit-fix" property and call it a day.

ACPI and DT have pretty much the exact same set of issues w.r.t.
long term maintenance and support.

The important step is raising issues early, rather than shipping FW with
values that are not known to work (or known to not work)...

> If you're saying that we should create a new ACPI HID, like
> QCOM0011, that probably would have worked as well, except hindsight
> is 20/20, and we already have firmware in the field.

There are other way of identifying the system, if you look further than
the device.

Thanks,
Thanks.

[PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Mark Rutland]

On Wed, Feb 08, 2017 at 07:27:29AM -0600, Timur Tabi wrote:
> Robin Murphy wrote:

> >Are we to take it that every SoC now and always with any Kryo or Falkor
> >core which also has an SBSA UART will require this workaround?
> 
> No, only Kryo and Falkor V1 based SOCs have this problem.  Falkor V2
> will have this fixed.  We intend to revert these fixes after Falkor
> V1 SOCs are no longer supported.

Supported by whom?

Generally, once something's upstreamed we expect it to remain supported.

> > There's a
> >guarantee that the UART itself will definitely never be fixed in some
> >future revision? That it'll never be integrated into, say, some
> >Kryo/Cortex-Axx big.LITTLE system where you do still need the
> >workaround, but might be making this check on the wrong core?
> 
> We are sure that this erratum is restricted to those specific SOCs.

Ok. Then please identify the system as a whole (i.e. the SoC), and not
the CPU.

> >If there's really no suitable ID register to identify this particular
> >UART implementation, it probably needs to be described appropriately by
> >the firmware - I can't claim knowledge of how that works under ACPI, but
> >I do note that the only device ID currently being matched is "ARMH0011",
> >which seems to me to be inappropriate to describe something which is not
> >an ARM PL011, bugs or no bugs.
> 
> ACPI is not like device tree.  You can't just define a
> "qcom,needs-busy-bit-fix" property and call it a day.

ACPI and DT have pretty much the exact same set of issues w.r.t.
long term maintenance and support.

The important step is raising issues early, rather than shipping FW with
values that are not known to work (or known to not work)...

> If you're saying that we should create a new ACPI HID, like
> QCOM0011, that probably would have worked as well, except hindsight
> is 20/20, and we already have firmware in the field.

There are other way of identifying the system, if you look further than
the device.

Thanks,
Thanks.

Re: [PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Timur Tabi]

Mark Rutland wrote:

>> No, only Kryo and Falkor V1 based SOCs have this problem.  Falkor V2
>> will have this fixed.  We intend to revert these fixes after Falkor
>> V1 SOCs are no longer supported.
>
> Supported by whom?

Qualcomm.

> Generally, once something's upstreamed we expect it to remain supported.

The V1 SOCs are technically pre-production, but still in wide use and it will be 
a while before they are all replaced by V2 SOCs.  It's only because of the 
"ugliness" of the erratum and its work-around that we want to git rid of it when 
we can.

Cov is better equipped to answer your other questions.

-- 
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.

Re: [PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Timur Tabi]

Mark Rutland wrote:

>> No, only Kryo and Falkor V1 based SOCs have this problem.  Falkor V2
>> will have this fixed.  We intend to revert these fixes after Falkor
>> V1 SOCs are no longer supported.
>
> Supported by whom?

Qualcomm.

> Generally, once something's upstreamed we expect it to remain supported.

The V1 SOCs are technically pre-production, but still in wide use and it will be 
a while before they are all replaced by V2 SOCs.  It's only because of the 
"ugliness" of the erratum and its work-around that we want to git rid of it when 
we can.

Cov is better equipped to answer your other questions.

-- 
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.

[PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Timur Tabi]

Mark Rutland wrote:

>> No, only Kryo and Falkor V1 based SOCs have this problem.  Falkor V2
>> will have this fixed.  We intend to revert these fixes after Falkor
>> V1 SOCs are no longer supported.
>
> Supported by whom?

Qualcomm.

> Generally, once something's upstreamed we expect it to remain supported.

The V1 SOCs are technically pre-production, but still in wide use and it will be 
a while before they are all replaced by V2 SOCs.  It's only because of the 
"ugliness" of the erratum and its work-around that we want to git rid of it when 
we can.

Cov is better equipped to answer your other questions.

-- 
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.

Re: [PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Marc Zyngier]

On Wed, Feb 08 2017 at  2:09:12 pm GMT, Timur Tabi <timur@codeaurora.org> wrote:
> Mark Rutland wrote:
>
>>> No, only Kryo and Falkor V1 based SOCs have this problem.  Falkor V2
>>> will have this fixed.  We intend to revert these fixes after Falkor
>>> V1 SOCs are no longer supported.
>>
>> Supported by whom?
>
> Qualcomm.
>
>> Generally, once something's upstreamed we expect it to remain supported.
>
> The V1 SOCs are technically pre-production, but still in wide use and
> it will be a while before they are all replaced by V2 SOCs.  It's only
> because of the "ugliness" of the erratum and its work-around that we
> want to git rid of it when we can.

Interesting. How will you guarantee that nobody will ever want to run a
mainline kernel on this box after a certain date? Self-destruct timer?
;-)

As someone who runs mainline on HW that exceeded its "sell-by" date by a
few decades, I'm genuinely interested.

Thanks,

	M.
-- 
Jazz is not dead, it just smell funny.

Re: [PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Marc Zyngier]

On Wed, Feb 08 2017 at  2:09:12 pm GMT, Timur Tabi <timur@codeaurora.org> wrote:
> Mark Rutland wrote:
>
>>> No, only Kryo and Falkor V1 based SOCs have this problem.  Falkor V2
>>> will have this fixed.  We intend to revert these fixes after Falkor
>>> V1 SOCs are no longer supported.
>>
>> Supported by whom?
>
> Qualcomm.
>
>> Generally, once something's upstreamed we expect it to remain supported.
>
> The V1 SOCs are technically pre-production, but still in wide use and
> it will be a while before they are all replaced by V2 SOCs.  It's only
> because of the "ugliness" of the erratum and its work-around that we
> want to git rid of it when we can.

Interesting. How will you guarantee that nobody will ever want to run a
mainline kernel on this box after a certain date? Self-destruct timer?
;-)

As someone who runs mainline on HW that exceeded its "sell-by" date by a
few decades, I'm genuinely interested.

Thanks,

	M.
-- 
Jazz is not dead, it just smell funny.

[PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Marc Zyngier]

On Wed, Feb 08 2017 at  2:09:12 pm GMT, Timur Tabi <timur@codeaurora.org> wrote:
> Mark Rutland wrote:
>
>>> No, only Kryo and Falkor V1 based SOCs have this problem.  Falkor V2
>>> will have this fixed.  We intend to revert these fixes after Falkor
>>> V1 SOCs are no longer supported.
>>
>> Supported by whom?
>
> Qualcomm.
>
>> Generally, once something's upstreamed we expect it to remain supported.
>
> The V1 SOCs are technically pre-production, but still in wide use and
> it will be a while before they are all replaced by V2 SOCs.  It's only
> because of the "ugliness" of the erratum and its work-around that we
> want to git rid of it when we can.

Interesting. How will you guarantee that nobody will ever want to run a
mainline kernel on this box after a certain date? Self-destruct timer?
;-)

As someone who runs mainline on HW that exceeded its "sell-by" date by a
few decades, I'm genuinely interested.

Thanks,

	M.
-- 
Jazz is not dead, it just smell funny.

Re: [PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Timur Tabi]

On 02/08/2017 09:35 AM, Marc Zyngier wrote:
> Interesting. How will you guarantee that nobody will ever want to run a
> mainline kernel on this box after a certain date? Self-destruct timer?
> ;-)
>
> As someone who runs mainline on HW that exceeded its "sell-by" date by a
> few decades, I'm genuinely interested.

The affected chips are pre-production and are only available to select 
customers for a limited time.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Timur Tabi]

On 02/08/2017 09:35 AM, Marc Zyngier wrote:
> Interesting. How will you guarantee that nobody will ever want to run a
> mainline kernel on this box after a certain date? Self-destruct timer?
> ;-)
>
> As someone who runs mainline on HW that exceeded its "sell-by" date by a
> few decades, I'm genuinely interested.

The affected chips are pre-production and are only available to select 
customers for a limited time.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Timur Tabi]

On 02/08/2017 09:35 AM, Marc Zyngier wrote:
> Interesting. How will you guarantee that nobody will ever want to run a
> mainline kernel on this box after a certain date? Self-destruct timer?
> ;-)
>
> As someone who runs mainline on HW that exceeded its "sell-by" date by a
> few decades, I'm genuinely interested.

The affected chips are pre-production and are only available to select 
customers for a limited time.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Christopher Covington]

Hi,

On 02/08/2017 08:27 AM, Timur Tabi wrote:
> Robin Murphy wrote:
>> Is there a reason anyone would ever want to turn this off? AFAICS you
>> save a few dozen bytes in return for a kernel image which you know won't
>> work properly on some hardware. That doesn't seem particularly
>> worthwhile, and it's not like the PL011 driver isn't already ripe with
>> unconditional vendor-specific data.

I'll make it unconditional in the next version.

>>> > +static bool qdf2400_e44(void) {
>>> > +    u32 cpu_var_model = read_cpuid_id() & ~MIDR_REVISION_MASK;
>>> > +
>>> > +    return (cpu_var_model == MIDR_QCOM_KRYO_V1 ||
>>> > +        cpu_var_model == MIDR_QCOM_FALKOR_V1);
> 
>> Are we to take it that every SoC now and always with any Kryo or Falkor
>> core which also has an SBSA UART will require this workaround?

I should have matched full 32 bit MIDR values, but I masked off the revision
number after careful investigation of current and future known MIDR values
and system pairings because MIDR_FOO isn't actually a 32 bit MIDR value as
the name would suggest but has variant and revision zeroed. The CPU in the
QDF2432 has a non-zero revision number and I didn't want to try to add an
unprecedented revision number to cputype.h after Will told me I wasn't
allowed to play with CPU toys for SoC games.

I'm going to stop using MIDR, but as an aside, there are two very different
CPUs, which may require different CPU errata workarounds, that (by accident)
only differ by the variant field. Robert Richter's recent MIDR_CPU_VAR_REV
is a welcome clarification.

> No, only Kryo and Falkor V1 based SOCs have this problem. Falkor V2
> will have this fixed.

A year ago, we were operating under the incorrect-in-hindsight assumption
that the QDF2400 v1 SoC with Falkor v1 CPU would have this fixed.

As for alternative means of identifying the errant hardware, I think that I
or a colleague will experiment with the approach used for ACPI PCI quirks:
checking the OEM ID, OEM Table ID, and OEM Revision. In this case, those
header fields would come from the Serial Port Console Redirection (SPCR)
table, rather than the PCI-specific MCFG table.

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5b69b85ba1ddd36be01f5c57830b37a3c8256009

Regarding ongoing support, I don't see any disagreement that the code must
be supported and maintained for the life of the hardware, however many years
that ends up being.

Thanks,
Cov

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code
Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Christopher Covington]

Hi,

On 02/08/2017 08:27 AM, Timur Tabi wrote:
> Robin Murphy wrote:
>> Is there a reason anyone would ever want to turn this off? AFAICS you
>> save a few dozen bytes in return for a kernel image which you know won't
>> work properly on some hardware. That doesn't seem particularly
>> worthwhile, and it's not like the PL011 driver isn't already ripe with
>> unconditional vendor-specific data.

I'll make it unconditional in the next version.

>>> > +static bool qdf2400_e44(void) {
>>> > +    u32 cpu_var_model = read_cpuid_id() & ~MIDR_REVISION_MASK;
>>> > +
>>> > +    return (cpu_var_model == MIDR_QCOM_KRYO_V1 ||
>>> > +        cpu_var_model == MIDR_QCOM_FALKOR_V1);
> 
>> Are we to take it that every SoC now and always with any Kryo or Falkor
>> core which also has an SBSA UART will require this workaround?

I should have matched full 32 bit MIDR values, but I masked off the revision
number after careful investigation of current and future known MIDR values
and system pairings because MIDR_FOO isn't actually a 32 bit MIDR value as
the name would suggest but has variant and revision zeroed. The CPU in the
QDF2432 has a non-zero revision number and I didn't want to try to add an
unprecedented revision number to cputype.h after Will told me I wasn't
allowed to play with CPU toys for SoC games.

I'm going to stop using MIDR, but as an aside, there are two very different
CPUs, which may require different CPU errata workarounds, that (by accident)
only differ by the variant field. Robert Richter's recent MIDR_CPU_VAR_REV
is a welcome clarification.

> No, only Kryo and Falkor V1 based SOCs have this problem. Falkor V2
> will have this fixed.

A year ago, we were operating under the incorrect-in-hindsight assumption
that the QDF2400 v1 SoC with Falkor v1 CPU would have this fixed.

As for alternative means of identifying the errant hardware, I think that I
or a colleague will experiment with the approach used for ACPI PCI quirks:
checking the OEM ID, OEM Table ID, and OEM Revision. In this case, those
header fields would come from the Serial Port Console Redirection (SPCR)
table, rather than the PCI-specific MCFG table.

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5b69b85ba1ddd36be01f5c57830b37a3c8256009

Regarding ongoing support, I don't see any disagreement that the code must
be supported and maintained for the life of the hardware, however many years
that ends up being.

Thanks,
Cov

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code
Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Christopher Covington]

On 02/07/2017 11:05 PM, Timur Tabi wrote:
> 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. To keep the
>> change small, this patch only works around the full console case, where UAP
>> data is available, and does not handle the erratum for earlycon, as the UAP
>> data is not available then.

>> +#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
>> +static struct vendor_data vendor_qdt_qdf2400_e44 = {
>> +    .reg_offset        = pl011_std_offsets,
>> +    .fr_busy        = UART011_FR_TXFE,
>> +    .fr_dsr            = UART01x_FR_DSR,
>> +    .fr_cts            = UART01x_FR_CTS,
>> +    .fr_ri            = UART011_FR_RI,
>> +    .inv_fr            = UART011_FR_TXFE,
>> +    .access_32b        = true,
>> +    .oversampling        = false,
>> +    .dma_threshold        = false,
>> +    .cts_event_workaround    = false,
>> +    .always_enabled        = true,
>> +    .fixed_options        = true,
>> +};
>> +#else
>> +#define vendor_qdt_qdf2400_e44 vendor_sbsa
>> +#endif
> 
> Instead of the #else, just put the #ifdef inside qdf2400_e44(). That
> way, the function always returns False if
> CONFIG_QCOM_QDF2400_ERRATUM_44 is not defined.

I'll consolidate the #ifdefery around qdf2400_e44().

> Also, don't you need to add a definition of .inv_fr in vendor_sbsa
> and the other vendor_xxx structures?

The current struct definitions appear to (ab)use implicit
zero-initialization, so I did too.

>> +
>>  static u16 pl011_st_offsets[REG_ARRAY_SIZE] = {
>>      [REG_DR] = UART01x_DR,
>>      [REG_ST_DMAWM] = ST_UART011_DMAWM,
>> @@ -1518,7 +1538,7 @@ 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);
>> +    unsigned int status = pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr;
>>      return status & (uap->vendor->fr_busy | UART01x_FR_TXFF) ?
>>                              0 : TIOCSER_TEMT;
>>  }
>> @@ -2218,7 +2238,8 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
>>       *    Finally, wait for transmitter to become empty
>>       *    and restore the TCR
>>       */
>> -    while (pl011_read(uap, REG_FR) & uap->vendor->fr_busy)
>> +    while ((pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr)
>> +                        & uap->vendor->fr_busy)
> 
> I really think the XOR logic needs to be documented wherever it's
> used. It's just too confusing.

References such as the following for basic bit operations like
setting, clearing, and toggling are easy enough to come by.

http://stackoverflow.com/questions/47981/how-do-you-set-clear-and-toggle-a-single-bit-in-c-c

Cov

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code
Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Christopher Covington]

On 02/07/2017 11:05 PM, Timur Tabi wrote:
> 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. To keep the
>> change small, this patch only works around the full console case, where UAP
>> data is available, and does not handle the erratum for earlycon, as the UAP
>> data is not available then.

>> +#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
>> +static struct vendor_data vendor_qdt_qdf2400_e44 = {
>> +    .reg_offset        = pl011_std_offsets,
>> +    .fr_busy        = UART011_FR_TXFE,
>> +    .fr_dsr            = UART01x_FR_DSR,
>> +    .fr_cts            = UART01x_FR_CTS,
>> +    .fr_ri            = UART011_FR_RI,
>> +    .inv_fr            = UART011_FR_TXFE,
>> +    .access_32b        = true,
>> +    .oversampling        = false,
>> +    .dma_threshold        = false,
>> +    .cts_event_workaround    = false,
>> +    .always_enabled        = true,
>> +    .fixed_options        = true,
>> +};
>> +#else
>> +#define vendor_qdt_qdf2400_e44 vendor_sbsa
>> +#endif
> 
> Instead of the #else, just put the #ifdef inside qdf2400_e44(). That
> way, the function always returns False if
> CONFIG_QCOM_QDF2400_ERRATUM_44 is not defined.

I'll consolidate the #ifdefery around qdf2400_e44().

> Also, don't you need to add a definition of .inv_fr in vendor_sbsa
> and the other vendor_xxx structures?

The current struct definitions appear to (ab)use implicit
zero-initialization, so I did too.

>> +
>>  static u16 pl011_st_offsets[REG_ARRAY_SIZE] = {
>>      [REG_DR] = UART01x_DR,
>>      [REG_ST_DMAWM] = ST_UART011_DMAWM,
>> @@ -1518,7 +1538,7 @@ 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);
>> +    unsigned int status = pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr;
>>      return status & (uap->vendor->fr_busy | UART01x_FR_TXFF) ?
>>                              0 : TIOCSER_TEMT;
>>  }
>> @@ -2218,7 +2238,8 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
>>       *    Finally, wait for transmitter to become empty
>>       *    and restore the TCR
>>       */
>> -    while (pl011_read(uap, REG_FR) & uap->vendor->fr_busy)
>> +    while ((pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr)
>> +                        & uap->vendor->fr_busy)
> 
> I really think the XOR logic needs to be documented wherever it's
> used. It's just too confusing.

References such as the following for basic bit operations like
setting, clearing, and toggling are easy enough to come by.

http://stackoverflow.com/questions/47981/how-do-you-set-clear-and-toggle-a-single-bit-in-c-c

Cov

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code
Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Timur Tabi]

On 02/08/2017 04:22 PM, Christopher Covington wrote:
>>> >> -    while (pl011_read(uap, REG_FR) & uap->vendor->fr_busy)
>>> >> +    while ((pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr)
>>> >> +                        & uap->vendor->fr_busy)
>> >
>> > I really think the XOR logic needs to be documented wherever it's
>> > used. It's just too confusing.
> References such as the following for basic bit operations like
> setting, clearing, and toggling are easy enough to come by.
>
> http://stackoverflow.com/questions/47981/how-do-you-set-clear-and-toggle-a-single-bit-in-c-c

I know what XOR does.  I was thinking a more high-level comment would be 
appropriate.  With E44, we want to ignore the BUSY bit and use TXFE instead. 
So was hoping to a comment that says that:

/* Normally, we poll until BUSY=0, but E44 says we should poll until TXFE=1 
instead. So with E44, we set fr_busy to TXFE, but we have to invert it. */

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Timur Tabi]

On 02/08/2017 04:22 PM, Christopher Covington wrote:
>>> >> -    while (pl011_read(uap, REG_FR) & uap->vendor->fr_busy)
>>> >> +    while ((pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr)
>>> >> +                        & uap->vendor->fr_busy)
>> >
>> > I really think the XOR logic needs to be documented wherever it's
>> > used. It's just too confusing.
> References such as the following for basic bit operations like
> setting, clearing, and toggling are easy enough to come by.
>
> http://stackoverflow.com/questions/47981/how-do-you-set-clear-and-toggle-a-single-bit-in-c-c

I know what XOR does.  I was thinking a more high-level comment would be 
appropriate.  With E44, we want to ignore the BUSY bit and use TXFE instead. 
So was hoping to a comment that says that:

/* Normally, we poll until BUSY=0, but E44 says we should poll until TXFE=1 
instead. So with E44, we set fr_busy to TXFE, but we have to invert it. */

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Christopher Covington]

On 02/08/2017 06:04 PM, Timur Tabi wrote:
> On 02/08/2017 04:22 PM, Christopher Covington wrote:
>>>> >> -    while (pl011_read(uap, REG_FR) & uap->vendor->fr_busy)
>>>> >> +    while ((pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr)
>>>> >> +                        & uap->vendor->fr_busy)
>>> >
>>> > I really think the XOR logic needs to be documented wherever it's
>>> > used. It's just too confusing.
>> References such as the following for basic bit operations like
>> setting, clearing, and toggling are easy enough to come by.
>>
>> http://stackoverflow.com/questions/47981/how-do-you-set-clear-and-toggle-a-single-bit-in-c-c
> 
> I know what XOR does. I was thinking a more high-level comment would
> be appropriate. With E44, we want to ignore the BUSY bit and use TXFE
> instead. So was hoping to a comment that says that:
> 
> /* Normally, we poll until BUSY=0, but E44 says we should poll until
> TXFE=1 instead. So with E44, we set fr_busy to TXFE, but we have to
> invert it. */

Oh, yeah, that'd be good. I'll include better comments in v2.

Cheers,
Cov

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code
Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit

[Christopher Covington]

On 02/08/2017 06:04 PM, Timur Tabi wrote:
> On 02/08/2017 04:22 PM, Christopher Covington wrote:
>>>> >> -    while (pl011_read(uap, REG_FR) & uap->vendor->fr_busy)
>>>> >> +    while ((pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr)
>>>> >> +                        & uap->vendor->fr_busy)
>>> >
>>> > I really think the XOR logic needs to be documented wherever it's
>>> > used. It's just too confusing.
>> References such as the following for basic bit operations like
>> setting, clearing, and toggling are easy enough to come by.
>>
>> http://stackoverflow.com/questions/47981/how-do-you-set-clear-and-toggle-a-single-bit-in-c-c
> 
> I know what XOR does. I was thinking a more high-level comment would
> be appropriate. With E44, we want to ignore the BUSY bit and use TXFE
> instead. So was hoping to a comment that says that:
> 
> /* Normally, we poll until BUSY=0, but E44 says we should poll until
> TXFE=1 instead. So with E44, we set fr_busy to TXFE, but we have to
> invert it. */

Oh, yeah, that'd be good. I'll include better comments in v2.

Cheers,
Cov

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code
Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH 2/2] tty: pl011: Work around QDF2400 E44 for earlycon

[Christopher Covington]

On 02/07/2017 11:31 PM, Shanker Donthineni wrote:
> Hi Cov,
> 
> The same PL011 driver will be used in virtutal machine, make sure
> your changes have no side effects in VM.

Fundamentally, this is the same workaround as has been tested in the qserver
downstream kernel for over a year.

Cheers,
Cov

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code
Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 2/2] tty: pl011: Work around QDF2400 E44 for earlycon

[Christopher Covington]

On 02/07/2017 11:31 PM, Shanker Donthineni wrote:
> Hi Cov,
> 
> The same PL011 driver will be used in virtutal machine, make sure
> your changes have no side effects in VM.

Fundamentally, this is the same workaround as has been tested in the qserver
downstream kernel for over a year.

Cheers,
Cov

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code
Aurora Forum, a Linux Foundation Collaborative Project.