diff mbox series

tty: serial: amba-pl011: Remove QDF2xxx workarounds

Message ID 20240122-topic-qdf_cleanup_tty-v1-1-0415503184be@linaro.org
State New
Headers
Series tty: serial: amba-pl011: Remove QDF2xxx workarounds |

Commit Message

Konrad Dybcio Jan. 22, 2024, 2:27 p.m. UTC 
  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@linaro.org>
---
Compile-tested only
---
 drivers/tty/serial/amba-pl011.c | 82 -----------------------------------------
 1 file changed, 82 deletions(-)


---
base-commit: 319fbd8fc6d339e0a1c7b067eed870c518a13a02
change-id: 20240122-topic-qdf_cleanup_tty-448ae426000d

Best regards,

Comments

Nathan Chancellor Jan. 30, 2024, 5:05 p.m. UTC | #1 
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@linaro.org>
> ---
> 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))
> -		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
Mark Brown Jan. 31, 2024, 1:44 p.m. UTC | #2 
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.

There's one in Linaro's test lab which gets used by KernelCI:

   https://lkft.validation.linaro.org/scheduler/device_type/qcom-qdf2400

While the platform is pretty old it works well enough and provides
coverage of server platforms, ideally more server vendors would arrange
for their boards to be available but that's not actually the case yet.
The fact that it's a unique implementation of the architecture is also
interesting from a test point of view.  Given how trivial the workaround
here is it seems more useful to keep the support, could we revert this
commit please?
Peter Hilber Feb. 1, 2024, 1:13 a.m. UTC | #3 
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@linaro.org>
>> ---
>> 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
>
Konrad Dybcio Feb. 1, 2024, 2:48 p.m. UTC | #4 
On 31.01.2024 14:44, Mark Brown wrote:
> 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.
> 
> There's one in Linaro's test lab which gets used by KernelCI:
> 
>    https://lkft.validation.linaro.org/scheduler/device_type/qcom-qdf2400
> 
> While the platform is pretty old it works well enough and provides
> coverage of server platforms, ideally more server vendors would arrange
> for their boards to be available but that's not actually the case yet.
> The fact that it's a unique implementation of the architecture is also
> interesting from a test point of view.  Given how trivial the workaround
> here is it seems more useful to keep the support, could we revert this
> commit please?

Right, sorry for the mess. It actually turned out that Qualcomm is apparently
still using some of these internally and was interested in still keeping it
alive, at least for the time being.. I mentioned this on another similarly-
themed patch threads, but forgot this one.

Should I submit a revert, or can this be dropped?

Konrad
Mark Brown Feb. 1, 2024, 4:28 p.m. UTC | #5 
On Thu, Feb 01, 2024 at 03:48:08PM +0100, Konrad Dybcio wrote:

> Right, sorry for the mess. It actually turned out that Qualcomm is apparently
> still using some of these internally and was interested in still keeping it
> alive, at least for the time being.. I mentioned this on another similarly-
> themed patch threads, but forgot this one.

> Should I submit a revert, or can this be dropped?

Might be as well to send the revert then if it's needed it's there?
It'd be good to get this fixed quickly due to the issue Nathan found,
that means it's pretty disruptive for testing if you're using platforms
with the pl011.
diff mbox series

Patch

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))
-		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;