Re: [PATCH] tty: serial: amba-pl011: Remove QDF2xxx workarounds

From: Peter Hilber
Date: Wed Jan 31 2024 - 20:15:49 EST

On 30.01.24 18:05, Nathan Chancellor wrote:
> Hi Konrad,
>
> On Mon, Jan 22, 2024 at 03:27:20PM +0100, Konrad Dybcio wrote:
>> This SoC family was destined for server use, featuring Qualcomm's very
>> interesting Kryo cores (before "Kryo" became a marketing term for Arm
>> cores with small modifications). It did however not leave the labs of
>> Qualcomm and presumably some partners, nor was it ever productized.
>>
>> Remove the workarounds, as they are long obsolete.
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
>> ---
>> Compile-tested only
>> ---
>> drivers/tty/serial/amba-pl011.c | 82 -----------------------------------------
>> 1 file changed, 82 deletions(-)
>>
>> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
>> index fccec1698a54..c4df9cbc264b 100644
>> --- a/drivers/tty/serial/amba-pl011.c
>> +++ b/drivers/tty/serial/amba-pl011.c
>> @@ -152,23 +152,6 @@ static const struct vendor_data vendor_sbsa = {
>> .fixed_options = true,
>> };
>>
>> -#ifdef CONFIG_ACPI_SPCR_TABLE
>> -static const 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,
>> -};
>> -#endif
>> -
>> static u16 pl011_st_offsets[REG_ARRAY_SIZE] = {
>> [REG_DR] = UART01x_DR,
>> [REG_ST_DMAWM] = ST_UART011_DMAWM,
>> @@ -2468,15 +2451,6 @@ static int pl011_console_match(struct console *co, char *name, int idx,
>> resource_size_t addr;
>> int i;
>>
>> - /*
>> - * Systems affected by the Qualcomm Technologies QDF2400 E44 erratum
>> - * have a distinct console name, so make sure we check for that.
>> - * The actual implementation of the erratum occurs in the probe
>> - * function.
>> - */
>> - if ((strcmp(name, "qdf2400_e44") != 0) && (strcmp(name, "pl011") != 0))

AFAIU the -ENODEV return for the "pl011" comparison needs to stay, since it
is not actually workaround related, cf. 10879ae5f12e ("serial: pl011: add
console matching function").

I suspect this is what might cause problems with QEMU.

Best regards,

Peter

>> - return -ENODEV;
>> -
>> if (uart_parse_earlycon(options, &iotype, &addr, &options))
>> return -ENODEV;
>>
>> @@ -2517,22 +2491,6 @@ static struct console amba_console = {
>>
>> #define AMBA_CONSOLE (&amba_console)
>>
>> -static void qdf2400_e44_putc(struct uart_port *port, unsigned char c)
>> -{
>> - while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
>> - cpu_relax();
>> - writel(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 int n)
>> -{
>> - struct earlycon_device *dev = con->data;
>> -
>> - uart_console_write(&dev->port, s, n, qdf2400_e44_putc);
>> -}
>> -
>> static void pl011_putc(struct uart_port *port, unsigned char c)
>> {
>> while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
>> @@ -2611,29 +2569,6 @@ OF_EARLYCON_DECLARE(pl011, "arm,pl011", pl011_early_console_setup);
>>
>> OF_EARLYCON_DECLARE(pl011, "arm,sbsa-uart", pl011_early_console_setup);
>>
>> -/*
>> - * On Qualcomm Datacenter Technologies QDF2400 SOCs affected by
>> - * Erratum 44, traditional earlycon can be enabled by specifying
>> - * "earlycon=qdf2400_e44,<address>". Any options are ignored.
>> - *
>> - * Alternatively, you can just specify "earlycon", and the early console
>> - * will be enabled with the information from the SPCR table. In this
>> - * case, the SPCR code will detect the need for the E44 work-around,
>> - * and set the console name to "qdf2400_e44".
>> - */
>> -static int __init
>> -qdf2400_e44_early_console_setup(struct earlycon_device *device,
>> - const char *opt)
>> -{
>> - if (!device->port.membase)
>> - return -ENODEV;
>> -
>> - device->con->write = qdf2400_e44_early_write;
>> - return 0;
>> -}
>> -
>> -EARLYCON_DECLARE(qdf2400_e44, qdf2400_e44_early_console_setup);
>> -
>> #else
>> #define AMBA_CONSOLE NULL
>> #endif
>> @@ -2869,22 +2804,6 @@ static int pl011_resume(struct device *dev)
>>
>> static SIMPLE_DEV_PM_OPS(pl011_dev_pm_ops, pl011_suspend, pl011_resume);
>>
>> -#ifdef CONFIG_ACPI_SPCR_TABLE
>> -static void qpdf2400_erratum44_workaround(struct device *dev,
>> - struct uart_amba_port *uap)
>> -{
>> - if (!qdf2400_e44_present)
>> - return;
>> -
>> - dev_info(dev, "working around QDF2400 SoC erratum 44\n");
>> - uap->vendor = &vendor_qdt_qdf2400_e44;
>> -}
>> -#else
>> -static void qpdf2400_erratum44_workaround(struct device *dev,
>> - struct uart_amba_port *uap)
>> -{ /* empty */ }
>> -#endif
>> -
>> static int sbsa_uart_probe(struct platform_device *pdev)
>> {
>> struct uart_amba_port *uap;
>> @@ -2921,7 +2840,6 @@ static int sbsa_uart_probe(struct platform_device *pdev)
>> uap->port.irq = ret;
>>
>> uap->vendor = &vendor_sbsa;
>> - qpdf2400_erratum44_workaround(&pdev->dev, uap);
>>
>> uap->reg_offset = uap->vendor->reg_offset;
>> uap->fifosize = 32;
>>
>
> I just bisected a crash that I see in -next to this change as commit
> 196f34af2bf4 ("tty: serial: amba-pl011: Remove QDF2xxx workarounds"). I
> see it when booting both ARCH=arm multi_v7_defconfig and ARCH=arm64
> virtconfig.
>
> $ qemu-system-aarch64 \
> -display none \
> -nodefaults \
> -cpu max,pauth-impdef=true \
> -machine virt,gic-version=max,virtualization=true \
> -append 'console=ttyAMA0 earlycon' \
> -kernel arch/arm64/boot/Image.gz \
> -initrd rootfs.cpio \
> -m 512m \
> -serial mon:stdio
> [ 0.000000] Booting Linux on physical CPU 0x0000000000 [0x000f0510]
> [ 0.000000] Linux version 6.8.0-rc1-00096-g196f34af2bf4 (nathan@dev-arch.thelio-3990X) (aarch64-linux-gcc (GCC) 13.2.0, GNU ld (GNU Binutils) 2.41) #1 SMP PREEMPT Tue Jan 30 09:42:53 MST 2024
> ..
> [ 0.210400] Serial: AMBA PL011 UART driver
> [ 0.245810] 9000000.pl011: ttyAMA0 at MMIO 0x9000000 (irq = 14, base_baud = 0) is a PL011 rev1
> [ 0.246789] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> [ 0.247015] Mem abort info:
> [ 0.247090] ESR = 0x0000000096000004
> [ 0.247204] EC = 0x25: DABT (current EL), IL = 32 bits
> [ 0.247338] SET = 0, FnV = 0
> [ 0.247423] EA = 0, S1PTW = 0
> [ 0.247512] FSC = 0x04: level 0 translation fault
> [ 0.247637] Data abort info:
> [ 0.247717] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> [ 0.247849] CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> [ 0.247975] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [ 0.248135] [0000000000000000] user address but active_mm is swapper
> [ 0.248391] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
> [ 0.248651] Modules linked in:
> [ 0.249219] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.8.0-rc1-00096-g196f34af2bf4 #1
> [ 0.249514] Hardware name: linux,dummy-virt (DT)
> [ 0.249757] pstate: 41400009 (nZcv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
> [ 0.249985] pc : __pi_strncmp+0x20/0x1b4
> [ 0.250611] lr : uart_parse_earlycon+0x34/0x174
> [ 0.250765] sp : ffff80008000b780
> [ 0.250869] x29: ffff80008000b780 x28: ffff000001c07b10 x27: ffffd8d528b2bfb0
> [ 0.251188] x26: ffff000001d57850 x25: 0000000000000000 x24: ffff000001ea8100
> [ 0.251410] x23: ffff000001ed5800 x22: ffff80008000b7ef x21: ffff80008000b7f0
> [ 0.251636] x20: ffff80008000b7d8 x19: 0000000000000000 x18: 0000000000000006
> [ 0.251855] x17: 203d20647561625f x16: 65736162202c3431 x15: 203d207172692820
> [ 0.252074] x14: 3030303030303978 x13: 0000000000000000 x12: 00000000000001b9
> [ 0.252293] x11: 0101010101010101 x10: ffffd8d528f48770 x9 : ffffd8d528ef0770
> [ 0.252531] x8 : ffffd8d528ac3c80 x7 : ffffd8d528f48770 x6 : 0000000000000000
> [ 0.252750] x5 : ffff000001ed5480 x4 : ffffd8d5280f4184 x3 : ffff80008000b7d8
> [ 0.252968] x2 : 0000000000000005 x1 : ffffd8d528ac3c80 x0 : 0000000000000000
> [ 0.253257] Call trace:
> [ 0.253439] __pi_strncmp+0x20/0x1b4
> [ 0.253642] pl011_console_match+0x44/0xd4
> [ 0.253792] try_enable_preferred_console+0x5c/0x10c
> [ 0.253940] register_console+0x124/0x438
> [ 0.254062] serial_core_register_port+0x618/0x654
> [ 0.254200] serial_ctrl_register_port+0x10/0x1c
> [ 0.254334] uart_add_one_port+0x10/0x1c
> [ 0.254452] pl011_register_port+0x68/0x11c
> [ 0.254574] pl011_probe+0x1a8/0x1dc
> [ 0.254682] amba_probe+0x110/0x1c4
> [ 0.254791] really_probe+0x148/0x2b0
> [ 0.254909] __driver_probe_device+0x78/0x12c
> [ 0.255037] driver_probe_device+0x3c/0x15c
> [ 0.255160] __device_attach_driver+0xb8/0x134
> [ 0.255289] bus_for_each_drv+0x84/0xe0
> [ 0.255404] __device_attach+0x9c/0x188
> [ 0.255519] device_initial_probe+0x14/0x20
> [ 0.255644] bus_probe_device+0xac/0xb0
> [ 0.255762] device_add+0x600/0x7b8
> [ 0.255868] amba_device_add+0x4c/0xb8
> [ 0.255981] of_platform_bus_create+0x2bc/0x37c
> [ 0.256116] of_platform_populate+0x50/0xfc
> [ 0.256240] of_platform_default_populate_init+0xe0/0x100
> [ 0.256395] do_one_initcall+0x7c/0x1c0
> [ 0.256511] kernel_init_freeable+0x1c0/0x288
> [ 0.256642] kernel_init+0x24/0x1dc
> [ 0.256754] ret_from_fork+0x10/0x20
> [ 0.257091] Code: f240091f 9240080d 540004c1 b500030d (f8408403)
> [ 0.257519] ---[ end trace 0000000000000000 ]---
> [ 0.258252] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> [ 0.258698] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
>
> If there is any more information I can provide or patches I can test,
> please let me know.
>
> Cheers,
> Nathan
>
> # bad: [41d66f96d0f15a0a2ad6fa2208f6bac1a66cbd52] Add linux-next specific files for 20240130
> # good: [41bccc98fb7931d63d03f326a746ac4d429c1dd3] Linux 6.8-rc2
> git bisect start '41d66f96d0f15a0a2ad6fa2208f6bac1a66cbd52' '41bccc98fb7931d63d03f326a746ac4d429c1dd3'
> # good: [f3f89885646036e16b325aea597fc9f375f1a56a] Merge branch 'main' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git
> git bisect good f3f89885646036e16b325aea597fc9f375f1a56a
> # good: [8042d32dd6c3730b0b4c8c9c811e204ed9f5f829] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
> git bisect good 8042d32dd6c3730b0b4c8c9c811e204ed9f5f829
> # bad: [f0edba72fe5ae932877f49796aaef8adf1a2eb8c] Merge branch 'togreg' of git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
> git bisect bad f0edba72fe5ae932877f49796aaef8adf1a2eb8c
> # good: [549632942a27cc3987473f8a8629200bc2ab0734] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/westeri/thunderbolt.git
> git bisect good 549632942a27cc3987473f8a8629200bc2ab0734
> # good: [92e32ed303a6c4dc5a5ad9ec54785a6245bcd8fa] tty: serial: samsung: change return type for s3c24xx_serial_rx_fifocnt()
> git bisect good 92e32ed303a6c4dc5a5ad9ec54785a6245bcd8fa
> # good: [41b5684e58b1286ae0fa180bc50b661a27efee33] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family
> git bisect good 41b5684e58b1286ae0fa180bc50b661a27efee33
> # bad: [098396f23ede72c3140a5ba33db38b130385a5fd] Merge branch 'habanalabs-next' of git://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/linux.git
> git bisect bad 098396f23ede72c3140a5ba33db38b130385a5fd
> # bad: [fccc9d9233f918ee50cf2955ae7134a7f3418351] tty: serial: uartps: Add rs485 support to uartps driver
> git bisect bad fccc9d9233f918ee50cf2955ae7134a7f3418351
> # bad: [59fdea965eee9908d904b94828c69f641f1e92fa] serial: txx9: Add missing #include <asm/txx9/generic.h>
> git bisect bad 59fdea965eee9908d904b94828c69f641f1e92fa
> # good: [79821c8a45807655c25d6bae2b13a3f98a5c01cd] tty: serial: samsung: shrink memory footprint of ``struct s3c24xx_uart_info``
> git bisect good 79821c8a45807655c25d6bae2b13a3f98a5c01cd
> # bad: [196f34af2bf4c87ac4299a9775503d81b446980c] tty: serial: amba-pl011: Remove QDF2xxx workarounds
> git bisect bad 196f34af2bf4c87ac4299a9775503d81b446980c
> # good: [09aec324b60f4309eeba8d2d5c897e882cce23bc] dt-bindings: serial: fsl-lpuart: support i.MX95
> git bisect good 09aec324b60f4309eeba8d2d5c897e882cce23bc
> # first bad commit: [196f34af2bf4c87ac4299a9775503d81b446980c] tty: serial: amba-pl011: Remove QDF2xxx workarounds
>